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

["Orzel, Michal" <michal.orzel@xxxxxxx>]

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).

Other than that:
Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>

NIT for the future: if you send a series of patches, please move the fixes/code
movement changes at the beginning of the series.

~Michal