Re: [PATCH v2 2/3] x86/EFI: replace ebmalloc()

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 27/07/2026 11:20 am, Jan Beulich wrote:
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -31,6 +31,7 @@
>  #include <asm/alternative.h>
>  #include <asm/apic.h>
>  #include <asm/bootinfo.h>
> +#include <asm/brk.h>
>  #include <asm/bzimage.h>
>  #include <asm/cpu-policy.h>
>  #include <asm/e820.h>
> @@ -164,6 +165,8 @@ cpumask_t __read_mostly cpu_present_map;
>  
>  unsigned long __read_mostly xen_phys_start;
>  
> +unsigned long __ro_after_init brk_end;
> +
>  /* Only used in asm code and within this source file */
>  char asmlinkage __section(".init.bss.stack_aligned") __aligned(STACK_SIZE)
>      cpu0_stack[STACK_SIZE];
> @@ -1141,7 +1144,6 @@ void asmlinkage __init noreturn __start_
>      struct boot_info *bi;
>      unsigned long nr_pages, raw_max_page;
>      int i, j, bytes = 0;
> -    unsigned long eb_start, eb_end;
>      bool acpi_boot_table_init_done = false, relocated = false;
>      bool vm_init_done = false;
>      int ret;
> @@ -1511,7 +1513,7 @@ void asmlinkage __init noreturn __start_
>          /*
>           * This needs to remain in sync with remove_xen_ranges() and the
>           * respective reserve_e820_ram() invocation below. No need to
> -         * query efi_boot_mem_unused() here, though.
> +         * query brk_get_unused_start() here, though.
>           */
>          xen->start = virt_to_maddr(_stext);
>          xen->size  = __2M_rwdata_end - _stext;
> @@ -1654,18 +1656,11 @@ void asmlinkage __init noreturn __start_
>      if ( !xen_phys_start )
>          panic("Not enough memory to relocate Xen\n");
>  
> -    /* FIXME: Putting a hole in .bss would shatter the large page mapping. */
> -    if ( using_2M_mapping() )
> -        efi_boot_mem_unused(NULL, NULL);
> -
>      /* This needs to remain in sync with remove_xen_ranges(). */
> -    if ( efi_boot_mem_unused(&eb_start, &eb_end) )
> -    {
> -        reserve_e820_ram(&boot_e820, __pa(_stext), __pa(eb_start));
> -        reserve_e820_ram(&boot_e820, __pa(eb_end), __pa(__2M_rwdata_end));
> -    }
> -    else
> -        reserve_e820_ram(&boot_e820, __pa(_stext), __pa(__2M_rwdata_end));
> +    brk_end = brk_get_unused_start();
> +    if ( using_2M_mapping() )
> +        brk_end = PAGE_ALIGN_2M(brk_end);
> +    reserve_e820_ram(&boot_e820, __pa(_stext), __pa(brk_end));

Hiding brk_end in setup.c like this is quite rude.  I guess it's because
you want to have brk.c be brk.init.o, but it really does live with the
other brk functions.

Furthermore, having brk_end right from the outset fixes the fact that
brk_get_unused_start() is doing things beyond retrieving a value.

With brk_end being the real bump pointer the allocator uses, then the
only function you need is brk_finish() (name subject to improvement)
which is now very clear about the point at which brk allocations cease
working.

The rest, dropping EFI's current ebmalloc() all looks fine now.

~Andrew