Re: [Xen-devel] [PATCH] x86/efi: Reserve EFI properties table

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Mon, May 08, 2017 at 05:17:18PM +0100, Ross Lagerwall wrote:
> Some EFI firmware implementations may place the EFI properties table in
> RAM marked as BootServicesData, which Xen does not consider as reserved.
> When dom0 tries to access the EFI properties table (which Linux >= 4.4
> does), it crashes with a page fault.  Fix this by unconditionally
> marking the EFI properties table as reserved in the E820, much like is
> done with the dmi regions.

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>

And also found who sets this:
https://github.com/tianocore/edk2/blob/1860cb00c18c6f0c58336ea15a63889dabd31d15/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c

Don't know if you want to include that in the blurb so somebody
can also look up why it is being set.


> 
> Signed-off-by: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
> ---
>  xen/arch/x86/efi/efi-boot.h |  8 ++++++++
>  xen/common/efi/boot.c       | 20 ++++++++++++++++++++
>  xen/common/efi/efi.h        |  4 ++++
>  3 files changed, 32 insertions(+)
> 
> diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
> index 34537d4..36ae464 100644
> --- a/xen/arch/x86/efi/efi-boot.h
> +++ b/xen/arch/x86/efi/efi-boot.h
> @@ -209,6 +209,14 @@ static void __init 
> efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
>          }
>      }
>  
> +    if ( efi_properties_tbl_addr && efi_properties_tbl_size )
> +    {
> +        ++e;
> +        e->addr = efi_properties_tbl_addr;
> +        e->size = efi_properties_tbl_size;
> +        e->type = E820_RESERVED;
> +        ++e820_raw.nr_map;
> +    }
>  }
>  
>  static void *__init efi_arch_allocate_mmap_buffer(UINTN map_size)
> diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
> index daf0c80..1fb396b 100644
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -40,6 +40,8 @@
>    { 0x605dab50, 0xe046, 0x4300, {0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 
> 0x23} }
>  #define APPLE_PROPERTIES_PROTOCOL_GUID \
>    { 0x91bd12fe, 0xf6c3, 0x44fb, { 0xa5, 0xb7, 0x51, 0x22, 0xab, 0x30, 0x3a, 
> 0xe0} }
> +#define EFI_PROPERTIES_TABLE_GUID \
> +  { 0x880aaca3, 0x4adc, 0x4a04, { 0x90, 0x79, 0xb7, 0x47, 0x34, 0x08, 0x25, 
> 0xe5} }
>  
>  typedef EFI_STATUS
>  (/* _not_ EFIAPI */ *EFI_SHIM_LOCK_VERIFY) (
> @@ -171,6 +173,15 @@ static char __section(".bss.page_aligned") 
> __aligned(PAGE_SIZE)
>      ebmalloc_mem[EBMALLOC_SIZE];
>  static unsigned long __initdata ebmalloc_allocated;
>  
> +struct efi_properties_table {
> +    u32 version;
> +    u32 length;
> +    u64 memory_protection_attribute;
> +};
> +
> +u64 __initdata efi_properties_tbl_addr;
> +u32 __initdata efi_properties_tbl_size;
> +
>  /* EFI boot allocator. */
>  static void __init __maybe_unused *ebmalloc(size_t size)
>  {
> @@ -809,6 +820,7 @@ static void __init efi_tables(void)
>          static EFI_GUID __initdata mps_guid = MPS_TABLE_GUID;
>          static EFI_GUID __initdata smbios_guid = SMBIOS_TABLE_GUID;
>          static EFI_GUID __initdata smbios3_guid = SMBIOS3_TABLE_GUID;
> +        static EFI_GUID __initdata properties_guid = 
> EFI_PROPERTIES_TABLE_GUID;
>  
>          if ( match_guid(&acpi2_guid, &efi_ct[i].VendorGuid) )
>              efi.acpi20 = (long)efi_ct[i].VendorTable;
> @@ -820,6 +832,14 @@ static void __init efi_tables(void)
>              efi.smbios = (long)efi_ct[i].VendorTable;
>          if ( match_guid(&smbios3_guid, &efi_ct[i].VendorGuid) )
>              efi.smbios3 = (long)efi_ct[i].VendorTable;
> +        if ( match_guid(&properties_guid, &efi_ct[i].VendorGuid) )
> +        {
> +            struct efi_properties_table *properties;
> +
> +            efi_properties_tbl_addr = (long)efi_ct[i].VendorTable;
> +            properties = (struct efi_properties_table 
> *)efi_properties_tbl_addr;
> +            efi_properties_tbl_size = properties->length;
> +        }
>      }
>  
>  #ifndef CONFIG_ARM /* TODO - disabled until implemented on ARM */
> diff --git a/xen/common/efi/efi.h b/xen/common/efi/efi.h
> index 6b9c56e..e509111 100644
> --- a/xen/common/efi/efi.h
> +++ b/xen/common/efi/efi.h
> @@ -5,6 +5,7 @@
>  #include <efi/efidevp.h>
>  #include <efi/eficapsule.h>
>  #include <efi/efiapi.h>
> +#include <xen/init.h>
>  #include <xen/efi.h>
>  #include <xen/spinlock.h>
>  #include <asm/page.h>
> @@ -39,3 +40,6 @@ extern UINT64 efi_boot_max_var_store_size, 
> efi_boot_remain_var_store_size,
>  
>  extern UINT64 efi_apple_properties_addr;
>  extern UINTN efi_apple_properties_len;
> +
> +extern u64 __initdata efi_properties_tbl_addr;
> +extern u32 __initdata efi_properties_tbl_size;
> -- 
> 2.7.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> https://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel