Re: [PATCH v2 5/7] xen/ppc: Enable bootfdt and boot allocator

[Julien Grall <julien@xxxxxxx>]

Hi Shawn,

On 18/01/2024 01:36, Shawn Anastasio wrote:
> Hi Julien,
>
> On 12/20/23 7:49 AM, Julien Grall wrote:
> > Hi,
> >
> > On 15/12/2023 02:44, Shawn Anastasio wrote:
> > > diff --git a/xen/common/device-tree/bootfdt.c
> > > b/xen/common/device-tree/bootfdt.c
> > > index 796ac01c18..7ddfcc7e2a 100644
> > > --- a/xen/common/device-tree/bootfdt.c
> > > +++ b/xen/common/device-tree/bootfdt.c
> > > @@ -543,12 +543,33 @@ size_t __init boot_fdt_info(const void *fdt,
> > > paddr_t paddr)
> > >        if ( ret < 0 )
> > >            panic("No valid device tree\n");
> > >    -    add_boot_module(BOOTMOD_FDT, paddr, fdt_totalsize(fdt), false);
> > > -
> > >        ret = device_tree_for_each_node((void *)fdt, 0, early_scan_node,
> > > NULL);
> > >        if ( ret )
> > >            panic("Early FDT parsing failed (%d)\n", ret);
> > >    +    /*
> > > +     * Add module for the FDT itself after the device tree has been
> > > parsed. This
> > > +     * is required on ppc64le where the device tree passed to Xen may
> > > have been
> > > +     * allocated by skiboot, in which case it will exist within a
> > > reserved
> > > +     * region and this call will fail. This is fine, however, since
> > > either way
> > > +     * the allocator will know not to step on the device tree.
> > > +     */
> > > +    add_boot_module(BOOTMOD_FDT, paddr, fdt_totalsize(fdt), false);
> > The consequence is BOOTMOD_FDT will not be added. AFAICT, Arm doesn't
> > try to access BOOTMOD_FDT, but I think it would be worth to print message.
> A message will already be printed by `meminfo_overlap_check` in setup.c
> when this condition is hit, like this:
>
> (XEN) Region: [0x0000003076e9b0, 0x00000030775215) overlapping with
> bank[3]: [0x00000030600000, 0x00000031000000)
This is somewhat non-descriptive. IOW it doesn't tell me that the 
overlap was with the FDT. But I guess this can be improved (this is not 
a request for you to send another patch).
Cheers,

--
Julien Grall