> -----Original Message----- > From: Auger Eric [mailto:eric.au...@redhat.com] > Sent: 29 March 2019 13:56 > To: Ard Biesheuvel <ard.biesheu...@linaro.org> > Cc: peter.mayd...@linaro.org; sa...@linux.intel.com; > qemu-devel@nongnu.org; Shameerali Kolothum Thodi > <shameerali.kolothum.th...@huawei.com>; Linuxarm > <linux...@huawei.com>; shannon.zha...@gmail.com; > qemu-...@nongnu.org; xuwei (O) <xuw...@huawei.com>; > imamm...@redhat.com; sebastien.bo...@intel.com; Laszlo Ersek > <ler...@redhat.com>; Leif Lindholm <leif.lindh...@arm.com> > Subject: Re: [Qemu-devel] [PATCH v3 07/10] hw/arm/virt: Introduce opt-in > feature "fdt" > > Hi Ard, > > On 3/29/19 2:14 PM, Ard Biesheuvel wrote: > > On Fri, 29 Mar 2019 at 14:12, Auger Eric <eric.au...@redhat.com> wrote: > >> > >> Hi Shameer, > >> > >> On 3/29/19 10:59 AM, Shameerali Kolothum Thodi wrote: > >>> > >>> > >>>> -----Original Message----- > >>>> From: Auger Eric [mailto:eric.au...@redhat.com] > >>>> Sent: 29 March 2019 09:32 > >>>> To: Shameerali Kolothum Thodi > <shameerali.kolothum.th...@huawei.com>; > >>>> qemu-devel@nongnu.org; qemu-...@nongnu.org; > imamm...@redhat.com; > >>>> peter.mayd...@linaro.org; shannon.zha...@gmail.com; > >>>> sa...@linux.intel.com; sebastien.bo...@intel.com > >>>> Cc: Linuxarm <linux...@huawei.com>; xuwei (O) > <xuw...@huawei.com>; > >>>> Laszlo Ersek <ler...@redhat.com>; Ard Biesheuvel > >>>> <ard.biesheu...@linaro.org>; Leif Lindholm <leif.lindh...@arm.com> > >>>> Subject: Re: [PATCH v3 07/10] hw/arm/virt: Introduce opt-in feature "fdt" > >>>> > >>>> Hi Shameer, > >>>> > >>>> [ + Laszlo, Ard, Leif ] > >>>> > >>>> On 3/21/19 11:47 AM, Shameer Kolothum wrote: > >>>>> This is to disable/enable populating DT nodes in case > >>>>> any conflict with acpi tables. The default is "off". > >>>> The name of the option sounds misleading to me. Also we don't really > >>>> know the scope of the disablement. At the moment this just aims to > >>>> prevent the hotpluggable dt nodes from being added if we boot in ACPI > mode. > >>>> > >>>>> > >>>>> This will be used in subsequent patch where cold plug > >>>>> device-memory support is added for DT boot. > >>>> I am concerned about the fact that in dt mode, by default, you won't see > >>>> any PCDIMM nodes. > >>>>> > >>>>> If DT memory node support is added for cold-plugged device > >>>>> memory, those memory will be visible to Guest kernel via > >>>>> UEFI GetMemoryMap() and gets treated as early boot memory. > >>>> Don't we have an issue in UEFI then. Normally the SRAT indicates whether > >>>> the slots are hotpluggable or not. Shouldn't the UEFI code look at this > >>>> info. > >>> > >>> Sorry I missed this part. Yes, that will be a more cleaner solution. > >>> > >>> Also, to be more clear on what happens, > >>> > >>> Guest ACPI boot with "fdt=on" , > >>> > >>> From kernel log, > >>> > >>> [ 0.000000] Early memory node ranges > >>> [ 0.000000] node 0: [mem > 0x0000000040000000-0x00000000bbf5ffff] > >>> [ 0.000000] node 0: [mem > 0x00000000bbf60000-0x00000000bbffffff] > >>> [ 0.000000] node 0: [mem > 0x00000000bc000000-0x00000000bc02ffff] > >>> [ 0.000000] node 0: [mem > 0x00000000bc030000-0x00000000bc36ffff] > >>> [ 0.000000] node 0: [mem > 0x00000000bc370000-0x00000000bf64ffff] > >>> [ 0.000000] node 0: [mem > 0x00000000bf650000-0x00000000bf6dffff] > >>> [ 0.000000] node 0: [mem > 0x00000000bf6e0000-0x00000000bf6effff] > >>> [ 0.000000] node 0: [mem > 0x00000000bf6f0000-0x00000000bf80ffff] > >>> [ 0.000000] node 0: [mem > 0x00000000bf810000-0x00000000bfffffff] > >>> [ 0.000000] node 0: [mem > 0x00000000c0000000-0x00000000ffffffff] --> This is the hotpluggable memory > node from DT. > >>> [ 0.000000] Zeroed struct page in unavailable ranges: 1040 pages > >>> [ 0.000000] Initmem setup node 0 [mem > 0x0000000040000000-0x00000000ffffffff] > >>> > >>> > >>> Guest ACPI boot with "fdt=off" , > >>> > >>> [ 0.000000] Movable zone start for each node > >>> [ 0.000000] Early memory node ranges > >>> [ 0.000000] node 0: [mem > 0x0000000040000000-0x00000000bbf5ffff] > >>> [ 0.000000] node 0: [mem > 0x00000000bbf60000-0x00000000bbffffff] > >>> [ 0.000000] node 0: [mem > 0x00000000bc000000-0x00000000bc02ffff] > >>> [ 0.000000] node 0: [mem > 0x00000000bc030000-0x00000000bc36ffff] > >>> [ 0.000000] node 0: [mem > 0x00000000bc370000-0x00000000bf64ffff] > >>> [ 0.000000] node 0: [mem > 0x00000000bf650000-0x00000000bf6dffff] > >>> [ 0.000000] node 0: [mem > 0x00000000bf6e0000-0x00000000bf6effff] > >>> [ 0.000000] node 0: [mem > 0x00000000bf6f0000-0x00000000bf80ffff] > >>> [ 0.000000] node 0: [mem > 0x00000000bf810000-0x00000000bfffffff] > >>> [ 0.000000] Zeroed struct page in unavailable ranges: 1040 pages > >>> [ 0.000000] Initmem setup node 0 [mem > 0x0000000040000000-0x00000000bfffffff > >>> > >>> The hotpluggable memory node is absent from early memory nodes here. > >> > >> OK thank you for the example illustrating the concern. > >>> > >>> As you said, it could be possible to detect this node using SRAT in UEFI. > >> > >> Let's wait for EDK2 experts on this. > >> > > > > Happy to chime in, but I need a bit more context here. > > > > What is the problem, how does this path try to solve it, and why is > > that a bad idea? > > > Sure, sorry. > > This series: > - [PATCH v3 00/10] ARM virt: ACPI memory hotplug support, > https://patchwork.kernel.org/cover/10863301/ > > aims to introduce PCDIMM support in qemu. In ACPI mode, it builds the > SRAT and DSDT parts and relies on GED to trigger the hotplug. > > We noticed that if we build the hotpluggable memory dt nodes on top of > the above ACPI tables, the DIMM slots are interpreted as not > hotpluggable memory slots (at least we think so). > > We think the EDK2 GetMemoryMap() uses the dt node info and ignores the > fact that those slots are exposed as hotpluggable in the SRAT for example. > > So in this series, we are forced to not generate the hotpluggable memory > dt nodes if we want the DIMM slots to be effectively recognized as > hotpluggable. > > Could you confirm we have a correct understanding of the EDK2 behaviour > and if so, would there be any solution for EDK2 to absorb both the DT > nodes and the relevant SRAT/DSDT tables and make the slots hotpluggable. > > At qemu level, detecting we are booting in ACPI mode and purposely > removing the above mentioned DT nodes does not look straightforward. > > Hope this clarifies
Thanks Eric for this. And this is where you can find the initial discussion on this, https://www.mail-archive.com/qemu-devel@nongnu.org/msg599076.html Thanks, Shameer
