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?
