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

Dave Young
2013-09-24T10:01:22Z

On 09/24/13 at 11:43am, Borislav Petkov wrote:
> Crap,
>
> I need to send from the web interface since the network here doesn't
> somehow let through port 587.
>
> On Tue, September 24, 2013 6:57 am, 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.
> >>
> >> 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;
> > ^^^^^^^^^^^^^^^
>
> Anyway, yes, this is wrong. We probably want to something like the
> following, instead (patch might be whitespace-damaged):
>
> --
> diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
> index a235dc95d629..ea0ea4fd3dab 100644
> --- a/arch/x86/platform/efi/efi_64.c
> +++ b/arch/x86/platform/efi/efi_64.c
> @@ -85,8 +85,7 @@ void __init efi_map_region(efi_memory_desc_t *md)
> {
> unsigned long size = md->num_pages << PAGE_SHIFT;
>
> - efi_va -= size;
> - if (efi_va < EFI_VA_END) {
> + if (efi_va - size < EFI_VA_END) {
> pr_warning(FW_WARN "VA address range overflow!\n");
> return;
> }
> @@ -101,6 +100,8 @@ void __init efi_map_region(efi_memory_desc_t *md)
> /* Do the VA map */
> __map_region(md, efi_va);
> md->virt_addr = efi_va;
> +
> + efi_va -= size;
> }
>
> void __iomem *__init efi_ioremap(unsigned long phys_addr, unsigned long
> size,
>

Ok, I got it, it it what what Peter mentioned problem 1.

--
Thanks
Dave