Re: [PATCH 2/2] xen/device-tree: move discard_initial_modules() to common code

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 10/8/26 1:19 PM, Orzel, Michal wrote:
> On 01-Oct-26 13:16, Oleksii Kurochko wrote:
> > Everything discard_initial_modules() touches is already common code:
> > bootinfo.modules, using_static_heap and fw_unreserved_regions(). Move it
> > out of Arm into common/device-tree/bootinfo.c, next to the boot allocator
> > handling of the same memory, and declare it in xen/bootinfo.h, so that
> > other device-tree based architectures can reuse it instead of growing a
> > copy of their own.
> >
> > The arch-specific remove_early_mappings() is not moved along with it:
> > how the early FDT mapping is torn down is up to the architecture. As the
> > DTB is one of the boot modules being released, the mapping has to go once
> > they are discarded, so call it from start_xen() right after
> > discard_initial_modules(), which keeps the common function free of any
> > arch dependency.
> >
> > While moving it, use unsigned int for the loop counter and replace the
> > "goto out" by a plain return, as there is nothing left to do at the label.
> >
> > No functional change.
> >
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> > ---
> >   xen/arch/arm/include/asm/setup.h  |  2 --
> >   xen/arch/arm/setup.c              | 35 ++-----------------------------
> >   xen/common/device-tree/bootinfo.c | 30 ++++++++++++++++++++++++++
> >   xen/include/xen/bootinfo.h        |  8 +++++++
> >   4 files changed, 40 insertions(+), 35 deletions(-)
> >
> > diff --git a/xen/arch/arm/include/asm/setup.h b/xen/arch/arm/include/asm/setup.h
> > index c491c567298f..02fdf2b56c9c 100644
> > --- a/xen/arch/arm/include/asm/setup.h
> > +++ b/xen/arch/arm/include/asm/setup.h
> > @@ -40,8 +40,6 @@ int acpi_make_efi_nodes(void *fdt, struct membank tbl_add[]);
> >   
> >   void create_dom0(void);
> >   
> > -void discard_initial_modules(void);
> > -
> >   void init_pdx(void);
> >   void setup_mm(void);
> >   
> > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> > index 16f899dff57b..a65c3d2a655e 100644
> > --- a/xen/arch/arm/setup.c
> > +++ b/xen/arch/arm/setup.c
> > @@ -210,39 +210,6 @@ static void __init processor_id(void)
> >       processor_setup();
> >   }
> >   
> > -void __init discard_initial_modules(void)
> > -{
> > -    struct boot_modules *mi = &bootinfo.modules;
> > -    int i;
> > -
> > -    /*
> > -     * When using static heap feature, don't give boot_modules memory back to
> > -     * the heap allocator
> > -     */
> > -    if ( using_static_heap )
> > -        goto out;
> > -
> > -    for ( i = 0; i < mi->nr_mods; i++ )
> > -    {
> > -        paddr_t s = mi->module[i].start;
> > -        paddr_t e = s + PAGE_ALIGN(mi->module[i].size);
> > -
> > -        if ( mi->module[i].kind == BOOTMOD_XEN )
> > -            continue;
> > -
> > -        if ( !mfn_valid(maddr_to_mfn(s)) ||
> > -             !mfn_valid(maddr_to_mfn(e)) )
> > -            continue;
> > -
> > -        fw_unreserved_regions(s, e, init_domheap_pages, 0);
> > -    }
> > -
> > -    mi->nr_mods = 0;
> > -
> > - out:
> > -    remove_early_mappings();
> > -}
> > -
> >   /* Relocate the FDT in Xen heap */
> >   static void __init relocate_fdt(const void **dtb_vaddr, size_t dtb_size)
> >   {
> > @@ -509,6 +476,8 @@ void asmlinkage __init noreturn start_xen(unsigned long 
> > fdt_paddr)
> >        */
> >       discard_initial_modules();
> The comment above is now duplicated with the one you added in xen/bootinfo.h.
> I'd drop it from here (can be done on commit).
I will drop it in v2 (if it won't merged at the moment when I will start 
address the comments for this patch series).
> Other than that:
> Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>
Thanks!


> NIT for the future: if you send a series of patches, please move the fixes/code
> movement changes at the beginning of the series.
I will take into account that in the future.

~ Oleksii