| Message ID | 1410310325-4509-4-git-send-email-roy.franz@linaro.org |
|---|---|
| State | New |
| Headers | show |
>>> On 10.09.14 at 02:51, <roy.franz@linaro.org> wrote: > --- a/xen/common/efi/boot.c > +++ b/xen/common/efi/boot.c > @@ -56,8 +56,6 @@ static EFI_HANDLE __initdata efi_ih; > static SIMPLE_TEXT_OUTPUT_INTERFACE *__initdata StdOut; > static SIMPLE_TEXT_OUTPUT_INTERFACE *__initdata StdErr; > > -static UINT32 __initdata mdesc_ver; Is this correct for the USE_SET_VIRTUAL_ADDRESS_MAP case? > @@ -1171,67 +1169,12 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) > } > } > > - efi_bs->GetMemoryMap(&efi_memmap_size, NULL, &map_key, > - &efi_mdesc_size, &mdesc_ver); > - mbi.mem_upper -= efi_memmap_size; > - mbi.mem_upper &= -__alignof__(EFI_MEMORY_DESCRIPTOR); > - if ( mbi.mem_upper < xen_phys_start ) > - blexit(L"Out of static memory"); > - efi_memmap = (void *)(long)mbi.mem_upper; > - status = efi_bs->GetMemoryMap(&efi_memmap_size, efi_memmap, &map_key, > - &efi_mdesc_size, &mdesc_ver); > - if ( EFI_ERROR(status) ) > - PrintErrMesg(L"Cannot obtain memory map", status); > + efi_arch_get_memory_map(&mmap_size, &mmap, &mmap_key, > + &mmap_desc_size, &mmap_desc_ver); The only arch-specific bit here is where to put the map. > --- a/xen/include/asm-x86/efi-boot.h > +++ b/xen/include/asm-x86/efi-boot.h > @@ -449,3 +449,89 @@ static void __init place_string(u32 *addr, const char *s) > } > *addr = (long)alloc; > } > + > +static void __init efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable, > + void *map, > + UINTN map_size, > + UINTN desc_size, > + UINT32 desc_ver) > +{ > + struct e820entry *e; > + int i; This was "unsigned int" originally, and should remain so. Please avoid type changes, or spell them out in the description if you find ones which indeed need adjustment. > +static void __init efi_arch_get_memory_map(UINTN *map_size, > + void **map, > + UINTN *map_key, UINTN *desc_size, > + UINT32 *desc_ver) > +{ > + EFI_STATUS status; Blank line after declarations please. Jan
On Thu, Sep 11, 2014 at 7:11 AM, Jan Beulich <JBeulich@suse.com> wrote: >>>> On 10.09.14 at 02:51, <roy.franz@linaro.org> wrote: >> --- a/xen/common/efi/boot.c >> +++ b/xen/common/efi/boot.c >> @@ -56,8 +56,6 @@ static EFI_HANDLE __initdata efi_ih; >> static SIMPLE_TEXT_OUTPUT_INTERFACE *__initdata StdOut; >> static SIMPLE_TEXT_OUTPUT_INTERFACE *__initdata StdErr; >> >> -static UINT32 __initdata mdesc_ver; > > Is this correct for the USE_SET_VIRTUAL_ADDRESS_MAP case? I'm not sure. That is more of a runtime services issue, and along with the unconditional disabling of using set virtual address map, I haven't really considered that case. > >> @@ -1171,67 +1169,12 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) >> } >> } >> >> - efi_bs->GetMemoryMap(&efi_memmap_size, NULL, &map_key, >> - &efi_mdesc_size, &mdesc_ver); >> - mbi.mem_upper -= efi_memmap_size; >> - mbi.mem_upper &= -__alignof__(EFI_MEMORY_DESCRIPTOR); >> - if ( mbi.mem_upper < xen_phys_start ) >> - blexit(L"Out of static memory"); >> - efi_memmap = (void *)(long)mbi.mem_upper; >> - status = efi_bs->GetMemoryMap(&efi_memmap_size, efi_memmap, &map_key, >> - &efi_mdesc_size, &mdesc_ver); >> - if ( EFI_ERROR(status) ) >> - PrintErrMesg(L"Cannot obtain memory map", status); >> + efi_arch_get_memory_map(&mmap_size, &mmap, &mmap_key, >> + &mmap_desc_size, &mmap_desc_ver); > > The only arch-specific bit here is where to put the map. Yes, but the ARM method of allocating the space can make the map bigger. I can re-arrange this so that only the memory allocation is in the arch code, but I think the current implementation is also reasonable. > >> --- a/xen/include/asm-x86/efi-boot.h >> +++ b/xen/include/asm-x86/efi-boot.h >> @@ -449,3 +449,89 @@ static void __init place_string(u32 *addr, const char *s) >> } >> *addr = (long)alloc; >> } >> + >> +static void __init efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable, >> + void *map, >> + UINTN map_size, >> + UINTN desc_size, >> + UINT32 desc_ver) >> +{ >> + struct e820entry *e; >> + int i; > > This was "unsigned int" originally, and should remain so. Please > avoid type changes, or spell them out in the description if you > find ones which indeed need adjustment. This was unintentional - I'll fix this. > >> +static void __init efi_arch_get_memory_map(UINTN *map_size, >> + void **map, >> + UINTN *map_key, UINTN *desc_size, >> + UINT32 *desc_ver) >> +{ >> + EFI_STATUS status; > > Blank line after declarations please. Noted - I'll review this patch-wide. > > Jan >
>>> On 11.09.14 at 19:40, <roy.franz@linaro.org> wrote: > On Thu, Sep 11, 2014 at 7:11 AM, Jan Beulich <JBeulich@suse.com> wrote: >>>>> On 10.09.14 at 02:51, <roy.franz@linaro.org> wrote: >>> @@ -1171,67 +1169,12 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) >>> } >>> } >>> >>> - efi_bs->GetMemoryMap(&efi_memmap_size, NULL, &map_key, >>> - &efi_mdesc_size, &mdesc_ver); >>> - mbi.mem_upper -= efi_memmap_size; >>> - mbi.mem_upper &= -__alignof__(EFI_MEMORY_DESCRIPTOR); >>> - if ( mbi.mem_upper < xen_phys_start ) >>> - blexit(L"Out of static memory"); >>> - efi_memmap = (void *)(long)mbi.mem_upper; >>> - status = efi_bs->GetMemoryMap(&efi_memmap_size, efi_memmap, &map_key, >>> - &efi_mdesc_size, &mdesc_ver); >>> - if ( EFI_ERROR(status) ) >>> - PrintErrMesg(L"Cannot obtain memory map", status); >>> + efi_arch_get_memory_map(&mmap_size, &mmap, &mmap_key, >>> + &mmap_desc_size, &mmap_desc_ver); >> >> The only arch-specific bit here is where to put the map. > Yes, but the ARM method of allocating the space can make the map > bigger. I can re-arrange > this so that only the memory allocation is in the arch code, but I > think the current implementation > is also reasonable. Hmm, yes, if ARM has no way of avoiding the growth of the map during allocation (which at a first glance seems suspicious to me), then yes. Is there a problem allocating a few more entries than the map's current size, so the possible growth can be accommodated? Jan
On Fri, 2014-09-12 at 08:07 +0100, Jan Beulich wrote: > >>> On 11.09.14 at 19:40, <roy.franz@linaro.org> wrote: > > On Thu, Sep 11, 2014 at 7:11 AM, Jan Beulich <JBeulich@suse.com> wrote: > >>>>> On 10.09.14 at 02:51, <roy.franz@linaro.org> wrote: > >>> @@ -1171,67 +1169,12 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) > >>> } > >>> } > >>> > >>> - efi_bs->GetMemoryMap(&efi_memmap_size, NULL, &map_key, > >>> - &efi_mdesc_size, &mdesc_ver); > >>> - mbi.mem_upper -= efi_memmap_size; > >>> - mbi.mem_upper &= -__alignof__(EFI_MEMORY_DESCRIPTOR); > >>> - if ( mbi.mem_upper < xen_phys_start ) > >>> - blexit(L"Out of static memory"); > >>> - efi_memmap = (void *)(long)mbi.mem_upper; > >>> - status = efi_bs->GetMemoryMap(&efi_memmap_size, efi_memmap, &map_key, > >>> - &efi_mdesc_size, &mdesc_ver); > >>> - if ( EFI_ERROR(status) ) > >>> - PrintErrMesg(L"Cannot obtain memory map", status); > >>> + efi_arch_get_memory_map(&mmap_size, &mmap, &mmap_key, > >>> + &mmap_desc_size, &mmap_desc_ver); > >> > >> The only arch-specific bit here is where to put the map. > > Yes, but the ARM method of allocating the space can make the map > > bigger. So something which ARM does can make the result of efi_bs->GetMemoryMap longer? Or are we talking about the bootinfo.mem array? > I can re-arrange > > this so that only the memory allocation is in the arch code, but I > > think the current implementation > > is also reasonable. > > Hmm, yes, if ARM has no way of avoiding the growth of the map > during allocation (which at a first glance seems suspicious to me), > then yes. Is there a problem allocating a few more entries than the > map's current size, so the possible growth can be accommodated? > > Jan >
>>> On 12.09.14 at 11:45, <Ian.Campbell@citrix.com> wrote: > On Fri, 2014-09-12 at 08:07 +0100, Jan Beulich wrote: >> >>> On 11.09.14 at 19:40, <roy.franz@linaro.org> wrote: >> > On Thu, Sep 11, 2014 at 7:11 AM, Jan Beulich <JBeulich@suse.com> wrote: >> >>>>> On 10.09.14 at 02:51, <roy.franz@linaro.org> wrote: >> >>> @@ -1171,67 +1169,12 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE > *SystemTable) >> >>> } >> >>> } >> >>> >> >>> - efi_bs->GetMemoryMap(&efi_memmap_size, NULL, &map_key, >> >>> - &efi_mdesc_size, &mdesc_ver); >> >>> - mbi.mem_upper -= efi_memmap_size; >> >>> - mbi.mem_upper &= -__alignof__(EFI_MEMORY_DESCRIPTOR); >> >>> - if ( mbi.mem_upper < xen_phys_start ) >> >>> - blexit(L"Out of static memory"); >> >>> - efi_memmap = (void *)(long)mbi.mem_upper; >> >>> - status = efi_bs->GetMemoryMap(&efi_memmap_size, efi_memmap, &map_key, >> >>> - &efi_mdesc_size, &mdesc_ver); >> >>> - if ( EFI_ERROR(status) ) >> >>> - PrintErrMesg(L"Cannot obtain memory map", status); >> >>> + efi_arch_get_memory_map(&mmap_size, &mmap, &mmap_key, >> >>> + &mmap_desc_size, &mmap_desc_ver); >> >> >> >> The only arch-specific bit here is where to put the map. >> > Yes, but the ARM method of allocating the space can make the map >> > bigger. > > So something which ARM does can make the result of efi_bs->GetMemoryMap > longer? Or are we talking about the bootinfo.mem array? Not having looked at the code, I suppose the issue is with the sequence of - get memory map size - alloc memory for the map - get memory map where the middle step may alter the memory map size. But with any kind of sane allocator it ought to be reasonable to expect it to not grow by more than two entries (if the allocator chooses to take the middle of an existing free block), which could be accounted for up front. Jan
On Fri, 2014-09-12 at 10:56 +0100, Jan Beulich wrote: > >>> On 12.09.14 at 11:45, <Ian.Campbell@citrix.com> wrote: > > On Fri, 2014-09-12 at 08:07 +0100, Jan Beulich wrote: > >> >>> On 11.09.14 at 19:40, <roy.franz@linaro.org> wrote: > >> > On Thu, Sep 11, 2014 at 7:11 AM, Jan Beulich <JBeulich@suse.com> wrote: > >> >>>>> On 10.09.14 at 02:51, <roy.franz@linaro.org> wrote: > >> >>> @@ -1171,67 +1169,12 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE > > *SystemTable) > >> >>> } > >> >>> } > >> >>> > >> >>> - efi_bs->GetMemoryMap(&efi_memmap_size, NULL, &map_key, > >> >>> - &efi_mdesc_size, &mdesc_ver); > >> >>> - mbi.mem_upper -= efi_memmap_size; > >> >>> - mbi.mem_upper &= -__alignof__(EFI_MEMORY_DESCRIPTOR); > >> >>> - if ( mbi.mem_upper < xen_phys_start ) > >> >>> - blexit(L"Out of static memory"); > >> >>> - efi_memmap = (void *)(long)mbi.mem_upper; > >> >>> - status = efi_bs->GetMemoryMap(&efi_memmap_size, efi_memmap, &map_key, > >> >>> - &efi_mdesc_size, &mdesc_ver); > >> >>> - if ( EFI_ERROR(status) ) > >> >>> - PrintErrMesg(L"Cannot obtain memory map", status); > >> >>> + efi_arch_get_memory_map(&mmap_size, &mmap, &mmap_key, > >> >>> + &mmap_desc_size, &mmap_desc_ver); > >> >> > >> >> The only arch-specific bit here is where to put the map. > >> > Yes, but the ARM method of allocating the space can make the map > >> > bigger. > > > > So something which ARM does can make the result of efi_bs->GetMemoryMap > > longer? Or are we talking about the bootinfo.mem array? > > Not having looked at the code, I suppose the issue is with the > sequence of > - get memory map size > - alloc memory for the map > - get memory map > where the middle step may alter the memory map size. But with any > kind of sane allocator it ought to be reasonable to expect it to not > grow by more than two entries (if the allocator chooses to take the > middle of an existing free block), which could be accounted for up > front. I see, thanks. Does x86 avoid this by using a static buffer or something? Or by the "account for it up front" trick which you mention? Ian.
>>> On 12.09.14 at 12:23, <Ian.Campbell@citrix.com> wrote: > On Fri, 2014-09-12 at 10:56 +0100, Jan Beulich wrote: >> >>> On 12.09.14 at 11:45, <Ian.Campbell@citrix.com> wrote: >> > On Fri, 2014-09-12 at 08:07 +0100, Jan Beulich wrote: >> >> >>> On 11.09.14 at 19:40, <roy.franz@linaro.org> wrote: >> >> > On Thu, Sep 11, 2014 at 7:11 AM, Jan Beulich <JBeulich@suse.com> wrote: >> >> >>>>> On 10.09.14 at 02:51, <roy.franz@linaro.org> wrote: >> >> >>> @@ -1171,67 +1169,12 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE > >> > *SystemTable) >> >> >>> } >> >> >>> } >> >> >>> >> >> >>> - efi_bs->GetMemoryMap(&efi_memmap_size, NULL, &map_key, >> >> >>> - &efi_mdesc_size, &mdesc_ver); >> >> >>> - mbi.mem_upper -= efi_memmap_size; >> >> >>> - mbi.mem_upper &= -__alignof__(EFI_MEMORY_DESCRIPTOR); >> >> >>> - if ( mbi.mem_upper < xen_phys_start ) >> >> >>> - blexit(L"Out of static memory"); >> >> >>> - efi_memmap = (void *)(long)mbi.mem_upper; >> >> >>> - status = efi_bs->GetMemoryMap(&efi_memmap_size, efi_memmap, &map_key, >> >> >>> - &efi_mdesc_size, &mdesc_ver); >> >> >>> - if ( EFI_ERROR(status) ) >> >> >>> - PrintErrMesg(L"Cannot obtain memory map", status); >> >> >>> + efi_arch_get_memory_map(&mmap_size, &mmap, &mmap_key, >> >> >>> + &mmap_desc_size, &mmap_desc_ver); >> >> >> >> >> >> The only arch-specific bit here is where to put the map. >> >> > Yes, but the ARM method of allocating the space can make the map >> >> > bigger. >> > >> > So something which ARM does can make the result of efi_bs->GetMemoryMap >> > longer? Or are we talking about the bootinfo.mem array? >> >> Not having looked at the code, I suppose the issue is with the >> sequence of >> - get memory map size >> - alloc memory for the map >> - get memory map >> where the middle step may alter the memory map size. But with any >> kind of sane allocator it ought to be reasonable to expect it to not >> grow by more than two entries (if the allocator chooses to take the >> middle of an existing free block), which could be accounted for up >> front. > > I see, thanks. > > Does x86 avoid this by using a static buffer or something? Or by the > "account for it up front" trick which you mention? It uses a kind of static buffer. Jan
On Fri, Sep 12, 2014 at 3:35 AM, Jan Beulich <JBeulich@suse.com> wrote: > >>> On 12.09.14 at 12:23, <Ian.Campbell@citrix.com> wrote: > > On Fri, 2014-09-12 at 10:56 +0100, Jan Beulich wrote: > >> >>> On 12.09.14 at 11:45, <Ian.Campbell@citrix.com> wrote: > >> > On Fri, 2014-09-12 at 08:07 +0100, Jan Beulich wrote: > >> >> >>> On 11.09.14 at 19:40, <roy.franz@linaro.org> wrote: > >> >> > On Thu, Sep 11, 2014 at 7:11 AM, Jan Beulich <JBeulich@suse.com> > wrote: > >> >> >>>>> On 10.09.14 at 02:51, <roy.franz@linaro.org> wrote: > >> >> >>> @@ -1171,67 +1169,12 @@ efi_start(EFI_HANDLE ImageHandle, > EFI_SYSTEM_TABLE > > > >> > *SystemTable) > >> >> >>> } > >> >> >>> } > >> >> >>> > >> >> >>> - efi_bs->GetMemoryMap(&efi_memmap_size, NULL, &map_key, > >> >> >>> - &efi_mdesc_size, &mdesc_ver); > >> >> >>> - mbi.mem_upper -= efi_memmap_size; > >> >> >>> - mbi.mem_upper &= -__alignof__(EFI_MEMORY_DESCRIPTOR); > >> >> >>> - if ( mbi.mem_upper < xen_phys_start ) > >> >> >>> - blexit(L"Out of static memory"); > >> >> >>> - efi_memmap = (void *)(long)mbi.mem_upper; > >> >> >>> - status = efi_bs->GetMemoryMap(&efi_memmap_size, efi_memmap, > &map_key, > >> >> >>> - &efi_mdesc_size, &mdesc_ver); > >> >> >>> - if ( EFI_ERROR(status) ) > >> >> >>> - PrintErrMesg(L"Cannot obtain memory map", status); > >> >> >>> + efi_arch_get_memory_map(&mmap_size, &mmap, &mmap_key, > >> >> >>> + &mmap_desc_size, > &mmap_desc_ver); > >> >> >> > >> >> >> The only arch-specific bit here is where to put the map. > >> >> > Yes, but the ARM method of allocating the space can make the map > >> >> > bigger. > >> > > >> > So something which ARM does can make the result of > efi_bs->GetMemoryMap > >> > longer? Or are we talking about the bootinfo.mem array? > >> > >> Not having looked at the code, I suppose the issue is with the > >> sequence of > >> - get memory map size > >> - alloc memory for the map > >> - get memory map > >> where the middle step may alter the memory map size. But with any > >> kind of sane allocator it ought to be reasonable to expect it to not > >> grow by more than two entries (if the allocator chooses to take the > >> middle of an existing free block), which could be accounted for up > >> front. > > > > I see, thanks. > > > > Does x86 avoid this by using a static buffer or something? Or by the > > "account for it up front" trick which you mention? > > It uses a kind of static buffer. > > Jan > > Increasing the buffer size should work. I will make it a page size larger, as that is the allocation granularity of the EFI allocator, and should be generous enough to avoid any problems, while not being excessively large. Roy
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c index ca604be..16ffe35 100644 --- a/xen/common/efi/boot.c +++ b/xen/common/efi/boot.c @@ -56,8 +56,6 @@ static EFI_HANDLE __initdata efi_ih; static SIMPLE_TEXT_OUTPUT_INTERFACE *__initdata StdOut; static SIMPLE_TEXT_OUTPUT_INTERFACE *__initdata StdErr; -static UINT32 __initdata mdesc_ver; - static struct file __initdata cfg; static struct file __initdata kernel; static struct file __initdata ramdisk; @@ -566,16 +564,18 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) EFI_STATUS status; unsigned int i, argc; CHAR16 **argv, *file_name, *cfg_file_name = NULL; - UINTN cols, rows, depth, size, map_key, info_size, gop_mode = ~0; + UINTN cols, rows, depth, size, info_size, gop_mode = ~0; EFI_HANDLE *handles = NULL; EFI_SHIM_LOCK_PROTOCOL *shim_lock; EFI_GRAPHICS_OUTPUT_PROTOCOL *gop = NULL; EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *mode_info; EFI_FILE_HANDLE dir_handle; union string section = { NULL }, name; - struct e820entry *e; u64 efer; bool_t base_video = 0; + UINT32 mmap_desc_ver = 0; + UINTN mmap_size, mmap_desc_size, mmap_key = 0; + void *mmap; efi_ih = ImageHandle; efi_bs = SystemTable->BootServices; @@ -875,8 +875,6 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) mbi.boot_loader_name = (long)"EFI"; mbi.mods_addr = (long)mb_modules; - place_string(&mbi.mem_upper, NULL); - /* Collect EDD info. */ BUILD_BUG_ON(offsetof(struct edd_info, edd_device_params) != EDDEXTSIZE); BUILD_BUG_ON(sizeof(struct edd_device_params) != EDDPARMSIZE); @@ -1171,67 +1169,12 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) } } - efi_bs->GetMemoryMap(&efi_memmap_size, NULL, &map_key, - &efi_mdesc_size, &mdesc_ver); - mbi.mem_upper -= efi_memmap_size; - mbi.mem_upper &= -__alignof__(EFI_MEMORY_DESCRIPTOR); - if ( mbi.mem_upper < xen_phys_start ) - blexit(L"Out of static memory"); - efi_memmap = (void *)(long)mbi.mem_upper; - status = efi_bs->GetMemoryMap(&efi_memmap_size, efi_memmap, &map_key, - &efi_mdesc_size, &mdesc_ver); - if ( EFI_ERROR(status) ) - PrintErrMesg(L"Cannot obtain memory map", status); + efi_arch_get_memory_map(&mmap_size, &mmap, &mmap_key, + &mmap_desc_size, &mmap_desc_ver); - /* Populate E820 table and check trampoline area availability. */ - e = e820map - 1; - for ( i = 0; i < efi_memmap_size; i += efi_mdesc_size ) - { - EFI_MEMORY_DESCRIPTOR *desc = efi_memmap + i; - u64 len = desc->NumberOfPages << EFI_PAGE_SHIFT; - u32 type; + efi_arch_process_memory_map(SystemTable, mmap, mmap_size, + mmap_desc_size, mmap_desc_ver); - switch ( desc->Type ) - { - default: - type = E820_RESERVED; - break; - case EfiConventionalMemory: - case EfiBootServicesCode: - case EfiBootServicesData: - if ( !trampoline_phys && desc->PhysicalStart + len <= 0x100000 && - len >= cfg.size && desc->PhysicalStart + len > cfg.addr ) - cfg.addr = (desc->PhysicalStart + len - cfg.size) & PAGE_MASK; - /* fall through */ - case EfiLoaderCode: - case EfiLoaderData: - if ( desc->Attribute & EFI_MEMORY_WB ) - type = E820_RAM; - else - case EfiUnusableMemory: - type = E820_UNUSABLE; - break; - case EfiACPIReclaimMemory: - type = E820_ACPI; - break; - case EfiACPIMemoryNVS: - type = E820_NVS; - break; - } - if ( e820nr && type == e->type && - desc->PhysicalStart == e->addr + e->size ) - e->size += len; - else if ( !len || e820nr >= E820MAX ) - continue; - else - { - ++e; - e->addr = desc->PhysicalStart; - e->size = len; - e->type = type; - ++e820nr; - } - } if ( !trampoline_phys ) { if ( !cfg.addr ) @@ -1239,7 +1182,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) relocate_trampoline(cfg.addr); } - status = efi_bs->ExitBootServices(ImageHandle, map_key); + status = efi_bs->ExitBootServices(ImageHandle, mmap_key); if ( EFI_ERROR(status) ) PrintErrMesg(L"Cannot exit boot services", status); diff --git a/xen/include/asm-x86/efi-boot.h b/xen/include/asm-x86/efi-boot.h index 4ad83ca..3fe6747 100644 --- a/xen/include/asm-x86/efi-boot.h +++ b/xen/include/asm-x86/efi-boot.h @@ -449,3 +449,89 @@ static void __init place_string(u32 *addr, const char *s) } *addr = (long)alloc; } + +static void __init efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable, + void *map, + UINTN map_size, + UINTN desc_size, + UINT32 desc_ver) +{ + struct e820entry *e; + int i; + + /* Set global EFI memory map variables */ + efi_memmap_size = map_size; + efi_mdesc_size = desc_size; + efi_memmap = map; + + /* Populate E820 table and check trampoline area availability. */ + e = e820map - 1; + for ( i = 0; i < map_size; i += desc_size ) + { + EFI_MEMORY_DESCRIPTOR *desc = map + i; + u64 len = desc->NumberOfPages << EFI_PAGE_SHIFT; + u32 type; + + switch ( desc->Type ) + { + default: + type = E820_RESERVED; + break; + case EfiConventionalMemory: + case EfiBootServicesCode: + case EfiBootServicesData: + if ( !trampoline_phys && desc->PhysicalStart + len <= 0x100000 && + len >= cfg.size && desc->PhysicalStart + len > cfg.addr ) + cfg.addr = (desc->PhysicalStart + len - cfg.size) & PAGE_MASK; + /* fall through */ + case EfiLoaderCode: + case EfiLoaderData: + if ( desc->Attribute & EFI_MEMORY_WB ) + type = E820_RAM; + else + case EfiUnusableMemory: + type = E820_UNUSABLE; + break; + case EfiACPIReclaimMemory: + type = E820_ACPI; + break; + case EfiACPIMemoryNVS: + type = E820_NVS; + break; + } + if ( e820nr && type == e->type && + desc->PhysicalStart == e->addr + e->size ) + e->size += len; + else if ( !len || e820nr >= E820MAX ) + continue; + else + { + ++e; + e->addr = desc->PhysicalStart; + e->size = len; + e->type = type; + ++e820nr; + } + } + +} + +static void __init efi_arch_get_memory_map(UINTN *map_size, + void **map, + UINTN *map_key, UINTN *desc_size, + UINT32 *desc_ver) +{ + EFI_STATUS status; + efi_bs->GetMemoryMap(map_size, NULL, map_key, + desc_size, desc_ver); + place_string(&mbi.mem_upper, NULL); + mbi.mem_upper -= *map_size; + mbi.mem_upper &= -__alignof__(EFI_MEMORY_DESCRIPTOR); + if ( mbi.mem_upper < xen_phys_start ) + blexit(L"Out of static memory"); + *map = (void *)(long)mbi.mem_upper; + status = efi_bs->GetMemoryMap(map_size, *map, map_key, + desc_size, desc_ver); + if ( EFI_ERROR(status) ) + PrintErrMesg(L"Cannot obtain memory map", status); +}
The memory used to store the EFI memory map is allocated in an architecture specific way, and the processing of the memory map itself uses x86 specific data structures. This patch adds architecture specific funtions so each architecture can provide its own implementation. Signed-off-by: Roy Franz <roy.franz@linaro.org> --- xen/common/efi/boot.c | 75 +++++------------------------------- xen/include/asm-x86/efi-boot.h | 86 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 66 deletions(-)