Re: [Qemu-devel] [PATCH v3 07/10] hw/arm/virt: Introduce opt-in feature "fdt"

[Ard Biesheuvel]

On Fri, 29 Mar 2019 at 14:12, Auger Eric <address@hidden> wrote:
>
> Hi Shameer,
>
> On 3/29/19 10:59 AM, Shameerali Kolothum Thodi wrote:
> >
> >
> >> -----Original Message-----
> >> From: Auger Eric [mailto:address@hidden
> >> Sent: 29 March 2019 09:32
> >> To: Shameerali Kolothum Thodi <address@hidden>;
> >> address@hidden; address@hidden; address@hidden;
> >> address@hidden; address@hidden;
> >> address@hidden; address@hidden
> >> Cc: Linuxarm <address@hidden>; xuwei (O) <address@hidden>;
> >> Laszlo Ersek <address@hidden>; Ard Biesheuvel
> >> <address@hidden>; Leif Lindholm <address@hidden>
> >> 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?