Re: [PATCH -v2] EFI: Runtime services virtual mapping

Dave Young
2013-09-24T08:57:31Z

On 09/24/13 at 01:23pm, Dave Young wrote:
> On 09/24/13 at 12:58pm, Dave Young wrote:
> > On 09/24/13 at 12:57pm, Dave Young wrote:
> > > On 09/23/13 at 08:06pm, H. Peter Anvin wrote:
> > > > Okay... I see two problems.
> > > >
> > > > 1. It looks like we subtract the region size after, rather than before, assigning an address.
>
> Could you explain more about this problem? Where is the code?
>
> > > >
> > > > 2. The second region is assigned the same address in the secondary kernel as in the first, implying the size of the first region was somehow set to zero.
> > >
> > > I find the reason, efi_reserve_boot_services will reserve the BOOT_SERVICE_DATA region
> > > thus the memmap size is changed to 0, so in 2nd kernel the virtual mapping addr after
> > > the md will be not same as 1st kernel, see below code:
> > >
> > > void __init efi_map_region(efi_memory_desc_t *md)
> > > {
> > > unsigned long size = md->num_pages << PAGE_SHIFT;
> > >
> > > efi_va -= size;
> > > ^^^^^^^^^^^^^^^
> > > [snip]
> > > }
> >
> > So how about just reserve BOOT_SERVICE_DATA region but keep the md.numpages as is?
>
> Hmm, num_pages = 0 is only set when boot service region reservation is imporsible, I'm
> lost.. But there must be somewhere set the size to 0.
>

digging more about it, it is indeed below code move the num_pages to 0:
void __init efi_reserve_boot_services(void)
{
[snip]
if ((start+size >= __pa_symbol(_text)
&& start <= __pa_symbol(_end)) ||
!e820_all_mapped(start, start+size, E820_RAM) ||
memblock_is_region_reserved(start, size)) {
/* Could not reserve, skip it */
md->num_pages = 0;
[snip]
}

During my test, the first region overlaps with kernel _text <-> _end, thus cause this issue.

I wonder if md->num_pages must be set to 0 here. If so I think we have to save the original memmap
for kexec use. Any better idea?