Re: [PATCH v5 04/44] x86/boot: move mmap info to boot info

["Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>]

On 10/9/24 11:13, Jan Beulich wrote:
> On 06.10.2024 23:49, Daniel P. Smith wrote:
> > --- a/xen/arch/x86/setup.c
> > +++ b/xen/arch/x86/setup.c
> > @@ -296,6 +296,12 @@ static struct boot_info __init 
> > *multiboot_fill_boot_info(unsigned long mbi_p)
> >       else
> >           bi->cmdline = "";
> >   
> > +    if ( mbi->flags & MBI_MEMMAP )
> > +    {
> > +        bi->memmap_addr = mbi->mmap_addr;
> > +        bi->memmap_length = mbi->mmap_length;
> > +    }
> > +
> >       return bi;
> >   }
> >   
> > @@ -1185,13 +1191,13 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
> >       {
> >           memmap_type = "Xen-e820";
> >       }
> > -    else if ( mbi->flags & MBI_MEMMAP )
> > +    else if ( bi->memmap_addr )
> I'd like to note that this isn't an exact transformation, as with the flag
> set the memory map could theoretically also like at address 0. As long as
> the legacy BIOS layout of low memory is as it is, that won't happen. I'm
> less certain going forward, for legacy-free hardware/firmware. Imo at the
> very least this needs mentioning as intentional in the description, for
> archeologists to later be able to tell whether this was an oversight.
>
> Or maybe it would be better to check ->memmap_length? That being zero
> clearly means there's effectively no map.
I think checking memmap_length is a better approach because as you say, 
the only time it will be zero is if no map was provided.
v/r,
dps