Re: [PATCH 28/32] x86/boot/e820: Make sure e820_search_gap() finds all gaps

[Nikolay Borisov]

On 5/15/25 15:05, Ingo Molnar wrote:
> The current implementation of e820_search_gap() searches gaps
> in a reverse search from MAX_GAP_END back to 0, contrary to
> what its main comment claims:
>
>      * Search for a gap in the E820 memory space from 0 to MAX_GAP_END (4GB).
>
> But gaps can not only be beyond E820 RAM ranges, they can be below
> them as well. For example this function will not find the proper
> PCI gap for simplified memory map layouts that have a single RAM
> range that crosses the 4GB boundary.
>
> Rework the function to have a proper forward search of
> E820 table entries.
>
> This makes the code somewhat bigger:
>
>     text       data        bss        dec        hex    filename
>     7613      44072          0      51685       c9e5    e820.o.before
>     7645      44072          0      51717       ca05    e820.o.after
>
> but it now both implements what it claims to do, and is more
> straightforward to read.
>
> ( This also allows 'idx' to be the regular u32 again, not an 'int'
>    underflowing to -1. )
>
> Signed-off-by: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: Andy Shevchenko <andy@xxxxxxxxxx>
> Cc: Arnd Bergmann <arnd@xxxxxxxxxx>
> Cc: David Woodhouse <dwmw@xxxxxxxxxxxx>
> Cc: H. Peter Anvin <hpa@xxxxxxxxx>
> Cc: Kees Cook <keescook@xxxxxxxxxxxx>
> Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> Cc: Mike Rapoport (Microsoft) <rppt@xxxxxxxxxx>
> ---
>   arch/x86/kernel/e820.c | 59 +++++++++++++++++++++++++++++++++++---------------
>   1 file changed, 41 insertions(+), 18 deletions(-)
>
> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> index 0d7e9794cd52..5260ce6ad466 100644
> --- a/arch/x86/kernel/e820.c
> +++ b/arch/x86/kernel/e820.c
> @@ -666,30 +666,52 @@ __init static void e820__update_table_kexec(void)
>    */
>   __init static int e820_search_gap(unsigned long *max_gap_start, unsigned long *max_gap_size)
>   {


Also it's not really searching for 'a gap' but rather it searched for a 
specific gap - one larger than the passed max_gap_size. So I wonder if 
find_gap would be a more apt name, given that you have a specific 
criterion you are searching against, the gap size. If you think I'm 
reading too much into it then it's fine to disregard this.


nit: Also this function ought to return boolean.

<snip>