[PATCH 00/10] arm64 EFI patches for 3.19

[PATCH 00/10] arm64 EFI patches for 3.19

[Ard Biesheuvel]

This is a bit of a mixed bag of patches that we would like to see merged for
3.19. Most of them have been posted and discussed on linux-efi and/or LAKML
before, and one of them has even been merged and reverted twice.

Patches #1 - #4 are fixes for compliance with the UEFI and PE/COFF specs.
No issues are known that require these patches, so there is no reason to
pull them into a stable release.

Patches #5 and #6 address minor issues in the arm64 EFI init code.

Patches #7 - #10 implement DMI/SMBIOS for arm64, both the existing 32-bit
version and the upcoming 3.0 version that allows the SMBIOS structure table
to reside at a physical offset that cannot be encoded in 32-bits. It also
install a 'Hardware: xxx' string that is printed along with oopses and
kernel call stack dumps on systems that implement DMI/SMBIOS.

Please refer to the patches themselves for version history. Acks and/or
comments appreciated.

Ard Biesheuvel (9):
  arm64/efi: efistub: jump to 'stext' directly, not through the header
  arm64/efi: set PE/COFF section alignment to 4 KB
  arm64/efi: set PE/COFF file alignment to 512 bytes
  arm64/efi: reserve regions of type ACPI_MEMORY_NVS
  arm64/efi: drop redundant set_bit(EFI_CONFIG_TABLES)
  arm64/efi: use UEFI memory map unconditionally if available
  efi: dmi: add support for SMBIOS 3.0 UEFI configuration table
  dmi: add support for SMBIOS 3.0 64-bit entry point
  arm64: dmi: set DMI string as dump stack arch description

Yi Li (1):
  arm64: dmi: Add SMBIOS/DMI support

 arch/arm64/Kconfig              | 11 +++++++
 arch/arm64/include/asm/dmi.h    | 31 ++++++++++++++++++
 arch/arm64/kernel/efi-entry.S   |  3 +-
 arch/arm64/kernel/efi.c         | 29 +++++++++++------
 arch/arm64/kernel/head.S        | 24 +++++++++-----
 arch/arm64/kernel/vmlinux.lds.S | 17 ++++++++++
 drivers/firmware/dmi_scan.c     | 70 +++++++++++++++++++++++++++++++++++++++--
 drivers/firmware/efi/efi.c      |  4 +++
 include/linux/efi.h             |  6 +++-
 9 files changed, 175 insertions(+), 20 deletions(-)
 create mode 100644 arch/arm64/include/asm/dmi.h

-- 
1.8.3.2

[PATCH 01/10] arm64/efi: efistub: jump to 'stext' directly, not through the header

[Ard Biesheuvel]

After the EFI stub has done its business, it jumps into the kernel by
branching to offset #0 of the loaded Image, which is where it expects
to find the header containing a 'branch to stext' instruction.

However, the UEFI spec 2.1.1 states the following regarding PE/COFF
image loading:
"A UEFI image is loaded into memory through the LoadImage() Boot
Service. This service loads an image with a PE32+ format into memory.
This PE32+ loader is required to load all sections of the PE32+ image
into memory."

In other words, it is /not/ required to load parts of the image that are
not covered by a PE/COFF section, so it may not have loaded the header
at the expected offset, as it is not covered by any PE/COFF section.

So instead, jump to 'stext' directly, which is at the base of the
PE/COFF .text section, by supplying a symbol 'stext_offset' to
efi-entry.o which contains the relative offset of stext into the Image.
Also replace other open coded calculations of the same value with a
reference to 'stext_offset'

Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
v3:
- rebased onto 3.17+
- added spec reference to commit message

v2:
- drop :lo12: relocation against stext_offset in favor of using a literal
  '=stext_offset' which is safer
---
 arch/arm64/kernel/efi-entry.S |  3 ++-
 arch/arm64/kernel/head.S      | 10 ++++++----
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/kernel/efi-entry.S b/arch/arm64/kernel/efi-entry.S
index 619b1dd7bcde..a0016d3a17da 100644
--- a/arch/arm64/kernel/efi-entry.S
+++ b/arch/arm64/kernel/efi-entry.S
@@ -61,7 +61,8 @@ ENTRY(efi_stub_entry)
 	 */
 	mov	x20, x0		// DTB address
 	ldr	x0, [sp, #16]	// relocated _text address
-	mov	x21, x0
+	ldr	x21, =stext_offset
+	add	x21, x0, x21
 
 	/*
 	 * Flush dcache covering current runtime addresses
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 0a6e4f924df8..8c06c9d269d2 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -132,6 +132,8 @@ efi_head:
 #endif
 
 #ifdef CONFIG_EFI
+	.globl	stext_offset
+	.set	stext_offset, stext - efi_head
 	.align 3
 pe_header:
 	.ascii	"PE"
@@ -155,7 +157,7 @@ optional_header:
 	.long	0				// SizeOfInitializedData
 	.long	0				// SizeOfUninitializedData
 	.long	efi_stub_entry - efi_head	// AddressOfEntryPoint
-	.long	stext - efi_head		// BaseOfCode
+	.long	stext_offset			// BaseOfCode
 
 extra_header_fields:
 	.quad	0				// ImageBase
@@ -172,7 +174,7 @@ extra_header_fields:
 	.long	_end - efi_head			// SizeOfImage
 
 	// Everything before the kernel image is considered part of the header
-	.long	stext - efi_head		// SizeOfHeaders
+	.long	stext_offset			// SizeOfHeaders
 	.long	0				// CheckSum
 	.short	0xa				// Subsystem (EFI application)
 	.short	0				// DllCharacteristics
@@ -217,9 +219,9 @@ section_table:
 	.byte	0
 	.byte	0        		// end of 0 padding of section name
 	.long	_end - stext		// VirtualSize
-	.long	stext - efi_head	// VirtualAddress
+	.long	stext_offset		// VirtualAddress
 	.long	_edata - stext		// SizeOfRawData
-	.long	stext - efi_head	// PointerToRawData
+	.long	stext_offset		// PointerToRawData
 
 	.long	0		// PointerToRelocations (0 for executables)
 	.long	0		// PointerToLineNumbers (0 for executables)
-- 
1.8.3.2

Re: [PATCH 01/10] arm64/efi: efistub: jump to 'stext' directly, not through the header

[Mark Rutland]

Hi Ard,

On Wed, Oct 22, 2014 at 03:21:44PM +0100, Ard Biesheuvel wrote:
> After the EFI stub has done its business, it jumps into the kernel by
> branching to offset #0 of the loaded Image, which is where it expects
> to find the header containing a 'branch to stext' instruction.
> 
> However, the UEFI spec 2.1.1 states the following regarding PE/COFF
> image loading:
> "A UEFI image is loaded into memory through the LoadImage() Boot
> Service. This service loads an image with a PE32+ format into memory.
> This PE32+ loader is required to load all sections of the PE32+ image
> into memory."
> 
> In other words, it is /not/ required to load parts of the image that are
> not covered by a PE/COFF section, so it may not have loaded the header
> at the expected offset, as it is not covered by any PE/COFF section.
> 
> So instead, jump to 'stext' directly, which is at the base of the
> PE/COFF .text section, by supplying a symbol 'stext_offset' to
> efi-entry.o which contains the relative offset of stext into the Image.
> Also replace other open coded calculations of the same value with a
> reference to 'stext_offset'
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Given the constraints you describe above, and prior discussions, this
looks sane to me:

Acked-by: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>

Mark.

> ---
> v3:
> - rebased onto 3.17+
> - added spec reference to commit message
> 
> v2:
> - drop :lo12: relocation against stext_offset in favor of using a literal
>   '=stext_offset' which is safer
> ---
>  arch/arm64/kernel/efi-entry.S |  3 ++-
>  arch/arm64/kernel/head.S      | 10 ++++++----
>  2 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/kernel/efi-entry.S b/arch/arm64/kernel/efi-entry.S
> index 619b1dd7bcde..a0016d3a17da 100644
> --- a/arch/arm64/kernel/efi-entry.S
> +++ b/arch/arm64/kernel/efi-entry.S
> @@ -61,7 +61,8 @@ ENTRY(efi_stub_entry)
>  	 */
>  	mov	x20, x0		// DTB address
>  	ldr	x0, [sp, #16]	// relocated _text address
> -	mov	x21, x0
> +	ldr	x21, =stext_offset
> +	add	x21, x0, x21
>  
>  	/*
>  	 * Flush dcache covering current runtime addresses
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 0a6e4f924df8..8c06c9d269d2 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -132,6 +132,8 @@ efi_head:
>  #endif
>  
>  #ifdef CONFIG_EFI
> +	.globl	stext_offset
> +	.set	stext_offset, stext - efi_head
>  	.align 3
>  pe_header:
>  	.ascii	"PE"
> @@ -155,7 +157,7 @@ optional_header:
>  	.long	0				// SizeOfInitializedData
>  	.long	0				// SizeOfUninitializedData
>  	.long	efi_stub_entry - efi_head	// AddressOfEntryPoint
> -	.long	stext - efi_head		// BaseOfCode
> +	.long	stext_offset			// BaseOfCode
>  
>  extra_header_fields:
>  	.quad	0				// ImageBase
> @@ -172,7 +174,7 @@ extra_header_fields:
>  	.long	_end - efi_head			// SizeOfImage
>  
>  	// Everything before the kernel image is considered part of the header
> -	.long	stext - efi_head		// SizeOfHeaders
> +	.long	stext_offset			// SizeOfHeaders
>  	.long	0				// CheckSum
>  	.short	0xa				// Subsystem (EFI application)
>  	.short	0				// DllCharacteristics
> @@ -217,9 +219,9 @@ section_table:
>  	.byte	0
>  	.byte	0        		// end of 0 padding of section name
>  	.long	_end - stext		// VirtualSize
> -	.long	stext - efi_head	// VirtualAddress
> +	.long	stext_offset		// VirtualAddress
>  	.long	_edata - stext		// SizeOfRawData
> -	.long	stext - efi_head	// PointerToRawData
> +	.long	stext_offset		// PointerToRawData
>  
>  	.long	0		// PointerToRelocations (0 for executables)
>  	.long	0		// PointerToLineNumbers (0 for executables)
> -- 
> 1.8.3.2
> 
> 

[PATCH 02/10] arm64/efi: set PE/COFF section alignment to 4 KB

[Ard Biesheuvel]

Position independent AArch64 code needs to be linked and loaded at the
same relative offset from a 4 KB boundary, or adrp/add and adrp/ldr
pairs will not work correctly. (This is how PC relative symbol
references with a 4 GB reach are emitted)

We need to declare this in the PE/COFF header, otherwise the PE/COFF
loader may load the Image and invoke the stub at an offset which
violates this rule.

Reviewed-by: Roy Franz <roy.franz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
v2: added comment explaining '.align 12' in head.S
---
 arch/arm64/kernel/head.S | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 8c06c9d269d2..8ae84d8c2a8c 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -161,7 +161,7 @@ optional_header:
 
 extra_header_fields:
 	.quad	0				// ImageBase
-	.long	0x20				// SectionAlignment
+	.long	0x1000				// SectionAlignment
 	.long	0x8				// FileAlignment
 	.short	0				// MajorOperatingSystemVersion
 	.short	0				// MinorOperatingSystemVersion
@@ -228,7 +228,15 @@ section_table:
 	.short	0		// NumberOfRelocations  (0 for executables)
 	.short	0		// NumberOfLineNumbers  (0 for executables)
 	.long	0xe0500020	// Characteristics (section flags)
-	.align 5
+
+	/*
+	 * EFI will load stext onwards at the 4k section alignment
+	 * described in the PE/COFF header. To ensure that instruction
+	 * sequences using an adrp and a :lo12: immediate will function
+	 * correctly at this alignment, we must ensure that stext is
+	 * placed at a 4k boundary in the Image to begin with.
+	 */
+	.align 12
 #endif
 
 ENTRY(stext)
-- 
1.8.3.2

Re: [PATCH 02/10] arm64/efi: set PE/COFF section alignment to 4 KB

[Mark Rutland]

On Wed, Oct 22, 2014 at 03:21:45PM +0100, Ard Biesheuvel wrote:
> Position independent AArch64 code needs to be linked and loaded at the
> same relative offset from a 4 KB boundary, or adrp/add and adrp/ldr
> pairs will not work correctly. (This is how PC relative symbol
> references with a 4 GB reach are emitted)
> 
> We need to declare this in the PE/COFF header, otherwise the PE/COFF
> loader may load the Image and invoke the stub at an offset which
> violates this rule.
> 
> Reviewed-by: Roy Franz <roy.franz@linaro.org>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Acked-by: Mark Rutland <mark.rutland@arm.com>

Mark.

> ---
> v2: added comment explaining '.align 12' in head.S
> ---
>  arch/arm64/kernel/head.S | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 8c06c9d269d2..8ae84d8c2a8c 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -161,7 +161,7 @@ optional_header:
>  
>  extra_header_fields:
>  	.quad	0				// ImageBase
> -	.long	0x20				// SectionAlignment
> +	.long	0x1000				// SectionAlignment
>  	.long	0x8				// FileAlignment
>  	.short	0				// MajorOperatingSystemVersion
>  	.short	0				// MinorOperatingSystemVersion
> @@ -228,7 +228,15 @@ section_table:
>  	.short	0		// NumberOfRelocations  (0 for executables)
>  	.short	0		// NumberOfLineNumbers  (0 for executables)
>  	.long	0xe0500020	// Characteristics (section flags)
> -	.align 5
> +
> +	/*
> +	 * EFI will load stext onwards at the 4k section alignment
> +	 * described in the PE/COFF header. To ensure that instruction
> +	 * sequences using an adrp and a :lo12: immediate will function
> +	 * correctly at this alignment, we must ensure that stext is
> +	 * placed at a 4k boundary in the Image to begin with.
> +	 */
> +	.align 12
>  #endif
>  
>  ENTRY(stext)
> -- 
> 1.8.3.2
> 
> 

[PATCH 03/10] arm64/efi: set PE/COFF file alignment to 512 bytes

[Ard Biesheuvel]

Change our PE/COFF header to use the minimum file alignment of
512 bytes (0x200), as mandated by the PE/COFF spec v8.3

Also update the linker script so that the Image file itself is also a
round multiple of FileAlignment.

Acked-by: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>
Acked-by: Roy Franz <roy.franz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 arch/arm64/kernel/head.S        |  2 +-
 arch/arm64/kernel/vmlinux.lds.S | 17 +++++++++++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 8ae84d8c2a8c..5a76e3ab788c 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -162,7 +162,7 @@ optional_header:
 extra_header_fields:
 	.quad	0				// ImageBase
 	.long	0x1000				// SectionAlignment
-	.long	0x8				// FileAlignment
+	.long	PECOFF_FILE_ALIGNMENT		// FileAlignment
 	.short	0				// MajorOperatingSystemVersion
 	.short	0				// MinorOperatingSystemVersion
 	.short	0				// MajorImageVersion
diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
index edf8715ba39b..4596f46d0244 100644
--- a/arch/arm64/kernel/vmlinux.lds.S
+++ b/arch/arm64/kernel/vmlinux.lds.S
@@ -32,6 +32,22 @@ jiffies = jiffies_64;
 	*(.hyp.text)					\
 	VMLINUX_SYMBOL(__hyp_text_end) = .;
 
+/*
+ * The size of the PE/COFF section that covers the kernel image, which
+ * runs from stext to _edata, must be a round multiple of the PE/COFF
+ * FileAlignment, which we set to its minimum value of 0x200. 'stext'
+ * itself is 4 KB aligned, so padding out _edata to a 0x200 aligned
+ * boundary should be sufficient.
+ */
+PECOFF_FILE_ALIGNMENT = 0x200;
+
+#ifdef CONFIG_EFI
+#define PECOFF_EDATA_PADDING	\
+	.pecoff_edata_padding : { BYTE(0); . = ALIGN(PECOFF_FILE_ALIGNMENT); }
+#else
+#define PECOFF_EDATA_PADDING
+#endif
+
 SECTIONS
 {
 	/*
@@ -103,6 +119,7 @@ SECTIONS
 	_data = .;
 	_sdata = .;
 	RW_DATA_SECTION(64, PAGE_SIZE, THREAD_SIZE)
+	PECOFF_EDATA_PADDING
 	_edata = .;
 
 	BSS_SECTION(0, 0, 0)
-- 
1.8.3.2

[PATCH 04/10] arm64/efi: reserve regions of type ACPI_MEMORY_NVS

[Ard Biesheuvel]

Memory regions of type ACPI_MEMORY_NVS should be preserved
by the OS, so make sure we reserve them at boot.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 arch/arm64/kernel/efi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 95c49ebc660d..71ea4fc0aa8a 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -132,6 +132,7 @@ static __init int is_reserve_region(efi_memory_desc_t *md)
 		return 1;
 
 	if (md->type == EFI_ACPI_RECLAIM_MEMORY ||
+	    md->type == EFI_ACPI_MEMORY_NVS ||
 	    md->type == EFI_RESERVED_TYPE)
 		return 1;
 
-- 
1.8.3.2

Re: [PATCH 04/10] arm64/efi: reserve regions of type ACPI_MEMORY_NVS

[Mark Rutland]

On Wed, Oct 22, 2014 at 03:21:47PM +0100, Ard Biesheuvel wrote:
> Memory regions of type ACPI_MEMORY_NVS should be preserved
> by the OS, so make sure we reserve them at boot.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  arch/arm64/kernel/efi.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index 95c49ebc660d..71ea4fc0aa8a 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -132,6 +132,7 @@ static __init int is_reserve_region(efi_memory_desc_t *md)
>  		return 1;
>  
>  	if (md->type == EFI_ACPI_RECLAIM_MEMORY ||
> +	    md->type == EFI_ACPI_MEMORY_NVS ||
>  	    md->type == EFI_RESERVED_TYPE)
>  		return 1;

Shouldn't we also filter out EFI_UNUSABLE_MEMORY? Or does that happen
elsewhere?

Perhaps instead we should invert this logic and assume memory should be
reserved if not EfiLoaderCode, EfiLoaderData, EfiBootServicesCode,
EfiBootServicesData, or EfiConventionalMemory. That looks to be what x86
does.

Mark.

Re: [PATCH 04/10] arm64/efi: reserve regions of type ACPI_MEMORY_NVS

[Ard Biesheuvel]

On 22 October 2014 18:15, Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> wrote:
> On Wed, Oct 22, 2014 at 03:21:47PM +0100, Ard Biesheuvel wrote:
>> Memory regions of type ACPI_MEMORY_NVS should be preserved
>> by the OS, so make sure we reserve them at boot.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>>  arch/arm64/kernel/efi.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
>> index 95c49ebc660d..71ea4fc0aa8a 100644
>> --- a/arch/arm64/kernel/efi.c
>> +++ b/arch/arm64/kernel/efi.c
>> @@ -132,6 +132,7 @@ static __init int is_reserve_region(efi_memory_desc_t *md)
>>               return 1;
>>
>>       if (md->type == EFI_ACPI_RECLAIM_MEMORY ||
>> +         md->type == EFI_ACPI_MEMORY_NVS ||
>>           md->type == EFI_RESERVED_TYPE)
>>               return 1;
>
> Shouldn't we also filter out EFI_UNUSABLE_MEMORY? Or does that happen
> elsewhere?
>

Yes, good point.

> Perhaps instead we should invert this logic and assume memory should be
> reserved if not EfiLoaderCode, EfiLoaderData, EfiBootServicesCode,
> EfiBootServicesData, or EfiConventionalMemory. That looks to be what x86
> does.
>

That would make it more robust against new types in future spec
changes, I suppose, although that would seem unlikely.

I am happy to change the patch to take that approach instead, if
others agree that it is preferable?

-- 
Ard.

Re: [PATCH 04/10] arm64/efi: reserve regions of type ACPI_MEMORY_NVS

[Ard Biesheuvel]

On 22 October 2014 18:33, Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On 22 October 2014 18:15, Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> wrote:
>> On Wed, Oct 22, 2014 at 03:21:47PM +0100, Ard Biesheuvel wrote:
>>> Memory regions of type ACPI_MEMORY_NVS should be preserved
>>> by the OS, so make sure we reserve them at boot.
>>>
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>> ---
>>>  arch/arm64/kernel/efi.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
>>> index 95c49ebc660d..71ea4fc0aa8a 100644
>>> --- a/arch/arm64/kernel/efi.c
>>> +++ b/arch/arm64/kernel/efi.c
>>> @@ -132,6 +132,7 @@ static __init int is_reserve_region(efi_memory_desc_t *md)
>>>               return 1;
>>>
>>>       if (md->type == EFI_ACPI_RECLAIM_MEMORY ||
>>> +         md->type == EFI_ACPI_MEMORY_NVS ||
>>>           md->type == EFI_RESERVED_TYPE)
>>>               return 1;
>>
>> Shouldn't we also filter out EFI_UNUSABLE_MEMORY? Or does that happen
>> elsewhere?
>>
>
> Yes, good point.
>
>> Perhaps instead we should invert this logic and assume memory should be
>> reserved if not EfiLoaderCode, EfiLoaderData, EfiBootServicesCode,
>> EfiBootServicesData, or EfiConventionalMemory. That looks to be what x86
>> does.
>>
>
> That would make it more robust against new types in future spec
> changes, I suppose, although that would seem unlikely.
>
> I am happy to change the patch to take that approach instead, if
> others agree that it is preferable?
>

As it turns out, there is another problem with this code:
is_reserve_region() currently identifies a region of type
EFI_RUNTIME_SERVICES_DATA as not reserved if it does not have the
EFI_MEMORY_RUNTIME attribute set. However, it is perfectly legal for
the firmware not to request a virtual mapping for such a region if it
contains things like configuration tables that are not used by any of
the Runtime Services themselves.

I will replace this patch with one that inverts the logic, as MarkR
suggests, but also drops the test against EFI_MEMORY_RUNTIME.

-- 
Ard.

[PATCH 05/10] arm64/efi: drop redundant set_bit(EFI_CONFIG_TABLES)

[Ard Biesheuvel]

The EFI_CONFIG_TABLES bit already gets set by efi_config_init(),
so there is no reason to set it again after this function returns
successfully.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 arch/arm64/kernel/efi.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 71ea4fc0aa8a..51522ab0c6da 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -112,8 +112,6 @@ static int __init uefi_init(void)
 		efi.systab->hdr.revision & 0xffff, vendor);
 
 	retval = efi_config_init(NULL);
-	if (retval == 0)
-		set_bit(EFI_CONFIG_TABLES, &efi.flags);
 
 out:
 	early_memunmap(efi.systab,  sizeof(efi_system_table_t));
-- 
1.8.3.2

Re: [PATCH 05/10] arm64/efi: drop redundant set_bit(EFI_CONFIG_TABLES)

[Will Deacon]

On Wed, Oct 22, 2014 at 03:21:48PM +0100, Ard Biesheuvel wrote:
> The EFI_CONFIG_TABLES bit already gets set by efi_config_init(),
> so there is no reason to set it again after this function returns
> successfully.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  arch/arm64/kernel/efi.c | 2 --
>  1 file changed, 2 deletions(-)

Since you mentioned it,

  Acked-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>

Will

> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index 71ea4fc0aa8a..51522ab0c6da 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -112,8 +112,6 @@ static int __init uefi_init(void)
>  		efi.systab->hdr.revision & 0xffff, vendor);
>  
>  	retval = efi_config_init(NULL);
> -	if (retval == 0)
> -		set_bit(EFI_CONFIG_TABLES, &efi.flags);
>  
>  out:
>  	early_memunmap(efi.systab,  sizeof(efi_system_table_t));
> -- 
> 1.8.3.2
> 
> 

[PATCH 06/10] arm64/efi: use UEFI memory map unconditionally if available

[Ard Biesheuvel]

On systems that boot via UEFI, all memory nodes are deleted from the
device tree, and instead, the size and location of system RAM is derived
from the UEFI memory map. This is handled by reserve_regions, which not only
reserves parts of memory that UEFI declares as reserved, but also installs
the memblocks that cover the remaining usable memory.

Currently, reserve_regions() is only called if uefi_init() succeeds.
However, it does not actually depend on anything that uefi_init() does,
and not calling reserve_regions() results in a broken boot, so it is
better to just call it unconditionally.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 arch/arm64/kernel/efi.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 51522ab0c6da..4cec21b1ecdd 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -313,8 +313,7 @@ void __init efi_init(void)
 	memmap.desc_size = params.desc_size;
 	memmap.desc_version = params.desc_ver;
 
-	if (uefi_init() < 0)
-		return;
+	WARN_ON(uefi_init() < 0);
 
 	reserve_regions();
 }
@@ -374,15 +373,13 @@ static int __init arm64_enter_virtual_mode(void)
 	int count = 0;
 	unsigned long flags;
 
-	if (!efi_enabled(EFI_BOOT)) {
-		pr_info("EFI services will not be available.\n");
-		return -1;
-	}
+	if (!efi_enabled(EFI_MEMMAP))
+		return 0;
 
 	mapsize = memmap.map_end - memmap.map;
 	early_memunmap(memmap.map, mapsize);
 
-	if (efi_runtime_disabled()) {
+	if (!efi_enabled(EFI_BOOT) || efi_runtime_disabled()) {
 		pr_info("EFI runtime services will be disabled.\n");
 		return -1;
 	}
-- 
1.8.3.2

Re: [PATCH 06/10] arm64/efi: use UEFI memory map unconditionally if available

[Mark Salter]

On Wed, 2014-10-22 at 16:21 +0200, Ard Biesheuvel wrote:
> On systems that boot via UEFI, all memory nodes are deleted from the
> device tree, and instead, the size and location of system RAM is derived
> from the UEFI memory map. This is handled by reserve_regions, which not only
> reserves parts of memory that UEFI declares as reserved, but also installs
> the memblocks that cover the remaining usable memory.
> 
> Currently, reserve_regions() is only called if uefi_init() succeeds.
> However, it does not actually depend on anything that uefi_init() does,
> and not calling reserve_regions() results in a broken boot, so it is
> better to just call it unconditionally.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  arch/arm64/kernel/efi.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index 51522ab0c6da..4cec21b1ecdd 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -313,8 +313,7 @@ void __init efi_init(void)
>  	memmap.desc_size = params.desc_size;
>  	memmap.desc_version = params.desc_ver;
>  
> -	if (uefi_init() < 0)
> -		return;
> +	WARN_ON(uefi_init() < 0);
>  
>  	reserve_regions();
>  }

It also looks like EFI_BOOT flag will be set even if uefi_init fails.
If uefi_init fails, we only need reserve_regions() for the purpose
of adding memblocks. Otherwise, we end up wasting a lot of memory.
So, something like this would also be needed:

diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 03aaa99..5a6e189 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -81,9 +81,6 @@ static int __init uefi_init(void)
 		return -ENOMEM;
 	}
 
-	set_bit(EFI_BOOT, &efi.flags);
-	set_bit(EFI_64BIT, &efi.flags);
-
 	/*
 	 * Verify the EFI Table
 	 */
@@ -109,6 +106,9 @@ static int __init uefi_init(void)
 		efi.systab->hdr.revision >> 16,
 		efi.systab->hdr.revision & 0xffff, vendor);
 
+	set_bit(EFI_BOOT, &efi.flags);
+	set_bit(EFI_64BIT, &efi.flags);
+
 	retval = efi_config_init(NULL);
 	if (retval == 0)
 		set_bit(EFI_CONFIG_TABLES, &efi.flags);
@@ -177,9 +177,10 @@ static __init void reserve_regions(void)
 		if (is_normal_ram(md))
 			early_init_dt_add_memory_arch(paddr, size);
 
-		if (is_reserve_region(md) ||
-		    md->type == EFI_BOOT_SERVICES_CODE ||
-		    md->type == EFI_BOOT_SERVICES_DATA) {
+		if (efi_enabled(EFI_BOOT) &&
+		    (is_reserve_region(md) ||
+		     md->type == EFI_BOOT_SERVICES_CODE ||
+		     md->type == EFI_BOOT_SERVICES_DATA)) {
 			memblock_reserve(paddr, size);
 			if (uefi_debug)
 				pr_cont("*");

> @@ -374,15 +373,13 @@ static int __init arm64_enter_virtual_mode(void)
>  	int count = 0;
>  	unsigned long flags;
>  
> -	if (!efi_enabled(EFI_BOOT)) {
> -		pr_info("EFI services will not be available.\n");
> -		return -1;
> -	}
> +	if (!efi_enabled(EFI_MEMMAP))
> +		return 0;
>  
>  	mapsize = memmap.map_end - memmap.map;
>  	early_memunmap(memmap.map, mapsize);
>  
> -	if (efi_runtime_disabled()) {
> +	if (!efi_enabled(EFI_BOOT) || efi_runtime_disabled()) {
>  		pr_info("EFI runtime services will be disabled.\n");
>  		return -1;
>  	}

Re: [PATCH 06/10] arm64/efi: use UEFI memory map unconditionally if available

[Ard Biesheuvel]

On 22 October 2014 19:06, Mark Salter <msalter-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Wed, 2014-10-22 at 16:21 +0200, Ard Biesheuvel wrote:
>> On systems that boot via UEFI, all memory nodes are deleted from the
>> device tree, and instead, the size and location of system RAM is derived
>> from the UEFI memory map. This is handled by reserve_regions, which not only
>> reserves parts of memory that UEFI declares as reserved, but also installs
>> the memblocks that cover the remaining usable memory.
>>
>> Currently, reserve_regions() is only called if uefi_init() succeeds.
>> However, it does not actually depend on anything that uefi_init() does,
>> and not calling reserve_regions() results in a broken boot, so it is
>> better to just call it unconditionally.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>>  arch/arm64/kernel/efi.c | 11 ++++-------
>>  1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
>> index 51522ab0c6da..4cec21b1ecdd 100644
>> --- a/arch/arm64/kernel/efi.c
>> +++ b/arch/arm64/kernel/efi.c
>> @@ -313,8 +313,7 @@ void __init efi_init(void)
>>       memmap.desc_size = params.desc_size;
>>       memmap.desc_version = params.desc_ver;
>>
>> -     if (uefi_init() < 0)
>> -             return;
>> +     WARN_ON(uefi_init() < 0);
>>
>>       reserve_regions();
>>  }
>
> It also looks like EFI_BOOT flag will be set even if uefi_init fails.
> If uefi_init fails, we only need reserve_regions() for the purpose
> of adding memblocks. Otherwise, we end up wasting a lot of memory.

Indeed. But perhaps it would be cleaner to add the memblocks in a
separate function that gets called before uefi_init(), let
reserve_regions() do just what it name implies, and retain the above
hunk so that the reserve_regions() call is only performed if UEFI
initialized correctly.

-- 
Ard.


> So, something like this would also be needed:
>
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index 03aaa99..5a6e189 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -81,9 +81,6 @@ static int __init uefi_init(void)
>                 return -ENOMEM;
>         }
>
> -       set_bit(EFI_BOOT, &efi.flags);
> -       set_bit(EFI_64BIT, &efi.flags);
> -
>         /*
>          * Verify the EFI Table
>          */
> @@ -109,6 +106,9 @@ static int __init uefi_init(void)
>                 efi.systab->hdr.revision >> 16,
>                 efi.systab->hdr.revision & 0xffff, vendor);
>
> +       set_bit(EFI_BOOT, &efi.flags);
> +       set_bit(EFI_64BIT, &efi.flags);
> +
>         retval = efi_config_init(NULL);
>         if (retval == 0)
>                 set_bit(EFI_CONFIG_TABLES, &efi.flags);
> @@ -177,9 +177,10 @@ static __init void reserve_regions(void)
>                 if (is_normal_ram(md))
>                         early_init_dt_add_memory_arch(paddr, size);
>
> -               if (is_reserve_region(md) ||
> -                   md->type == EFI_BOOT_SERVICES_CODE ||
> -                   md->type == EFI_BOOT_SERVICES_DATA) {
> +               if (efi_enabled(EFI_BOOT) &&
> +                   (is_reserve_region(md) ||
> +                    md->type == EFI_BOOT_SERVICES_CODE ||
> +                    md->type == EFI_BOOT_SERVICES_DATA)) {
>                         memblock_reserve(paddr, size);
>                         if (uefi_debug)
>                                 pr_cont("*");
>
>> @@ -374,15 +373,13 @@ static int __init arm64_enter_virtual_mode(void)
>>       int count = 0;
>>       unsigned long flags;
>>
>> -     if (!efi_enabled(EFI_BOOT)) {
>> -             pr_info("EFI services will not be available.\n");
>> -             return -1;
>> -     }
>> +     if (!efi_enabled(EFI_MEMMAP))
>> +             return 0;
>>
>>       mapsize = memmap.map_end - memmap.map;
>>       early_memunmap(memmap.map, mapsize);
>>
>> -     if (efi_runtime_disabled()) {
>> +     if (!efi_enabled(EFI_BOOT) || efi_runtime_disabled()) {
>>               pr_info("EFI runtime services will be disabled.\n");
>>               return -1;
>>       }
>
>

Re: [PATCH 06/10] arm64/efi: use UEFI memory map unconditionally if available

[Mark Salter]

On Wed, 2014-10-22 at 19:20 +0200, Ard Biesheuvel wrote:
> On 22 October 2014 19:06, Mark Salter <msalter-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > On Wed, 2014-10-22 at 16:21 +0200, Ard Biesheuvel wrote:
> >> On systems that boot via UEFI, all memory nodes are deleted from the
> >> device tree, and instead, the size and location of system RAM is derived
> >> from the UEFI memory map. This is handled by reserve_regions, which not only
> >> reserves parts of memory that UEFI declares as reserved, but also installs
> >> the memblocks that cover the remaining usable memory.
> >>
> >> Currently, reserve_regions() is only called if uefi_init() succeeds.
> >> However, it does not actually depend on anything that uefi_init() does,
> >> and not calling reserve_regions() results in a broken boot, so it is
> >> better to just call it unconditionally.
> >>
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> >> ---
> >>  arch/arm64/kernel/efi.c | 11 ++++-------
> >>  1 file changed, 4 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> >> index 51522ab0c6da..4cec21b1ecdd 100644
> >> --- a/arch/arm64/kernel/efi.c
> >> +++ b/arch/arm64/kernel/efi.c
> >> @@ -313,8 +313,7 @@ void __init efi_init(void)
> >>       memmap.desc_size = params.desc_size;
> >>       memmap.desc_version = params.desc_ver;
> >>
> >> -     if (uefi_init() < 0)
> >> -             return;
> >> +     WARN_ON(uefi_init() < 0);
> >>
> >>       reserve_regions();
> >>  }
> >
> > It also looks like EFI_BOOT flag will be set even if uefi_init fails.
> > If uefi_init fails, we only need reserve_regions() for the purpose
> > of adding memblocks. Otherwise, we end up wasting a lot of memory.
> 
> Indeed. But perhaps it would be cleaner to add the memblocks in a
> separate function that gets called before uefi_init(), let
> reserve_regions() do just what it name implies, and retain the above
> hunk so that the reserve_regions() call is only performed if UEFI
> initialized correctly.
> 

That works for me.

Re: [PATCH 06/10] arm64/efi: use UEFI memory map unconditionally if available

[Mark Rutland]

On Wed, Oct 22, 2014 at 06:06:56PM +0100, Mark Salter wrote:
> On Wed, 2014-10-22 at 16:21 +0200, Ard Biesheuvel wrote:
> > On systems that boot via UEFI, all memory nodes are deleted from the
> > device tree, and instead, the size and location of system RAM is derived
> > from the UEFI memory map. This is handled by reserve_regions, which not only
> > reserves parts of memory that UEFI declares as reserved, but also installs
> > the memblocks that cover the remaining usable memory.
> > 
> > Currently, reserve_regions() is only called if uefi_init() succeeds.
> > However, it does not actually depend on anything that uefi_init() does,
> > and not calling reserve_regions() results in a broken boot, so it is
> > better to just call it unconditionally.
> > 
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > ---
> >  arch/arm64/kernel/efi.c | 11 ++++-------
> >  1 file changed, 4 insertions(+), 7 deletions(-)
> > 
> > diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> > index 51522ab0c6da..4cec21b1ecdd 100644
> > --- a/arch/arm64/kernel/efi.c
> > +++ b/arch/arm64/kernel/efi.c
> > @@ -313,8 +313,7 @@ void __init efi_init(void)
> >  	memmap.desc_size = params.desc_size;
> >  	memmap.desc_version = params.desc_ver;
> >  
> > -	if (uefi_init() < 0)
> > -		return;
> > +	WARN_ON(uefi_init() < 0);
> >  
> >  	reserve_regions();
> >  }
> 
> It also looks like EFI_BOOT flag will be set even if uefi_init fails.
> If uefi_init fails, we only need reserve_regions() for the purpose
> of adding memblocks. Otherwise, we end up wasting a lot of memory.

What memory are we wasting in that case? Surely the only items that we
could choose to throw away if we failed uefi_init are
EFI_ACPI_RECLAIM_MEMORY and EFI_MEMORY_RUNTIME?

We might want to keep those around so we can kexec into a kernel where
we can make use of them. Surely they shouldn't take up a significant
proportion of the available memory?

Thanks,
Mark.

Re: [PATCH 06/10] arm64/efi: use UEFI memory map unconditionally if available

[Mark Salter]

On Thu, 2014-10-23 at 16:54 +0100, Mark Rutland wrote:
> On Wed, Oct 22, 2014 at 06:06:56PM +0100, Mark Salter wrote:
> > On Wed, 2014-10-22 at 16:21 +0200, Ard Biesheuvel wrote:
> > > On systems that boot via UEFI, all memory nodes are deleted from the
> > > device tree, and instead, the size and location of system RAM is derived
> > > from the UEFI memory map. This is handled by reserve_regions, which not only
> > > reserves parts of memory that UEFI declares as reserved, but also installs
> > > the memblocks that cover the remaining usable memory.
> > > 
> > > Currently, reserve_regions() is only called if uefi_init() succeeds.
> > > However, it does not actually depend on anything that uefi_init() does,
> > > and not calling reserve_regions() results in a broken boot, so it is
> > > better to just call it unconditionally.
> > > 
> > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > > ---
> > >  arch/arm64/kernel/efi.c | 11 ++++-------
> > >  1 file changed, 4 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> > > index 51522ab0c6da..4cec21b1ecdd 100644
> > > --- a/arch/arm64/kernel/efi.c
> > > +++ b/arch/arm64/kernel/efi.c
> > > @@ -313,8 +313,7 @@ void __init efi_init(void)
> > >  	memmap.desc_size = params.desc_size;
> > >  	memmap.desc_version = params.desc_ver;
> > >  
> > > -	if (uefi_init() < 0)
> > > -		return;
> > > +	WARN_ON(uefi_init() < 0);
> > >  
> > >  	reserve_regions();
> > >  }
> > 
> > It also looks like EFI_BOOT flag will be set even if uefi_init fails.
> > If uefi_init fails, we only need reserve_regions() for the purpose
> > of adding memblocks. Otherwise, we end up wasting a lot of memory.
> 
> What memory are we wasting in that case? Surely the only items that we
> could choose to throw away if we failed uefi_init are
> EFI_ACPI_RECLAIM_MEMORY and EFI_MEMORY_RUNTIME?
> 
> We might want to keep those around so we can kexec into a kernel where
> we can make use of them. Surely they shouldn't take up a significant
> proportion of the available memory?

In addition, reserve_regions() also reserves BOOT_SERVICES (which gets
freed up later after set_virtual_address_map(). That won't happen if
uefi_init() fails. In any case, if uefi_init() fails, we won't be able
to find the ACPI tables kexec kernel won't be able to either. The
BOOT_SERVICES stuff is the big chunk. There was some discussion of
not reserving that but no one has gotten around to writing the patch
to clean out the code that does it.

Re: [PATCH 06/10] arm64/efi: use UEFI memory map unconditionally if available

[Ard Biesheuvel]

On 23 October 2014 18:19, Mark Salter <msalter-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Thu, 2014-10-23 at 16:54 +0100, Mark Rutland wrote:
>> On Wed, Oct 22, 2014 at 06:06:56PM +0100, Mark Salter wrote:
>> > On Wed, 2014-10-22 at 16:21 +0200, Ard Biesheuvel wrote:
>> > > On systems that boot via UEFI, all memory nodes are deleted from the
>> > > device tree, and instead, the size and location of system RAM is derived
>> > > from the UEFI memory map. This is handled by reserve_regions, which not only
>> > > reserves parts of memory that UEFI declares as reserved, but also installs
>> > > the memblocks that cover the remaining usable memory.
>> > >
>> > > Currently, reserve_regions() is only called if uefi_init() succeeds.
>> > > However, it does not actually depend on anything that uefi_init() does,
>> > > and not calling reserve_regions() results in a broken boot, so it is
>> > > better to just call it unconditionally.
>> > >
>> > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> > > ---
>> > >  arch/arm64/kernel/efi.c | 11 ++++-------
>> > >  1 file changed, 4 insertions(+), 7 deletions(-)
>> > >
>> > > diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
>> > > index 51522ab0c6da..4cec21b1ecdd 100644
>> > > --- a/arch/arm64/kernel/efi.c
>> > > +++ b/arch/arm64/kernel/efi.c
>> > > @@ -313,8 +313,7 @@ void __init efi_init(void)
>> > >   memmap.desc_size = params.desc_size;
>> > >   memmap.desc_version = params.desc_ver;
>> > >
>> > > - if (uefi_init() < 0)
>> > > -         return;
>> > > + WARN_ON(uefi_init() < 0);
>> > >
>> > >   reserve_regions();
>> > >  }
>> >
>> > It also looks like EFI_BOOT flag will be set even if uefi_init fails.
>> > If uefi_init fails, we only need reserve_regions() for the purpose
>> > of adding memblocks. Otherwise, we end up wasting a lot of memory.
>>
>> What memory are we wasting in that case? Surely the only items that we
>> could choose to throw away if we failed uefi_init are
>> EFI_ACPI_RECLAIM_MEMORY and EFI_MEMORY_RUNTIME?
>>
>> We might want to keep those around so we can kexec into a kernel where
>> we can make use of them. Surely they shouldn't take up a significant
>> proportion of the available memory?
>
> In addition, reserve_regions() also reserves BOOT_SERVICES (which gets
> freed up later after set_virtual_address_map(). That won't happen if
> uefi_init() fails. In any case, if uefi_init() fails, we won't be able
> to find the ACPI tables kexec kernel won't be able to either. The
> BOOT_SERVICES stuff is the big chunk. There was some discussion of
> not reserving that but no one has gotten around to writing the patch
> to clean out the code that does it.
>

I am working on some patches to move the call to
SetVirtualAddressMap() to the stub, allowing us to handle the first
boot and subsequent kexec boots uniformly. This also means there will
be no need to reserve the BOOT_SERVICES regions anymore.

Perhaps we should drop this patch from this series, and I can revisit
it as part of the kexec series. That can wait for 3.20, I suppose, as
I don't see kexec being merged for 3.19

-- 
Ard.

Re: [PATCH 06/10] arm64/efi: use UEFI memory map unconditionally if available

[Mark Rutland]

[...]

> > > It also looks like EFI_BOOT flag will be set even if uefi_init fails.
> > > If uefi_init fails, we only need reserve_regions() for the purpose
> > > of adding memblocks. Otherwise, we end up wasting a lot of memory.
> > 
> > What memory are we wasting in that case? Surely the only items that we
> > could choose to throw away if we failed uefi_init are
> > EFI_ACPI_RECLAIM_MEMORY and EFI_MEMORY_RUNTIME?
> > 
> > We might want to keep those around so we can kexec into a kernel where
> > we can make use of them. Surely they shouldn't take up a significant
> > proportion of the available memory?
> 
> In addition, reserve_regions() also reserves BOOT_SERVICES (which gets
> freed up later after set_virtual_address_map(). That won't happen if
> uefi_init() fails. In any case, if uefi_init() fails, we won't be able
> to find the ACPI tables kexec kernel won't be able to either.

If you need the ACPI tables, the first kernel won't boot. However, if we
fail to map the runtime services data and code, that doesn't mean the
next kernel can't. We might have failed to map the runtime services due
to an early_ioremap bug or similar, and that could be fixed in the
kernel we're about to kexec to.

If we're going to nuke runtime regions, we should nuke them in the
memory map descriptors as a courtesy to the next kernel. Otherwise we
could be more courteous and simply leave them be (which is my
preference).

> The BOOT_SERVICES stuff is the big chunk. There was some discussion of
> not reserving that but no one has gotten around to writing the patch
> to clean out the code that does it.

I've just spent some time doing so; patch below. It boots happily on my
Juno (with 4KiB pages at least), but I haven't given it much of a stress
test.

I wasn't sure if unusable memory could show up with EFI_MEMORY_WB
attributes. If so, this patch still won't prevent that from being mapped
as cacheable, but that should be easy to fix.

Thanks,
Mark.

---->8----
>From 581d5bac5b4a6f93e23737012b71c58d809af6bb Mon Sep 17 00:00:00 2001
From: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Date: Thu, 23 Oct 2014 19:41:55 +0100
Subject: [PATCH] arm64: efi: Simplify memory descriptor handling

Currently discovering the memory map from UEFI is a multi-stage affair,
where the boot services code and data are kept around for much longer
than necessary. Additionally, potential overlap between memory and IO
mappings at kernel page granularity is not handled.

This patch attempts to solve both of these issues, with the addition and
filtering of memory regions being handled in a single function.

First, all normal memory (i.e. anything which can be mapped writeback
cacheable) is added to memblock. Second, IO and reserved regions are
filtered out of memblock at kernel page granularity, to prevent
potential conflicting mappings.

As some reserved regions must be present in the idmap these are
reserved. Everything else that's not normal memory is removed,
preventing the possibility of a cacheable mapping covering something it
shouldn't.

Signed-off-by: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org
Cc: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>
Cc: Leif Lindholm <leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org
Cc: Mark Salter <msalter-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 arch/arm64/kernel/efi.c | 181 +++++++++++-------------------------------------
 1 file changed, 42 insertions(+), 139 deletions(-)

diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 03aaa99..8873c28 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -154,161 +154,66 @@ static __init int is_reserve_region(efi_memory_desc_t *md)
 	return 0;
 }
 
-static __init void reserve_regions(void)
+static __init void efi_add_memory(void)
 {
 	efi_memory_desc_t *md;
-	u64 paddr, npages, size;
+	u64 paddr, size;
 
 	if (uefi_debug)
-		pr_info("Processing EFI memory map:\n");
+		pr_info("EFI: adding normal memory:\n");
 
 	for_each_efi_memory_desc(&memmap, md) {
+		if (!is_normal_ram(md))
+			continue;
+
 		paddr = md->phys_addr;
-		npages = md->num_pages;
+		size = md->num_pages << EFI_PAGE_SHIFT;
 
 		if (uefi_debug)
-			pr_info("  0x%012llx-0x%012llx [%s]",
-				paddr, paddr + (npages << EFI_PAGE_SHIFT) - 1,
+			pr_info("  0x%012llx-0x%012llx [%s]\n",
+				paddr, size - 1,
 				memory_type_name[md->type]);
 
-		memrange_efi_to_native(&paddr, &npages);
-		size = npages << PAGE_SHIFT;
+		early_init_dt_add_memory_arch(paddr, size);
+	}
+
+	/*
+	 * Regions describe in an EFI_MEMORY_DESCRIPTOR are only guaranteed to
+	 * be 4KiB aligned but the kernel page size could be larger, and
+	 * regions with different attributes could fall into the same kernel
+	 * page. Thus we must remove any memory in the same kernel page as an
+	 * IO region.
+	 *
+	 * Reserved regions of memory need to be in the idmap, so just reserve
+	 * them.
+	 */
+	if (uefi_debug)
+		pr_info("EFI: correcting for overlapping regions:\n");
+
+	for_each_efi_memory_desc(&memmap, md) {
+		if (!is_reserve_region(md) && is_normal_ram(md))
+			continue;
+
+		paddr = md->phys_addr;
+		size = md->num_pages << EFI_PAGE_SHIFT;
+
+		size = PAGE_ALIGN(size + (paddr & ~PAGE_MASK));
+		paddr &= PAGE_MASK;
 
-		if (is_normal_ram(md))
-			early_init_dt_add_memory_arch(paddr, size);
+		if (uefi_debug)
+			pr_info("  0x%012llx-0x%012llx [%s]\n",
+				paddr, size - 1,
+				memory_type_name[md->type]);
 
-		if (is_reserve_region(md) ||
-		    md->type == EFI_BOOT_SERVICES_CODE ||
-		    md->type == EFI_BOOT_SERVICES_DATA) {
+		if (is_reserve_region(md))
 			memblock_reserve(paddr, size);
-			if (uefi_debug)
-				pr_cont("*");
-		}
-
-		if (uefi_debug)
-			pr_cont("\n");
+		else
+			memblock_remove(paddr, size);
 	}
 
 	set_bit(EFI_MEMMAP, &efi.flags);
 }
 
-
-static u64 __init free_one_region(u64 start, u64 end)
-{
-	u64 size = end - start;
-
-	if (uefi_debug)
-		pr_info("  EFI freeing: 0x%012llx-0x%012llx\n",	start, end - 1);
-
-	free_bootmem_late(start, size);
-	return size;
-}
-
-static u64 __init free_region(u64 start, u64 end)
-{
-	u64 map_start, map_end, total = 0;
-
-	if (end <= start)
-		return total;
-
-	map_start = (u64)memmap.phys_map;
-	map_end = PAGE_ALIGN(map_start + (memmap.map_end - memmap.map));
-	map_start &= PAGE_MASK;
-
-	if (start < map_end && end > map_start) {
-		/* region overlaps UEFI memmap */
-		if (start < map_start)
-			total += free_one_region(start, map_start);
-
-		if (map_end < end)
-			total += free_one_region(map_end, end);
-	} else
-		total += free_one_region(start, end);
-
-	return total;
-}
-
-static void __init free_boot_services(void)
-{
-	u64 total_freed = 0;
-	u64 keep_end, free_start, free_end;
-	efi_memory_desc_t *md;
-
-	/*
-	 * If kernel uses larger pages than UEFI, we have to be careful
-	 * not to inadvertantly free memory we want to keep if there is
-	 * overlap at the kernel page size alignment. We do not want to
-	 * free is_reserve_region() memory nor the UEFI memmap itself.
-	 *
-	 * The memory map is sorted, so we keep track of the end of
-	 * any previous region we want to keep, remember any region
-	 * we want to free and defer freeing it until we encounter
-	 * the next region we want to keep. This way, before freeing
-	 * it, we can clip it as needed to avoid freeing memory we
-	 * want to keep for UEFI.
-	 */
-
-	keep_end = 0;
-	free_start = 0;
-
-	for_each_efi_memory_desc(&memmap, md) {
-		u64 paddr, npages, size;
-
-		if (is_reserve_region(md)) {
-			/*
-			 * We don't want to free any memory from this region.
-			 */
-			if (free_start) {
-				/* adjust free_end then free region */
-				if (free_end > md->phys_addr)
-					free_end -= PAGE_SIZE;
-				total_freed += free_region(free_start, free_end);
-				free_start = 0;
-			}
-			keep_end = md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT);
-			continue;
-		}
-
-		if (md->type != EFI_BOOT_SERVICES_CODE &&
-		    md->type != EFI_BOOT_SERVICES_DATA) {
-			/* no need to free this region */
-			continue;
-		}
-
-		/*
-		 * We want to free memory from this region.
-		 */
-		paddr = md->phys_addr;
-		npages = md->num_pages;
-		memrange_efi_to_native(&paddr, &npages);
-		size = npages << PAGE_SHIFT;
-
-		if (free_start) {
-			if (paddr <= free_end)
-				free_end = paddr + size;
-			else {
-				total_freed += free_region(free_start, free_end);
-				free_start = paddr;
-				free_end = paddr + size;
-			}
-		} else {
-			free_start = paddr;
-			free_end = paddr + size;
-		}
-		if (free_start < keep_end) {
-			free_start += PAGE_SIZE;
-			if (free_start >= free_end)
-				free_start = 0;
-		}
-	}
-	if (free_start)
-		total_freed += free_region(free_start, free_end);
-
-	if (total_freed)
-		pr_info("Freed 0x%llx bytes of EFI boot services memory",
-			total_freed);
-}
-
 void __init efi_init(void)
 {
 	struct efi_fdt_params params;
@@ -330,7 +235,7 @@ void __init efi_init(void)
 	if (uefi_init() < 0)
 		return;
 
-	reserve_regions();
+	efi_add_memory();
 }
 
 void __init efi_idmap_init(void)
@@ -452,8 +357,6 @@ static int __init arm64_enter_virtual_mode(void)
 
 	kfree(virtmap);
 
-	free_boot_services();
-
 	if (status != EFI_SUCCESS) {
 		pr_err("Failed to set EFI virtual address map! [%lx]\n",
 			status);
-- 
1.9.1

Re: [PATCH 06/10] arm64/efi: use UEFI memory map unconditionally if available

[Ard Biesheuvel]

On 23 October 2014 21:14, Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> wrote:
> [...]
>
>> > > It also looks like EFI_BOOT flag will be set even if uefi_init fails.
>> > > If uefi_init fails, we only need reserve_regions() for the purpose
>> > > of adding memblocks. Otherwise, we end up wasting a lot of memory.
>> >
>> > What memory are we wasting in that case? Surely the only items that we
>> > could choose to throw away if we failed uefi_init are
>> > EFI_ACPI_RECLAIM_MEMORY and EFI_MEMORY_RUNTIME?
>> >
>> > We might want to keep those around so we can kexec into a kernel where
>> > we can make use of them. Surely they shouldn't take up a significant
>> > proportion of the available memory?
>>
>> In addition, reserve_regions() also reserves BOOT_SERVICES (which gets
>> freed up later after set_virtual_address_map(). That won't happen if
>> uefi_init() fails. In any case, if uefi_init() fails, we won't be able
>> to find the ACPI tables kexec kernel won't be able to either.
>
> If you need the ACPI tables, the first kernel won't boot. However, if we
> fail to map the runtime services data and code, that doesn't mean the
> next kernel can't. We might have failed to map the runtime services due
> to an early_ioremap bug or similar, and that could be fixed in the
> kernel we're about to kexec to.
>
> If we're going to nuke runtime regions, we should nuke them in the
> memory map descriptors as a courtesy to the next kernel. Otherwise we
> could be more courteous and simply leave them be (which is my
> preference).
>
>> The BOOT_SERVICES stuff is the big chunk. There was some discussion of
>> not reserving that but no one has gotten around to writing the patch
>> to clean out the code that does it.
>
> I've just spent some time doing so; patch below. It boots happily on my
> Juno (with 4KiB pages at least), but I haven't given it much of a stress
> test.
>
> I wasn't sure if unusable memory could show up with EFI_MEMORY_WB
> attributes. If so, this patch still won't prevent that from being mapped
> as cacheable, but that should be easy to fix.
>
> Thanks,
> Mark.
>
> ---->8----
> From 581d5bac5b4a6f93e23737012b71c58d809af6bb Mon Sep 17 00:00:00 2001
> From: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> Date: Thu, 23 Oct 2014 19:41:55 +0100
> Subject: [PATCH] arm64: efi: Simplify memory descriptor handling
>
> Currently discovering the memory map from UEFI is a multi-stage affair,
> where the boot services code and data are kept around for much longer
> than necessary. Additionally, potential overlap between memory and IO
> mappings at kernel page granularity is not handled.
>
> This patch attempts to solve both of these issues, with the addition and
> filtering of memory regions being handled in a single function.
>
> First, all normal memory (i.e. anything which can be mapped writeback
> cacheable) is added to memblock. Second, IO and reserved regions are
> filtered out of memblock at kernel page granularity, to prevent
> potential conflicting mappings.
>

Fortunately, this is being addressed in an upcoming UEFI spec
revision: it will not be allowed for memory regions that could
potentially conflict in terms of cache attributes to share the same 64
KB page frame.

As I mentioned, my kexec patches will move SetVirtualAddressMap() to
the stub, which means we will also be able to drop the id map bits
entirely. As I said, let's revisit this in that context, and drop it
for now.

-- 
Ard.


> As some reserved regions must be present in the idmap these are
> reserved. Everything else that's not normal memory is removed,
> preventing the possibility of a cacheable mapping covering something it
> shouldn't.
>
> Signed-off-by: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> Cc: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org
> Cc: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>
> Cc: Leif Lindholm <leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org
> Cc: Mark Salter <msalter-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  arch/arm64/kernel/efi.c | 181 +++++++++++-------------------------------------
>  1 file changed, 42 insertions(+), 139 deletions(-)
>
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index 03aaa99..8873c28 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -154,161 +154,66 @@ static __init int is_reserve_region(efi_memory_desc_t *md)
>         return 0;
>  }
>
> -static __init void reserve_regions(void)
> +static __init void efi_add_memory(void)
>  {
>         efi_memory_desc_t *md;
> -       u64 paddr, npages, size;
> +       u64 paddr, size;
>
>         if (uefi_debug)
> -               pr_info("Processing EFI memory map:\n");
> +               pr_info("EFI: adding normal memory:\n");
>
>         for_each_efi_memory_desc(&memmap, md) {
> +               if (!is_normal_ram(md))
> +                       continue;
> +
>                 paddr = md->phys_addr;
> -               npages = md->num_pages;
> +               size = md->num_pages << EFI_PAGE_SHIFT;
>
>                 if (uefi_debug)
> -                       pr_info("  0x%012llx-0x%012llx [%s]",
> -                               paddr, paddr + (npages << EFI_PAGE_SHIFT) - 1,
> +                       pr_info("  0x%012llx-0x%012llx [%s]\n",
> +                               paddr, size - 1,
>                                 memory_type_name[md->type]);
>
> -               memrange_efi_to_native(&paddr, &npages);
> -               size = npages << PAGE_SHIFT;
> +               early_init_dt_add_memory_arch(paddr, size);
> +       }
> +
> +       /*
> +        * Regions describe in an EFI_MEMORY_DESCRIPTOR are only guaranteed to
> +        * be 4KiB aligned but the kernel page size could be larger, and
> +        * regions with different attributes could fall into the same kernel
> +        * page. Thus we must remove any memory in the same kernel page as an
> +        * IO region.
> +        *
> +        * Reserved regions of memory need to be in the idmap, so just reserve
> +        * them.
> +        */
> +       if (uefi_debug)
> +               pr_info("EFI: correcting for overlapping regions:\n");
> +
> +       for_each_efi_memory_desc(&memmap, md) {
> +               if (!is_reserve_region(md) && is_normal_ram(md))
> +                       continue;
> +
> +               paddr = md->phys_addr;
> +               size = md->num_pages << EFI_PAGE_SHIFT;
> +
> +               size = PAGE_ALIGN(size + (paddr & ~PAGE_MASK));
> +               paddr &= PAGE_MASK;
>
> -               if (is_normal_ram(md))
> -                       early_init_dt_add_memory_arch(paddr, size);
> +               if (uefi_debug)
> +                       pr_info("  0x%012llx-0x%012llx [%s]\n",
> +                               paddr, size - 1,
> +                               memory_type_name[md->type]);
>
> -               if (is_reserve_region(md) ||
> -                   md->type == EFI_BOOT_SERVICES_CODE ||
> -                   md->type == EFI_BOOT_SERVICES_DATA) {
> +               if (is_reserve_region(md))
>                         memblock_reserve(paddr, size);
> -                       if (uefi_debug)
> -                               pr_cont("*");
> -               }
> -
> -               if (uefi_debug)
> -                       pr_cont("\n");
> +               else
> +                       memblock_remove(paddr, size);
>         }
>
>         set_bit(EFI_MEMMAP, &efi.flags);
>  }
>
> -
> -static u64 __init free_one_region(u64 start, u64 end)
> -{
> -       u64 size = end - start;
> -
> -       if (uefi_debug)
> -               pr_info("  EFI freeing: 0x%012llx-0x%012llx\n", start, end - 1);
> -
> -       free_bootmem_late(start, size);
> -       return size;
> -}
> -
> -static u64 __init free_region(u64 start, u64 end)
> -{
> -       u64 map_start, map_end, total = 0;
> -
> -       if (end <= start)
> -               return total;
> -
> -       map_start = (u64)memmap.phys_map;
> -       map_end = PAGE_ALIGN(map_start + (memmap.map_end - memmap.map));
> -       map_start &= PAGE_MASK;
> -
> -       if (start < map_end && end > map_start) {
> -               /* region overlaps UEFI memmap */
> -               if (start < map_start)
> -                       total += free_one_region(start, map_start);
> -
> -               if (map_end < end)
> -                       total += free_one_region(map_end, end);
> -       } else
> -               total += free_one_region(start, end);
> -
> -       return total;
> -}
> -
> -static void __init free_boot_services(void)
> -{
> -       u64 total_freed = 0;
> -       u64 keep_end, free_start, free_end;
> -       efi_memory_desc_t *md;
> -
> -       /*
> -        * If kernel uses larger pages than UEFI, we have to be careful
> -        * not to inadvertantly free memory we want to keep if there is
> -        * overlap at the kernel page size alignment. We do not want to
> -        * free is_reserve_region() memory nor the UEFI memmap itself.
> -        *
> -        * The memory map is sorted, so we keep track of the end of
> -        * any previous region we want to keep, remember any region
> -        * we want to free and defer freeing it until we encounter
> -        * the next region we want to keep. This way, before freeing
> -        * it, we can clip it as needed to avoid freeing memory we
> -        * want to keep for UEFI.
> -        */
> -
> -       keep_end = 0;
> -       free_start = 0;
> -
> -       for_each_efi_memory_desc(&memmap, md) {
> -               u64 paddr, npages, size;
> -
> -               if (is_reserve_region(md)) {
> -                       /*
> -                        * We don't want to free any memory from this region.
> -                        */
> -                       if (free_start) {
> -                               /* adjust free_end then free region */
> -                               if (free_end > md->phys_addr)
> -                                       free_end -= PAGE_SIZE;
> -                               total_freed += free_region(free_start, free_end);
> -                               free_start = 0;
> -                       }
> -                       keep_end = md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT);
> -                       continue;
> -               }
> -
> -               if (md->type != EFI_BOOT_SERVICES_CODE &&
> -                   md->type != EFI_BOOT_SERVICES_DATA) {
> -                       /* no need to free this region */
> -                       continue;
> -               }
> -
> -               /*
> -                * We want to free memory from this region.
> -                */
> -               paddr = md->phys_addr;
> -               npages = md->num_pages;
> -               memrange_efi_to_native(&paddr, &npages);
> -               size = npages << PAGE_SHIFT;
> -
> -               if (free_start) {
> -                       if (paddr <= free_end)
> -                               free_end = paddr + size;
> -                       else {
> -                               total_freed += free_region(free_start, free_end);
> -                               free_start = paddr;
> -                               free_end = paddr + size;
> -                       }
> -               } else {
> -                       free_start = paddr;
> -                       free_end = paddr + size;
> -               }
> -               if (free_start < keep_end) {
> -                       free_start += PAGE_SIZE;
> -                       if (free_start >= free_end)
> -                               free_start = 0;
> -               }
> -       }
> -       if (free_start)
> -               total_freed += free_region(free_start, free_end);
> -
> -       if (total_freed)
> -               pr_info("Freed 0x%llx bytes of EFI boot services memory",
> -                       total_freed);
> -}
> -
>  void __init efi_init(void)
>  {
>         struct efi_fdt_params params;
> @@ -330,7 +235,7 @@ void __init efi_init(void)
>         if (uefi_init() < 0)
>                 return;
>
> -       reserve_regions();
> +       efi_add_memory();
>  }
>
>  void __init efi_idmap_init(void)
> @@ -452,8 +357,6 @@ static int __init arm64_enter_virtual_mode(void)
>
>         kfree(virtmap);
>
> -       free_boot_services();
> -
>         if (status != EFI_SUCCESS) {
>                 pr_err("Failed to set EFI virtual address map! [%lx]\n",
>                         status);
> --
> 1.9.1
>

[PATCH 07/10] efi: dmi: add support for SMBIOS 3.0 UEFI configuration table

[Ard Biesheuvel]

This adds support to the UEFI side for detecting the presence of
a SMBIOS 3.0 64-bit entry point. This allows the actual SMBIOS
structure table to reside at a physical offset over 4 GB, which
cannot be supported by the legacy SMBIOS 32-bit entry point.

Since the firmware can legally provide both entry points, store
the SMBIOS 3.0 entry point in a separate variable, and let the
DMI decoding layer decide which one will be used.

Tested-by: Suravee Suthikulpanit <suravee.suthikulpanit-5C7GfCeVMHo@public.gmane.org>
Acked-by: Leif Lindholm <leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/firmware/efi/efi.c | 4 ++++
 include/linux/efi.h        | 6 +++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 8590099ac148..9035c1b74d58 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -30,6 +30,7 @@ struct efi __read_mostly efi = {
 	.acpi       = EFI_INVALID_TABLE_ADDR,
 	.acpi20     = EFI_INVALID_TABLE_ADDR,
 	.smbios     = EFI_INVALID_TABLE_ADDR,
+	.smbios3    = EFI_INVALID_TABLE_ADDR,
 	.sal_systab = EFI_INVALID_TABLE_ADDR,
 	.boot_info  = EFI_INVALID_TABLE_ADDR,
 	.hcdp       = EFI_INVALID_TABLE_ADDR,
@@ -86,6 +87,8 @@ static ssize_t systab_show(struct kobject *kobj,
 		str += sprintf(str, "ACPI=0x%lx\n", efi.acpi);
 	if (efi.smbios != EFI_INVALID_TABLE_ADDR)
 		str += sprintf(str, "SMBIOS=0x%lx\n", efi.smbios);
+	if (efi.smbios3 != EFI_INVALID_TABLE_ADDR)
+		str += sprintf(str, "SMBIOS3=0x%lx\n", efi.smbios3);
 	if (efi.hcdp != EFI_INVALID_TABLE_ADDR)
 		str += sprintf(str, "HCDP=0x%lx\n", efi.hcdp);
 	if (efi.boot_info != EFI_INVALID_TABLE_ADDR)
@@ -260,6 +263,7 @@ static __initdata efi_config_table_type_t common_tables[] = {
 	{MPS_TABLE_GUID, "MPS", &efi.mps},
 	{SAL_SYSTEM_TABLE_GUID, "SALsystab", &efi.sal_systab},
 	{SMBIOS_TABLE_GUID, "SMBIOS", &efi.smbios},
+	{SMBIOS3_TABLE_GUID, "SMBIOS 3.0", &efi.smbios3},
 	{UGA_IO_PROTOCOL_GUID, "UGA", &efi.uga},
 	{NULL_GUID, NULL, NULL},
 };
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 0949f9c7e872..27d69388ded0 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -547,6 +547,9 @@ void efi_native_runtime_setup(void);
 #define SMBIOS_TABLE_GUID    \
     EFI_GUID(  0xeb9d2d31, 0x2d88, 0x11d3, 0x9a, 0x16, 0x0, 0x90, 0x27, 0x3f, 0xc1, 0x4d )
 
+#define SMBIOS3_TABLE_GUID    \
+    EFI_GUID(  0xf2fd1544, 0x9794, 0x4a2c, 0x99, 0x2e, 0xe5, 0xbb, 0xcf, 0x20, 0xe3, 0x94 )
+
 #define SAL_SYSTEM_TABLE_GUID    \
     EFI_GUID(  0xeb9d2d32, 0x2d88, 0x11d3, 0x9a, 0x16, 0x0, 0x90, 0x27, 0x3f, 0xc1, 0x4d )
 
@@ -810,7 +813,8 @@ extern struct efi {
 	unsigned long mps;		/* MPS table */
 	unsigned long acpi;		/* ACPI table  (IA64 ext 0.71) */
 	unsigned long acpi20;		/* ACPI table  (ACPI 2.0) */
-	unsigned long smbios;		/* SM BIOS table */
+	unsigned long smbios;		/* SM BIOS table (32 bit entry point) */
+	unsigned long smbios3;		/* SM BIOS table (64 bit entry point) */
 	unsigned long sal_systab;	/* SAL system table */
 	unsigned long boot_info;	/* boot info table */
 	unsigned long hcdp;		/* HCDP table */
-- 
1.8.3.2

Re: [PATCH 07/10] efi: dmi: add support for SMBIOS 3.0 UEFI configuration table

[Matt Fleming]

On Wed, 22 Oct, at 04:21:50PM, Ard Biesheuvel wrote:
> This adds support to the UEFI side for detecting the presence of
> a SMBIOS 3.0 64-bit entry point. This allows the actual SMBIOS
> structure table to reside at a physical offset over 4 GB, which
> cannot be supported by the legacy SMBIOS 32-bit entry point.
> 
> Since the firmware can legally provide both entry points, store
> the SMBIOS 3.0 entry point in a separate variable, and let the
> DMI decoding layer decide which one will be used.
> 
> Tested-by: Suravee Suthikulpanit <suravee.suthikulpanit-5C7GfCeVMHo@public.gmane.org>
> Acked-by: Leif Lindholm <leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  drivers/firmware/efi/efi.c | 4 ++++
>  include/linux/efi.h        | 6 +++++-
>  2 files changed, 9 insertions(+), 1 deletion(-)

Looks like you might also need to change driver/xen/efi.c to set
EFI_INVALID_TABLE_ADDR for the new table address?

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH 07/10] efi: dmi: add support for SMBIOS 3.0 UEFI configuration table

[Ard Biesheuvel]

On 27 October 2014 16:26, Matt Fleming <matt-HNK1S37rvNbeXh+fF434Mdi2O/JbrIOy@public.gmane.org> wrote:
> On Wed, 22 Oct, at 04:21:50PM, Ard Biesheuvel wrote:
>> This adds support to the UEFI side for detecting the presence of
>> a SMBIOS 3.0 64-bit entry point. This allows the actual SMBIOS
>> structure table to reside at a physical offset over 4 GB, which
>> cannot be supported by the legacy SMBIOS 32-bit entry point.
>>
>> Since the firmware can legally provide both entry points, store
>> the SMBIOS 3.0 entry point in a separate variable, and let the
>> DMI decoding layer decide which one will be used.
>>
>> Tested-by: Suravee Suthikulpanit <suravee.suthikulpanit-5C7GfCeVMHo@public.gmane.org>
>> Acked-by: Leif Lindholm <leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>>  drivers/firmware/efi/efi.c | 4 ++++
>>  include/linux/efi.h        | 6 +++++-
>>  2 files changed, 9 insertions(+), 1 deletion(-)
>
> Looks like you might also need to change driver/xen/efi.c to set
> EFI_INVALID_TABLE_ADDR for the new table address?
>

You're quite right. Will add it in v2

-- 
Ard.

[PATCH 08/10] dmi: add support for SMBIOS 3.0 64-bit entry point

[Ard Biesheuvel]

The DMTF SMBIOS reference spec v3.0.0 defines a new 64-bit entry point,
which enables support for SMBIOS structure tables residing at a physical
offset over 4 GB. This is especially important for upcoming arm64
platforms whose system RAM resides entirely above the 4 GB boundary.

For the UEFI case, this code attempts to detect the new SMBIOS 3.0
header magic at the offset passed in the SMBIOS3_TABLE_GUID UEFI
configuration table. If this configuration table is not provided, or
if we fail to parse the header, we fall back to using the legacy
SMBIOS_TABLE_GUID configuration table. This is in line with the spec,
that allows both configuration tables to be provided, but mandates that
they must point to the same structure table, unless the version pointed
to by the 64-bit entry point is a superset of the 32-bit one.

For the non-UEFI case, the detection logic is modified to look for the
SMBIOS 3.0 header magic before it looks for the legacy header magic.

Note that this patch is based on version 3.0.0d [draft] of the
specification, which is expected not to deviate from the final version
in ways that would affect the correctness of this implementation.

Tested-by: Suravee Suthikulpanit <suravee.suthikulpanit-5C7GfCeVMHo@public.gmane.org>
Acked-by: Leif Lindholm <leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/firmware/dmi_scan.c | 70 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 68 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
index 17afc51f3054..a698b90ed14f 100644
--- a/drivers/firmware/dmi_scan.c
+++ b/drivers/firmware/dmi_scan.c
@@ -93,6 +93,12 @@ static void dmi_table(u8 *buf, int len, int num,
 		const struct dmi_header *dm = (const struct dmi_header *)data;
 
 		/*
+		 * 7.45 End-of-Table (Type 127) [SMBIOS reference spec v3.0.0]
+		 */
+		if (dm->type == 127)
+			break;
+
+		/*
 		 *  We want to know the total length (formatted area and
 		 *  strings) before decoding to make sure we won't run off the
 		 *  table in dmi_decode or dmi_string
@@ -107,7 +113,7 @@ static void dmi_table(u8 *buf, int len, int num,
 	}
 }
 
-static u32 dmi_base;
+static phys_addr_t dmi_base;
 static u16 dmi_len;
 static u16 dmi_num;
 
@@ -514,12 +520,72 @@ static int __init dmi_present(const u8 *buf)
 	return 1;
 }
 
+/*
+ * Check for the SMBIOS 3.0 64-bit entry point signature. Unlike the legacy
+ * 32-bit entry point, there is no embedded DMI header (_DMI_) in here.
+ */
+static int __init dmi_smbios3_present(const u8 *buf)
+{
+	if (memcmp(buf, "_SM3_", 5) == 0 &&
+	    buf[6] < 32 && dmi_checksum(buf, buf[6])) {
+		dmi_ver = get_unaligned_be16(buf + 7);
+		dmi_len = get_unaligned_le32(buf + 12);
+		dmi_base = get_unaligned_le64(buf + 16);
+
+		/*
+		 * The 64-bit SMBIOS 3.0 entry point no longer has a field
+		 * containing the number of structures present in the table.
+		 * Instead, it defines the table size as a maximum size, and
+		 * relies on the end-of-table structure type (#127) to be used
+		 * to signal the end of the table.
+		 * So let's define dmi_num as an upper bound as well: each
+		 * structure has a 4 byte header, so dmi_len / 4 is an upper
+		 * bound for the number of structures in the table.
+		 */
+		dmi_num = dmi_len / 4;
+
+		if (dmi_walk_early(dmi_decode) == 0) {
+			pr_info("SMBIOS %d.%d present.\n",
+			        dmi_ver >> 8, dmi_ver & 0xFF);
+			dmi_format_ids(dmi_ids_string, sizeof(dmi_ids_string));
+			printk(KERN_DEBUG "DMI: %s\n", dmi_ids_string);
+			return 0;
+		}
+	}
+	return 1;
+}
+
 void __init dmi_scan_machine(void)
 {
 	char __iomem *p, *q;
 	char buf[32];
 
 	if (efi_enabled(EFI_CONFIG_TABLES)) {
+		/*
+		 * According to the DMTF SMBIOS reference spec v3.0.0, it is
+		 * allowed to define both the 64-bit entry point (smbios3) and
+		 * the 32-bit entry point (smbios), in which case they should
+		 * either both point to the same SMBIOS structure table, or the
+		 * table pointed to by the 64-bit entry point should contain a
+		 * superset of the table contents pointed to by the 32-bit entry
+		 * point (section 5.2)
+		 * This implies that the 64-bit entry point should have
+		 * precedence if it is defined and supported by the OS. If we
+		 * have the 64-bit entry point, but fail to decode it, fall
+		 * back to the legacy one (if available)
+		 */
+		if (efi.smbios3 != EFI_INVALID_TABLE_ADDR) {
+			p = dmi_early_remap(efi.smbios3, 32);
+			if (p == NULL)
+				goto error;
+			memcpy_fromio(buf, p, 32);
+			dmi_early_unmap(p, 32);
+
+			if (!dmi_smbios3_present(buf)) {
+				dmi_available = 1;
+				goto out;
+			}
+		}
 		if (efi.smbios == EFI_INVALID_TABLE_ADDR)
 			goto error;
 
@@ -552,7 +618,7 @@ void __init dmi_scan_machine(void)
 		memset(buf, 0, 16);
 		for (q = p; q < p + 0x10000; q += 16) {
 			memcpy_fromio(buf + 16, q, 16);
-			if (!dmi_present(buf)) {
+			if (!dmi_smbios3_present(buf) || !dmi_present(buf)) {
 				dmi_available = 1;
 				dmi_early_unmap(p, 0x10000);
 				goto out;
-- 
1.8.3.2

[PATCH 09/10] arm64: dmi: Add SMBIOS/DMI support

[Ard Biesheuvel]

From: Yi Li <yi.li-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

SMBIOS is important for server hardware vendors. It implements a spec for
providing descriptive information about the platform. Things like serial
numbers, physical layout of the ports, build configuration data, and the like.

Signed-off-by: Yi Li <yi.li-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Tested-by: Suravee Suthikulpanit <suravee.suthikulpanit-5C7GfCeVMHo@public.gmane.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
A short history of this patch:

v4: Moved call to dmi_scan_machine() to separate core_initcall(), so that it is
    called unconditionally, i.e., even if UEFI fails to initialize. Otherwise,
    any drivers that attempt to consult DMI info for quirks handling start
    spewing errors, as Catalin unfortunately found out after merging (and
    subsequently reverting) this patch into arm64 for-next/core for the second
    time.

v3: Moved call to dmi_scan_machine() into arm64_enter_virtual_mode(). This is
    to ensure that it is called early enough, because dmi_scan_machine() needs
    to be called before dmi_id_init(), which itself is invoked using an
    arch_initcall(). DMI depends on UEFI on arm64, so it is legal to only invoke
    dmi_scan_machine() when building with UEFI support. However, calling it from
    arm64_enter_virtual_mode() was a mistake, as it could result in
    dmi_scan_machine() not being called at all, resulting in the issues found
    by Catalin.

v2: Use efi_lookup_mapped_addr() to obtain the virtual address of the SMBIOS
    structure table instead of calling ioremap_cache(). This seemed a good idea
    at the time, as the UEFI memory map covers those regions, so the virtual
    mapping should be known as well. However, this is only true if the firmware
    has requested a virtual remapping for the region by setting the
    EFI_MEMORY_RUNTIME bit, which Tianocore/EDK2 appears to do, but violates
    the UEFI spec. ("In general, UEFI Configuration Tables loaded at boot time
    (e.g., SMBIOS table) can be contained in memory of type
    EfiRuntimeServicesData (recommended and the system firmware must not request
    a virtual mapping), [...]", section 2.3.6, UEFI spec v2.4B). This version
    was merged into the arm64 for-next/core branch and reverted again later per
    our request.
---
 arch/arm64/Kconfig           | 11 +++++++++++
 arch/arm64/include/asm/dmi.h | 31 +++++++++++++++++++++++++++++++
 arch/arm64/kernel/efi.c      | 13 +++++++++++++
 3 files changed, 55 insertions(+)
 create mode 100644 arch/arm64/include/asm/dmi.h

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index ac9afde76dea..211401e8a1d5 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -400,6 +400,17 @@ config EFI
 	  allow the kernel to be booted as an EFI application. This
 	  is only useful on systems that have UEFI firmware.
 
+config DMI
+	bool "Enable support for SMBIOS (DMI) tables"
+	depends on EFI
+	default y
+	help
+	  This enables SMBIOS/DMI feature for systems.
+
+	  This option is only useful on systems that have UEFI firmware.
+	  However, even with this option, the resultant kernel should
+	  continue to boot on existing non-UEFI platforms.
+
 endmenu
 
 menu "Userspace binary formats"
diff --git a/arch/arm64/include/asm/dmi.h b/arch/arm64/include/asm/dmi.h
new file mode 100644
index 000000000000..69d37d87b159
--- /dev/null
+++ b/arch/arm64/include/asm/dmi.h
@@ -0,0 +1,31 @@
+/*
+ * arch/arm64/include/asm/dmi.h
+ *
+ * Copyright (C) 2013 Linaro Limited.
+ * Written by: Yi Li (yi.li-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org)
+ *
+ * based on arch/ia64/include/asm/dmi.h
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+
+#ifndef __ASM_DMI_H
+#define __ASM_DMI_H
+
+#include <linux/io.h>
+#include <linux/slab.h>
+
+/*
+ * According to section 2.3.6 of the UEFI spec, the firmware should not
+ * request a virtual mapping for configuration tables such as SMBIOS.
+ * This means we have to map them before use.
+ */
+#define dmi_early_remap(x, l)		ioremap_cache(x, l)
+#define dmi_early_unmap(x, l)		iounmap(x)
+#define dmi_remap(x, l)			ioremap_cache(x, l)
+#define dmi_unmap(x)			iounmap(x)
+#define dmi_alloc(l)			kzalloc(l, GFP_KERNEL)
+
+#endif
diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 4cec21b1ecdd..0e9da0067ef2 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -11,6 +11,7 @@
  *
  */
 
+#include <linux/dmi.h>
 #include <linux/efi.h>
 #include <linux/export.h>
 #include <linux/memblock.h>
@@ -467,3 +468,15 @@ err_unmap:
 	return -1;
 }
 early_initcall(arm64_enter_virtual_mode);
+
+static int __init arm64_dmi_init(void)
+{
+	/*
+	 * On arm64, DMI depends on UEFI, and dmi_scan_machine() needs to
+	 * be called early because dmi_id_init(), which is an arch_initcall
+	 * itself, depends on dmi_scan_machine() having been called already.
+	 */
+	dmi_scan_machine();
+	return 0;
+}
+core_initcall(arm64_dmi_init);
-- 
1.8.3.2

[PATCH 10/10] arm64: dmi: set DMI string as dump stack arch description

[Ard Biesheuvel]

This sets the DMI string, containing system type, serial number,
firmware version etc. as dump stack arch description, so that oopses
and other kernel stack dumps automatically have this information
included, if available.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 arch/arm64/kernel/efi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 0e9da0067ef2..baab9344a32b 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -477,6 +477,8 @@ static int __init arm64_dmi_init(void)
 	 * itself, depends on dmi_scan_machine() having been called already.
 	 */
 	dmi_scan_machine();
+	if (dmi_available)
+		dmi_set_dump_stack_arch_desc();
 	return 0;
 }
 core_initcall(arm64_dmi_init);
-- 
1.8.3.2

Re: [PATCH 10/10] arm64: dmi: set DMI string as dump stack arch description

[Will Deacon]

On Wed, Oct 22, 2014 at 03:21:53PM +0100, Ard Biesheuvel wrote:
> This sets the DMI string, containing system type, serial number,
> firmware version etc. as dump stack arch description, so that oopses
> and other kernel stack dumps automatically have this information
> included, if available.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  arch/arm64/kernel/efi.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index 0e9da0067ef2..baab9344a32b 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -477,6 +477,8 @@ static int __init arm64_dmi_init(void)
>  	 * itself, depends on dmi_scan_machine() having been called already.
>  	 */
>  	dmi_scan_machine();
> +	if (dmi_available)
> +		dmi_set_dump_stack_arch_desc();

This looks fine, but I don't understand why you would ever *not* want to
call this when DMI is available. In other words, why can't this be part
of dmi_scan_machine?

Will

Re: [PATCH 10/10] arm64: dmi: set DMI string as dump stack arch description

[Ard Biesheuvel]

On 27 October 2014 13:24, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> wrote:
> On Wed, Oct 22, 2014 at 03:21:53PM +0100, Ard Biesheuvel wrote:
>> This sets the DMI string, containing system type, serial number,
>> firmware version etc. as dump stack arch description, so that oopses
>> and other kernel stack dumps automatically have this information
>> included, if available.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>>  arch/arm64/kernel/efi.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
>> index 0e9da0067ef2..baab9344a32b 100644
>> --- a/arch/arm64/kernel/efi.c
>> +++ b/arch/arm64/kernel/efi.c
>> @@ -477,6 +477,8 @@ static int __init arm64_dmi_init(void)
>>        * itself, depends on dmi_scan_machine() having been called already.
>>        */
>>       dmi_scan_machine();
>> +     if (dmi_available)
>> +             dmi_set_dump_stack_arch_desc();
>
> This looks fine, but I don't understand why you would ever *not* want to
> call this when DMI is available. In other words, why can't this be part
> of dmi_scan_machine?
>

/**
 * dmi_set_dump_stack_arch_desc - set arch description for dump_stack()
 *
 * Invoke dump_stack_set_arch_desc() with DMI system information so that
 * DMI identifiers are printed out on task dumps.  Arch boot code should
 * call this function after dmi_scan_machine() if it wants to print out DMI
 * identifiers on task dumps.
 */

Apparently, the author of the function had reason to assume that not
all architectures that implement DMI would choose to use the DMI
string as arch description even if it is available.

Currently, ia64 and x86 call this function unconditionally, which
means the arch description is cleared if it was set before, and moving
the call to dmi_scan_machine() should retain the same behavior. For
the arm64 case, I think calling it conditionally is better: if we ever
want to populate the arch description from DT, the unconditional call
would clear its contents even on non-DMI systems.

-- 
Ard.

Re: [PATCH 00/10] arm64 EFI patches for 3.19

[Will Deacon]

Hi Ard,

On Wed, Oct 22, 2014 at 03:21:43PM +0100, Ard Biesheuvel wrote:
> This is a bit of a mixed bag of patches that we would like to see merged for
> 3.19. Most of them have been posted and discussed on linux-efi and/or LAKML
> before, and one of them has even been merged and reverted twice.
> 
> Patches #1 - #4 are fixes for compliance with the UEFI and PE/COFF specs.
> No issues are known that require these patches, so there is no reason to
> pull them into a stable release.
> 
> Patches #5 and #6 address minor issues in the arm64 EFI init code.
> 
> Patches #7 - #10 implement DMI/SMBIOS for arm64, both the existing 32-bit
> version and the upcoming 3.0 version that allows the SMBIOS structure table
> to reside at a physical offset that cannot be encoded in 32-bits. It also
> install a 'Hardware: xxx' string that is printed along with oopses and
> kernel call stack dumps on systems that implement DMI/SMBIOS.
> 
> Please refer to the patches themselves for version history. Acks and/or
> comments appreciated.

It would be helpful for me if you could split this into two series -- one
for the arm64 tree and the other for the efi tree. That would also make the
dependencies clearer, so I know whether any of the arch bits need to go via
Matt.

Cheers,

Will

Re: [PATCH 00/10] arm64 EFI patches for 3.19

[Ard Biesheuvel]

On 27 October 2014 12:50, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> wrote:
> Hi Ard,
>
> On Wed, Oct 22, 2014 at 03:21:43PM +0100, Ard Biesheuvel wrote:
>> This is a bit of a mixed bag of patches that we would like to see merged for
>> 3.19. Most of them have been posted and discussed on linux-efi and/or LAKML
>> before, and one of them has even been merged and reverted twice.
>>
>> Patches #1 - #4 are fixes for compliance with the UEFI and PE/COFF specs.
>> No issues are known that require these patches, so there is no reason to
>> pull them into a stable release.
>>
>> Patches #5 and #6 address minor issues in the arm64 EFI init code.
>>
>> Patches #7 - #10 implement DMI/SMBIOS for arm64, both the existing 32-bit
>> version and the upcoming 3.0 version that allows the SMBIOS structure table
>> to reside at a physical offset that cannot be encoded in 32-bits. It also
>> install a 'Hardware: xxx' string that is printed along with oopses and
>> kernel call stack dumps on systems that implement DMI/SMBIOS.
>>
>> Please refer to the patches themselves for version history. Acks and/or
>> comments appreciated.
>
> It would be helpful for me if you could split this into two series -- one
> for the arm64 tree and the other for the efi tree. That would also make the
> dependencies clearer, so I know whether any of the arch bits need to go via
> Matt.
>

Yeah, I was struggling a bit with that. I think the agreement was that
everything EFI related goes through Matt's tree, but I don't think
that necessarily makes sense for patches that only touch arch/arm64,
unless there are interdependencies with the generic code.

>From this series, only patches #7 and #8 need to go through Matt's
tree, and even if #9 and #10 are also related to SMBIOS, they are in
fact orthogonal to #7 and #8, so those can still go through the arm64
tree without any merge order issues later on.

@Matt: any thoughts?

In addition, #4 and #6 are still being discussed, and I haven't
received any acks on #5 and #10.

-- 
Ard.

Re: [PATCH 00/10] arm64 EFI patches for 3.19

[Matt Fleming]

On Mon, 2014-10-27 at 13:03 +0100, Ard Biesheuvel wrote:
> 
> Yeah, I was struggling a bit with that. I think the agreement was that
> everything EFI related goes through Matt's tree, but I don't think
> that necessarily makes sense for patches that only touch arch/arm64,
> unless there are interdependencies with the generic code.
> 
> From this series, only patches #7 and #8 need to go through Matt's
> tree, and even if #9 and #10 are also related to SMBIOS, they are in
> fact orthogonal to #7 and #8, so those can still go through the arm64
> tree without any merge order issues later on.
> 
> @Matt: any thoughts?

In this case, if it makes things easier for you folks in terms of
dependencies to take this *all* through the arm64 tree, that seems
sensible to me.

Re: [PATCH 00/10] arm64 EFI patches for 3.19

[Will Deacon]

On Mon, Oct 27, 2014 at 05:45:27PM +0000, Matt Fleming wrote:
> On Mon, 2014-10-27 at 13:03 +0100, Ard Biesheuvel wrote:
> > 
> > Yeah, I was struggling a bit with that. I think the agreement was that
> > everything EFI related goes through Matt's tree, but I don't think
> > that necessarily makes sense for patches that only touch arch/arm64,
> > unless there are interdependencies with the generic code.
> > 
> > From this series, only patches #7 and #8 need to go through Matt's
> > tree, and even if #9 and #10 are also related to SMBIOS, they are in
> > fact orthogonal to #7 and #8, so those can still go through the arm64
> > tree without any merge order issues later on.
> > 
> > @Matt: any thoughts?
> 
> In this case, if it makes things easier for you folks in terms of
> dependencies to take this *all* through the arm64 tree, that seems
> sensible to me.

Thanks, Matt. I won't merge anything that touches files outside of
arch/arm64 without your ack, but then I'll be happy to take this lot via
the arm64 tree.

Will