linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [GIT PULL 00/13] EFI changes for v4.6 part 2
@ 2016-02-17 12:35 Matt Fleming
  2016-02-17 12:35 ` [PATCH 01/13] efi: Reformat GUID tables to follow the format in UEFI spec Matt Fleming
                   ` (12 more replies)
  0 siblings, 13 replies; 54+ messages in thread
From: Matt Fleming @ 2016-02-17 12:35 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner, H . Peter Anvin
  Cc: Ard Biesheuvel, Matt Fleming, linux-kernel, linux-efi,
	Andy Lutomirski, Borislav Petkov, Jeremy Linton, Lee, Chun-Yi,
	Linus Torvalds, Mark Rutland, Peter Jones, Peter Zijlstra,
	Ravi Shankar, Ricardo Neri, Sai Praneeth Prakhya,
	Suzuki K Poulose, Will Deacon

Folks, here are the remaining EFI changes for v4.6.

Two of the patches were sent in the previous v4.6 pull request on 1st
February. Those include running EFI runtime services with interrupts
enabled (just like Windows does) and aligning the EFI GUID formats in
include/linux/efi.h to match the way they're written in the UEFI spec.
They've both been reworked to address comments.

The rest of the changes are all over the tree, and range from simple
fixes and cleanups to support for EFI_PROPERTIES_TABLE on x86.

The following changes since commit 35575e0e8ba633fc8276509a21f89b599b4f9006:

  efi: Add Persistent Memory type name (2016-02-03 11:41:20 +0100)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git tags/efi-next

for you to fetch changes up to 83a901db98e790e5682b85b8ee85f9e545d84e27:

  x86/efi: Only map kernel text for EFI mixed mode (2016-02-15 11:37:02 +0000)

----------------------------------------------------------------
 * checkpatch.pl cleanup of the GUIDs in efi.h which has the added
   benefit of making them more closely resemble how they're presented
   in the UEFI specification - Peter Jones

 * Now that we've verified that Windows works this way, leave
   interrupts enabled when invoking EFI runtime services which
   also reduces interrupt latencies - Ard Biesheuvel

 * Reduce page table attribute inconsistencies between the EFI page
   tables and the standard kernel page tables by ensuring we also set
   _PAGE_GLOBAL in the EFI-specific paths - Sai Praneeth Prakhya

 * A bunch of small fixes to the generic EFI stub and some early boot
   platform compatibility checks for ARM and arm64 - Ard Biesheuvel

 * Add support for EFI_PROPERTIES_TABLE to x86, allowing us to apply
   more secure memory mapping permissions for firmware that ships with
   the feature enabled - Sai Praneeth Prakhya

 * Fix an EFI mixed mode bug where we intend to only map the kernel
   image's text but end up mapping the entire image - Sai Praneeth Prakhya

----------------------------------------------------------------
Ard Biesheuvel (8):
      efi/runtime-wrappers: Run UEFI Runtime Services with interrupts enabled
      efi/arm64: Drop __init annotation from handle_kernel_image()
      arm64: vmlinux.lds.S: Handle .init.rodata.xxx and .init.bss sections
      efi/efistub: Prevent __init annotations from being used
      efi/arm-init: Use read-only early mappings
      efi/arm: Check for LPAE support before booting a LPAE kernel
      efi/arm64: Check for h/w support before booting a >4 KB granule kernel
      efi/arm*: Perform hardware compatibility check

Peter Jones (1):
      efi: Reformat GUID tables to follow the format in UEFI spec

Sai Praneeth (4):
      x86/mm/pageattr: Use _PAGE_GLOBAL bit for EFI page table mappings
      x86/mm/pageattr: Don't implicitly allow _PAGE_RW in kernel_map_pages_in_pgd()
      x86/efi: Map EFI_MEMORY_{XP,RO} memory region bits to EFI page tables
      x86/efi: Only map kernel text for EFI mixed mode

 arch/arm64/kernel/vmlinux.lds.S           |  1 +
 arch/x86/include/asm/efi.h                |  2 +-
 arch/x86/mm/pageattr.c                    | 17 ++++++++
 arch/x86/platform/efi/efi.c               |  9 +++-
 arch/x86/platform/efi/efi_32.c            |  2 +-
 arch/x86/platform/efi/efi_64.c            | 55 ++++++++++++++++++++----
 drivers/firmware/efi/arm-init.c           | 14 +++---
 drivers/firmware/efi/libstub/arm-stub.c   |  4 ++
 drivers/firmware/efi/libstub/arm32-stub.c | 17 ++++++++
 drivers/firmware/efi/libstub/arm64-stub.c | 34 ++++++++++++---
 drivers/firmware/efi/libstub/efistub.h    | 12 ++++++
 drivers/firmware/efi/runtime-wrappers.c   | 71 ++++++++++++-------------------
 include/linux/efi.h                       | 63 ++++++++++++++++++---------
 13 files changed, 210 insertions(+), 91 deletions(-)

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH 01/13] efi: Reformat GUID tables to follow the format in UEFI spec
  2016-02-17 12:35 [GIT PULL 00/13] EFI changes for v4.6 part 2 Matt Fleming
@ 2016-02-17 12:35 ` Matt Fleming
  2016-02-22 12:12   ` [tip:efi/core] " =?UTF-8?B?dGlwLWJvdCBmb3IgUGV0ZXIgSm9uZXMgPHRpcGJvdEB6eXRvci5jb20+?=
  2016-02-17 12:35 ` [PATCH 02/13] efi/runtime-wrappers: Run UEFI Runtime Services with interrupts enabled Matt Fleming
                   ` (11 subsequent siblings)
  12 siblings, 1 reply; 54+ messages in thread
From: Matt Fleming @ 2016-02-17 12:35 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner, H . Peter Anvin
  Cc: Ard Biesheuvel, Peter Jones, linux-kernel, linux-efi, Matt Fleming

From: Peter Jones <pjones@redhat.com>

This makes it much easier to hunt for typos in the GUID definitions.

It also makes checkpatch complain less about efi.h GUID additions, so
that if you add another one with the same style, checkpatch won't
complain about it.

Signed-off-by: Peter Jones <pjones@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
---
 include/linux/efi.h | 63 +++++++++++++++++++++++++++++++++++------------------
 1 file changed, 42 insertions(+), 21 deletions(-)

diff --git a/include/linux/efi.h b/include/linux/efi.h
index 3c6cbbdae4aa..99b88c5a6770 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -536,67 +536,88 @@ void efi_native_runtime_setup(void);
  *  EFI Configuration Table and GUID definitions
  */
 #define NULL_GUID \
-    EFI_GUID(  0x00000000, 0x0000, 0x0000, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 )
+	EFI_GUID(0x00000000, 0x0000, 0x0000, \
+		 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00)
 
 #define MPS_TABLE_GUID    \
-    EFI_GUID(  0xeb9d2d2f, 0x2d88, 0x11d3, 0x9a, 0x16, 0x0, 0x90, 0x27, 0x3f, 0xc1, 0x4d )
+	EFI_GUID(0xeb9d2d2f, 0x2d88, 0x11d3, \
+		 0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
 
 #define ACPI_TABLE_GUID    \
-    EFI_GUID(  0xeb9d2d30, 0x2d88, 0x11d3, 0x9a, 0x16, 0x0, 0x90, 0x27, 0x3f, 0xc1, 0x4d )
+	EFI_GUID(0xeb9d2d30, 0x2d88, 0x11d3, \
+		 0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
 
 #define ACPI_20_TABLE_GUID    \
-    EFI_GUID(  0x8868e871, 0xe4f1, 0x11d3, 0xbc, 0x22, 0x0, 0x80, 0xc7, 0x3c, 0x88, 0x81 )
+	EFI_GUID(0x8868e871, 0xe4f1, 0x11d3, \
+		 0xbc, 0x22, 0x00, 0x80, 0xc7, 0x3c, 0x88, 0x81)
 
 #define SMBIOS_TABLE_GUID    \
-    EFI_GUID(  0xeb9d2d31, 0x2d88, 0x11d3, 0x9a, 0x16, 0x0, 0x90, 0x27, 0x3f, 0xc1, 0x4d )
+	EFI_GUID(0xeb9d2d31, 0x2d88, 0x11d3, \
+		 0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
 
 #define SMBIOS3_TABLE_GUID    \
-    EFI_GUID(  0xf2fd1544, 0x9794, 0x4a2c, 0x99, 0x2e, 0xe5, 0xbb, 0xcf, 0x20, 0xe3, 0x94 )
+	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 )
+	EFI_GUID(0xeb9d2d32, 0x2d88, 0x11d3, \
+		 0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
 
 #define HCDP_TABLE_GUID	\
-    EFI_GUID(  0xf951938d, 0x620b, 0x42ef, 0x82, 0x79, 0xa8, 0x4b, 0x79, 0x61, 0x78, 0x98 )
+	EFI_GUID(0xf951938d, 0x620b, 0x42ef, \
+		 0x82, 0x79, 0xa8, 0x4b, 0x79, 0x61, 0x78, 0x98)
 
 #define UGA_IO_PROTOCOL_GUID \
-    EFI_GUID(  0x61a4d49e, 0x6f68, 0x4f1b, 0xb9, 0x22, 0xa8, 0x6e, 0xed, 0xb, 0x7, 0xa2 )
+	EFI_GUID(0x61a4d49e, 0x6f68, 0x4f1b, \
+		 0xb9, 0x22, 0xa8, 0x6e, 0xed, 0x0b, 0x07, 0xa2)
 
 #define EFI_GLOBAL_VARIABLE_GUID \
-    EFI_GUID(  0x8be4df61, 0x93ca, 0x11d2, 0xaa, 0x0d, 0x00, 0xe0, 0x98, 0x03, 0x2b, 0x8c )
+	EFI_GUID(0x8be4df61, 0x93ca, 0x11d2, \
+		 0xaa, 0x0d, 0x00, 0xe0, 0x98, 0x03, 0x2b, 0x8c)
 
 #define UV_SYSTEM_TABLE_GUID \
-    EFI_GUID(  0x3b13a7d4, 0x633e, 0x11dd, 0x93, 0xec, 0xda, 0x25, 0x56, 0xd8, 0x95, 0x93 )
+	EFI_GUID(0x3b13a7d4, 0x633e, 0x11dd, \
+		 0x93, 0xec, 0xda, 0x25, 0x56, 0xd8, 0x95, 0x93)
 
 #define LINUX_EFI_CRASH_GUID \
-    EFI_GUID(  0xcfc8fc79, 0xbe2e, 0x4ddc, 0x97, 0xf0, 0x9f, 0x98, 0xbf, 0xe2, 0x98, 0xa0 )
+	EFI_GUID(0xcfc8fc79, 0xbe2e, 0x4ddc, \
+		 0x97, 0xf0, 0x9f, 0x98, 0xbf, 0xe2, 0x98, 0xa0)
 
 #define LOADED_IMAGE_PROTOCOL_GUID \
-    EFI_GUID(  0x5b1b31a1, 0x9562, 0x11d2, 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b )
+	EFI_GUID(0x5b1b31a1, 0x9562, 0x11d2, \
+		 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
 
 #define EFI_GRAPHICS_OUTPUT_PROTOCOL_GUID \
-    EFI_GUID(  0x9042a9de, 0x23dc, 0x4a38, 0x96, 0xfb, 0x7a, 0xde, 0xd0, 0x80, 0x51, 0x6a )
+	EFI_GUID(0x9042a9de, 0x23dc, 0x4a38, \
+		 0x96, 0xfb, 0x7a, 0xde, 0xd0, 0x80, 0x51, 0x6a)
 
 #define EFI_UGA_PROTOCOL_GUID \
-    EFI_GUID(  0x982c298b, 0xf4fa, 0x41cb, 0xb8, 0x38, 0x77, 0xaa, 0x68, 0x8f, 0xb8, 0x39 )
+	EFI_GUID(0x982c298b, 0xf4fa, 0x41cb, \
+		 0xb8, 0x38, 0x77, 0xaa, 0x68, 0x8f, 0xb8, 0x39)
 
 #define EFI_PCI_IO_PROTOCOL_GUID \
-    EFI_GUID(  0x4cf5b200, 0x68b8, 0x4ca5, 0x9e, 0xec, 0xb2, 0x3e, 0x3f, 0x50, 0x2, 0x9a )
+	EFI_GUID(0x4cf5b200, 0x68b8, 0x4ca5, \
+		 0x9e, 0xec, 0xb2, 0x3e, 0x3f, 0x50, 0x02, 0x9a)
 
 #define EFI_FILE_INFO_ID \
-    EFI_GUID(  0x9576e92, 0x6d3f, 0x11d2, 0x8e, 0x39, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b )
+	EFI_GUID(0x9576e92, 0x6d3f, 0x11d2, \
+		 0x8e, 0x39, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
 
 #define EFI_SYSTEM_RESOURCE_TABLE_GUID \
-    EFI_GUID(  0xb122a263, 0x3661, 0x4f68, 0x99, 0x29, 0x78, 0xf8, 0xb0, 0xd6, 0x21, 0x80 )
+	EFI_GUID(0xb122a263, 0x3661, 0x4f68, \
+		 0x99, 0x29, 0x78, 0xf8, 0xb0, 0xd6, 0x21, 0x80)
 
 #define EFI_FILE_SYSTEM_GUID \
-    EFI_GUID(  0x964e5b22, 0x6459, 0x11d2, 0x8e, 0x39, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b )
+	EFI_GUID(0x964e5b22, 0x6459, 0x11d2, \
+		 0x8e, 0x39, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
 
 #define DEVICE_TREE_GUID \
-    EFI_GUID(  0xb1b621d5, 0xf19c, 0x41a5, 0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0 )
+	EFI_GUID(0xb1b621d5, 0xf19c, 0x41a5, \
+		 0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0)
 
 #define EFI_PROPERTIES_TABLE_GUID \
-    EFI_GUID(  0x880aaca3, 0x4adc, 0x4a04, 0x90, 0x79, 0xb7, 0x47, 0x34, 0x08, 0x25, 0xe5 )
+	EFI_GUID(0x880aaca3, 0x4adc, 0x4a04, \
+		 0x90, 0x79, 0xb7, 0x47, 0x34, 0x08, 0x25, 0xe5)
 
 typedef struct {
 	efi_guid_t guid;
-- 
2.6.2

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH 02/13] efi/runtime-wrappers: Run UEFI Runtime Services with interrupts enabled
  2016-02-17 12:35 [GIT PULL 00/13] EFI changes for v4.6 part 2 Matt Fleming
  2016-02-17 12:35 ` [PATCH 01/13] efi: Reformat GUID tables to follow the format in UEFI spec Matt Fleming
@ 2016-02-17 12:35 ` Matt Fleming
  2016-02-22 12:13   ` [tip:efi/core] " =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?=
  2016-02-17 12:35 ` [PATCH 03/13] x86/mm/pageattr: Use _PAGE_GLOBAL bit for EFI page table mappings Matt Fleming
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 54+ messages in thread
From: Matt Fleming @ 2016-02-17 12:35 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner, H . Peter Anvin
  Cc: Ard Biesheuvel, linux-kernel, linux-efi, Andy Lutomirski,
	Linus Torvalds, Sai Praneeth Prakhya, Peter Zijlstra,
	Matt Fleming

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

The UEFI spec allows Runtime Services to be invoked with interrupts
enabled. The only reason we were disabling interrupts was to prevent
recursive calls into the services on the same CPU, which will lead to
deadlock. However, the only context where such invocations may occur
legally is from efi-pstore via efivars, and that code has been updated
to call a non-blocking alternative when invoked from a non-interruptible
context.

So instead, update the ordinary, blocking UEFI Runtime Services wrappers
to execute with interrupts enabled. This aims to prevent excessive interrupt
latencies on uniprocessor platforms with slow variable stores.

Note that other OSes such as Windows call UEFI Runtime Services with
interrupts enabled as well.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
---
 drivers/firmware/efi/runtime-wrappers.c | 71 +++++++++++++--------------------
 1 file changed, 28 insertions(+), 43 deletions(-)

diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c
index 7b8b2f2702ca..de6953039af6 100644
--- a/drivers/firmware/efi/runtime-wrappers.c
+++ b/drivers/firmware/efi/runtime-wrappers.c
@@ -63,23 +63,21 @@ static DEFINE_SPINLOCK(efi_runtime_lock);
 
 static efi_status_t virt_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc)
 {
-	unsigned long flags;
 	efi_status_t status;
 
-	spin_lock_irqsave(&efi_runtime_lock, flags);
+	spin_lock(&efi_runtime_lock);
 	status = efi_call_virt(get_time, tm, tc);
-	spin_unlock_irqrestore(&efi_runtime_lock, flags);
+	spin_unlock(&efi_runtime_lock);
 	return status;
 }
 
 static efi_status_t virt_efi_set_time(efi_time_t *tm)
 {
-	unsigned long flags;
 	efi_status_t status;
 
-	spin_lock_irqsave(&efi_runtime_lock, flags);
+	spin_lock(&efi_runtime_lock);
 	status = efi_call_virt(set_time, tm);
-	spin_unlock_irqrestore(&efi_runtime_lock, flags);
+	spin_unlock(&efi_runtime_lock);
 	return status;
 }
 
@@ -87,23 +85,21 @@ static efi_status_t virt_efi_get_wakeup_time(efi_bool_t *enabled,
 					     efi_bool_t *pending,
 					     efi_time_t *tm)
 {
-	unsigned long flags;
 	efi_status_t status;
 
-	spin_lock_irqsave(&efi_runtime_lock, flags);
+	spin_lock(&efi_runtime_lock);
 	status = efi_call_virt(get_wakeup_time, enabled, pending, tm);
-	spin_unlock_irqrestore(&efi_runtime_lock, flags);
+	spin_unlock(&efi_runtime_lock);
 	return status;
 }
 
 static efi_status_t virt_efi_set_wakeup_time(efi_bool_t enabled, efi_time_t *tm)
 {
-	unsigned long flags;
 	efi_status_t status;
 
-	spin_lock_irqsave(&efi_runtime_lock, flags);
+	spin_lock(&efi_runtime_lock);
 	status = efi_call_virt(set_wakeup_time, enabled, tm);
-	spin_unlock_irqrestore(&efi_runtime_lock, flags);
+	spin_unlock(&efi_runtime_lock);
 	return status;
 }
 
@@ -113,13 +109,12 @@ static efi_status_t virt_efi_get_variable(efi_char16_t *name,
 					  unsigned long *data_size,
 					  void *data)
 {
-	unsigned long flags;
 	efi_status_t status;
 
-	spin_lock_irqsave(&efi_runtime_lock, flags);
+	spin_lock(&efi_runtime_lock);
 	status = efi_call_virt(get_variable, name, vendor, attr, data_size,
 			       data);
-	spin_unlock_irqrestore(&efi_runtime_lock, flags);
+	spin_unlock(&efi_runtime_lock);
 	return status;
 }
 
@@ -127,12 +122,11 @@ static efi_status_t virt_efi_get_next_variable(unsigned long *name_size,
 					       efi_char16_t *name,
 					       efi_guid_t *vendor)
 {
-	unsigned long flags;
 	efi_status_t status;
 
-	spin_lock_irqsave(&efi_runtime_lock, flags);
+	spin_lock(&efi_runtime_lock);
 	status = efi_call_virt(get_next_variable, name_size, name, vendor);
-	spin_unlock_irqrestore(&efi_runtime_lock, flags);
+	spin_unlock(&efi_runtime_lock);
 	return status;
 }
 
@@ -142,13 +136,12 @@ static efi_status_t virt_efi_set_variable(efi_char16_t *name,
 					  unsigned long data_size,
 					  void *data)
 {
-	unsigned long flags;
 	efi_status_t status;
 
-	spin_lock_irqsave(&efi_runtime_lock, flags);
+	spin_lock(&efi_runtime_lock);
 	status = efi_call_virt(set_variable, name, vendor, attr, data_size,
 			       data);
-	spin_unlock_irqrestore(&efi_runtime_lock, flags);
+	spin_unlock(&efi_runtime_lock);
 	return status;
 }
 
@@ -157,15 +150,14 @@ virt_efi_set_variable_nonblocking(efi_char16_t *name, efi_guid_t *vendor,
 				  u32 attr, unsigned long data_size,
 				  void *data)
 {
-	unsigned long flags;
 	efi_status_t status;
 
-	if (!spin_trylock_irqsave(&efi_runtime_lock, flags))
+	if (!spin_trylock(&efi_runtime_lock))
 		return EFI_NOT_READY;
 
 	status = efi_call_virt(set_variable, name, vendor, attr, data_size,
 			       data);
-	spin_unlock_irqrestore(&efi_runtime_lock, flags);
+	spin_unlock(&efi_runtime_lock);
 	return status;
 }
 
@@ -175,16 +167,15 @@ static efi_status_t virt_efi_query_variable_info(u32 attr,
 						 u64 *remaining_space,
 						 u64 *max_variable_size)
 {
-	unsigned long flags;
 	efi_status_t status;
 
 	if (efi.runtime_version < EFI_2_00_SYSTEM_TABLE_REVISION)
 		return EFI_UNSUPPORTED;
 
-	spin_lock_irqsave(&efi_runtime_lock, flags);
+	spin_lock(&efi_runtime_lock);
 	status = efi_call_virt(query_variable_info, attr, storage_space,
 			       remaining_space, max_variable_size);
-	spin_unlock_irqrestore(&efi_runtime_lock, flags);
+	spin_unlock(&efi_runtime_lock);
 	return status;
 }
 
@@ -194,29 +185,27 @@ virt_efi_query_variable_info_nonblocking(u32 attr,
 					 u64 *remaining_space,
 					 u64 *max_variable_size)
 {
-	unsigned long flags;
 	efi_status_t status;
 
 	if (efi.runtime_version < EFI_2_00_SYSTEM_TABLE_REVISION)
 		return EFI_UNSUPPORTED;
 
-	if (!spin_trylock_irqsave(&efi_runtime_lock, flags))
+	if (!spin_trylock(&efi_runtime_lock))
 		return EFI_NOT_READY;
 
 	status = efi_call_virt(query_variable_info, attr, storage_space,
 			       remaining_space, max_variable_size);
-	spin_unlock_irqrestore(&efi_runtime_lock, flags);
+	spin_unlock(&efi_runtime_lock);
 	return status;
 }
 
 static efi_status_t virt_efi_get_next_high_mono_count(u32 *count)
 {
-	unsigned long flags;
 	efi_status_t status;
 
-	spin_lock_irqsave(&efi_runtime_lock, flags);
+	spin_lock(&efi_runtime_lock);
 	status = efi_call_virt(get_next_high_mono_count, count);
-	spin_unlock_irqrestore(&efi_runtime_lock, flags);
+	spin_unlock(&efi_runtime_lock);
 	return status;
 }
 
@@ -225,26 +214,23 @@ static void virt_efi_reset_system(int reset_type,
 				  unsigned long data_size,
 				  efi_char16_t *data)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&efi_runtime_lock, flags);
+	spin_lock(&efi_runtime_lock);
 	__efi_call_virt(reset_system, reset_type, status, data_size, data);
-	spin_unlock_irqrestore(&efi_runtime_lock, flags);
+	spin_unlock(&efi_runtime_lock);
 }
 
 static efi_status_t virt_efi_update_capsule(efi_capsule_header_t **capsules,
 					    unsigned long count,
 					    unsigned long sg_list)
 {
-	unsigned long flags;
 	efi_status_t status;
 
 	if (efi.runtime_version < EFI_2_00_SYSTEM_TABLE_REVISION)
 		return EFI_UNSUPPORTED;
 
-	spin_lock_irqsave(&efi_runtime_lock, flags);
+	spin_lock(&efi_runtime_lock);
 	status = efi_call_virt(update_capsule, capsules, count, sg_list);
-	spin_unlock_irqrestore(&efi_runtime_lock, flags);
+	spin_unlock(&efi_runtime_lock);
 	return status;
 }
 
@@ -253,16 +239,15 @@ static efi_status_t virt_efi_query_capsule_caps(efi_capsule_header_t **capsules,
 						u64 *max_size,
 						int *reset_type)
 {
-	unsigned long flags;
 	efi_status_t status;
 
 	if (efi.runtime_version < EFI_2_00_SYSTEM_TABLE_REVISION)
 		return EFI_UNSUPPORTED;
 
-	spin_lock_irqsave(&efi_runtime_lock, flags);
+	spin_lock(&efi_runtime_lock);
 	status = efi_call_virt(query_capsule_caps, capsules, count, max_size,
 			       reset_type);
-	spin_unlock_irqrestore(&efi_runtime_lock, flags);
+	spin_unlock(&efi_runtime_lock);
 	return status;
 }
 
-- 
2.6.2

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH 03/13] x86/mm/pageattr: Use _PAGE_GLOBAL bit for EFI page table mappings
  2016-02-17 12:35 [GIT PULL 00/13] EFI changes for v4.6 part 2 Matt Fleming
  2016-02-17 12:35 ` [PATCH 01/13] efi: Reformat GUID tables to follow the format in UEFI spec Matt Fleming
  2016-02-17 12:35 ` [PATCH 02/13] efi/runtime-wrappers: Run UEFI Runtime Services with interrupts enabled Matt Fleming
@ 2016-02-17 12:35 ` Matt Fleming
  2016-02-22 12:13   ` [tip:efi/core] x86/mm/pat: " =?UTF-8?B?dGlwLWJvdCBmb3IgU2FpIFByYW5lZXRoIDx0aXBib3RAenl0b3IuY29tPg==?=
  2016-02-17 12:35 ` [PATCH 04/13] efi/arm64: Drop __init annotation from handle_kernel_image() Matt Fleming
                   ` (9 subsequent siblings)
  12 siblings, 1 reply; 54+ messages in thread
From: Matt Fleming @ 2016-02-17 12:35 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner, H . Peter Anvin
  Cc: Ard Biesheuvel, Sai Praneeth, linux-kernel, linux-efi,
	Ricardo Neri, Ravi Shankar, Borislav Petkov, Matt Fleming

From: Sai Praneeth <sai.praneeth.prakhya@intel.com>

Since EFI page tables can be treated as kernel page tables they should
be global. All the other page mapping functions in pageattr.c set the
_PAGE_GLOBAL bit and we want to avoid inconsistencies when we map a page
in the EFI code paths, for example when that page is split in
__split_large_page(), etc. It also makes it easier to validate that the
EFI region mappings have the correct attributes because there are fewer
differences compared with regular kernel mappings.

Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Cc: Ricardo Neri <ricardo.neri@intel.com>
Cc: Ravi Shankar <ravi.v.shankar@intel.com>
Cc: Borislav Petkov <bp@alien8.de>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
---
 arch/x86/mm/pageattr.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index 632d34d20237..bf312da41a6d 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -909,6 +909,20 @@ static void populate_pte(struct cpa_data *cpa,
 
 	pte = pte_offset_kernel(pmd, start);
 
+	/*
+	 * Set the GLOBAL flags only if the PRESENT flag is
+	 * set otherwise pte_present will return true even on
+	 * a non present pte. The canon_pgprot will clear
+	 * _PAGE_GLOBAL for the ancient hardware that doesn't
+	 * support it.
+	 */
+	if (pgprot_val(pgprot) & _PAGE_PRESENT)
+		pgprot_val(pgprot) |= _PAGE_GLOBAL;
+	else
+		pgprot_val(pgprot) &= ~_PAGE_GLOBAL;
+
+	pgprot = canon_pgprot(pgprot);
+
 	while (num_pages-- && start < end) {
 		set_pte(pte, pfn_pte(cpa->pfn, pgprot));
 
-- 
2.6.2

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH 04/13] efi/arm64: Drop __init annotation from handle_kernel_image()
  2016-02-17 12:35 [GIT PULL 00/13] EFI changes for v4.6 part 2 Matt Fleming
                   ` (2 preceding siblings ...)
  2016-02-17 12:35 ` [PATCH 03/13] x86/mm/pageattr: Use _PAGE_GLOBAL bit for EFI page table mappings Matt Fleming
@ 2016-02-17 12:35 ` Matt Fleming
  2016-02-22 12:14   ` [tip:efi/core] " =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?=
  2016-02-17 12:35 ` [PATCH 05/13] arm64: vmlinux.lds.S: Handle .init.rodata.xxx and .init.bss sections Matt Fleming
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 54+ messages in thread
From: Matt Fleming @ 2016-02-17 12:35 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner, H . Peter Anvin
  Cc: Ard Biesheuvel, linux-kernel, linux-efi, Matt Fleming,
	Mark Rutland, Will Deacon

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

After moving arm64-stub.c to libstub/, all of its sections are emitted
as .init.xxx sections automatically, and the __init annotation of
handle_kernel_image() causes it to end up in .init.init.text, which is
not recognized as an __init section by the linker scripts. So drop the
annotation.

Acked-by: Will Deacon <will.deacon@arm.com>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
---
 drivers/firmware/efi/libstub/arm64-stub.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
index 78dfbd34b6bf..9e0342745e4f 100644
--- a/drivers/firmware/efi/libstub/arm64-stub.c
+++ b/drivers/firmware/efi/libstub/arm64-stub.c
@@ -13,13 +13,13 @@
 #include <asm/efi.h>
 #include <asm/sections.h>
 
-efi_status_t __init handle_kernel_image(efi_system_table_t *sys_table_arg,
-					unsigned long *image_addr,
-					unsigned long *image_size,
-					unsigned long *reserve_addr,
-					unsigned long *reserve_size,
-					unsigned long dram_base,
-					efi_loaded_image_t *image)
+efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
+				 unsigned long *image_addr,
+				 unsigned long *image_size,
+				 unsigned long *reserve_addr,
+				 unsigned long *reserve_size,
+				 unsigned long dram_base,
+				 efi_loaded_image_t *image)
 {
 	efi_status_t status;
 	unsigned long kernel_size, kernel_memsize = 0;
-- 
2.6.2

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH 05/13] arm64: vmlinux.lds.S: Handle .init.rodata.xxx and .init.bss sections
  2016-02-17 12:35 [GIT PULL 00/13] EFI changes for v4.6 part 2 Matt Fleming
                   ` (3 preceding siblings ...)
  2016-02-17 12:35 ` [PATCH 04/13] efi/arm64: Drop __init annotation from handle_kernel_image() Matt Fleming
@ 2016-02-17 12:35 ` Matt Fleming
  2016-02-22 12:14   ` [tip:efi/core] arm64/vmlinux.lds.S: " =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?=
  2016-02-17 12:35 ` [PATCH 06/13] efi/efistub: Prevent __init annotations from being used Matt Fleming
                   ` (7 subsequent siblings)
  12 siblings, 1 reply; 54+ messages in thread
From: Matt Fleming @ 2016-02-17 12:35 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner, H . Peter Anvin
  Cc: Ard Biesheuvel, linux-kernel, linux-efi, Matt Fleming,
	Mark Rutland, Will Deacon

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

The EFI stub is typically built into the decompressor (x86, ARM) so none
of its symbols are annotated as __init. However, on arm64, the stub is
linked into the kernel proper, and the code is __init annotated at the
section level by prepending all names of SHF_ALLOC sections with '.init'.

This results in section names like .init.rodata.str1.8 (for string literals)
and .init.bss (which is tiny), both of which can be moved into the .init.data
output section.

Acked-by: Will Deacon <will.deacon@arm.com>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
---
 arch/arm64/kernel/vmlinux.lds.S | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
index e3928f578891..cbf4db440e9c 100644
--- a/arch/arm64/kernel/vmlinux.lds.S
+++ b/arch/arm64/kernel/vmlinux.lds.S
@@ -134,6 +134,7 @@ SECTIONS
 		CON_INITCALL
 		SECURITY_INITCALL
 		INIT_RAM_FS
+		*(.init.rodata.* .init.bss)	/* from the EFI stub */
 	}
 	.exit.data : {
 		ARM_EXIT_KEEP(EXIT_DATA)
-- 
2.6.2

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH 06/13] efi/efistub: Prevent __init annotations from being used
  2016-02-17 12:35 [GIT PULL 00/13] EFI changes for v4.6 part 2 Matt Fleming
                   ` (4 preceding siblings ...)
  2016-02-17 12:35 ` [PATCH 05/13] arm64: vmlinux.lds.S: Handle .init.rodata.xxx and .init.bss sections Matt Fleming
@ 2016-02-17 12:35 ` Matt Fleming
  2016-02-22 12:14   ` [tip:efi/core] " =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?=
  2016-02-17 12:36 ` [PATCH 07/13] efi/arm-init: Use read-only early mappings Matt Fleming
                   ` (6 subsequent siblings)
  12 siblings, 1 reply; 54+ messages in thread
From: Matt Fleming @ 2016-02-17 12:35 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner, H . Peter Anvin
  Cc: Ard Biesheuvel, linux-kernel, linux-efi, Matt Fleming, Mark Rutland

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

__init annotations should not be used in the EFI stub, since the code is
either included in the decompressor (x86, ARM) where they have no effect,
or the whole stub is __init annotated at the section level (arm64), by
renaming the sections, in which case the __init annotations will be
redundant, and will result in section names like .init.init.text, and our
linker script does not expect that. So un-#define __init so that its
inadvertent use will force a build error.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
---
 drivers/firmware/efi/libstub/efistub.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 6b6548fda089..86ff7bfa6ace 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -5,6 +5,16 @@
 /* error code which can't be mistaken for valid address */
 #define EFI_ERROR	(~0UL)
 
+/*
+ * __init annotations should not be used in the EFI stub, since the code is
+ * either included in the decompressor (x86, ARM) where they have no effect,
+ * or the whole stub is __init annotated at the section level (arm64), by
+ * renaming the sections, in which case the __init annotation will be
+ * redundant, and will result in section names like .init.init.text, and our
+ * linker script does not expect that.
+ */
+#undef __init
+
 void efi_char16_printk(efi_system_table_t *, efi_char16_t *);
 
 efi_status_t efi_open_volume(efi_system_table_t *sys_table_arg, void *__image,
-- 
2.6.2

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH 07/13] efi/arm-init: Use read-only early mappings
  2016-02-17 12:35 [GIT PULL 00/13] EFI changes for v4.6 part 2 Matt Fleming
                   ` (5 preceding siblings ...)
  2016-02-17 12:35 ` [PATCH 06/13] efi/efistub: Prevent __init annotations from being used Matt Fleming
@ 2016-02-17 12:36 ` Matt Fleming
  2016-02-22 12:15   ` [tip:efi/core] " =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?=
  2016-02-17 12:36 ` [PATCH 08/13] efi/arm: Check for LPAE support before booting a LPAE kernel Matt Fleming
                   ` (5 subsequent siblings)
  12 siblings, 1 reply; 54+ messages in thread
From: Matt Fleming @ 2016-02-17 12:36 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner, H . Peter Anvin
  Cc: Ard Biesheuvel, linux-kernel, linux-efi, Matt Fleming, Mark Rutland

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

The early mappings of the EFI system table contents and the UEFI memory
map are read-only from the OS point of view. So map them read-only to
protect them from inadvertent modification.

Acked-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
---
 drivers/firmware/efi/arm-init.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
index 9e15d571b53c..aa1f743152a2 100644
--- a/drivers/firmware/efi/arm-init.c
+++ b/drivers/firmware/efi/arm-init.c
@@ -61,8 +61,8 @@ static int __init uefi_init(void)
 	char vendor[100] = "unknown";
 	int i, retval;
 
-	efi.systab = early_memremap(efi_system_table,
-				    sizeof(efi_system_table_t));
+	efi.systab = early_memremap_ro(efi_system_table,
+				       sizeof(efi_system_table_t));
 	if (efi.systab == NULL) {
 		pr_warn("Unable to map EFI system table.\n");
 		return -ENOMEM;
@@ -86,8 +86,8 @@ static int __init uefi_init(void)
 			efi.systab->hdr.revision & 0xffff);
 
 	/* Show what we know for posterity */
-	c16 = early_memremap(efi_to_phys(efi.systab->fw_vendor),
-			     sizeof(vendor) * sizeof(efi_char16_t));
+	c16 = early_memremap_ro(efi_to_phys(efi.systab->fw_vendor),
+				sizeof(vendor) * sizeof(efi_char16_t));
 	if (c16) {
 		for (i = 0; i < (int) sizeof(vendor) - 1 && *c16; ++i)
 			vendor[i] = c16[i];
@@ -100,8 +100,8 @@ static int __init uefi_init(void)
 		efi.systab->hdr.revision & 0xffff, vendor);
 
 	table_size = sizeof(efi_config_table_64_t) * efi.systab->nr_tables;
-	config_tables = early_memremap(efi_to_phys(efi.systab->tables),
-				       table_size);
+	config_tables = early_memremap_ro(efi_to_phys(efi.systab->tables),
+					  table_size);
 	if (config_tables == NULL) {
 		pr_warn("Unable to map EFI config table array.\n");
 		retval = -ENOMEM;
@@ -185,7 +185,7 @@ void __init efi_init(void)
 	efi_system_table = params.system_table;
 
 	memmap.phys_map = params.mmap;
-	memmap.map = early_memremap(params.mmap, params.mmap_size);
+	memmap.map = early_memremap_ro(params.mmap, params.mmap_size);
 	if (memmap.map == NULL) {
 		/*
 		* If we are booting via UEFI, the UEFI memory map is the only
-- 
2.6.2

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH 08/13] efi/arm: Check for LPAE support before booting a LPAE kernel
  2016-02-17 12:35 [GIT PULL 00/13] EFI changes for v4.6 part 2 Matt Fleming
                   ` (6 preceding siblings ...)
  2016-02-17 12:36 ` [PATCH 07/13] efi/arm-init: Use read-only early mappings Matt Fleming
@ 2016-02-17 12:36 ` Matt Fleming
  2016-02-22 12:15   ` [tip:efi/core] " =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?=
  2016-02-17 12:36 ` [PATCH 09/13] efi/arm64: Check for h/w support before booting a >4 KB granule kernel Matt Fleming
                   ` (4 subsequent siblings)
  12 siblings, 1 reply; 54+ messages in thread
From: Matt Fleming @ 2016-02-17 12:36 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner, H . Peter Anvin
  Cc: Ard Biesheuvel, linux-kernel, linux-efi, Matt Fleming,
	Jeremy Linton, Mark Rutland

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

A kernel built with support for LPAE cannot boot to a state where it
can inform the user about it if it fails due to missing LPAE support
in the hardware.

If we happen to be booting via UEFI, we can fail gracefully so check
for LPAE support in the hardware on CONFIG_ARM_LPAE builds before
entering the kernel proper.

Reviewed-by: Jeremy Linton <jeremy.linton@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
---
 drivers/firmware/efi/libstub/arm32-stub.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/firmware/efi/libstub/arm32-stub.c b/drivers/firmware/efi/libstub/arm32-stub.c
index 495ebd657e38..6f42be4d0084 100644
--- a/drivers/firmware/efi/libstub/arm32-stub.c
+++ b/drivers/firmware/efi/libstub/arm32-stub.c
@@ -9,6 +9,23 @@
 #include <linux/efi.h>
 #include <asm/efi.h>
 
+efi_status_t check_platform_features(efi_system_table_t *sys_table_arg)
+{
+	int block;
+
+	/* non-LPAE kernels can run anywhere */
+	if (!IS_ENABLED(CONFIG_ARM_LPAE))
+		return EFI_SUCCESS;
+
+	/* LPAE kernels need compatible hardware */
+	block = cpuid_feature_extract(CPUID_EXT_MMFR0, 0);
+	if (block < 5) {
+		pr_efi_err(sys_table_arg, "This LPAE kernel is not supported by your CPU\n");
+		return EFI_UNSUPPORTED;
+	}
+	return EFI_SUCCESS;
+}
+
 efi_status_t handle_kernel_image(efi_system_table_t *sys_table,
 				 unsigned long *image_addr,
 				 unsigned long *image_size,
-- 
2.6.2

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH 09/13] efi/arm64: Check for h/w support before booting a >4 KB granule kernel
  2016-02-17 12:35 [GIT PULL 00/13] EFI changes for v4.6 part 2 Matt Fleming
                   ` (7 preceding siblings ...)
  2016-02-17 12:36 ` [PATCH 08/13] efi/arm: Check for LPAE support before booting a LPAE kernel Matt Fleming
@ 2016-02-17 12:36 ` Matt Fleming
  2016-02-22 12:16   ` [tip:efi/core] efi/arm64: Check for h/w support before booting a >4 KB granular kernel =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?=
  2016-03-06  3:35   ` [PATCH 09/13] efi/arm64: Check for h/w support before booting a >4 KB granule kernel Ard Biesheuvel
  2016-02-17 12:36 ` [PATCH 10/13] efi/arm*: Perform hardware compatibility check Matt Fleming
                   ` (3 subsequent siblings)
  12 siblings, 2 replies; 54+ messages in thread
From: Matt Fleming @ 2016-02-17 12:36 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner, H . Peter Anvin
  Cc: Ard Biesheuvel, linux-kernel, linux-efi, Matt Fleming,
	Jeremy Linton, Mark Rutland, Suzuki K Poulose

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

A kernel built with support for a page size that is not supported by the
hardware it runs on cannot boot to a state where it can inform the user
about it.

If we happen to be booting via UEFI, we can fail gracefully so check
if the currently configured page size is supported by the hardware before
entering the kernel proper. Note that UEFI mandates support for 4 KB pages,
so in that case, no check is needed.

Reviewed-by: Jeremy Linton <jeremy.linton@arm.com>
Tested-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
---
 drivers/firmware/efi/libstub/arm64-stub.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
index 9e0342745e4f..aef04ad60e0d 100644
--- a/drivers/firmware/efi/libstub/arm64-stub.c
+++ b/drivers/firmware/efi/libstub/arm64-stub.c
@@ -12,6 +12,26 @@
 #include <linux/efi.h>
 #include <asm/efi.h>
 #include <asm/sections.h>
+#include <asm/sysreg.h>
+
+efi_status_t check_platform_features(efi_system_table_t *sys_table_arg)
+{
+	u64 tg;
+
+	/* UEFI mandates support for 4 KB granule, no need to check */
+	if (IS_ENABLED(CONFIG_ARM64_4K_PAGES))
+		return EFI_SUCCESS;
+
+	tg = (read_cpuid(ID_AA64MMFR0_EL1) >> ID_AA64MMFR0_TGRAN_SHIFT) & 0xf;
+	if (tg != ID_AA64MMFR0_TGRAN_SUPPORTED) {
+		if (IS_ENABLED(CONFIG_ARM64_64K_PAGES))
+			pr_efi_err(sys_table_arg, "This 64 KB granule kernel is not supported by your CPU\n");
+		else
+			pr_efi_err(sys_table_arg, "This 16 KB granule kernel is not supported by your CPU\n");
+		return EFI_UNSUPPORTED;
+	}
+	return EFI_SUCCESS;
+}
 
 efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
 				 unsigned long *image_addr,
-- 
2.6.2

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH 10/13] efi/arm*: Perform hardware compatibility check
  2016-02-17 12:35 [GIT PULL 00/13] EFI changes for v4.6 part 2 Matt Fleming
                   ` (8 preceding siblings ...)
  2016-02-17 12:36 ` [PATCH 09/13] efi/arm64: Check for h/w support before booting a >4 KB granule kernel Matt Fleming
@ 2016-02-17 12:36 ` Matt Fleming
  2016-02-22 12:16   ` [tip:efi/core] " =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?=
  2016-02-17 12:36 ` [PATCH 11/13] x86/mm/pageattr: Don't implicitly allow _PAGE_RW in kernel_map_pages_in_pgd() Matt Fleming
                   ` (2 subsequent siblings)
  12 siblings, 1 reply; 54+ messages in thread
From: Matt Fleming @ 2016-02-17 12:36 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner, H . Peter Anvin
  Cc: Ard Biesheuvel, linux-kernel, linux-efi, Matt Fleming,
	Jeremy Linton, Mark Rutland, Suzuki K Poulose

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Before proceeding with relocating the kernel and parsing the command line,
insert a call to check_platform_features() to allow an arch specific check
to be performed whether the current kernel can execute on the current
hardware.

Reviewed-by: Jeremy Linton <jeremy.linton@arm.com>
Tested-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
---
 drivers/firmware/efi/libstub/arm-stub.c | 4 ++++
 drivers/firmware/efi/libstub/efistub.h  | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
index 3397902e4040..6086a874fa3c 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/arm-stub.c
@@ -190,6 +190,10 @@ unsigned long efi_entry(void *handle, efi_system_table_t *sys_table,
 
 	pr_efi(sys_table, "Booting Linux Kernel...\n");
 
+	status = check_platform_features(sys_table);
+	if (status != EFI_SUCCESS)
+		goto fail;
+
 	/*
 	 * Get a handle to the loaded image protocol.  This is used to get
 	 * information about the running image, such as size and the command
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 86ff7bfa6ace..981c6035ce09 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -53,4 +53,6 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
 		     unsigned long desc_size, efi_memory_desc_t *runtime_map,
 		     int *count);
 
+efi_status_t check_platform_features(efi_system_table_t *sys_table_arg);
+
 #endif
-- 
2.6.2

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH 11/13] x86/mm/pageattr: Don't implicitly allow _PAGE_RW in kernel_map_pages_in_pgd()
  2016-02-17 12:35 [GIT PULL 00/13] EFI changes for v4.6 part 2 Matt Fleming
                   ` (9 preceding siblings ...)
  2016-02-17 12:36 ` [PATCH 10/13] efi/arm*: Perform hardware compatibility check Matt Fleming
@ 2016-02-17 12:36 ` Matt Fleming
  2016-02-22 12:16   ` [tip:efi/core] x86/mm/pat: " =?UTF-8?B?dGlwLWJvdCBmb3IgU2FpIFByYW5lZXRoIDx0aXBib3RAenl0b3IuY29tPg==?=
  2016-02-17 12:36 ` [PATCH 12/13] x86/efi: Map EFI_MEMORY_{XP,RO} memory region bits to EFI page tables Matt Fleming
  2016-02-17 12:36 ` [PATCH 13/13] x86/efi: Only map kernel text for EFI mixed mode Matt Fleming
  12 siblings, 1 reply; 54+ messages in thread
From: Matt Fleming @ 2016-02-17 12:36 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner, H . Peter Anvin
  Cc: Ard Biesheuvel, Sai Praneeth, linux-kernel, linux-efi,
	Borislav Petkov, Lee, Chun-Yi, Ricardo Neri, Ravi Shankar,
	Matt Fleming

From: Sai Praneeth <sai.praneeth.prakhya@intel.com>

As part of the preparation for the EFI_MEMORY_RO flag added in the UEFI
2.5 specification, we need the ability to map pages in kernel page
tables without _PAGE_RW being set.

Modify kernel_map_pages_in_pgd() to require its callers to pass _PAGE_RW
if the pages need to be mapped read/write. Otherwise, we'll map the
pages as read-only.

Cc: Borislav Petkov <bp@alien8.de>
Cc: "Lee, Chun-Yi" <jlee@suse.com>
Cc: Ricardo Neri <ricardo.neri@intel.com>
Cc: Ravi Shankar <ravi.v.shankar@intel.com>
Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
---
 arch/x86/mm/pageattr.c         | 3 +++
 arch/x86/platform/efi/efi_64.c | 8 ++++----
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index bf312da41a6d..14c38ae80409 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -1971,6 +1971,9 @@ int kernel_map_pages_in_pgd(pgd_t *pgd, u64 pfn, unsigned long address,
 	if (!(page_flags & _PAGE_NX))
 		cpa.mask_clr = __pgprot(_PAGE_NX);
 
+	if (!(page_flags & _PAGE_RW))
+		cpa.mask_clr = __pgprot(_PAGE_RW);
+
 	cpa.mask_set = __pgprot(_PAGE_PRESENT | page_flags);
 
 	retval = __change_page_attr_set_clr(&cpa, 0);
diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
index b492521503fe..b0965b27e47f 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -233,7 +233,7 @@ int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages)
 	 * phys_efi_set_virtual_address_map().
 	 */
 	pfn = pa_memmap >> PAGE_SHIFT;
-	if (kernel_map_pages_in_pgd(pgd, pfn, pa_memmap, num_pages, _PAGE_NX)) {
+	if (kernel_map_pages_in_pgd(pgd, pfn, pa_memmap, num_pages, _PAGE_NX | _PAGE_RW)) {
 		pr_err("Error ident-mapping new memmap (0x%lx)!\n", pa_memmap);
 		return 1;
 	}
@@ -262,7 +262,7 @@ int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages)
 		pfn = md->phys_addr >> PAGE_SHIFT;
 		npages = md->num_pages;
 
-		if (kernel_map_pages_in_pgd(pgd, pfn, md->phys_addr, npages, 0)) {
+		if (kernel_map_pages_in_pgd(pgd, pfn, md->phys_addr, npages, _PAGE_RW)) {
 			pr_err("Failed to map 1:1 memory\n");
 			return 1;
 		}
@@ -279,7 +279,7 @@ int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages)
 	text = __pa(_text);
 	pfn = text >> PAGE_SHIFT;
 
-	if (kernel_map_pages_in_pgd(pgd, pfn, text, npages, 0)) {
+	if (kernel_map_pages_in_pgd(pgd, pfn, text, npages, _PAGE_RW)) {
 		pr_err("Failed to map kernel text 1:1\n");
 		return 1;
 	}
@@ -294,7 +294,7 @@ void __init efi_cleanup_page_tables(unsigned long pa_memmap, unsigned num_pages)
 
 static void __init __map_region(efi_memory_desc_t *md, u64 va)
 {
-	unsigned long flags = 0;
+	unsigned long flags = _PAGE_RW;
 	unsigned long pfn;
 	pgd_t *pgd = efi_pgd;
 
-- 
2.6.2

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH 12/13] x86/efi: Map EFI_MEMORY_{XP,RO} memory region bits to EFI page tables
  2016-02-17 12:35 [GIT PULL 00/13] EFI changes for v4.6 part 2 Matt Fleming
                   ` (10 preceding siblings ...)
  2016-02-17 12:36 ` [PATCH 11/13] x86/mm/pageattr: Don't implicitly allow _PAGE_RW in kernel_map_pages_in_pgd() Matt Fleming
@ 2016-02-17 12:36 ` Matt Fleming
  2016-02-22 12:17   ` [tip:efi/core] " =?UTF-8?B?dGlwLWJvdCBmb3IgU2FpIFByYW5lZXRoIDx0aXBib3RAenl0b3IuY29tPg==?=
  2016-02-17 12:36 ` [PATCH 13/13] x86/efi: Only map kernel text for EFI mixed mode Matt Fleming
  12 siblings, 1 reply; 54+ messages in thread
From: Matt Fleming @ 2016-02-17 12:36 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner, H . Peter Anvin
  Cc: Ard Biesheuvel, Sai Praneeth, linux-kernel, linux-efi,
	Borislav Petkov, Lee, Chun-Yi, Ricardo Neri, Ravi Shankar,
	Matt Fleming

From: Sai Praneeth <sai.praneeth.prakhya@intel.com>

Now that we have EFI memory region bits that indicate which regions do
not need execute permission or read/write permission in the page tables,
let's use them.

We also check for EFI_NX_PE_DATA and only enforce the restrictive
mappings if it's present (to allow us to ignore buggy firmware that sets
bits it didn't mean to and to preserve backwards compatibility).

Instead of assuming that firmware would set appropriate attributes in
memory descriptor like EFI_MEMORY_RO for code and EFI_MEMORY_XP for
data, we can expect some firmware out there which might only set *type*
in memory descriptor to be EFI_RUNTIME_SERVICES_CODE or
EFI_RUNTIME_SERVICES_DATA leaving away attribute. This will lead to
improper mappings of EFI runtime regions. In order to avoid it, we check
attribute and type of memory descriptor to update mappings and moreover
Windows works this way.

Cc: Borislav Petkov <bp@alien8.de>
Cc: "Lee, Chun-Yi" <jlee@suse.com>
Cc: Ricardo Neri <ricardo.neri@intel.com>
Cc: Ravi Shankar <ravi.v.shankar@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
---
 arch/x86/include/asm/efi.h     |  2 +-
 arch/x86/platform/efi/efi.c    |  9 +++++++--
 arch/x86/platform/efi/efi_32.c |  2 +-
 arch/x86/platform/efi/efi_64.c | 45 ++++++++++++++++++++++++++++++++++++++----
 4 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index 8fd9e637629a..7bb206f73915 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -141,7 +141,7 @@ extern int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pa
 extern void __init efi_cleanup_page_tables(unsigned long pa_memmap, unsigned num_pages);
 extern void __init old_map_region(efi_memory_desc_t *md);
 extern void __init runtime_code_page_mkexec(void);
-extern void __init efi_runtime_mkexec(void);
+extern void __init efi_runtime_update_mappings(void);
 extern void __init efi_dump_pagetable(void);
 extern void __init efi_apply_memmap_quirks(void);
 extern int __init efi_reuse_config(u64 tables, int nr_tables);
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index e80826e6f3a9..994a7df84a7b 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -934,7 +934,6 @@ static void __init __efi_enter_virtual_mode(void)
 	}
 
 	efi_sync_low_kernel_mappings();
-	efi_dump_pagetable();
 
 	if (efi_is_native()) {
 		status = phys_efi_set_virtual_address_map(
@@ -972,7 +971,13 @@ static void __init __efi_enter_virtual_mode(void)
 
 	efi.set_virtual_address_map = NULL;
 
-	efi_runtime_mkexec();
+	/*
+	 * Apply more restrictive page table mapping attributes now that
+	 * SVAM() has been called and the firmware has performed all
+	 * necessary relocation fixups for the new virtual addresses.
+	 */
+	efi_runtime_update_mappings();
+	efi_dump_pagetable();
 
 	/*
 	 * We mapped the descriptor array into the EFI pagetable above
diff --git a/arch/x86/platform/efi/efi_32.c b/arch/x86/platform/efi/efi_32.c
index 58d669bc8250..338402b91d2e 100644
--- a/arch/x86/platform/efi/efi_32.c
+++ b/arch/x86/platform/efi/efi_32.c
@@ -90,7 +90,7 @@ void __init efi_call_phys_epilog(pgd_t *save_pgd)
 	__flush_tlb_all();
 }
 
-void __init efi_runtime_mkexec(void)
+void __init efi_runtime_update_mappings(void)
 {
 	if (__supported_pte_mask & _PAGE_NX)
 		runtime_code_page_mkexec();
diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
index b0965b27e47f..40d2f447a9dd 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -393,13 +393,50 @@ void __init parse_efi_setup(u64 phys_addr, u32 data_len)
 	efi_setup = phys_addr + sizeof(struct setup_data);
 }
 
-void __init efi_runtime_mkexec(void)
+void __init efi_runtime_update_mappings(void)
 {
-	if (!efi_enabled(EFI_OLD_MEMMAP))
+	unsigned long pfn;
+	pgd_t *pgd = efi_pgd;
+	efi_memory_desc_t *md;
+	void *p;
+
+	if (efi_enabled(EFI_OLD_MEMMAP)) {
+		if (__supported_pte_mask & _PAGE_NX)
+			runtime_code_page_mkexec();
+		return;
+	}
+
+	if (!efi_enabled(EFI_NX_PE_DATA))
 		return;
 
-	if (__supported_pte_mask & _PAGE_NX)
-		runtime_code_page_mkexec();
+	for (p = memmap.map; p < memmap.map_end; p += memmap.desc_size) {
+		unsigned long pf = 0;
+		md = p;
+
+		if (!(md->attribute & EFI_MEMORY_RUNTIME))
+			continue;
+
+		if (!(md->attribute & EFI_MEMORY_WB))
+			pf |= _PAGE_PCD;
+
+		if ((md->attribute & EFI_MEMORY_XP) ||
+			(md->type == EFI_RUNTIME_SERVICES_DATA))
+			pf |= _PAGE_NX;
+
+		if (!(md->attribute & EFI_MEMORY_RO) &&
+			(md->type != EFI_RUNTIME_SERVICES_CODE))
+			pf |= _PAGE_RW;
+
+		/* Update the 1:1 mapping */
+		pfn = md->phys_addr >> PAGE_SHIFT;
+		if (kernel_map_pages_in_pgd(pgd, pfn, md->phys_addr, md->num_pages, pf))
+			pr_warn("Error mapping PA 0x%llx -> VA 0x%llx!\n",
+				   md->phys_addr, md->virt_addr);
+
+		if (kernel_map_pages_in_pgd(pgd, pfn, md->virt_addr, md->num_pages, pf))
+			pr_warn("Error mapping PA 0x%llx -> VA 0x%llx!\n",
+				   md->phys_addr, md->virt_addr);
+	}
 }
 
 void __init efi_dump_pagetable(void)
-- 
2.6.2

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH 13/13] x86/efi: Only map kernel text for EFI mixed mode
  2016-02-17 12:35 [GIT PULL 00/13] EFI changes for v4.6 part 2 Matt Fleming
                   ` (11 preceding siblings ...)
  2016-02-17 12:36 ` [PATCH 12/13] x86/efi: Map EFI_MEMORY_{XP,RO} memory region bits to EFI page tables Matt Fleming
@ 2016-02-17 12:36 ` Matt Fleming
  2016-02-22 12:17   ` [tip:efi/core] " =?UTF-8?B?dGlwLWJvdCBmb3IgU2FpIFByYW5lZXRoIDx0aXBib3RAenl0b3IuY29tPg==?=
  12 siblings, 1 reply; 54+ messages in thread
From: Matt Fleming @ 2016-02-17 12:36 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner, H . Peter Anvin
  Cc: Ard Biesheuvel, Sai Praneeth, linux-kernel, linux-efi,
	Borislav Petkov, Ricardo Neri, Ravi Shankar, Matt Fleming

From: Sai Praneeth <sai.praneeth.prakhya@intel.com>

The correct symbol to use when figuring out the size of the kernel
text is '_etext', not '_end' which is the symbol for the entire kernel
image includes data and debug sections.

Cc: Borislav Petkov <bp@alien8.de>
Cc: Ricardo Neri <ricardo.neri@intel.com>
Cc: Ravi Shankar <ravi.v.shankar@intel.com>
Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
---
 arch/x86/platform/efi/efi_64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
index 40d2f447a9dd..49e4dd4a1f58 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -275,7 +275,7 @@ int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages)
 	efi_scratch.phys_stack = virt_to_phys(page_address(page));
 	efi_scratch.phys_stack += PAGE_SIZE; /* stack grows down */
 
-	npages = (_end - _text) >> PAGE_SHIFT;
+	npages = (_etext - _text) >> PAGE_SHIFT;
 	text = __pa(_text);
 	pfn = text >> PAGE_SHIFT;
 
-- 
2.6.2

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [tip:efi/core] efi: Reformat GUID tables to follow the format in UEFI spec
  2016-02-17 12:35 ` [PATCH 01/13] efi: Reformat GUID tables to follow the format in UEFI spec Matt Fleming
@ 2016-02-22 12:12   ` =?UTF-8?B?dGlwLWJvdCBmb3IgUGV0ZXIgSm9uZXMgPHRpcGJvdEB6eXRvci5jb20+?=
  0 siblings, 0 replies; 54+ messages in thread
From: =?UTF-8?B?dGlwLWJvdCBmb3IgUGV0ZXIgSm9uZXMgPHRpcGJvdEB6eXRvci5jb20+?= @ 2016-02-22 12:12 UTC (permalink / raw)
  To: =?UTF-8?B?bGludXgtdGlwLWNvbW1pdHNAdmdlci5rZXJuZWwub3Jn?=
  Cc: matt, hpa, torvalds, mingo, pjones, tglx, linux-kernel, peterz,
	ard.biesheuvel

Commit-ID:  662b1d890c593673964758fe5b6f22067bffba7a
Gitweb:     http://git.kernel.org/tip/662b1d890c593673964758fe5b6f22067bffba7a
Author:     Peter Jones <pjones@redhat.com>
AuthorDate: Wed, 17 Feb 2016 12:35:54 +0000
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 22 Feb 2016 08:26:25 +0100

efi: Reformat GUID tables to follow the format in UEFI spec

This makes it much easier to hunt for typos in the GUID definitions.

It also makes checkpatch complain less about efi.h GUID additions, so
that if you add another one with the same style, checkpatch won't
complain about it.

Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/1455712566-16727-2-git-send-email-matt@codeblueprint.co.uk
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 include/linux/efi.h | 63 +++++++++++++++++++++++++++++++++++------------------
 1 file changed, 42 insertions(+), 21 deletions(-)

diff --git a/include/linux/efi.h b/include/linux/efi.h
index 1acd723..42be9c9 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -536,67 +536,88 @@ void efi_native_runtime_setup(void);
  *  EFI Configuration Table and GUID definitions
  */
 #define NULL_GUID \
-    EFI_GUID(  0x00000000, 0x0000, 0x0000, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 )
+	EFI_GUID(0x00000000, 0x0000, 0x0000, \
+		 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00)
 
 #define MPS_TABLE_GUID    \
-    EFI_GUID(  0xeb9d2d2f, 0x2d88, 0x11d3, 0x9a, 0x16, 0x0, 0x90, 0x27, 0x3f, 0xc1, 0x4d )
+	EFI_GUID(0xeb9d2d2f, 0x2d88, 0x11d3, \
+		 0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
 
 #define ACPI_TABLE_GUID    \
-    EFI_GUID(  0xeb9d2d30, 0x2d88, 0x11d3, 0x9a, 0x16, 0x0, 0x90, 0x27, 0x3f, 0xc1, 0x4d )
+	EFI_GUID(0xeb9d2d30, 0x2d88, 0x11d3, \
+		 0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
 
 #define ACPI_20_TABLE_GUID    \
-    EFI_GUID(  0x8868e871, 0xe4f1, 0x11d3, 0xbc, 0x22, 0x0, 0x80, 0xc7, 0x3c, 0x88, 0x81 )
+	EFI_GUID(0x8868e871, 0xe4f1, 0x11d3, \
+		 0xbc, 0x22, 0x00, 0x80, 0xc7, 0x3c, 0x88, 0x81)
 
 #define SMBIOS_TABLE_GUID    \
-    EFI_GUID(  0xeb9d2d31, 0x2d88, 0x11d3, 0x9a, 0x16, 0x0, 0x90, 0x27, 0x3f, 0xc1, 0x4d )
+	EFI_GUID(0xeb9d2d31, 0x2d88, 0x11d3, \
+		 0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
 
 #define SMBIOS3_TABLE_GUID    \
-    EFI_GUID(  0xf2fd1544, 0x9794, 0x4a2c, 0x99, 0x2e, 0xe5, 0xbb, 0xcf, 0x20, 0xe3, 0x94 )
+	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 )
+	EFI_GUID(0xeb9d2d32, 0x2d88, 0x11d3, \
+		 0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
 
 #define HCDP_TABLE_GUID	\
-    EFI_GUID(  0xf951938d, 0x620b, 0x42ef, 0x82, 0x79, 0xa8, 0x4b, 0x79, 0x61, 0x78, 0x98 )
+	EFI_GUID(0xf951938d, 0x620b, 0x42ef, \
+		 0x82, 0x79, 0xa8, 0x4b, 0x79, 0x61, 0x78, 0x98)
 
 #define UGA_IO_PROTOCOL_GUID \
-    EFI_GUID(  0x61a4d49e, 0x6f68, 0x4f1b, 0xb9, 0x22, 0xa8, 0x6e, 0xed, 0xb, 0x7, 0xa2 )
+	EFI_GUID(0x61a4d49e, 0x6f68, 0x4f1b, \
+		 0xb9, 0x22, 0xa8, 0x6e, 0xed, 0x0b, 0x07, 0xa2)
 
 #define EFI_GLOBAL_VARIABLE_GUID \
-    EFI_GUID(  0x8be4df61, 0x93ca, 0x11d2, 0xaa, 0x0d, 0x00, 0xe0, 0x98, 0x03, 0x2b, 0x8c )
+	EFI_GUID(0x8be4df61, 0x93ca, 0x11d2, \
+		 0xaa, 0x0d, 0x00, 0xe0, 0x98, 0x03, 0x2b, 0x8c)
 
 #define UV_SYSTEM_TABLE_GUID \
-    EFI_GUID(  0x3b13a7d4, 0x633e, 0x11dd, 0x93, 0xec, 0xda, 0x25, 0x56, 0xd8, 0x95, 0x93 )
+	EFI_GUID(0x3b13a7d4, 0x633e, 0x11dd, \
+		 0x93, 0xec, 0xda, 0x25, 0x56, 0xd8, 0x95, 0x93)
 
 #define LINUX_EFI_CRASH_GUID \
-    EFI_GUID(  0xcfc8fc79, 0xbe2e, 0x4ddc, 0x97, 0xf0, 0x9f, 0x98, 0xbf, 0xe2, 0x98, 0xa0 )
+	EFI_GUID(0xcfc8fc79, 0xbe2e, 0x4ddc, \
+		 0x97, 0xf0, 0x9f, 0x98, 0xbf, 0xe2, 0x98, 0xa0)
 
 #define LOADED_IMAGE_PROTOCOL_GUID \
-    EFI_GUID(  0x5b1b31a1, 0x9562, 0x11d2, 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b )
+	EFI_GUID(0x5b1b31a1, 0x9562, 0x11d2, \
+		 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
 
 #define EFI_GRAPHICS_OUTPUT_PROTOCOL_GUID \
-    EFI_GUID(  0x9042a9de, 0x23dc, 0x4a38, 0x96, 0xfb, 0x7a, 0xde, 0xd0, 0x80, 0x51, 0x6a )
+	EFI_GUID(0x9042a9de, 0x23dc, 0x4a38, \
+		 0x96, 0xfb, 0x7a, 0xde, 0xd0, 0x80, 0x51, 0x6a)
 
 #define EFI_UGA_PROTOCOL_GUID \
-    EFI_GUID(  0x982c298b, 0xf4fa, 0x41cb, 0xb8, 0x38, 0x77, 0xaa, 0x68, 0x8f, 0xb8, 0x39 )
+	EFI_GUID(0x982c298b, 0xf4fa, 0x41cb, \
+		 0xb8, 0x38, 0x77, 0xaa, 0x68, 0x8f, 0xb8, 0x39)
 
 #define EFI_PCI_IO_PROTOCOL_GUID \
-    EFI_GUID(  0x4cf5b200, 0x68b8, 0x4ca5, 0x9e, 0xec, 0xb2, 0x3e, 0x3f, 0x50, 0x2, 0x9a )
+	EFI_GUID(0x4cf5b200, 0x68b8, 0x4ca5, \
+		 0x9e, 0xec, 0xb2, 0x3e, 0x3f, 0x50, 0x02, 0x9a)
 
 #define EFI_FILE_INFO_ID \
-    EFI_GUID(  0x9576e92, 0x6d3f, 0x11d2, 0x8e, 0x39, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b )
+	EFI_GUID(0x9576e92, 0x6d3f, 0x11d2, \
+		 0x8e, 0x39, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
 
 #define EFI_SYSTEM_RESOURCE_TABLE_GUID \
-    EFI_GUID(  0xb122a263, 0x3661, 0x4f68, 0x99, 0x29, 0x78, 0xf8, 0xb0, 0xd6, 0x21, 0x80 )
+	EFI_GUID(0xb122a263, 0x3661, 0x4f68, \
+		 0x99, 0x29, 0x78, 0xf8, 0xb0, 0xd6, 0x21, 0x80)
 
 #define EFI_FILE_SYSTEM_GUID \
-    EFI_GUID(  0x964e5b22, 0x6459, 0x11d2, 0x8e, 0x39, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b )
+	EFI_GUID(0x964e5b22, 0x6459, 0x11d2, \
+		 0x8e, 0x39, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
 
 #define DEVICE_TREE_GUID \
-    EFI_GUID(  0xb1b621d5, 0xf19c, 0x41a5, 0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0 )
+	EFI_GUID(0xb1b621d5, 0xf19c, 0x41a5, \
+		 0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0)
 
 #define EFI_PROPERTIES_TABLE_GUID \
-    EFI_GUID(  0x880aaca3, 0x4adc, 0x4a04, 0x90, 0x79, 0xb7, 0x47, 0x34, 0x08, 0x25, 0xe5 )
+	EFI_GUID(0x880aaca3, 0x4adc, 0x4a04, \
+		 0x90, 0x79, 0xb7, 0x47, 0x34, 0x08, 0x25, 0xe5)
 
 typedef struct {
 	efi_guid_t guid;

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [tip:efi/core] efi/runtime-wrappers: Run UEFI Runtime Services with interrupts enabled
  2016-02-17 12:35 ` [PATCH 02/13] efi/runtime-wrappers: Run UEFI Runtime Services with interrupts enabled Matt Fleming
@ 2016-02-22 12:13   ` =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?=
  0 siblings, 0 replies; 54+ messages in thread
From: =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?= @ 2016-02-22 12:13 UTC (permalink / raw)
  To: =?UTF-8?B?bGludXgtdGlwLWNvbW1pdHNAdmdlci5rZXJuZWwub3Jn?=
  Cc: linux-kernel, peterz, torvalds, ard.biesheuvel, hpa, luto,
	sai.praneeth.prakhya, mingo, tglx, matt

Commit-ID:  fe3244945c47161e2486412d6412c87ba279305d
Gitweb:     http://git.kernel.org/tip/fe3244945c47161e2486412d6412c87ba279305d
Author:     Ard Biesheuvel <ard.biesheuvel@linaro.org>
AuthorDate: Wed, 17 Feb 2016 12:35:55 +0000
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 22 Feb 2016 08:26:25 +0100

efi/runtime-wrappers: Run UEFI Runtime Services with interrupts enabled

The UEFI spec allows Runtime Services to be invoked with interrupts
enabled. The only reason we were disabling interrupts was to prevent
recursive calls into the services on the same CPU, which will lead to
deadlock. However, the only context where such invocations may occur
legally is from efi-pstore via efivars, and that code has been updated
to call a non-blocking alternative when invoked from a non-interruptible
context.

So instead, update the ordinary, blocking UEFI Runtime Services wrappers
to execute with interrupts enabled. This aims to prevent excessive interrupt
latencies on uniprocessor platforms with slow variable stores.

Note that other OSes such as Windows call UEFI Runtime Services with
interrupts enabled as well.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/1455712566-16727-3-git-send-email-matt@codeblueprint.co.uk
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 drivers/firmware/efi/runtime-wrappers.c | 71 +++++++++++++--------------------
 1 file changed, 28 insertions(+), 43 deletions(-)

diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c
index 7b8b2f2..de69530 100644
--- a/drivers/firmware/efi/runtime-wrappers.c
+++ b/drivers/firmware/efi/runtime-wrappers.c
@@ -63,23 +63,21 @@ static DEFINE_SPINLOCK(efi_runtime_lock);
 
 static efi_status_t virt_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc)
 {
-	unsigned long flags;
 	efi_status_t status;
 
-	spin_lock_irqsave(&efi_runtime_lock, flags);
+	spin_lock(&efi_runtime_lock);
 	status = efi_call_virt(get_time, tm, tc);
-	spin_unlock_irqrestore(&efi_runtime_lock, flags);
+	spin_unlock(&efi_runtime_lock);
 	return status;
 }
 
 static efi_status_t virt_efi_set_time(efi_time_t *tm)
 {
-	unsigned long flags;
 	efi_status_t status;
 
-	spin_lock_irqsave(&efi_runtime_lock, flags);
+	spin_lock(&efi_runtime_lock);
 	status = efi_call_virt(set_time, tm);
-	spin_unlock_irqrestore(&efi_runtime_lock, flags);
+	spin_unlock(&efi_runtime_lock);
 	return status;
 }
 
@@ -87,23 +85,21 @@ static efi_status_t virt_efi_get_wakeup_time(efi_bool_t *enabled,
 					     efi_bool_t *pending,
 					     efi_time_t *tm)
 {
-	unsigned long flags;
 	efi_status_t status;
 
-	spin_lock_irqsave(&efi_runtime_lock, flags);
+	spin_lock(&efi_runtime_lock);
 	status = efi_call_virt(get_wakeup_time, enabled, pending, tm);
-	spin_unlock_irqrestore(&efi_runtime_lock, flags);
+	spin_unlock(&efi_runtime_lock);
 	return status;
 }
 
 static efi_status_t virt_efi_set_wakeup_time(efi_bool_t enabled, efi_time_t *tm)
 {
-	unsigned long flags;
 	efi_status_t status;
 
-	spin_lock_irqsave(&efi_runtime_lock, flags);
+	spin_lock(&efi_runtime_lock);
 	status = efi_call_virt(set_wakeup_time, enabled, tm);
-	spin_unlock_irqrestore(&efi_runtime_lock, flags);
+	spin_unlock(&efi_runtime_lock);
 	return status;
 }
 
@@ -113,13 +109,12 @@ static efi_status_t virt_efi_get_variable(efi_char16_t *name,
 					  unsigned long *data_size,
 					  void *data)
 {
-	unsigned long flags;
 	efi_status_t status;
 
-	spin_lock_irqsave(&efi_runtime_lock, flags);
+	spin_lock(&efi_runtime_lock);
 	status = efi_call_virt(get_variable, name, vendor, attr, data_size,
 			       data);
-	spin_unlock_irqrestore(&efi_runtime_lock, flags);
+	spin_unlock(&efi_runtime_lock);
 	return status;
 }
 
@@ -127,12 +122,11 @@ static efi_status_t virt_efi_get_next_variable(unsigned long *name_size,
 					       efi_char16_t *name,
 					       efi_guid_t *vendor)
 {
-	unsigned long flags;
 	efi_status_t status;
 
-	spin_lock_irqsave(&efi_runtime_lock, flags);
+	spin_lock(&efi_runtime_lock);
 	status = efi_call_virt(get_next_variable, name_size, name, vendor);
-	spin_unlock_irqrestore(&efi_runtime_lock, flags);
+	spin_unlock(&efi_runtime_lock);
 	return status;
 }
 
@@ -142,13 +136,12 @@ static efi_status_t virt_efi_set_variable(efi_char16_t *name,
 					  unsigned long data_size,
 					  void *data)
 {
-	unsigned long flags;
 	efi_status_t status;
 
-	spin_lock_irqsave(&efi_runtime_lock, flags);
+	spin_lock(&efi_runtime_lock);
 	status = efi_call_virt(set_variable, name, vendor, attr, data_size,
 			       data);
-	spin_unlock_irqrestore(&efi_runtime_lock, flags);
+	spin_unlock(&efi_runtime_lock);
 	return status;
 }
 
@@ -157,15 +150,14 @@ virt_efi_set_variable_nonblocking(efi_char16_t *name, efi_guid_t *vendor,
 				  u32 attr, unsigned long data_size,
 				  void *data)
 {
-	unsigned long flags;
 	efi_status_t status;
 
-	if (!spin_trylock_irqsave(&efi_runtime_lock, flags))
+	if (!spin_trylock(&efi_runtime_lock))
 		return EFI_NOT_READY;
 
 	status = efi_call_virt(set_variable, name, vendor, attr, data_size,
 			       data);
-	spin_unlock_irqrestore(&efi_runtime_lock, flags);
+	spin_unlock(&efi_runtime_lock);
 	return status;
 }
 
@@ -175,16 +167,15 @@ static efi_status_t virt_efi_query_variable_info(u32 attr,
 						 u64 *remaining_space,
 						 u64 *max_variable_size)
 {
-	unsigned long flags;
 	efi_status_t status;
 
 	if (efi.runtime_version < EFI_2_00_SYSTEM_TABLE_REVISION)
 		return EFI_UNSUPPORTED;
 
-	spin_lock_irqsave(&efi_runtime_lock, flags);
+	spin_lock(&efi_runtime_lock);
 	status = efi_call_virt(query_variable_info, attr, storage_space,
 			       remaining_space, max_variable_size);
-	spin_unlock_irqrestore(&efi_runtime_lock, flags);
+	spin_unlock(&efi_runtime_lock);
 	return status;
 }
 
@@ -194,29 +185,27 @@ virt_efi_query_variable_info_nonblocking(u32 attr,
 					 u64 *remaining_space,
 					 u64 *max_variable_size)
 {
-	unsigned long flags;
 	efi_status_t status;
 
 	if (efi.runtime_version < EFI_2_00_SYSTEM_TABLE_REVISION)
 		return EFI_UNSUPPORTED;
 
-	if (!spin_trylock_irqsave(&efi_runtime_lock, flags))
+	if (!spin_trylock(&efi_runtime_lock))
 		return EFI_NOT_READY;
 
 	status = efi_call_virt(query_variable_info, attr, storage_space,
 			       remaining_space, max_variable_size);
-	spin_unlock_irqrestore(&efi_runtime_lock, flags);
+	spin_unlock(&efi_runtime_lock);
 	return status;
 }
 
 static efi_status_t virt_efi_get_next_high_mono_count(u32 *count)
 {
-	unsigned long flags;
 	efi_status_t status;
 
-	spin_lock_irqsave(&efi_runtime_lock, flags);
+	spin_lock(&efi_runtime_lock);
 	status = efi_call_virt(get_next_high_mono_count, count);
-	spin_unlock_irqrestore(&efi_runtime_lock, flags);
+	spin_unlock(&efi_runtime_lock);
 	return status;
 }
 
@@ -225,26 +214,23 @@ static void virt_efi_reset_system(int reset_type,
 				  unsigned long data_size,
 				  efi_char16_t *data)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&efi_runtime_lock, flags);
+	spin_lock(&efi_runtime_lock);
 	__efi_call_virt(reset_system, reset_type, status, data_size, data);
-	spin_unlock_irqrestore(&efi_runtime_lock, flags);
+	spin_unlock(&efi_runtime_lock);
 }
 
 static efi_status_t virt_efi_update_capsule(efi_capsule_header_t **capsules,
 					    unsigned long count,
 					    unsigned long sg_list)
 {
-	unsigned long flags;
 	efi_status_t status;
 
 	if (efi.runtime_version < EFI_2_00_SYSTEM_TABLE_REVISION)
 		return EFI_UNSUPPORTED;
 
-	spin_lock_irqsave(&efi_runtime_lock, flags);
+	spin_lock(&efi_runtime_lock);
 	status = efi_call_virt(update_capsule, capsules, count, sg_list);
-	spin_unlock_irqrestore(&efi_runtime_lock, flags);
+	spin_unlock(&efi_runtime_lock);
 	return status;
 }
 
@@ -253,16 +239,15 @@ static efi_status_t virt_efi_query_capsule_caps(efi_capsule_header_t **capsules,
 						u64 *max_size,
 						int *reset_type)
 {
-	unsigned long flags;
 	efi_status_t status;
 
 	if (efi.runtime_version < EFI_2_00_SYSTEM_TABLE_REVISION)
 		return EFI_UNSUPPORTED;
 
-	spin_lock_irqsave(&efi_runtime_lock, flags);
+	spin_lock(&efi_runtime_lock);
 	status = efi_call_virt(query_capsule_caps, capsules, count, max_size,
 			       reset_type);
-	spin_unlock_irqrestore(&efi_runtime_lock, flags);
+	spin_unlock(&efi_runtime_lock);
 	return status;
 }
 

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings
  2016-02-17 12:35 ` [PATCH 03/13] x86/mm/pageattr: Use _PAGE_GLOBAL bit for EFI page table mappings Matt Fleming
@ 2016-02-22 12:13   ` =?UTF-8?B?dGlwLWJvdCBmb3IgU2FpIFByYW5lZXRoIDx0aXBib3RAenl0b3IuY29tPg==?=
  2016-02-23 17:47     ` Andy Lutomirski
  0 siblings, 1 reply; 54+ messages in thread
From: =?UTF-8?B?dGlwLWJvdCBmb3IgU2FpIFByYW5lZXRoIDx0aXBib3RAenl0b3IuY29tPg==?= @ 2016-02-22 12:13 UTC (permalink / raw)
  To: =?UTF-8?B?bGludXgtdGlwLWNvbW1pdHNAdmdlci5rZXJuZWwub3Jn?=
  Cc: mingo, tglx, brgerst, torvalds, peterz, ard.biesheuvel, luto,
	hughd, ricardo.neri, mcgrof, sai.praneeth.prakhya, linux-kernel,
	bp, akpm, hpa, dvlasenk, toshi.kani, matt, ravi.v.shankar

Commit-ID:  397630150632639b3ca5b4414accd5011c45e276
Gitweb:     http://git.kernel.org/tip/397630150632639b3ca5b4414accd5011c45e276
Author:     Sai Praneeth <sai.praneeth.prakhya@intel.com>
AuthorDate: Wed, 17 Feb 2016 12:35:56 +0000
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 22 Feb 2016 08:26:26 +0100

x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings

Since EFI page tables can be treated as kernel page tables they should
be global. All the other page mapping functions in pageattr.c set the
_PAGE_GLOBAL bit and we want to avoid inconsistencies when we map a page
in the EFI code paths, for example when that page is split in
__split_large_page(), etc. It also makes it easier to validate that the
EFI region mappings have the correct attributes because there are fewer
differences compared with regular kernel mappings.

Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Luis R. Rodriguez <mcgrof@suse.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ravi Shankar <ravi.v.shankar@intel.com>
Cc: Ricardo Neri <ricardo.neri@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Toshi Kani <toshi.kani@hp.com>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/1455712566-16727-4-git-send-email-matt@codeblueprint.co.uk
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/mm/pageattr.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index 632d34d..bf312da 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -909,6 +909,20 @@ static void populate_pte(struct cpa_data *cpa,
 
 	pte = pte_offset_kernel(pmd, start);
 
+	/*
+	 * Set the GLOBAL flags only if the PRESENT flag is
+	 * set otherwise pte_present will return true even on
+	 * a non present pte. The canon_pgprot will clear
+	 * _PAGE_GLOBAL for the ancient hardware that doesn't
+	 * support it.
+	 */
+	if (pgprot_val(pgprot) & _PAGE_PRESENT)
+		pgprot_val(pgprot) |= _PAGE_GLOBAL;
+	else
+		pgprot_val(pgprot) &= ~_PAGE_GLOBAL;
+
+	pgprot = canon_pgprot(pgprot);
+
 	while (num_pages-- && start < end) {
 		set_pte(pte, pfn_pte(cpa->pfn, pgprot));
 

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [tip:efi/core] efi/arm64: Drop __init annotation from handle_kernel_image()
  2016-02-17 12:35 ` [PATCH 04/13] efi/arm64: Drop __init annotation from handle_kernel_image() Matt Fleming
@ 2016-02-22 12:14   ` =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?=
  0 siblings, 0 replies; 54+ messages in thread
From: =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?= @ 2016-02-22 12:14 UTC (permalink / raw)
  To: =?UTF-8?B?bGludXgtdGlwLWNvbW1pdHNAdmdlci5rZXJuZWwub3Jn?=
  Cc: peterz, hpa, tglx, mark.rutland, mingo, matt, will.deacon,
	ard.biesheuvel, linux-kernel, torvalds

Commit-ID:  dae31fd2b74c35cc84128733bc210bf6b26ae408
Gitweb:     http://git.kernel.org/tip/dae31fd2b74c35cc84128733bc210bf6b26ae408
Author:     Ard Biesheuvel <ard.biesheuvel@linaro.org>
AuthorDate: Wed, 17 Feb 2016 12:35:57 +0000
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 22 Feb 2016 08:26:26 +0100

efi/arm64: Drop __init annotation from handle_kernel_image()

After moving arm64-stub.c to libstub/, all of its sections are emitted
as .init.xxx sections automatically, and the __init annotation of
handle_kernel_image() causes it to end up in .init.init.text, which is
not recognized as an __init section by the linker scripts. So drop the
annotation.

Tested-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
Acked-by: Will Deacon <will.deacon@arm.com>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/1455712566-16727-5-git-send-email-matt@codeblueprint.co.uk
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 drivers/firmware/efi/libstub/arm64-stub.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
index 78dfbd3..9e03427 100644
--- a/drivers/firmware/efi/libstub/arm64-stub.c
+++ b/drivers/firmware/efi/libstub/arm64-stub.c
@@ -13,13 +13,13 @@
 #include <asm/efi.h>
 #include <asm/sections.h>
 
-efi_status_t __init handle_kernel_image(efi_system_table_t *sys_table_arg,
-					unsigned long *image_addr,
-					unsigned long *image_size,
-					unsigned long *reserve_addr,
-					unsigned long *reserve_size,
-					unsigned long dram_base,
-					efi_loaded_image_t *image)
+efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
+				 unsigned long *image_addr,
+				 unsigned long *image_size,
+				 unsigned long *reserve_addr,
+				 unsigned long *reserve_size,
+				 unsigned long dram_base,
+				 efi_loaded_image_t *image)
 {
 	efi_status_t status;
 	unsigned long kernel_size, kernel_memsize = 0;

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [tip:efi/core] arm64/vmlinux.lds.S: Handle .init.rodata.xxx and .init.bss sections
  2016-02-17 12:35 ` [PATCH 05/13] arm64: vmlinux.lds.S: Handle .init.rodata.xxx and .init.bss sections Matt Fleming
@ 2016-02-22 12:14   ` =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?=
  0 siblings, 0 replies; 54+ messages in thread
From: =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?= @ 2016-02-22 12:14 UTC (permalink / raw)
  To: =?UTF-8?B?bGludXgtdGlwLWNvbW1pdHNAdmdlci5rZXJuZWwub3Jn?=
  Cc: hpa, mark.rutland, will.deacon, ard.biesheuvel, torvalds, peterz,
	matt, mingo, tglx, linux-kernel

Commit-ID:  1ce99bf45306ba889faadced6baabebf7770c546
Gitweb:     http://git.kernel.org/tip/1ce99bf45306ba889faadced6baabebf7770c546
Author:     Ard Biesheuvel <ard.biesheuvel@linaro.org>
AuthorDate: Wed, 17 Feb 2016 12:35:58 +0000
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 22 Feb 2016 08:26:26 +0100

arm64/vmlinux.lds.S: Handle .init.rodata.xxx and .init.bss sections

The EFI stub is typically built into the decompressor (x86, ARM) so none
of its symbols are annotated as __init. However, on arm64, the stub is
linked into the kernel proper, and the code is __init annotated at the
section level by prepending all names of SHF_ALLOC sections with '.init'.

This results in section names like .init.rodata.str1.8 (for string literals)
and .init.bss (which is tiny), both of which can be moved into the .init.data
output section.

Tested-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
Acked-by: Will Deacon <will.deacon@arm.com>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/1455712566-16727-6-git-send-email-matt@codeblueprint.co.uk
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/arm64/kernel/vmlinux.lds.S | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
index e3928f5..cbf4db4 100644
--- a/arch/arm64/kernel/vmlinux.lds.S
+++ b/arch/arm64/kernel/vmlinux.lds.S
@@ -134,6 +134,7 @@ SECTIONS
 		CON_INITCALL
 		SECURITY_INITCALL
 		INIT_RAM_FS
+		*(.init.rodata.* .init.bss)	/* from the EFI stub */
 	}
 	.exit.data : {
 		ARM_EXIT_KEEP(EXIT_DATA)

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [tip:efi/core] efi/efistub: Prevent __init annotations from being used
  2016-02-17 12:35 ` [PATCH 06/13] efi/efistub: Prevent __init annotations from being used Matt Fleming
@ 2016-02-22 12:14   ` =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?=
  0 siblings, 0 replies; 54+ messages in thread
From: =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?= @ 2016-02-22 12:14 UTC (permalink / raw)
  To: =?UTF-8?B?bGludXgtdGlwLWNvbW1pdHNAdmdlci5rZXJuZWwub3Jn?=
  Cc: ard.biesheuvel, mark.rutland, matt, mingo, torvalds, hpa, peterz,
	linux-kernel, tglx

Commit-ID:  07e83dbb75865b016f6493c119a30aac7c25051a
Gitweb:     http://git.kernel.org/tip/07e83dbb75865b016f6493c119a30aac7c25051a
Author:     Ard Biesheuvel <ard.biesheuvel@linaro.org>
AuthorDate: Wed, 17 Feb 2016 12:35:59 +0000
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 22 Feb 2016 08:26:26 +0100

efi/efistub: Prevent __init annotations from being used

__init annotations should not be used in the EFI stub, since the code is
either included in the decompressor (x86, ARM) where they have no effect,
or the whole stub is __init annotated at the section level (arm64), by
renaming the sections.

In the second case the __init annotations will be redundant, and will
result in section names like .init.init.text, and our linker script does
not expect that.

So un-#define __init so that its inadvertent use will force a build error.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/1455712566-16727-7-git-send-email-matt@codeblueprint.co.uk
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 drivers/firmware/efi/libstub/efistub.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 6b6548f..86ff7bf 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -5,6 +5,16 @@
 /* error code which can't be mistaken for valid address */
 #define EFI_ERROR	(~0UL)
 
+/*
+ * __init annotations should not be used in the EFI stub, since the code is
+ * either included in the decompressor (x86, ARM) where they have no effect,
+ * or the whole stub is __init annotated at the section level (arm64), by
+ * renaming the sections, in which case the __init annotation will be
+ * redundant, and will result in section names like .init.init.text, and our
+ * linker script does not expect that.
+ */
+#undef __init
+
 void efi_char16_printk(efi_system_table_t *, efi_char16_t *);
 
 efi_status_t efi_open_volume(efi_system_table_t *sys_table_arg, void *__image,

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [tip:efi/core] efi/arm-init: Use read-only early mappings
  2016-02-17 12:36 ` [PATCH 07/13] efi/arm-init: Use read-only early mappings Matt Fleming
@ 2016-02-22 12:15   ` =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?=
  0 siblings, 0 replies; 54+ messages in thread
From: =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?= @ 2016-02-22 12:15 UTC (permalink / raw)
  To: =?UTF-8?B?bGludXgtdGlwLWNvbW1pdHNAdmdlci5rZXJuZWwub3Jn?=
  Cc: torvalds, peterz, ard.biesheuvel, tglx, matt, mingo,
	linux-kernel, hpa, mark.rutland

Commit-ID:  2eec5dedf770dc85c1fdf6b86873165e61bb1fff
Gitweb:     http://git.kernel.org/tip/2eec5dedf770dc85c1fdf6b86873165e61bb1fff
Author:     Ard Biesheuvel <ard.biesheuvel@linaro.org>
AuthorDate: Wed, 17 Feb 2016 12:36:00 +0000
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 22 Feb 2016 08:26:27 +0100

efi/arm-init: Use read-only early mappings

The early mappings of the EFI system table contents and the UEFI memory
map are read-only from the OS point of view. So map them read-only to
protect them from inadvertent modification.

Tested-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/1455712566-16727-8-git-send-email-matt@codeblueprint.co.uk
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 drivers/firmware/efi/arm-init.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
index 9e15d57..aa1f743 100644
--- a/drivers/firmware/efi/arm-init.c
+++ b/drivers/firmware/efi/arm-init.c
@@ -61,8 +61,8 @@ static int __init uefi_init(void)
 	char vendor[100] = "unknown";
 	int i, retval;
 
-	efi.systab = early_memremap(efi_system_table,
-				    sizeof(efi_system_table_t));
+	efi.systab = early_memremap_ro(efi_system_table,
+				       sizeof(efi_system_table_t));
 	if (efi.systab == NULL) {
 		pr_warn("Unable to map EFI system table.\n");
 		return -ENOMEM;
@@ -86,8 +86,8 @@ static int __init uefi_init(void)
 			efi.systab->hdr.revision & 0xffff);
 
 	/* Show what we know for posterity */
-	c16 = early_memremap(efi_to_phys(efi.systab->fw_vendor),
-			     sizeof(vendor) * sizeof(efi_char16_t));
+	c16 = early_memremap_ro(efi_to_phys(efi.systab->fw_vendor),
+				sizeof(vendor) * sizeof(efi_char16_t));
 	if (c16) {
 		for (i = 0; i < (int) sizeof(vendor) - 1 && *c16; ++i)
 			vendor[i] = c16[i];
@@ -100,8 +100,8 @@ static int __init uefi_init(void)
 		efi.systab->hdr.revision & 0xffff, vendor);
 
 	table_size = sizeof(efi_config_table_64_t) * efi.systab->nr_tables;
-	config_tables = early_memremap(efi_to_phys(efi.systab->tables),
-				       table_size);
+	config_tables = early_memremap_ro(efi_to_phys(efi.systab->tables),
+					  table_size);
 	if (config_tables == NULL) {
 		pr_warn("Unable to map EFI config table array.\n");
 		retval = -ENOMEM;
@@ -185,7 +185,7 @@ void __init efi_init(void)
 	efi_system_table = params.system_table;
 
 	memmap.phys_map = params.mmap;
-	memmap.map = early_memremap(params.mmap, params.mmap_size);
+	memmap.map = early_memremap_ro(params.mmap, params.mmap_size);
 	if (memmap.map == NULL) {
 		/*
 		* If we are booting via UEFI, the UEFI memory map is the only

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [tip:efi/core] efi/arm: Check for LPAE support before booting a LPAE kernel
  2016-02-17 12:36 ` [PATCH 08/13] efi/arm: Check for LPAE support before booting a LPAE kernel Matt Fleming
@ 2016-02-22 12:15   ` =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?=
  0 siblings, 0 replies; 54+ messages in thread
From: =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?= @ 2016-02-22 12:15 UTC (permalink / raw)
  To: =?UTF-8?B?bGludXgtdGlwLWNvbW1pdHNAdmdlci5rZXJuZWwub3Jn?=
  Cc: matt, linux-kernel, tglx, hpa, jeremy.linton, mark.rutland,
	torvalds, peterz, ard.biesheuvel, mingo

Commit-ID:  2ec0f0a3a4bfab90eda8b81656f62e07abf2321f
Gitweb:     http://git.kernel.org/tip/2ec0f0a3a4bfab90eda8b81656f62e07abf2321f
Author:     Ard Biesheuvel <ard.biesheuvel@linaro.org>
AuthorDate: Wed, 17 Feb 2016 12:36:01 +0000
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 22 Feb 2016 08:26:27 +0100

efi/arm: Check for LPAE support before booting a LPAE kernel

A kernel built with support for LPAE cannot boot to a state where it
can inform the user about if it has to fail due to missing LPAE support
in the hardware.

If we happen to be booting via UEFI, we can fail gracefully so check
for LPAE support in the hardware on CONFIG_ARM_LPAE builds before
entering the kernel proper.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
Reviewed-by: Jeremy Linton <jeremy.linton@arm.com>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/1455712566-16727-9-git-send-email-matt@codeblueprint.co.uk
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 drivers/firmware/efi/libstub/arm32-stub.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/firmware/efi/libstub/arm32-stub.c b/drivers/firmware/efi/libstub/arm32-stub.c
index 495ebd6..6f42be4 100644
--- a/drivers/firmware/efi/libstub/arm32-stub.c
+++ b/drivers/firmware/efi/libstub/arm32-stub.c
@@ -9,6 +9,23 @@
 #include <linux/efi.h>
 #include <asm/efi.h>
 
+efi_status_t check_platform_features(efi_system_table_t *sys_table_arg)
+{
+	int block;
+
+	/* non-LPAE kernels can run anywhere */
+	if (!IS_ENABLED(CONFIG_ARM_LPAE))
+		return EFI_SUCCESS;
+
+	/* LPAE kernels need compatible hardware */
+	block = cpuid_feature_extract(CPUID_EXT_MMFR0, 0);
+	if (block < 5) {
+		pr_efi_err(sys_table_arg, "This LPAE kernel is not supported by your CPU\n");
+		return EFI_UNSUPPORTED;
+	}
+	return EFI_SUCCESS;
+}
+
 efi_status_t handle_kernel_image(efi_system_table_t *sys_table,
 				 unsigned long *image_addr,
 				 unsigned long *image_size,

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [tip:efi/core] efi/arm64: Check for h/w support before booting a >4 KB granular kernel
  2016-02-17 12:36 ` [PATCH 09/13] efi/arm64: Check for h/w support before booting a >4 KB granule kernel Matt Fleming
@ 2016-02-22 12:16   ` =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?=
  2016-03-06  3:35   ` [PATCH 09/13] efi/arm64: Check for h/w support before booting a >4 KB granule kernel Ard Biesheuvel
  1 sibling, 0 replies; 54+ messages in thread
From: =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?= @ 2016-02-22 12:16 UTC (permalink / raw)
  To: =?UTF-8?B?bGludXgtdGlwLWNvbW1pdHNAdmdlci5rZXJuZWwub3Jn?=
  Cc: peterz, jeremy.linton, mingo, matt, ard.biesheuvel, mark.rutland,
	torvalds, hpa, linux-kernel, suzuki.poulose, tglx

Commit-ID:  42b55734030c1f724d5f47aeb872e2cccd650d79
Gitweb:     http://git.kernel.org/tip/42b55734030c1f724d5f47aeb872e2cccd650d79
Author:     Ard Biesheuvel <ard.biesheuvel@linaro.org>
AuthorDate: Wed, 17 Feb 2016 12:36:02 +0000
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 22 Feb 2016 08:26:27 +0100

efi/arm64: Check for h/w support before booting a >4 KB granular kernel

A kernel built with support for a page size that is not supported by the
hardware it runs on cannot boot to a state where it can inform the user
about the failure.

If we happen to be booting via UEFI, we can fail gracefully so check
if the currently configured page size is supported by the hardware before
entering the kernel proper. Note that UEFI mandates support for 4 KB pages,
so in that case, no check is needed.

Tested-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
Reviewed-by: Jeremy Linton <jeremy.linton@arm.com>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/1455712566-16727-10-git-send-email-matt@codeblueprint.co.uk
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 drivers/firmware/efi/libstub/arm64-stub.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
index 9e03427..047fc34 100644
--- a/drivers/firmware/efi/libstub/arm64-stub.c
+++ b/drivers/firmware/efi/libstub/arm64-stub.c
@@ -12,6 +12,26 @@
 #include <linux/efi.h>
 #include <asm/efi.h>
 #include <asm/sections.h>
+#include <asm/sysreg.h>
+
+efi_status_t check_platform_features(efi_system_table_t *sys_table_arg)
+{
+	u64 tg;
+
+	/* UEFI mandates support for 4 KB granularity, no need to check */
+	if (IS_ENABLED(CONFIG_ARM64_4K_PAGES))
+		return EFI_SUCCESS;
+
+	tg = (read_cpuid(ID_AA64MMFR0_EL1) >> ID_AA64MMFR0_TGRAN_SHIFT) & 0xf;
+	if (tg != ID_AA64MMFR0_TGRAN_SUPPORTED) {
+		if (IS_ENABLED(CONFIG_ARM64_64K_PAGES))
+			pr_efi_err(sys_table_arg, "This 64 KB granular kernel is not supported by your CPU\n");
+		else
+			pr_efi_err(sys_table_arg, "This 16 KB granular kernel is not supported by your CPU\n");
+		return EFI_UNSUPPORTED;
+	}
+	return EFI_SUCCESS;
+}
 
 efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
 				 unsigned long *image_addr,

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [tip:efi/core] efi/arm*: Perform hardware compatibility check
  2016-02-17 12:36 ` [PATCH 10/13] efi/arm*: Perform hardware compatibility check Matt Fleming
@ 2016-02-22 12:16   ` =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?=
  0 siblings, 0 replies; 54+ messages in thread
From: =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?= @ 2016-02-22 12:16 UTC (permalink / raw)
  To: =?UTF-8?B?bGludXgtdGlwLWNvbW1pdHNAdmdlci5rZXJuZWwub3Jn?=
  Cc: mingo, mark.rutland, hpa, linux-kernel, peterz, jeremy.linton,
	tglx, matt, suzuki.poulose, ard.biesheuvel, torvalds

Commit-ID:  b9d6769b5678dbd6cb328d20716561d35b2b1510
Gitweb:     http://git.kernel.org/tip/b9d6769b5678dbd6cb328d20716561d35b2b1510
Author:     Ard Biesheuvel <ard.biesheuvel@linaro.org>
AuthorDate: Wed, 17 Feb 2016 12:36:03 +0000
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 22 Feb 2016 08:26:27 +0100

efi/arm*: Perform hardware compatibility check

Before proceeding with relocating the kernel and parsing the command line,
insert a call to check_platform_features() to allow an arch specific check
to be performed whether the current kernel can execute on the current
hardware.

Tested-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
Reviewed-by: Jeremy Linton <jeremy.linton@arm.com>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/1455712566-16727-11-git-send-email-matt@codeblueprint.co.uk
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 drivers/firmware/efi/libstub/arm-stub.c | 4 ++++
 drivers/firmware/efi/libstub/efistub.h  | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
index 3397902..6086a87 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/arm-stub.c
@@ -190,6 +190,10 @@ unsigned long efi_entry(void *handle, efi_system_table_t *sys_table,
 
 	pr_efi(sys_table, "Booting Linux Kernel...\n");
 
+	status = check_platform_features(sys_table);
+	if (status != EFI_SUCCESS)
+		goto fail;
+
 	/*
 	 * Get a handle to the loaded image protocol.  This is used to get
 	 * information about the running image, such as size and the command
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 86ff7bf..981c603 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -53,4 +53,6 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
 		     unsigned long desc_size, efi_memory_desc_t *runtime_map,
 		     int *count);
 
+efi_status_t check_platform_features(efi_system_table_t *sys_table_arg);
+
 #endif

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [tip:efi/core] x86/mm/pat: Don't implicitly allow _PAGE_RW in kernel_map_pages_in_pgd()
  2016-02-17 12:36 ` [PATCH 11/13] x86/mm/pageattr: Don't implicitly allow _PAGE_RW in kernel_map_pages_in_pgd() Matt Fleming
@ 2016-02-22 12:16   ` =?UTF-8?B?dGlwLWJvdCBmb3IgU2FpIFByYW5lZXRoIDx0aXBib3RAenl0b3IuY29tPg==?=
  0 siblings, 0 replies; 54+ messages in thread
From: =?UTF-8?B?dGlwLWJvdCBmb3IgU2FpIFByYW5lZXRoIDx0aXBib3RAenl0b3IuY29tPg==?= @ 2016-02-22 12:16 UTC (permalink / raw)
  To: =?UTF-8?B?bGludXgtdGlwLWNvbW1pdHNAdmdlci5rZXJuZWwub3Jn?=
  Cc: dvlasenk, luto, ard.biesheuvel, akpm, bp, matt, mcgrof,
	ricardo.neri, toshi.kani, hughd, sai.praneeth.prakhya, torvalds,
	ravi.v.shankar, tglx, linux-kernel, jlee, hpa, brgerst, peterz,
	mingo

Commit-ID:  15f003d20782a4079e078d16df57081ebd1fc150
Gitweb:     http://git.kernel.org/tip/15f003d20782a4079e078d16df57081ebd1fc150
Author:     Sai Praneeth <sai.praneeth.prakhya@intel.com>
AuthorDate: Wed, 17 Feb 2016 12:36:04 +0000
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 22 Feb 2016 08:26:28 +0100

x86/mm/pat: Don't implicitly allow _PAGE_RW in kernel_map_pages_in_pgd()

As part of the preparation for the EFI_MEMORY_RO flag added in the UEFI
2.5 specification, we need the ability to map pages in kernel page
tables without _PAGE_RW being set.

Modify kernel_map_pages_in_pgd() to require its callers to pass _PAGE_RW
if the pages need to be mapped read/write. Otherwise, we'll map the
pages as read-only.

Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Lee, Chun-Yi <jlee@suse.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Luis R. Rodriguez <mcgrof@suse.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ravi Shankar <ravi.v.shankar@intel.com>
Cc: Ricardo Neri <ricardo.neri@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Toshi Kani <toshi.kani@hp.com>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/1455712566-16727-12-git-send-email-matt@codeblueprint.co.uk
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/mm/pageattr.c         | 3 +++
 arch/x86/platform/efi/efi_64.c | 8 ++++----
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index bf312da..14c38ae 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -1971,6 +1971,9 @@ int kernel_map_pages_in_pgd(pgd_t *pgd, u64 pfn, unsigned long address,
 	if (!(page_flags & _PAGE_NX))
 		cpa.mask_clr = __pgprot(_PAGE_NX);
 
+	if (!(page_flags & _PAGE_RW))
+		cpa.mask_clr = __pgprot(_PAGE_RW);
+
 	cpa.mask_set = __pgprot(_PAGE_PRESENT | page_flags);
 
 	retval = __change_page_attr_set_clr(&cpa, 0);
diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
index b492521..b0965b2 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -233,7 +233,7 @@ int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages)
 	 * phys_efi_set_virtual_address_map().
 	 */
 	pfn = pa_memmap >> PAGE_SHIFT;
-	if (kernel_map_pages_in_pgd(pgd, pfn, pa_memmap, num_pages, _PAGE_NX)) {
+	if (kernel_map_pages_in_pgd(pgd, pfn, pa_memmap, num_pages, _PAGE_NX | _PAGE_RW)) {
 		pr_err("Error ident-mapping new memmap (0x%lx)!\n", pa_memmap);
 		return 1;
 	}
@@ -262,7 +262,7 @@ int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages)
 		pfn = md->phys_addr >> PAGE_SHIFT;
 		npages = md->num_pages;
 
-		if (kernel_map_pages_in_pgd(pgd, pfn, md->phys_addr, npages, 0)) {
+		if (kernel_map_pages_in_pgd(pgd, pfn, md->phys_addr, npages, _PAGE_RW)) {
 			pr_err("Failed to map 1:1 memory\n");
 			return 1;
 		}
@@ -279,7 +279,7 @@ int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages)
 	text = __pa(_text);
 	pfn = text >> PAGE_SHIFT;
 
-	if (kernel_map_pages_in_pgd(pgd, pfn, text, npages, 0)) {
+	if (kernel_map_pages_in_pgd(pgd, pfn, text, npages, _PAGE_RW)) {
 		pr_err("Failed to map kernel text 1:1\n");
 		return 1;
 	}
@@ -294,7 +294,7 @@ void __init efi_cleanup_page_tables(unsigned long pa_memmap, unsigned num_pages)
 
 static void __init __map_region(efi_memory_desc_t *md, u64 va)
 {
-	unsigned long flags = 0;
+	unsigned long flags = _PAGE_RW;
 	unsigned long pfn;
 	pgd_t *pgd = efi_pgd;
 

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [tip:efi/core] x86/efi: Map EFI_MEMORY_{XP,RO} memory region bits to EFI page tables
  2016-02-17 12:36 ` [PATCH 12/13] x86/efi: Map EFI_MEMORY_{XP,RO} memory region bits to EFI page tables Matt Fleming
@ 2016-02-22 12:17   ` =?UTF-8?B?dGlwLWJvdCBmb3IgU2FpIFByYW5lZXRoIDx0aXBib3RAenl0b3IuY29tPg==?=
  0 siblings, 0 replies; 54+ messages in thread
From: =?UTF-8?B?dGlwLWJvdCBmb3IgU2FpIFByYW5lZXRoIDx0aXBib3RAenl0b3IuY29tPg==?= @ 2016-02-22 12:17 UTC (permalink / raw)
  To: =?UTF-8?B?bGludXgtdGlwLWNvbW1pdHNAdmdlci5rZXJuZWwub3Jn?=
  Cc: dvlasenk, toshi.kani, tglx, peterz, akpm, ricardo.neri,
	ard.biesheuvel, matt, mingo, luto, sai.praneeth.prakhya, bp,
	ravi.v.shankar, keescook, jlee, torvalds, mcgrof, brgerst,
	linux-kernel, hpa

Commit-ID:  6d0cc887d571e96f928be83f094322451fd4bf6f
Gitweb:     http://git.kernel.org/tip/6d0cc887d571e96f928be83f094322451fd4bf6f
Author:     Sai Praneeth <sai.praneeth.prakhya@intel.com>
AuthorDate: Wed, 17 Feb 2016 12:36:05 +0000
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 22 Feb 2016 08:26:28 +0100

x86/efi: Map EFI_MEMORY_{XP,RO} memory region bits to EFI page tables

Now that we have EFI memory region bits that indicate which regions do
not need execute permission or read/write permission in the page tables,
let's use them.

We also check for EFI_NX_PE_DATA and only enforce the restrictive
mappings if it's present (to allow us to ignore buggy firmware that sets
bits it didn't mean to and to preserve backwards compatibility).

Instead of assuming that firmware would set appropriate attributes in
memory descriptor like EFI_MEMORY_RO for code and EFI_MEMORY_XP for
data, we can expect some firmware out there which might only set *type*
in memory descriptor to be EFI_RUNTIME_SERVICES_CODE or
EFI_RUNTIME_SERVICES_DATA leaving away attribute. This will lead to
improper mappings of EFI runtime regions. In order to avoid it, we check
attribute and type of memory descriptor to update mappings and moreover
Windows works this way.

Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Lee, Chun-Yi <jlee@suse.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Luis R. Rodriguez <mcgrof@suse.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ravi Shankar <ravi.v.shankar@intel.com>
Cc: Ricardo Neri <ricardo.neri@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Toshi Kani <toshi.kani@hp.com>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/1455712566-16727-13-git-send-email-matt@codeblueprint.co.uk
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/efi.h     |  2 +-
 arch/x86/platform/efi/efi.c    |  9 +++++++--
 arch/x86/platform/efi/efi_32.c |  2 +-
 arch/x86/platform/efi/efi_64.c | 45 ++++++++++++++++++++++++++++++++++++++----
 4 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index 8fd9e63..7bb206f 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -141,7 +141,7 @@ extern int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pa
 extern void __init efi_cleanup_page_tables(unsigned long pa_memmap, unsigned num_pages);
 extern void __init old_map_region(efi_memory_desc_t *md);
 extern void __init runtime_code_page_mkexec(void);
-extern void __init efi_runtime_mkexec(void);
+extern void __init efi_runtime_update_mappings(void);
 extern void __init efi_dump_pagetable(void);
 extern void __init efi_apply_memmap_quirks(void);
 extern int __init efi_reuse_config(u64 tables, int nr_tables);
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index e80826e..994a7df8 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -934,7 +934,6 @@ static void __init __efi_enter_virtual_mode(void)
 	}
 
 	efi_sync_low_kernel_mappings();
-	efi_dump_pagetable();
 
 	if (efi_is_native()) {
 		status = phys_efi_set_virtual_address_map(
@@ -972,7 +971,13 @@ static void __init __efi_enter_virtual_mode(void)
 
 	efi.set_virtual_address_map = NULL;
 
-	efi_runtime_mkexec();
+	/*
+	 * Apply more restrictive page table mapping attributes now that
+	 * SVAM() has been called and the firmware has performed all
+	 * necessary relocation fixups for the new virtual addresses.
+	 */
+	efi_runtime_update_mappings();
+	efi_dump_pagetable();
 
 	/*
 	 * We mapped the descriptor array into the EFI pagetable above
diff --git a/arch/x86/platform/efi/efi_32.c b/arch/x86/platform/efi/efi_32.c
index 58d669b..338402b 100644
--- a/arch/x86/platform/efi/efi_32.c
+++ b/arch/x86/platform/efi/efi_32.c
@@ -90,7 +90,7 @@ void __init efi_call_phys_epilog(pgd_t *save_pgd)
 	__flush_tlb_all();
 }
 
-void __init efi_runtime_mkexec(void)
+void __init efi_runtime_update_mappings(void)
 {
 	if (__supported_pte_mask & _PAGE_NX)
 		runtime_code_page_mkexec();
diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
index b0965b2..40d2f44 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -393,13 +393,50 @@ void __init parse_efi_setup(u64 phys_addr, u32 data_len)
 	efi_setup = phys_addr + sizeof(struct setup_data);
 }
 
-void __init efi_runtime_mkexec(void)
+void __init efi_runtime_update_mappings(void)
 {
-	if (!efi_enabled(EFI_OLD_MEMMAP))
+	unsigned long pfn;
+	pgd_t *pgd = efi_pgd;
+	efi_memory_desc_t *md;
+	void *p;
+
+	if (efi_enabled(EFI_OLD_MEMMAP)) {
+		if (__supported_pte_mask & _PAGE_NX)
+			runtime_code_page_mkexec();
+		return;
+	}
+
+	if (!efi_enabled(EFI_NX_PE_DATA))
 		return;
 
-	if (__supported_pte_mask & _PAGE_NX)
-		runtime_code_page_mkexec();
+	for (p = memmap.map; p < memmap.map_end; p += memmap.desc_size) {
+		unsigned long pf = 0;
+		md = p;
+
+		if (!(md->attribute & EFI_MEMORY_RUNTIME))
+			continue;
+
+		if (!(md->attribute & EFI_MEMORY_WB))
+			pf |= _PAGE_PCD;
+
+		if ((md->attribute & EFI_MEMORY_XP) ||
+			(md->type == EFI_RUNTIME_SERVICES_DATA))
+			pf |= _PAGE_NX;
+
+		if (!(md->attribute & EFI_MEMORY_RO) &&
+			(md->type != EFI_RUNTIME_SERVICES_CODE))
+			pf |= _PAGE_RW;
+
+		/* Update the 1:1 mapping */
+		pfn = md->phys_addr >> PAGE_SHIFT;
+		if (kernel_map_pages_in_pgd(pgd, pfn, md->phys_addr, md->num_pages, pf))
+			pr_warn("Error mapping PA 0x%llx -> VA 0x%llx!\n",
+				   md->phys_addr, md->virt_addr);
+
+		if (kernel_map_pages_in_pgd(pgd, pfn, md->virt_addr, md->num_pages, pf))
+			pr_warn("Error mapping PA 0x%llx -> VA 0x%llx!\n",
+				   md->phys_addr, md->virt_addr);
+	}
 }
 
 void __init efi_dump_pagetable(void)

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [tip:efi/core] x86/efi: Only map kernel text for EFI mixed mode
  2016-02-17 12:36 ` [PATCH 13/13] x86/efi: Only map kernel text for EFI mixed mode Matt Fleming
@ 2016-02-22 12:17   ` =?UTF-8?B?dGlwLWJvdCBmb3IgU2FpIFByYW5lZXRoIDx0aXBib3RAenl0b3IuY29tPg==?=
  0 siblings, 0 replies; 54+ messages in thread
From: =?UTF-8?B?dGlwLWJvdCBmb3IgU2FpIFByYW5lZXRoIDx0aXBib3RAenl0b3IuY29tPg==?= @ 2016-02-22 12:17 UTC (permalink / raw)
  To: =?UTF-8?B?bGludXgtdGlwLWNvbW1pdHNAdmdlci5rZXJuZWwub3Jn?=
  Cc: ravi.v.shankar, sai.praneeth.prakhya, ard.biesheuvel, peterz,
	tglx, ricardo.neri, matt, torvalds, hpa, bp, linux-kernel, mingo

Commit-ID:  2ad510dc372c2caac9aada9ff6dd10e787616e1d
Gitweb:     http://git.kernel.org/tip/2ad510dc372c2caac9aada9ff6dd10e787616e1d
Author:     Sai Praneeth <sai.praneeth.prakhya@intel.com>
AuthorDate: Wed, 17 Feb 2016 12:36:06 +0000
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 22 Feb 2016 08:26:28 +0100

x86/efi: Only map kernel text for EFI mixed mode

The correct symbol to use when figuring out the size of the kernel
text is '_etext', not '_end' which is the symbol for the entire kernel
image includes data and debug sections.

Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ravi Shankar <ravi.v.shankar@intel.com>
Cc: Ricardo Neri <ricardo.neri@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/1455712566-16727-14-git-send-email-matt@codeblueprint.co.uk
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/platform/efi/efi_64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
index 40d2f44..49e4dd4 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -275,7 +275,7 @@ int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages)
 	efi_scratch.phys_stack = virt_to_phys(page_address(page));
 	efi_scratch.phys_stack += PAGE_SIZE; /* stack grows down */
 
-	npages = (_end - _text) >> PAGE_SHIFT;
+	npages = (_etext - _text) >> PAGE_SHIFT;
 	text = __pa(_text);
 	pfn = text >> PAGE_SHIFT;
 

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings
  2016-02-22 12:13   ` [tip:efi/core] x86/mm/pat: " =?UTF-8?B?dGlwLWJvdCBmb3IgU2FpIFByYW5lZXRoIDx0aXBib3RAenl0b3IuY29tPg==?=
@ 2016-02-23 17:47     ` Andy Lutomirski
  2016-02-23 18:08       ` Linus Torvalds
  2016-02-24  0:50       ` Sai Praneeth Prakhya
  0 siblings, 2 replies; 54+ messages in thread
From: Andy Lutomirski @ 2016-02-23 17:47 UTC (permalink / raw)
  To: Ravi V. Shankar, Toshi Kani, Brian Gerst, Thomas Gleixner,
	Ingo Molnar, Sai Praneeth Prakhya, Linus Torvalds,
	Luis Rodriguez, Andrew Morton, Denys Vlasenko, Matt Fleming,
	H. Peter Anvin, linux-kernel, Peter Zijlstra, Borislav Petkov,
	ricardo.neri, Hugh Dickins, Ard Biesheuvel

On Feb 23, 2016 1:09 AM, <&quot;tip-bot for Sai Praneeth
&lt;tipbot@zytor.com&gt;&quot;@zytor.com> wrote:

Something's wrong with tip-bot.  This should say:


commit 397630150632639b3ca5b4414accd5011c45e276
Author: Sai Praneeth <sai.praneeth.prakhya@intel.com>
Date:   Wed Feb 17 12:35:56 2016 +0000

    x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings

    Since EFI page tables can be treated as kernel page tables they should
    be global. All the other page mapping functions in pageattr.c set the
    _PAGE_GLOBAL bit and we want to avoid inconsistencies when we map a page
    in the EFI code paths, for example when that page is split in
    __split_large_page(), etc. It also makes it easier to validate that the
    EFI region mappings have the correct attributes because there are fewer
    differences compared with regular kernel mappings.

But the actual patch is:


@@ -909,6 +909,20 @@ static void populate_pte(struct cpa_data *cpa,

        pte = pte_offset_kernel(pmd, start);

+       /*
+        * Set the GLOBAL flags only if the PRESENT flag is
+        * set otherwise pte_present will return true even on
+        * a non present pte. The canon_pgprot will clear
+        * _PAGE_GLOBAL for the ancient hardware that doesn't
+        * support it.
+        */
+       if (pgprot_val(pgprot) & _PAGE_PRESENT)
+               pgprot_val(pgprot) |= _PAGE_GLOBAL;
+       else
+               pgprot_val(pgprot) &= ~_PAGE_GLOBAL;
+
+       pgprot = canon_pgprot(pgprot);
+

The comment is confusing.  This code is setting GLOBAL if PRESENT is
set even if not requested, but the comment is about setting GLOBAL
*only* if PRESENT is set.

Can you explain:

a) Why this wasn't already broken. (were there no callers who set
GLOBAL but not PRESENT?  If there weren't any, why is that part
needed?)

b) Why setting GLOBAL for EFI mappings is useful.

c) Why setting GLOBAL for EFI mappings is safe.  Don't we unmap the
EFI mappings when we're not actively using them in new kernels?  If
so, don't we explicitly want them *not* to be GLOBAL to avoid needing
an extra-expensive global flush?

d) Why this doesn't break any non-EFI code.

--Andy

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings
  2016-02-23 17:47     ` Andy Lutomirski
@ 2016-02-23 18:08       ` Linus Torvalds
  2016-02-23 18:12         ` Borislav Petkov
                           ` (2 more replies)
  2016-02-24  0:50       ` Sai Praneeth Prakhya
  1 sibling, 3 replies; 54+ messages in thread
From: Linus Torvalds @ 2016-02-23 18:08 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Ravi V. Shankar, Toshi Kani, Brian Gerst, Thomas Gleixner,
	Ingo Molnar, Sai Praneeth Prakhya, Luis Rodriguez, Andrew Morton,
	Denys Vlasenko, Matt Fleming, H. Peter Anvin, linux-kernel,
	Peter Zijlstra, Borislav Petkov, ricardo.neri, Hugh Dickins,
	Ard Biesheuvel

On Tue, Feb 23, 2016 at 9:47 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Feb 23, 2016 1:09 AM, <&quot;tip-bot for Sai Praneeth
> &lt;tipbot@zytor.com&gt;&quot;@zytor.com> wrote:
>
> Something's wrong with tip-bot.  This should say:

Yeah, there's about 50 tipbot emails that are just pure garbage. They
don't even show in my mailers, because they are so corrupt.

The raw email has some insane encoding too, for reasons I can't begin to fathom.

This is an example of what tipbot *used* to send out in the headers:

  From: tip-bot for Dave Hansen <tipbot@zytor.com>
  Subject: [tip:mm/pkeys] mm/core, x86/mm/pkeys:
    Add execute-only protection keys support

and this is what it sent out in the last crazy setup:

  From:   =?UTF-8?B?dGlwLWJvdCBmb3IgU2FpIFByYW5lZXRoIDx0aXBib3RAenl0b3IuY29tPg==?=@zytor.com
  Subject:
   =?UTF-8?B?W3RpcDplZmkvY29yZV0geDg2L21tL3BhdDogVXNlIF9QQUdFX0dMT0JBTCBiaXQ=?=
   =?UTF-8?B?IGZvciBFRkkgcGFnZSB0YWJsZSBtYXBwaW5ncw==?=

despite neither subject nor author having any odd characters in them.

(That's just two header lines - all the other ones are corrupt in
similar ways too)

The thing that seems to really make things unreadable is that the
content encoding lines have this corrupted quoting too:

  MIME-Version: =?UTF-8?B?MS4w?=
  Content-Transfer-Encoding: =?UTF-8?B?OGJpdA==?=
  Content-Type: =?UTF-8?B?dGV4dC9wbGFpbjsgY2hhcnNldD1VVEYtOA==?=
  Content-Disposition: =?UTF-8?B?aW5saW5l?=

rather than what it *should* be:

  Content-Transfer-Encoding: 8bit
  Content-Type: text/plain; charset=UTF-8
  Content-Disposition: inline

so the whole header situation is a complete mess.

The fact that you can see the patch at all and comment on the
*contents* of the email is impressive. My mail reader just says "this
is garbage" and shows me nothing at all.

               Linus

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings
  2016-02-23 18:08       ` Linus Torvalds
@ 2016-02-23 18:12         ` Borislav Petkov
  2016-02-24  2:09         ` H. Peter Anvin
  2016-02-25  8:59         ` Ingo Molnar
  2 siblings, 0 replies; 54+ messages in thread
From: Borislav Petkov @ 2016-02-23 18:12 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Andy Lutomirski, Ravi V. Shankar, Toshi Kani, Brian Gerst,
	Thomas Gleixner, Ingo Molnar, Sai Praneeth Prakhya,
	Luis Rodriguez, Andrew Morton, Denys Vlasenko, Matt Fleming,
	H. Peter Anvin, linux-kernel, Peter Zijlstra, ricardo.neri,
	Hugh Dickins, Ard Biesheuvel

On Tue, Feb 23, 2016 at 10:08:06AM -0800, Linus Torvalds wrote:
> The fact that you can see the patch at all and comment on the
> *contents* of the email is impressive. My mail reader just says "this
> is garbage" and shows me nothing at all.

Yeah, mutt says:

[-- application/x-=?UTF-8?B?dGV4dC9wbGFpbjsgY2hhcnNldD1VVEYtOA==?= is unsupported (use 'v' to view this part) --]

in the mail body. But then one can open it and it defaults to text:

---Attachment: application/x-=?UTF-8?B?dGV4dC9wbGFpbjsgY2hhcnNldD1VVEYtOA==?= (all)
No matching mailcap entry found.  Viewing as text.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings
  2016-02-23 17:47     ` Andy Lutomirski
  2016-02-23 18:08       ` Linus Torvalds
@ 2016-02-24  0:50       ` Sai Praneeth Prakhya
  2016-02-24  2:43         ` Andy Lutomirski
  1 sibling, 1 reply; 54+ messages in thread
From: Sai Praneeth Prakhya @ 2016-02-24  0:50 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Ravi V. Shankar, Toshi Kani, Brian Gerst, Thomas Gleixner,
	Ingo Molnar, Linus Torvalds, Luis Rodriguez, Andrew Morton,
	Denys Vlasenko, Matt Fleming, H. Peter Anvin, linux-kernel,
	Peter Zijlstra, Borislav Petkov, ricardo.neri, Hugh Dickins,
	Ard Biesheuvel

On Tue, 2016-02-23 at 09:47 -0800, Andy Lutomirski wrote:
> On Feb 23, 2016 1:09 AM, <&quot;tip-bot for Sai Praneeth
> &lt;tipbot@zytor.com&gt;&quot;@zytor.com> wrote:
> 
> Something's wrong with tip-bot.  This should say:
> 
> 
> commit 397630150632639b3ca5b4414accd5011c45e276
> Author: Sai Praneeth <sai.praneeth.prakhya@intel.com>
> Date:   Wed Feb 17 12:35:56 2016 +0000
> 
>     x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings
> 
>     Since EFI page tables can be treated as kernel page tables they should
>     be global. All the other page mapping functions in pageattr.c set the
>     _PAGE_GLOBAL bit and we want to avoid inconsistencies when we map a page
>     in the EFI code paths, for example when that page is split in
>     __split_large_page(), etc. It also makes it easier to validate that the
>     EFI region mappings have the correct attributes because there are fewer
>     differences compared with regular kernel mappings.
> 
> But the actual patch is:
> 
> 
> @@ -909,6 +909,20 @@ static void populate_pte(struct cpa_data *cpa,
> 
>         pte = pte_offset_kernel(pmd, start);
> 
> +       /*
> +        * Set the GLOBAL flags only if the PRESENT flag is
> +        * set otherwise pte_present will return true even on
> +        * a non present pte. The canon_pgprot will clear
> +        * _PAGE_GLOBAL for the ancient hardware that doesn't
> +        * support it.
> +        */
> +       if (pgprot_val(pgprot) & _PAGE_PRESENT)
> +               pgprot_val(pgprot) |= _PAGE_GLOBAL;
> +       else
> +               pgprot_val(pgprot) &= ~_PAGE_GLOBAL;
> +
> +       pgprot = canon_pgprot(pgprot);
> +
> 
> The comment is confusing.  This code is setting GLOBAL if PRESENT is
> set even if not requested, but the comment is about setting GLOBAL
> *only* if PRESENT is set.

As you rightly said the code is about making a page GLOBAL if PRESENT is
set and we do set PRESENT bit before mapping so that page is GLOBAL.
This code was taken from the other parts of pageattr.c. The point is
that we don't want differences between whether things were mapped in the
EFI page tables directly (i.e. using populate_pte()) or later split from
large pages via the split_large_page() code path. If this is still
confusing could you please elaborate on it further.

> 
> Can you explain:
> 
> a) Why this wasn't already broken. (were there no callers who set
> GLOBAL but not PRESENT?  If there weren't any, why is that part
> needed?)
> 

I don't think previous implementation is broken and this is not a bug
fix as such. Before this patch some EFI region mappings had GLOBAL bit
set (which followed split large page path) and some aren't (which used
populate_pte). This patch just aligns the mappings done via two
different code paths as mentioned above. As a whole it also maintains
consistency with kernel mappings.

> b) Why setting GLOBAL for EFI mappings is useful.

We did this for consistency among EFI mappings. This has some advantages
as mentioned in the commit message. It also makes it less confusing when
starting at the PGT_DUMP traces if _PAGE_GLOBAL is used consistently.

We don't actually do anything special with _PAGE_GLOBAL in EFI.

> c) Why setting GLOBAL for EFI mappings is safe.  Don't we unmap the
> EFI mappings when we're not actively using them in new kernels?  If
> so, don't we explicitly want them *not* to be GLOBAL to avoid needing
> an extra-expensive global flush?

This is a valid point. I know that EFI runtime regions persist during
and after boot if we have a UEFI firmware and other commits made EFI
regions have separate page table but I am not clear about the effect of
global flush. I think Matt/Boris could comment on it.

> d) Why this doesn't break any non-EFI code.

We touch this code path only when mapping EFI runtime regions to VA
space, i.e. we added pgd field in cpa only as a support for mapping efi
runtime regions.

> --Andy

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings
  2016-02-23 18:08       ` Linus Torvalds
  2016-02-23 18:12         ` Borislav Petkov
@ 2016-02-24  2:09         ` H. Peter Anvin
  2016-02-24  2:13           ` H. Peter Anvin
  2016-02-25  8:59         ` Ingo Molnar
  2 siblings, 1 reply; 54+ messages in thread
From: H. Peter Anvin @ 2016-02-24  2:09 UTC (permalink / raw)
  To: Linus Torvalds, Andy Lutomirski
  Cc: Ravi V. Shankar, Toshi Kani, Brian Gerst, Thomas Gleixner,
	Ingo Molnar, Sai Praneeth Prakhya, Luis Rodriguez, Andrew Morton,
	Denys Vlasenko, Matt Fleming, linux-kernel, Peter Zijlstra,
	Borislav Petkov, ricardo.neri, Hugh Dickins, Ard Biesheuvel

On February 23, 2016 10:08:06 AM PST, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>On Tue, Feb 23, 2016 at 9:47 AM, Andy Lutomirski <luto@amacapital.net>
>wrote:
>> On Feb 23, 2016 1:09 AM, <&quot;tip-bot for Sai Praneeth
>> &lt;tipbot@zytor.com&gt;&quot;@zytor.com> wrote:
>>
>> Something's wrong with tip-bot.  This should say:
>
>Yeah, there's about 50 tipbot emails that are just pure garbage. They
>don't even show in my mailers, because they are so corrupt.
>
>The raw email has some insane encoding too, for reasons I can't begin
>to fathom.
>
>This is an example of what tipbot *used* to send out in the headers:
>
>  From: tip-bot for Dave Hansen <tipbot@zytor.com>
>  Subject: [tip:mm/pkeys] mm/core, x86/mm/pkeys:
>    Add execute-only protection keys support
>
>and this is what it sent out in the last crazy setup:
>
>From:  
>=?UTF-8?B?dGlwLWJvdCBmb3IgU2FpIFByYW5lZXRoIDx0aXBib3RAenl0b3IuY29tPg==?=@zytor.com
>  Subject:
>=?UTF-8?B?W3RpcDplZmkvY29yZV0geDg2L21tL3BhdDogVXNlIF9QQUdFX0dMT0JBTCBiaXQ=?=
>   =?UTF-8?B?IGZvciBFRkkgcGFnZSB0YWJsZSBtYXBwaW5ncw==?=
>
>despite neither subject nor author having any odd characters in them.
>
>(That's just two header lines - all the other ones are corrupt in
>similar ways too)
>
>The thing that seems to really make things unreadable is that the
>content encoding lines have this corrupted quoting too:
>
>  MIME-Version: =?UTF-8?B?MS4w?=
>  Content-Transfer-Encoding: =?UTF-8?B?OGJpdA==?=
>  Content-Type: =?UTF-8?B?dGV4dC9wbGFpbjsgY2hhcnNldD1VVEYtOA==?=
>  Content-Disposition: =?UTF-8?B?aW5saW5l?=
>
>rather than what it *should* be:
>
>  Content-Transfer-Encoding: 8bit
>  Content-Type: text/plain; charset=UTF-8
>  Content-Disposition: inline
>
>so the whole header situation is a complete mess.
>
>The fact that you can see the patch at all and comment on the
>*contents* of the email is impressive. My mail reader just says "this
>is garbage" and shows me nothing at all.
>
>               Linus

Someone decided to change the behavior of the Perl module I used for encoding to unconditionally encode almost everything, claiming some kind of strict RFC compliance.  An upgrade caused this to happen.  I have switched modules to one which should do what one actually wants.
-- 
Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings
  2016-02-24  2:09         ` H. Peter Anvin
@ 2016-02-24  2:13           ` H. Peter Anvin
  0 siblings, 0 replies; 54+ messages in thread
From: H. Peter Anvin @ 2016-02-24  2:13 UTC (permalink / raw)
  To: Linus Torvalds, Andy Lutomirski
  Cc: Ravi V. Shankar, Toshi Kani, Brian Gerst, Thomas Gleixner,
	Ingo Molnar, Sai Praneeth Prakhya, Luis Rodriguez, Andrew Morton,
	Denys Vlasenko, Matt Fleming, linux-kernel, Peter Zijlstra,
	Borislav Petkov, ricardo.neri, Hugh Dickins, Ard Biesheuvel

On February 23, 2016 6:09:19 PM PST, "H. Peter Anvin" <hpa@zytor.com> wrote:
>On February 23, 2016 10:08:06 AM PST, Linus Torvalds
><torvalds@linux-foundation.org> wrote:
>>On Tue, Feb 23, 2016 at 9:47 AM, Andy Lutomirski <luto@amacapital.net>
>>wrote:
>>> On Feb 23, 2016 1:09 AM, <&quot;tip-bot for Sai Praneeth
>>> &lt;tipbot@zytor.com&gt;&quot;@zytor.com> wrote:
>>>
>>> Something's wrong with tip-bot.  This should say:
>>
>>Yeah, there's about 50 tipbot emails that are just pure garbage. They
>>don't even show in my mailers, because they are so corrupt.
>>
>>The raw email has some insane encoding too, for reasons I can't begin
>>to fathom.
>>
>>This is an example of what tipbot *used* to send out in the headers:
>>
>>  From: tip-bot for Dave Hansen <tipbot@zytor.com>
>>  Subject: [tip:mm/pkeys] mm/core, x86/mm/pkeys:
>>    Add execute-only protection keys support
>>
>>and this is what it sent out in the last crazy setup:
>>
>>From:  
>>=?UTF-8?B?dGlwLWJvdCBmb3IgU2FpIFByYW5lZXRoIDx0aXBib3RAenl0b3IuY29tPg==?=@zytor.com
>>  Subject:
>>=?UTF-8?B?W3RpcDplZmkvY29yZV0geDg2L21tL3BhdDogVXNlIF9QQUdFX0dMT0JBTCBiaXQ=?=
>>   =?UTF-8?B?IGZvciBFRkkgcGFnZSB0YWJsZSBtYXBwaW5ncw==?=
>>
>>despite neither subject nor author having any odd characters in them.
>>
>>(That's just two header lines - all the other ones are corrupt in
>>similar ways too)
>>
>>The thing that seems to really make things unreadable is that the
>>content encoding lines have this corrupted quoting too:
>>
>>  MIME-Version: =?UTF-8?B?MS4w?=
>>  Content-Transfer-Encoding: =?UTF-8?B?OGJpdA==?=
>>  Content-Type: =?UTF-8?B?dGV4dC9wbGFpbjsgY2hhcnNldD1VVEYtOA==?=
>>  Content-Disposition: =?UTF-8?B?aW5saW5l?=
>>
>>rather than what it *should* be:
>>
>>  Content-Transfer-Encoding: 8bit
>>  Content-Type: text/plain; charset=UTF-8
>>  Content-Disposition: inline
>>
>>so the whole header situation is a complete mess.
>>
>>The fact that you can see the patch at all and comment on the
>>*contents* of the email is impressive. My mail reader just says "this
>>is garbage" and shows me nothing at all.
>>
>>               Linus
>
>Someone decided to change the behavior of the Perl module I used for
>encoding to unconditionally encode almost everything, claiming some
>kind of strict RFC compliance.  An upgrade caused this to happen.  I
>have switched modules to one which should do what one actually wants.

For the record: I implemented escaping only to placate vger's spam filters...
-- 
Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings
  2016-02-24  0:50       ` Sai Praneeth Prakhya
@ 2016-02-24  2:43         ` Andy Lutomirski
  2016-02-24 14:10           ` Matt Fleming
  0 siblings, 1 reply; 54+ messages in thread
From: Andy Lutomirski @ 2016-02-24  2:43 UTC (permalink / raw)
  To: Sai Praneeth Prakhya
  Cc: Ravi V. Shankar, Toshi Kani, Brian Gerst, Thomas Gleixner,
	Ingo Molnar, Linus Torvalds, Luis Rodriguez, Andrew Morton,
	Denys Vlasenko, Matt Fleming, H. Peter Anvin, linux-kernel,
	Peter Zijlstra, Borislav Petkov, ricardo.neri, Hugh Dickins,
	Ard Biesheuvel

On Tue, Feb 23, 2016 at 4:50 PM, Sai Praneeth Prakhya
<sai.praneeth.prakhya@intel.com> wrote:
> On Tue, 2016-02-23 at 09:47 -0800, Andy Lutomirski wrote:
>> On Feb 23, 2016 1:09 AM, <&quot;tip-bot for Sai Praneeth
>> &lt;tipbot@zytor.com&gt;&quot;@zytor.com> wrote:
>>
>> Something's wrong with tip-bot.  This should say:
>>
>>
>> commit 397630150632639b3ca5b4414accd5011c45e276
>> Author: Sai Praneeth <sai.praneeth.prakhya@intel.com>
>> Date:   Wed Feb 17 12:35:56 2016 +0000
>>
>>     x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings
>>
>>     Since EFI page tables can be treated as kernel page tables they should
>>     be global. All the other page mapping functions in pageattr.c set the
>>     _PAGE_GLOBAL bit and we want to avoid inconsistencies when we map a page
>>     in the EFI code paths, for example when that page is split in
>>     __split_large_page(), etc. It also makes it easier to validate that the
>>     EFI region mappings have the correct attributes because there are fewer
>>     differences compared with regular kernel mappings.
>>
>> But the actual patch is:
>>
>>
>> @@ -909,6 +909,20 @@ static void populate_pte(struct cpa_data *cpa,
>>
>>         pte = pte_offset_kernel(pmd, start);
>>
>> +       /*
>> +        * Set the GLOBAL flags only if the PRESENT flag is
>> +        * set otherwise pte_present will return true even on
>> +        * a non present pte. The canon_pgprot will clear
>> +        * _PAGE_GLOBAL for the ancient hardware that doesn't
>> +        * support it.
>> +        */
>> +       if (pgprot_val(pgprot) & _PAGE_PRESENT)
>> +               pgprot_val(pgprot) |= _PAGE_GLOBAL;
>> +       else
>> +               pgprot_val(pgprot) &= ~_PAGE_GLOBAL;
>> +
>> +       pgprot = canon_pgprot(pgprot);
>> +
>>
>> The comment is confusing.  This code is setting GLOBAL if PRESENT is
>> set even if not requested, but the comment is about setting GLOBAL
>> *only* if PRESENT is set.
>
> As you rightly said the code is about making a page GLOBAL if PRESENT is
> set and we do set PRESENT bit before mapping so that page is GLOBAL.
> This code was taken from the other parts of pageattr.c. The point is
> that we don't want differences between whether things were mapped in the
> EFI page tables directly (i.e. using populate_pte()) or later split from
> large pages via the split_large_page() code path. If this is still
> confusing could you please elaborate on it further.

At least the comment should say "Set the GLOBAL flag if and only
if...".  But why is this code here in the first place?  What is
passing a pgprot with global unset into this code in the first place?

>
>>
>> Can you explain:
>>
>> a) Why this wasn't already broken. (were there no callers who set
>> GLOBAL but not PRESENT?  If there weren't any, why is that part
>> needed?)
>>
>
> I don't think previous implementation is broken and this is not a bug
> fix as such. Before this patch some EFI region mappings had GLOBAL bit
> set (which followed split large page path) and some aren't (which used
> populate_pte). This patch just aligns the mappings done via two
> different code paths as mentioned above. As a whole it also maintains
> consistency with kernel mappings.

But your code also *clears* GLOBAL if PRESENT is clear, and your
comment talks about that.  Does this ever actually happen in practice?

I'd much rather see WARN_ON_ONCE((pgprot_val(pgprot) & (_PAGE_PRESENT
| _PAGE_GLOBAL)) == _PAGE_GLOBAL) in here, if that makes sense in the
context of the callers of the function.

>
>> b) Why setting GLOBAL for EFI mappings is useful.
>
> We did this for consistency among EFI mappings. This has some advantages
> as mentioned in the commit message. It also makes it less confusing when
> starting at the PGT_DUMP traces if _PAGE_GLOBAL is used consistently.
>
> We don't actually do anything special with _PAGE_GLOBAL in EFI.

You're making a choice of whether to set _PAGE_GLOBAL, and I think
you've made the wrong choice.

Normally, the only pages with are _PAGE_GLOBAL are those that are in
the normal kernel mappings (swapper_pg_dir and normal mm_struct pgds).
By allowing _PAGE_GLOBAL to be set in EFI mappings, you're breaking
that convention, which forces you to use extra-expensive
__flush_tlb_all calls in efi_call_virt.

I think you should explicitly *clear* _PAGE_GLOBAL in the EFI mappings
instead.  This would allow you to use write_cr3 by itself, which would
make the code simpler and faster.

>
>> c) Why setting GLOBAL for EFI mappings is safe.  Don't we unmap the
>> EFI mappings when we're not actively using them in new kernels?  If
>> so, don't we explicitly want them *not* to be GLOBAL to avoid needing
>> an extra-expensive global flush?
>
> This is a valid point. I know that EFI runtime regions persist during
> and after boot if we have a UEFI firmware and other commits made EFI
> regions have separate page table but I am not clear about the effect of
> global flush. I think Matt/Boris could comment on it.

It's straightfoward on existing kernels.  If _PAGE_GLOBAL is set, TLB
entries persist across cr3 writes.  If _PAGE_GLOBAL is clear, then TLB
entries are flushed by cr3 writes.

With PCID enabled (which is only in a not-quite-ready patch set I
have), the story is a bit more complicated, but it works essentially
the same way unless you explicitly opt out.

>
>> d) Why this doesn't break any non-EFI code.
>
> We touch this code path only when mapping EFI runtime regions to VA
> space, i.e. we added pgd field in cpa only as a support for mapping efi
> runtime regions.

populate_pgd is called from non-EFI code as well though, isn't it?

--Andy

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings
  2016-02-24  2:43         ` Andy Lutomirski
@ 2016-02-24 14:10           ` Matt Fleming
  2016-02-24 16:20             ` Borislav Petkov
  2016-02-24 16:39             ` Andy Lutomirski
  0 siblings, 2 replies; 54+ messages in thread
From: Matt Fleming @ 2016-02-24 14:10 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Sai Praneeth Prakhya, Ravi V. Shankar, Toshi Kani, Brian Gerst,
	Thomas Gleixner, Ingo Molnar, Linus Torvalds, Luis Rodriguez,
	Andrew Morton, Denys Vlasenko, H. Peter Anvin, linux-kernel,
	Peter Zijlstra, Borislav Petkov, ricardo.neri, Hugh Dickins,
	Ard Biesheuvel

On Tue, 23 Feb, at 06:43:04PM, Andy Lutomirski wrote:
> On Tue, Feb 23, 2016 at 4:50 PM, Sai Praneeth Prakhya
> <sai.praneeth.prakhya@intel.com> wrote:
> >
> > As you rightly said the code is about making a page GLOBAL if PRESENT is
> > set and we do set PRESENT bit before mapping so that page is GLOBAL.
> > This code was taken from the other parts of pageattr.c. The point is
> > that we don't want differences between whether things were mapped in the
> > EFI page tables directly (i.e. using populate_pte()) or later split from
> > large pages via the split_large_page() code path. If this is still
> > confusing could you please elaborate on it further.
> 
> At least the comment should say "Set the GLOBAL flag if and only
> if...".  But why is this code here in the first place?  What is
> passing a pgprot with global unset into this code in the first place?
 
This comes from populate_pgd(),

  static int populate_pgd(struct cpa_data *cpa, unsigned long addr)
  {
	  pgprot_t pgprot = __pgprot(_KERNPG_TABLE);

> > I don't think previous implementation is broken and this is not a bug
> > fix as such. Before this patch some EFI region mappings had GLOBAL bit
> > set (which followed split large page path) and some aren't (which used
> > populate_pte). This patch just aligns the mappings done via two
> > different code paths as mentioned above. As a whole it also maintains
> > consistency with kernel mappings.
> 
> But your code also *clears* GLOBAL if PRESENT is clear, and your
> comment talks about that.  Does this ever actually happen in practice?
 
Not when mapping EFI regions, no (we explicitly set _PAGE_PRESENT in
kernel_map_pages_in_pgd(), but this logic is duplicated from
__split_large_page() which can be called from non-EFI code.

> I'd much rather see WARN_ON_ONCE((pgprot_val(pgprot) & (_PAGE_PRESENT
> | _PAGE_GLOBAL)) == _PAGE_GLOBAL) in here, if that makes sense in the
> context of the callers of the function.
 
I think that'd be a nice cleanup, along with pulling all the pgprot
twiddling out into a single function.

> > We did this for consistency among EFI mappings. This has some advantages
> > as mentioned in the commit message. It also makes it less confusing when
> > starting at the PGT_DUMP traces if _PAGE_GLOBAL is used consistently.
> >
> > We don't actually do anything special with _PAGE_GLOBAL in EFI.
> 
> You're making a choice of whether to set _PAGE_GLOBAL, and I think
> you've made the wrong choice.
> 
> Normally, the only pages with are _PAGE_GLOBAL are those that are in
> the normal kernel mappings (swapper_pg_dir and normal mm_struct pgds).
> By allowing _PAGE_GLOBAL to be set in EFI mappings, you're breaking
> that convention, which forces you to use extra-expensive
> __flush_tlb_all calls in efi_call_virt.
> 
> I think you should explicitly *clear* _PAGE_GLOBAL in the EFI mappings
> instead.  This would allow you to use write_cr3 by itself, which would
> make the code simpler and faster.
 
This is interesting.

When I suggested to Sai that he write this patch my main motivation
was consistency for all mappings. We've got that now, but perhaps it's
the wrong consistency ;)

If we go with the no-PAGE_GLOBAL approach, we need changes to ensure
we never set _PAGE_GLOBAL for the EFI mappings, because before this
patch was applied sometimes we did and sometimes we didn't, depending
on whether a page was split or not.

I'm racking my brain to think of how your suggestion might have
unintended consequences because diagnosing stale TLB entry bugs is
simply the worst job ever. I can't think of anything. The only
scenarios where we'd see problems is if a) we have new global mappings
in the EFI page tables or b) we have different global mappings.

Since we reference swapper_pg_dir from the PMD level downwards b)
shouldn't be a problem, and the only differences between
swapper_pg_dir and efi_pgd should be the EFI mappings, which saves us
from a).

> > This is a valid point. I know that EFI runtime regions persist during
> > and after boot if we have a UEFI firmware and other commits made EFI
> > regions have separate page table but I am not clear about the effect of
> > global flush. I think Matt/Boris could comment on it.
> 
> It's straightfoward on existing kernels.  If _PAGE_GLOBAL is set, TLB
> entries persist across cr3 writes.  If _PAGE_GLOBAL is clear, then TLB
> entries are flushed by cr3 writes.
 
This is safe for EFI right now because of the big __flush_tlb_all() in
efi_call_virt().

> With PCID enabled (which is only in a not-quite-ready patch set I
> have), the story is a bit more complicated, but it works essentially
> the same way unless you explicitly opt out.
 
Hmm... is series that posted somewhere?

> > We touch this code path only when mapping EFI runtime regions to VA
> > space, i.e. we added pgd field in cpa only as a support for mapping efi
> > runtime regions.
> 
> populate_pgd is called from non-EFI code as well though, isn't it?

Nope. The "if (cpa->pgd)" guard ensures that we only call that
function for the EFI mapping code - no one else sets ->pgd.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings
  2016-02-24 14:10           ` Matt Fleming
@ 2016-02-24 16:20             ` Borislav Petkov
  2016-02-24 16:36               ` Andy Lutomirski
  2016-02-24 16:41               ` Borislav Petkov
  2016-02-24 16:39             ` Andy Lutomirski
  1 sibling, 2 replies; 54+ messages in thread
From: Borislav Petkov @ 2016-02-24 16:20 UTC (permalink / raw)
  To: Matt Fleming, Andy Lutomirski
  Cc: Sai Praneeth Prakhya, Ravi V. Shankar, Toshi Kani, Brian Gerst,
	Thomas Gleixner, Ingo Molnar, Linus Torvalds, Luis Rodriguez,
	Andrew Morton, Denys Vlasenko, H. Peter Anvin, linux-kernel,
	Peter Zijlstra, ricardo.neri, Hugh Dickins, Ard Biesheuvel

On Wed, Feb 24, 2016 at 02:10:46PM +0000, Matt Fleming wrote:
> > Normally, the only pages with are _PAGE_GLOBAL are those that are in
> > the normal kernel mappings (swapper_pg_dir and normal mm_struct pgds).
> > By allowing _PAGE_GLOBAL to be set in EFI mappings, you're breaking
> > that convention, which forces you to use extra-expensive
> > __flush_tlb_all calls in efi_call_virt.

Hold on, do you mean the __flush_tlb_all() in the CONFIG_EFI_MIXED code?

That's mixed mode. I think you mean the FLUSH_TLB_ALL in efi_call.
That's EFI on 64-bit but that is mandated by the spec, AFAIR.

So the EFI runtime crap should not change once it is mapped. And those
should be global. It is only natural.

> Nope. The "if (cpa->pgd)" guard ensures that we only call that
> function for the EFI mapping code - no one else sets ->pgd.

But it could - there's no guarantee. kernel_map_pages_in_pgd() is an
exported facility.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings
  2016-02-24 16:20             ` Borislav Petkov
@ 2016-02-24 16:36               ` Andy Lutomirski
  2016-02-24 18:56                 ` Linus Torvalds
  2016-02-24 19:33                 ` Matt Fleming
  2016-02-24 16:41               ` Borislav Petkov
  1 sibling, 2 replies; 54+ messages in thread
From: Andy Lutomirski @ 2016-02-24 16:36 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Matt Fleming, Sai Praneeth Prakhya, Ravi V. Shankar, Toshi Kani,
	Brian Gerst, Thomas Gleixner, Ingo Molnar, Linus Torvalds,
	Luis Rodriguez, Andrew Morton, Denys Vlasenko, H. Peter Anvin,
	linux-kernel, Peter Zijlstra, ricardo.neri, Hugh Dickins,
	Ard Biesheuvel

On Wed, Feb 24, 2016 at 8:20 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Wed, Feb 24, 2016 at 02:10:46PM +0000, Matt Fleming wrote:
>> > Normally, the only pages with are _PAGE_GLOBAL are those that are in
>> > the normal kernel mappings (swapper_pg_dir and normal mm_struct pgds).
>> > By allowing _PAGE_GLOBAL to be set in EFI mappings, you're breaking
>> > that convention, which forces you to use extra-expensive
>> > __flush_tlb_all calls in efi_call_virt.
>
> Hold on, do you mean the __flush_tlb_all() in the CONFIG_EFI_MIXED code?
>
> That's mixed mode. I think you mean the FLUSH_TLB_ALL in efi_call.
> That's EFI on 64-bit but that is mandated by the spec, AFAIR.

I mean the one in efi_call_virt.  Why would the spec mandate a TLB
flush at all?  EFI runtime services have no business touching the
paging structures directly.  Heck, the 32-bit ones don't even know the
*format* of the paging structures.

>
> So the EFI runtime crap should not change once it is mapped. And those
> should be global. It is only natural.

Why is it natural?

Long-term, I'd rather see EFI runtime services use an actual mm_struct
and use_mm.

--Andy

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings
  2016-02-24 14:10           ` Matt Fleming
  2016-02-24 16:20             ` Borislav Petkov
@ 2016-02-24 16:39             ` Andy Lutomirski
  2016-02-25 16:00               ` Matt Fleming
  1 sibling, 1 reply; 54+ messages in thread
From: Andy Lutomirski @ 2016-02-24 16:39 UTC (permalink / raw)
  To: Matt Fleming
  Cc: Sai Praneeth Prakhya, Ravi V. Shankar, Toshi Kani, Brian Gerst,
	Thomas Gleixner, Ingo Molnar, Linus Torvalds, Luis Rodriguez,
	Andrew Morton, Denys Vlasenko, H. Peter Anvin, linux-kernel,
	Peter Zijlstra, Borislav Petkov, ricardo.neri, Hugh Dickins,
	Ard Biesheuvel

On Wed, Feb 24, 2016 at 6:10 AM, Matt Fleming <matt@codeblueprint.co.uk> wrote:
> On Tue, 23 Feb, at 06:43:04PM, Andy Lutomirski wrote:
>> On Tue, Feb 23, 2016 at 4:50 PM, Sai Praneeth Prakhya
>> <sai.praneeth.prakhya@intel.com> wrote:
>> >
>> > As you rightly said the code is about making a page GLOBAL if PRESENT is
>> > set and we do set PRESENT bit before mapping so that page is GLOBAL.
>> > This code was taken from the other parts of pageattr.c. The point is
>> > that we don't want differences between whether things were mapped in the
>> > EFI page tables directly (i.e. using populate_pte()) or later split from
>> > large pages via the split_large_page() code path. If this is still
>> > confusing could you please elaborate on it further.
>>
>> At least the comment should say "Set the GLOBAL flag if and only
>> if...".  But why is this code here in the first place?  What is
>> passing a pgprot with global unset into this code in the first place?
>
> This comes from populate_pgd(),
>
>   static int populate_pgd(struct cpa_data *cpa, unsigned long addr)
>   {
>           pgprot_t pgprot = __pgprot(_KERNPG_TABLE);

which reminds me: aren't we supposed to *not* set GLOBAL on _PAGE_TABLE entries?

>>
>> You're making a choice of whether to set _PAGE_GLOBAL, and I think
>> you've made the wrong choice.
>>
>> Normally, the only pages with are _PAGE_GLOBAL are those that are in
>> the normal kernel mappings (swapper_pg_dir and normal mm_struct pgds).
>> By allowing _PAGE_GLOBAL to be set in EFI mappings, you're breaking
>> that convention, which forces you to use extra-expensive
>> __flush_tlb_all calls in efi_call_virt.
>>
>> I think you should explicitly *clear* _PAGE_GLOBAL in the EFI mappings
>> instead.  This would allow you to use write_cr3 by itself, which would
>> make the code simpler and faster.
>
> This is interesting.
>
> When I suggested to Sai that he write this patch my main motivation
> was consistency for all mappings. We've got that now, but perhaps it's
> the wrong consistency ;)
>
> If we go with the no-PAGE_GLOBAL approach, we need changes to ensure
> we never set _PAGE_GLOBAL for the EFI mappings, because before this
> patch was applied sometimes we did and sometimes we didn't, depending
> on whether a page was split or not.
>
> I'm racking my brain to think of how your suggestion might have
> unintended consequences because diagnosing stale TLB entry bugs is
> simply the worst job ever. I can't think of anything. The only
> scenarios where we'd see problems is if a) we have new global mappings
> in the EFI page tables or b) we have different global mappings.
>
> Since we reference swapper_pg_dir from the PMD level downwards b)
> shouldn't be a problem, and the only differences between
> swapper_pg_dir and efi_pgd should be the EFI mappings, which saves us
> from a).

:)

Anyway, there's certainly no need to do this right now.

>
>> > This is a valid point. I know that EFI runtime regions persist during
>> > and after boot if we have a UEFI firmware and other commits made EFI
>> > regions have separate page table but I am not clear about the effect of
>> > global flush. I think Matt/Boris could comment on it.
>>
>> It's straightfoward on existing kernels.  If _PAGE_GLOBAL is set, TLB
>> entries persist across cr3 writes.  If _PAGE_GLOBAL is clear, then TLB
>> entries are flushed by cr3 writes.
>
> This is safe for EFI right now because of the big __flush_tlb_all() in
> efi_call_virt().
>
>> With PCID enabled (which is only in a not-quite-ready patch set I
>> have), the story is a bit more complicated, but it works essentially
>> the same way unless you explicitly opt out.
>
> Hmm... is series that posted somewhere?

It's living here:

https://git.kernel.org/cgit/linux/kernel/git/luto/linux.git/log/?h=x86/pcid

at least until someone figures out how to squash the races in the bookkeeping.

I intentionally never load PCID == 0 with the "don't flush" bit set
specifically so that direct PCID-unaware CR3 loads (like the EFI code
does all over the place) keep working.

>
>> > We touch this code path only when mapping EFI runtime regions to VA
>> > space, i.e. we added pgd field in cpa only as a support for mapping efi
>> > runtime regions.
>>
>> populate_pgd is called from non-EFI code as well though, isn't it?
>
> Nope. The "if (cpa->pgd)" guard ensures that we only call that
> function for the EFI mapping code - no one else sets ->pgd.

OK, although a comment might be nice.

--Andy

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings
  2016-02-24 16:20             ` Borislav Petkov
  2016-02-24 16:36               ` Andy Lutomirski
@ 2016-02-24 16:41               ` Borislav Petkov
  2016-02-24 16:47                 ` Andy Lutomirski
  1 sibling, 1 reply; 54+ messages in thread
From: Borislav Petkov @ 2016-02-24 16:41 UTC (permalink / raw)
  To: Matt Fleming, Andy Lutomirski
  Cc: Sai Praneeth Prakhya, Ravi V. Shankar, Toshi Kani, Brian Gerst,
	Thomas Gleixner, Ingo Molnar, Linus Torvalds, Luis Rodriguez,
	Andrew Morton, Denys Vlasenko, H. Peter Anvin, linux-kernel,
	Peter Zijlstra, ricardo.neri, Hugh Dickins, Ard Biesheuvel

On Wed, Feb 24, 2016 at 05:20:02PM +0100, Borislav Petkov wrote:
> That's mixed mode. I think you mean the FLUSH_TLB_ALL in efi_call.
> That's EFI on 64-bit but that is mandated by the spec, AFAIR.

Ok, so mfleming set me straight on IRC - that's tip/master I should be
staring at.

In any case, I think we should do __flush_tlb_all() in efi_call_cirt()
just in case, for the simple reason that EFI could be installing some
funky TLB entries which we don't want. I'm not saying it does and it
probably won't but what's stopping it?

Or am I being overly paranoid?

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings
  2016-02-24 16:41               ` Borislav Petkov
@ 2016-02-24 16:47                 ` Andy Lutomirski
  0 siblings, 0 replies; 54+ messages in thread
From: Andy Lutomirski @ 2016-02-24 16:47 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Matt Fleming, Sai Praneeth Prakhya, Ravi V. Shankar, Toshi Kani,
	Brian Gerst, Thomas Gleixner, Ingo Molnar, Linus Torvalds,
	Luis Rodriguez, Andrew Morton, Denys Vlasenko, H. Peter Anvin,
	linux-kernel, Peter Zijlstra, ricardo.neri, Hugh Dickins,
	Ard Biesheuvel

On Wed, Feb 24, 2016 at 8:41 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Wed, Feb 24, 2016 at 05:20:02PM +0100, Borislav Petkov wrote:
>> That's mixed mode. I think you mean the FLUSH_TLB_ALL in efi_call.
>> That's EFI on 64-bit but that is mandated by the spec, AFAIR.
>
> Ok, so mfleming set me straight on IRC - that's tip/master I should be
> staring at.
>
> In any case, I think we should do __flush_tlb_all() in efi_call_cirt()
> just in case, for the simple reason that EFI could be installing some
> funky TLB entries which we don't want. I'm not saying it does and it
> probably won't but what's stopping it?
>
> Or am I being overly paranoid?

I think you may be overly paranoid here.  At least no working 32-bit
EFI does this because we run it in compat mode.  Any paging entries it
inserts would be misinterpreted and likely immediately cause a crash.
Also, the EFI code doesn't know a virtual address through which to
reference the paging structures in the first place -- it could read
CR3, but that gives a physical address, and it's not at all clear to
me what even the crazier firmware authors would do with a physical
address that doesn't live in EFI-defined ranges.

--Andy

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings
  2016-02-24 16:36               ` Andy Lutomirski
@ 2016-02-24 18:56                 ` Linus Torvalds
  2016-02-24 19:45                   ` Matt Fleming
  2016-02-24 19:33                 ` Matt Fleming
  1 sibling, 1 reply; 54+ messages in thread
From: Linus Torvalds @ 2016-02-24 18:56 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Borislav Petkov, Matt Fleming, Sai Praneeth Prakhya,
	Ravi V. Shankar, Toshi Kani, Brian Gerst, Thomas Gleixner,
	Ingo Molnar, Luis Rodriguez, Andrew Morton, Denys Vlasenko,
	H. Peter Anvin, linux-kernel, Peter Zijlstra, ricardo.neri,
	Hugh Dickins, Ard Biesheuvel

On Wed, Feb 24, 2016 at 8:36 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>>
>> So the EFI runtime crap should not change once it is mapped. And those
>> should be global. It is only natural.
>
> Why is it natural?
>
> Long-term, I'd rather see EFI runtime services use an actual mm_struct
> and use_mm.

Definitely.

The EFI runtime page mapping may be unchanging, but that doesn't mean
we should be mapping it all the time - the mapping may not change, but
we will change away from it.

So marking those pages global is very wrong.

                 Linus

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings
  2016-02-24 16:36               ` Andy Lutomirski
  2016-02-24 18:56                 ` Linus Torvalds
@ 2016-02-24 19:33                 ` Matt Fleming
  2016-02-24 19:49                   ` Andy Lutomirski
  1 sibling, 1 reply; 54+ messages in thread
From: Matt Fleming @ 2016-02-24 19:33 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Borislav Petkov, Sai Praneeth Prakhya, Ravi V. Shankar,
	Toshi Kani, Brian Gerst, Thomas Gleixner, Ingo Molnar,
	Linus Torvalds, Luis Rodriguez, Andrew Morton, Denys Vlasenko,
	H. Peter Anvin, linux-kernel, Peter Zijlstra, ricardo.neri,
	Hugh Dickins, Ard Biesheuvel

On Wed, 24 Feb, at 08:36:33AM, Andy Lutomirski wrote:
> On Wed, Feb 24, 2016 at 8:20 AM, Borislav Petkov <bp@alien8.de> wrote:
> > On Wed, Feb 24, 2016 at 02:10:46PM +0000, Matt Fleming wrote:
> >> > Normally, the only pages with are _PAGE_GLOBAL are those that are in
> >> > the normal kernel mappings (swapper_pg_dir and normal mm_struct pgds).
> >> > By allowing _PAGE_GLOBAL to be set in EFI mappings, you're breaking
> >> > that convention, which forces you to use extra-expensive
> >> > __flush_tlb_all calls in efi_call_virt.
> >
> > Hold on, do you mean the __flush_tlb_all() in the CONFIG_EFI_MIXED code?
> >
> > That's mixed mode. I think you mean the FLUSH_TLB_ALL in efi_call.
> > That's EFI on 64-bit but that is mandated by the spec, AFAIR.
> 
> I mean the one in efi_call_virt.  Why would the spec mandate a TLB
> flush at all?  EFI runtime services have no business touching the
> paging structures directly.  Heck, the 32-bit ones don't even know the
> *format* of the paging structures.
 
Right, and it would necessitate copying out arguments because the
firmware won't understand where/how the kernel has mapped things.

No firmware is going to be doing that.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings
  2016-02-24 18:56                 ` Linus Torvalds
@ 2016-02-24 19:45                   ` Matt Fleming
  2016-02-24 19:50                     ` Andy Lutomirski
  2016-02-29 10:56                     ` Sylvain Chouleur
  0 siblings, 2 replies; 54+ messages in thread
From: Matt Fleming @ 2016-02-24 19:45 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Andy Lutomirski, Borislav Petkov, Sai Praneeth Prakhya,
	Ravi V. Shankar, Toshi Kani, Brian Gerst, Thomas Gleixner,
	Ingo Molnar, Luis Rodriguez, Andrew Morton, Denys Vlasenko,
	H. Peter Anvin, linux-kernel, Peter Zijlstra, ricardo.neri,
	Hugh Dickins, Ard Biesheuvel, Sylvain Chouleur

On Wed, 24 Feb, at 10:56:13AM, Linus Torvalds wrote:
> On Wed, Feb 24, 2016 at 8:36 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> >>
> >> So the EFI runtime crap should not change once it is mapped. And those
> >> should be global. It is only natural.
> >
> > Why is it natural?
> >
> > Long-term, I'd rather see EFI runtime services use an actual mm_struct
> > and use_mm.
> 
> Definitely.
> 
> The EFI runtime page mapping may be unchanging, but that doesn't mean
> we should be mapping it all the time - the mapping may not change, but
> we will change away from it.
 
There is movement towards hanging the EFI memory map off of mm_struct
for x86. ARM and arm64 already do this and there were some patches
from Sylvain (Cc'd) to do this for the purposes of having a task
context that could be preempted while in the middle of an EFI runtime
call for some Intel platforms,

  https://lkml.kernel.org/r/1452702762-27216-4-git-send-email-sylvain.chouleur@gmail.com

Apart from the code simplification and not being required to open-code
the %cr3 diddling, are there other benefits of mm_struct and use_mm()
that make it appealing in the non-preemptible case?

Not that those aren't reasons enough.

> So marking those pages global is very wrong.

Ingo, Andy, how do you want to handle this patch? Maybe just drop it
from tip/efi/core while we prod around making all the EFI mappings
non-global? Nothing else depends on it, it can be dropped without any
harm.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings
  2016-02-24 19:33                 ` Matt Fleming
@ 2016-02-24 19:49                   ` Andy Lutomirski
  2016-02-25  9:06                     ` Ingo Molnar
  2016-02-25 15:27                     ` Matt Fleming
  0 siblings, 2 replies; 54+ messages in thread
From: Andy Lutomirski @ 2016-02-24 19:49 UTC (permalink / raw)
  To: Matt Fleming
  Cc: Borislav Petkov, Sai Praneeth Prakhya, Ravi V. Shankar,
	Toshi Kani, Brian Gerst, Thomas Gleixner, Ingo Molnar,
	Linus Torvalds, Luis Rodriguez, Andrew Morton, Denys Vlasenko,
	H. Peter Anvin, linux-kernel, Peter Zijlstra, ricardo.neri,
	Hugh Dickins, Ard Biesheuvel

On Wed, Feb 24, 2016 at 11:33 AM, Matt Fleming <matt@codeblueprint.co.uk> wrote:
> On Wed, 24 Feb, at 08:36:33AM, Andy Lutomirski wrote:
>> On Wed, Feb 24, 2016 at 8:20 AM, Borislav Petkov <bp@alien8.de> wrote:
>> > On Wed, Feb 24, 2016 at 02:10:46PM +0000, Matt Fleming wrote:
>> >> > Normally, the only pages with are _PAGE_GLOBAL are those that are in
>> >> > the normal kernel mappings (swapper_pg_dir and normal mm_struct pgds).
>> >> > By allowing _PAGE_GLOBAL to be set in EFI mappings, you're breaking
>> >> > that convention, which forces you to use extra-expensive
>> >> > __flush_tlb_all calls in efi_call_virt.
>> >
>> > Hold on, do you mean the __flush_tlb_all() in the CONFIG_EFI_MIXED code?
>> >
>> > That's mixed mode. I think you mean the FLUSH_TLB_ALL in efi_call.
>> > That's EFI on 64-bit but that is mandated by the spec, AFAIR.
>>
>> I mean the one in efi_call_virt.  Why would the spec mandate a TLB
>> flush at all?  EFI runtime services have no business touching the
>> paging structures directly.  Heck, the 32-bit ones don't even know the
>> *format* of the paging structures.
>
> Right, and it would necessitate copying out arguments because the
> firmware won't understand where/how the kernel has mapped things.
>
> No firmware is going to be doing that.

Just so I understand correctly: could we get away with putting the EFI
virtual runtime mappings at positive (user) addresses for 64-bit UEFI,
or is there some reason that we need the high bit set?

If we could use positive addresses, then we could use the existing
use_mm infrastructure directly with no funny business at all except to
the extent that we might need to use unusual APIs to set up the VMAs
(if we use real VMAs) in the first place.  (We could cheat and
allocate a single monstrous VM_MIXEDMAP or VM_PFNMAP vma with a .fault
handler that always fails.)  If we have to use negative addresses,
then we'll always be stuck with a funny pgd, but we could still
probably use use_mm instead of manually fiddling with cr3.

Some day I want to experiment with calling runtime services at CPL 3,
too :)  We'd want to add some infrastructure to permit kernel threads
to run through the entry/exit code as if they were user processes, but
there's nothing conceptually wrong with that.  We already allow kernel
threads to call execve and "return" to real user mode, so it's not
much of a stretch.  The main issue would be dealing with signal
handling and such -- we'd want to report faults back to the kernel
thread's CPL3-invocation thunks rather than delivering a signal at CPL
3.

Hmm, now it's time to muse about how the interface would work.  If we
kept it in line with existing practice, we'd add an API to make a
special kernel thread with an attached user context.  To enter CPL3,
you'd return from the thread's main function.  When user mode was done
(fault or syscall), new hooks in the entry code (similar to seccomp
and the die notifier stuff) would re-enter the main function with some
arguments indicating what happened.

If we wanted to make it a bit easier to use, we'd have to allocate an
extra kernel sack, and we could have:

void invoke_cpl3(struct cpl3_context *ctx);

where ctx contains memory for an extra stack as well as a bunch of
data indicating the reason that it returned.

The latter is harder to implement but probably much easier to use.

If anyone wants to work on this, ping me and I'll help and do a bunch of review.

--Andy

-- 
Andy Lutomirski
AMA Capital Management, LLC

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings
  2016-02-24 19:45                   ` Matt Fleming
@ 2016-02-24 19:50                     ` Andy Lutomirski
  2016-02-29 10:56                     ` Sylvain Chouleur
  1 sibling, 0 replies; 54+ messages in thread
From: Andy Lutomirski @ 2016-02-24 19:50 UTC (permalink / raw)
  To: Matt Fleming
  Cc: Linus Torvalds, Borislav Petkov, Sai Praneeth Prakhya,
	Ravi V. Shankar, Toshi Kani, Brian Gerst, Thomas Gleixner,
	Ingo Molnar, Luis Rodriguez, Andrew Morton, Denys Vlasenko,
	H. Peter Anvin, linux-kernel, Peter Zijlstra, ricardo.neri,
	Hugh Dickins, Ard Biesheuvel, Sylvain Chouleur

On Wed, Feb 24, 2016 at 11:45 AM, Matt Fleming <matt@codeblueprint.co.uk> wrote:
> On Wed, 24 Feb, at 10:56:13AM, Linus Torvalds wrote:
>> On Wed, Feb 24, 2016 at 8:36 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>> >>
>> >> So the EFI runtime crap should not change once it is mapped. And those
>> >> should be global. It is only natural.
>> >
>> > Why is it natural?
>> >
>> > Long-term, I'd rather see EFI runtime services use an actual mm_struct
>> > and use_mm.
>>
>> Definitely.
>>
>> The EFI runtime page mapping may be unchanging, but that doesn't mean
>> we should be mapping it all the time - the mapping may not change, but
>> we will change away from it.
>
> There is movement towards hanging the EFI memory map off of mm_struct
> for x86. ARM and arm64 already do this and there were some patches
> from Sylvain (Cc'd) to do this for the purposes of having a task
> context that could be preempted while in the middle of an EFI runtime
> call for some Intel platforms,
>
>   https://lkml.kernel.org/r/1452702762-27216-4-git-send-email-sylvain.chouleur@gmail.com
>
> Apart from the code simplification and not being required to open-code
> the %cr3 diddling, are there other benefits of mm_struct and use_mm()
> that make it appealing in the non-preemptible case?
>
> Not that those aren't reasons enough.

If we add PCID support, then use_mm will get the benefits (~200ns
savings for a round trip) for free.

>
>> So marking those pages global is very wrong.
>
> Ingo, Andy, how do you want to handle this patch? Maybe just drop it
> from tip/efi/core while we prod around making all the EFI mappings
> non-global? Nothing else depends on it, it can be dropped without any
> harm.

If the patch is harmless as is, I'm okay with letting it stay.

--Andy

-- 
Andy Lutomirski
AMA Capital Management, LLC

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings
  2016-02-23 18:08       ` Linus Torvalds
  2016-02-23 18:12         ` Borislav Petkov
  2016-02-24  2:09         ` H. Peter Anvin
@ 2016-02-25  8:59         ` Ingo Molnar
  2 siblings, 0 replies; 54+ messages in thread
From: Ingo Molnar @ 2016-02-25  8:59 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Andy Lutomirski, Ravi V. Shankar, Toshi Kani, Brian Gerst,
	Thomas Gleixner, Sai Praneeth Prakhya, Luis Rodriguez,
	Andrew Morton, Denys Vlasenko, Matt Fleming, H. Peter Anvin,
	linux-kernel, Peter Zijlstra, Borislav Petkov, ricardo.neri,
	Hugh Dickins, Ard Biesheuvel


* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Tue, Feb 23, 2016 at 9:47 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> > On Feb 23, 2016 1:09 AM, <&quot;tip-bot for Sai Praneeth
> > &lt;tipbot@zytor.com&gt;&quot;@zytor.com> wrote:
> >
> > Something's wrong with tip-bot.  This should say:
> 
> Yeah, there's about 50 tipbot emails that are just pure garbage. They don't even 
> show in my mailers, because they are so corrupt.

It should now all be fixed.

We were unlucky in that I just happened to push out a bunch of new commits after 
the script broke. Normally I'd have noticed this after just a few commits.

Sorry about this!

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings
  2016-02-24 19:49                   ` Andy Lutomirski
@ 2016-02-25  9:06                     ` Ingo Molnar
  2016-02-25 15:27                     ` Matt Fleming
  1 sibling, 0 replies; 54+ messages in thread
From: Ingo Molnar @ 2016-02-25  9:06 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Matt Fleming, Borislav Petkov, Sai Praneeth Prakhya,
	Ravi V. Shankar, Toshi Kani, Brian Gerst, Thomas Gleixner,
	Linus Torvalds, Luis Rodriguez, Andrew Morton, Denys Vlasenko,
	H. Peter Anvin, linux-kernel, Peter Zijlstra, ricardo.neri,
	Hugh Dickins, Ard Biesheuvel


* Andy Lutomirski <luto@amacapital.net> wrote:

> >> I mean the one in efi_call_virt.  Why would the spec mandate a TLB flush at 
> >> all?  EFI runtime services have no business touching the paging structures 
> >> directly.  Heck, the 32-bit ones don't even know the *format* of the paging 
> >> structures.
> >
> > Right, and it would necessitate copying out arguments because the firmware 
> > won't understand where/how the kernel has mapped things.
> >
> > No firmware is going to be doing that.
> 
> Just so I understand correctly: could we get away with putting the EFI virtual 
> runtime mappings at positive (user) addresses for 64-bit UEFI, or is there some 
> reason that we need the high bit set?
> 
> If we could use positive addresses, then we could use the existing use_mm 
> infrastructure directly with no funny business at all except to the extent that 
> we might need to use unusual APIs to set up the VMAs (if we use real VMAs) in 
> the first place.  (We could cheat and allocate a single monstrous VM_MIXEDMAP or 
> VM_PFNMAP vma with a .fault handler that always fails.)  If we have to use 
> negative addresses, then we'll always be stuck with a funny pgd, but we could 
> still probably use use_mm instead of manually fiddling with cr3.

Would be nice to get an answer to these questions. The more we isolate firmware 
execution into 'regular' MM concepts, the more robust it all becomes.

> Some day I want to experiment with calling runtime services at CPL 3, too :)

That would be an interesting isolation method as well ...

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings
  2016-02-24 19:49                   ` Andy Lutomirski
  2016-02-25  9:06                     ` Ingo Molnar
@ 2016-02-25 15:27                     ` Matt Fleming
  1 sibling, 0 replies; 54+ messages in thread
From: Matt Fleming @ 2016-02-25 15:27 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Borislav Petkov, Sai Praneeth Prakhya, Ravi V. Shankar,
	Toshi Kani, Brian Gerst, Thomas Gleixner, Ingo Molnar,
	Linus Torvalds, Luis Rodriguez, Andrew Morton, Denys Vlasenko,
	H. Peter Anvin, linux-kernel, Peter Zijlstra, ricardo.neri,
	Hugh Dickins, Ard Biesheuvel

On Wed, 24 Feb, at 11:49:23AM, Andy Lutomirski wrote:
> On Wed, Feb 24, 2016 at 11:33 AM, Matt Fleming <matt@codeblueprint.co.uk> wrote:
> > On Wed, 24 Feb, at 08:36:33AM, Andy Lutomirski wrote:
> >> On Wed, Feb 24, 2016 at 8:20 AM, Borislav Petkov <bp@alien8.de> wrote:
> >> > On Wed, Feb 24, 2016 at 02:10:46PM +0000, Matt Fleming wrote:
> >> >> > Normally, the only pages with are _PAGE_GLOBAL are those that are in
> >> >> > the normal kernel mappings (swapper_pg_dir and normal mm_struct pgds).
> >> >> > By allowing _PAGE_GLOBAL to be set in EFI mappings, you're breaking
> >> >> > that convention, which forces you to use extra-expensive
> >> >> > __flush_tlb_all calls in efi_call_virt.
> >> >
> >> > Hold on, do you mean the __flush_tlb_all() in the CONFIG_EFI_MIXED code?
> >> >
> >> > That's mixed mode. I think you mean the FLUSH_TLB_ALL in efi_call.
> >> > That's EFI on 64-bit but that is mandated by the spec, AFAIR.
> >>
> >> I mean the one in efi_call_virt.  Why would the spec mandate a TLB
> >> flush at all?  EFI runtime services have no business touching the
> >> paging structures directly.  Heck, the 32-bit ones don't even know the
> >> *format* of the paging structures.
> >
> > Right, and it would necessitate copying out arguments because the
> > firmware won't understand where/how the kernel has mapped things.
> >
> > No firmware is going to be doing that.
> 
> Just so I understand correctly: could we get away with putting the EFI
> virtual runtime mappings at positive (user) addresses for 64-bit UEFI,
> or is there some reason that we need the high bit set?
 
Good question. There are multiple parts to this answer:

 1) Some firmware is known to break when entered via the identity
    addresses (VA==PA) 

 2) Kexec cares the most about where we map things because the region
    has to be static across Kexec reboots. We do pass the kernel's EFI
    memory map regions between Kexec kernels but that region clearly
    needs to be available across kernel versions

It shouldn't be possible to conflict with userspace mappings or
anything like that because we should never be accessing userspace
addresses during EFI runtime services calls - all relevant data is
copied to a kernel buffer or such. Userspace isn't even mapped now
we've got completely separate EFI page tables.

I don't think there's anything else that would stop us clearing the
high bit and moving the EFI virtual mapping region somewhere else.
Boris?

> If we could use positive addresses, then we could use the existing
> use_mm infrastructure directly with no funny business at all except to
> the extent that we might need to use unusual APIs to set up the VMAs
> (if we use real VMAs) in the first place.  (We could cheat and
> allocate a single monstrous VM_MIXEDMAP or VM_PFNMAP vma with a .fault
> handler that always fails.)  If we have to use negative addresses,
> then we'll always be stuck with a funny pgd, but we could still
> probably use use_mm instead of manually fiddling with cr3.
 
We don't use VMAs at the moment.

Having a custom .fault handler could be a very interesting idea
because we've talked about wanting to do EFI-specific things in the
past if we fault while executing firmware, e.g. printing warnings in
the kernel log indicating the firmware is known to be buggy because it
performed an access not compliant with the spec. See 1) above.

> Some day I want to experiment with calling runtime services at CPL 3,
> too :)  We'd want to add some infrastructure to permit kernel threads
> to run through the entry/exit code as if they were user processes, but
> there's nothing conceptually wrong with that.  We already allow kernel
> threads to call execve and "return" to real user mode, so it's not
> much of a stretch.  The main issue would be dealing with signal
> handling and such -- we'd want to report faults back to the kernel
> thread's CPL3-invocation thunks rather than delivering a signal at CPL
> 3.
 
Right, more isolation is better. I'm not sure we could get all the way
to CPL 3 but I wouldn't begrudge anyone trying.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings
  2016-02-24 16:39             ` Andy Lutomirski
@ 2016-02-25 16:00               ` Matt Fleming
  0 siblings, 0 replies; 54+ messages in thread
From: Matt Fleming @ 2016-02-25 16:00 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Sai Praneeth Prakhya, Ravi V. Shankar, Toshi Kani, Brian Gerst,
	Thomas Gleixner, Ingo Molnar, Linus Torvalds, Luis Rodriguez,
	Andrew Morton, Denys Vlasenko, H. Peter Anvin, linux-kernel,
	Peter Zijlstra, Borislav Petkov, ricardo.neri, Hugh Dickins,
	Ard Biesheuvel

On Wed, 24 Feb, at 08:39:56AM, Andy Lutomirski wrote:
> 
> OK, although a comment might be nice.

Something like this?

---
>From ac40fc0269d4d8cc9051982c177ee140d6e1b761 Mon Sep 17 00:00:00 2001
From: Matt Fleming <matt@codeblueprint.co.uk>
Date: Thu, 25 Feb 2016 15:54:50 +0000
Subject: [PATCH] x86/mm/pat: Document the (currently) EFI-only code path

It's not at all obvious that populate_pgd() and friends are only
executed when mapping EFI virtual memory regions or that no other
pageattr callers pass a ->pgd value.

Reported-by: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
---
 arch/x86/mm/pageattr.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index 14c38ae80409..8fee5b6f8f66 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -1125,8 +1125,14 @@ static int populate_pgd(struct cpa_data *cpa, unsigned long addr)
 static int __cpa_process_fault(struct cpa_data *cpa, unsigned long vaddr,
 			       int primary)
 {
-	if (cpa->pgd)
+	if (cpa->pgd) {
+		/*
+		 * Right now, we only execute this code path when mapping
+		 * the EFI virtual memory map regions, no other users
+		 * provide a ->pgd value. This may change in the future.
+		 */
 		return populate_pgd(cpa, vaddr);
+	}
 
 	/*
 	 * Ignore all non primary paths.
-- 
2.6.2

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings
  2016-02-24 19:45                   ` Matt Fleming
  2016-02-24 19:50                     ` Andy Lutomirski
@ 2016-02-29 10:56                     ` Sylvain Chouleur
  2016-03-02 11:20                       ` Matt Fleming
  1 sibling, 1 reply; 54+ messages in thread
From: Sylvain Chouleur @ 2016-02-29 10:56 UTC (permalink / raw)
  To: Matt Fleming
  Cc: Linus Torvalds, Andy Lutomirski, Borislav Petkov,
	Sai Praneeth Prakhya, Ravi V. Shankar, Toshi Kani, Brian Gerst,
	Thomas Gleixner, Ingo Molnar, Luis Rodriguez, Andrew Morton,
	Denys Vlasenko, H. Peter Anvin, linux-kernel, Peter Zijlstra,
	ricardo.neri, Hugh Dickins, Ard Biesheuvel

2016-02-24 20:45 GMT+01:00 Matt Fleming <matt@codeblueprint.co.uk>:
> On Wed, 24 Feb, at 10:56:13AM, Linus Torvalds wrote:
>> On Wed, Feb 24, 2016 at 8:36 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>> >>
>> >> So the EFI runtime crap should not change once it is mapped. And those
>> >> should be global. It is only natural.
>> >
>> > Why is it natural?
>> >
>> > Long-term, I'd rather see EFI runtime services use an actual mm_struct
>> > and use_mm.
>>
>> Definitely.
>>
>> The EFI runtime page mapping may be unchanging, but that doesn't mean
>> we should be mapping it all the time - the mapping may not change, but
>> we will change away from it.
>
> There is movement towards hanging the EFI memory map off of mm_struct
> for x86. ARM and arm64 already do this and there were some patches
> from Sylvain (Cc'd) to do this for the purposes of having a task
> context that could be preempted while in the middle of an EFI runtime
> call for some Intel platforms,
>
>   https://lkml.kernel.org/r/1452702762-27216-4-git-send-email-sylvain.chouleur@gmail.com

I was thinking we could use the efi kthread to handle the efi services
generically, not only for the interruptible case, and have a way to decide if we
allow interruptions inside the efi call itself or not.

Then all runtime services would use an mm_struct. The drawback is that you will
need two context switchs to be able to execute the runtime service.

-- 
Sylvain

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings
  2016-02-29 10:56                     ` Sylvain Chouleur
@ 2016-03-02 11:20                       ` Matt Fleming
  0 siblings, 0 replies; 54+ messages in thread
From: Matt Fleming @ 2016-03-02 11:20 UTC (permalink / raw)
  To: Sylvain Chouleur
  Cc: Linus Torvalds, Andy Lutomirski, Borislav Petkov,
	Sai Praneeth Prakhya, Ravi V. Shankar, Toshi Kani, Brian Gerst,
	Thomas Gleixner, Ingo Molnar, Luis Rodriguez, Andrew Morton,
	Denys Vlasenko, H. Peter Anvin, linux-kernel, Peter Zijlstra,
	ricardo.neri, Hugh Dickins, Ard Biesheuvel

On Mon, 29 Feb, at 11:56:56AM, Sylvain Chouleur wrote:
> 2016-02-24 20:45 GMT+01:00 Matt Fleming <matt@codeblueprint.co.uk>:
> > On Wed, 24 Feb, at 10:56:13AM, Linus Torvalds wrote:
> >> On Wed, Feb 24, 2016 at 8:36 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> >> >>
> >> >> So the EFI runtime crap should not change once it is mapped. And those
> >> >> should be global. It is only natural.
> >> >
> >> > Why is it natural?
> >> >
> >> > Long-term, I'd rather see EFI runtime services use an actual mm_struct
> >> > and use_mm.
> >>
> >> Definitely.
> >>
> >> The EFI runtime page mapping may be unchanging, but that doesn't mean
> >> we should be mapping it all the time - the mapping may not change, but
> >> we will change away from it.
> >
> > There is movement towards hanging the EFI memory map off of mm_struct
> > for x86. ARM and arm64 already do this and there were some patches
> > from Sylvain (Cc'd) to do this for the purposes of having a task
> > context that could be preempted while in the middle of an EFI runtime
> > call for some Intel platforms,
> >
> >   https://lkml.kernel.org/r/1452702762-27216-4-git-send-email-sylvain.chouleur@gmail.com
> 
> I was thinking we could use the efi kthread to handle the efi services
> generically, not only for the interruptible case, and have a way to decide if we
> allow interruptions inside the efi call itself or not.
> 
> Then all runtime services would use an mm_struct. The drawback is that you will
> need two context switchs to be able to execute the runtime service.

I would be surprised if the asynchronous nature of having a special
EFI kthread would buy you any benefit in general. And in fact, in the
efi-pstore code you can be invoked in IRQ context and you really don't
want to start talking to a kthread.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 09/13] efi/arm64: Check for h/w support before booting a >4 KB granule kernel
  2016-02-17 12:36 ` [PATCH 09/13] efi/arm64: Check for h/w support before booting a >4 KB granule kernel Matt Fleming
  2016-02-22 12:16   ` [tip:efi/core] efi/arm64: Check for h/w support before booting a >4 KB granular kernel =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?=
@ 2016-03-06  3:35   ` Ard Biesheuvel
  2016-03-07 11:02     ` Matt Fleming
  1 sibling, 1 reply; 54+ messages in thread
From: Ard Biesheuvel @ 2016-03-06  3:35 UTC (permalink / raw)
  To: Matt Fleming, Ingo Molnar
  Cc: Thomas Gleixner, H . Peter Anvin, linux-kernel, linux-efi,
	Jeremy Linton, Mark Rutland, Suzuki K Poulose

On 17 February 2016 at 13:36, Matt Fleming <matt@codeblueprint.co.uk> wrote:
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> A kernel built with support for a page size that is not supported by the
> hardware it runs on cannot boot to a state where it can inform the user
> about it.
>
> If we happen to be booting via UEFI, we can fail gracefully so check
> if the currently configured page size is supported by the hardware before
> entering the kernel proper. Note that UEFI mandates support for 4 KB pages,
> so in that case, no check is needed.
>
> Reviewed-by: Jeremy Linton <jeremy.linton@arm.com>
> Tested-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>

Hi Matt,

This patch turned up in -next with 'granule' replaced with 'granular',
both in the commit log and in the patch itself. The term 'granule' is
part of the idiom used by the ARM Architecture Reference Manual, and
so changing it silently to 'granular' is not entirely appropriate here
(although harmless in practice, obviously). In general, I would
appreciate it if in the future, such changes were not made silently
somewhere in the merge pipeline.

Thanks,
Ard.


> ---
>  drivers/firmware/efi/libstub/arm64-stub.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
> index 9e0342745e4f..aef04ad60e0d 100644
> --- a/drivers/firmware/efi/libstub/arm64-stub.c
> +++ b/drivers/firmware/efi/libstub/arm64-stub.c
> @@ -12,6 +12,26 @@
>  #include <linux/efi.h>
>  #include <asm/efi.h>
>  #include <asm/sections.h>
> +#include <asm/sysreg.h>
> +
> +efi_status_t check_platform_features(efi_system_table_t *sys_table_arg)
> +{
> +       u64 tg;
> +
> +       /* UEFI mandates support for 4 KB granule, no need to check */
> +       if (IS_ENABLED(CONFIG_ARM64_4K_PAGES))
> +               return EFI_SUCCESS;
> +
> +       tg = (read_cpuid(ID_AA64MMFR0_EL1) >> ID_AA64MMFR0_TGRAN_SHIFT) & 0xf;
> +       if (tg != ID_AA64MMFR0_TGRAN_SUPPORTED) {
> +               if (IS_ENABLED(CONFIG_ARM64_64K_PAGES))
> +                       pr_efi_err(sys_table_arg, "This 64 KB granule kernel is not supported by your CPU\n");
> +               else
> +                       pr_efi_err(sys_table_arg, "This 16 KB granule kernel is not supported by your CPU\n");
> +               return EFI_UNSUPPORTED;
> +       }
> +       return EFI_SUCCESS;
> +}
>
>  efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
>                                  unsigned long *image_addr,
> --
> 2.6.2
>

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 09/13] efi/arm64: Check for h/w support before booting a >4 KB granule kernel
  2016-03-06  3:35   ` [PATCH 09/13] efi/arm64: Check for h/w support before booting a >4 KB granule kernel Ard Biesheuvel
@ 2016-03-07 11:02     ` Matt Fleming
  2016-03-07 11:05       ` Ard Biesheuvel
  0 siblings, 1 reply; 54+ messages in thread
From: Matt Fleming @ 2016-03-07 11:02 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Ingo Molnar, Thomas Gleixner, H . Peter Anvin, linux-kernel,
	linux-efi, Jeremy Linton, Mark Rutland, Suzuki K Poulose

On Sun, 06 Mar, at 04:35:32AM, Ard Biesheuvel wrote:
> 
> Hi Matt,
> 
> This patch turned up in -next with 'granule' replaced with 'granular',
> both in the commit log and in the patch itself. The term 'granule' is
> part of the idiom used by the ARM Architecture Reference Manual, and
> so changing it silently to 'granular' is not entirely appropriate here
> (although harmless in practice, obviously). In general, I would
> appreciate it if in the future, such changes were not made silently
> somewhere in the merge pipeline.

Sorry about this Ard. I'll make sure this doesn't happen again in
future.

Ingo, is there any chance we can fixup this patch in-place and revert
back to the original wording before it gets to Linus' during the merge
window?

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 09/13] efi/arm64: Check for h/w support before booting a >4 KB granule kernel
  2016-03-07 11:02     ` Matt Fleming
@ 2016-03-07 11:05       ` Ard Biesheuvel
  0 siblings, 0 replies; 54+ messages in thread
From: Ard Biesheuvel @ 2016-03-07 11:05 UTC (permalink / raw)
  To: Matt Fleming
  Cc: Ingo Molnar, Thomas Gleixner, H . Peter Anvin, linux-kernel,
	linux-efi, Jeremy Linton, Mark Rutland, Suzuki K Poulose

On 7 March 2016 at 18:02, Matt Fleming <matt@codeblueprint.co.uk> wrote:
> On Sun, 06 Mar, at 04:35:32AM, Ard Biesheuvel wrote:
>>
>> Hi Matt,
>>
>> This patch turned up in -next with 'granule' replaced with 'granular',
>> both in the commit log and in the patch itself. The term 'granule' is
>> part of the idiom used by the ARM Architecture Reference Manual, and
>> so changing it silently to 'granular' is not entirely appropriate here
>> (although harmless in practice, obviously). In general, I would
>> appreciate it if in the future, such changes were not made silently
>> somewhere in the merge pipeline.
>
> Sorry about this Ard. I'll make sure this doesn't happen again in
> future.
>

Thanks

> Ingo, is there any chance we can fixup this patch in-place and revert
> back to the original wording before it gets to Linus' during the merge
> window?

To be honest, I don't care deeply about the commit log, as long as the
code changes are reverted.

Thanks,
Ard.

^ permalink raw reply	[flat|nested] 54+ messages in thread

end of thread, other threads:[~2016-03-07 11:05 UTC | newest]

Thread overview: 54+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-17 12:35 [GIT PULL 00/13] EFI changes for v4.6 part 2 Matt Fleming
2016-02-17 12:35 ` [PATCH 01/13] efi: Reformat GUID tables to follow the format in UEFI spec Matt Fleming
2016-02-22 12:12   ` [tip:efi/core] " =?UTF-8?B?dGlwLWJvdCBmb3IgUGV0ZXIgSm9uZXMgPHRpcGJvdEB6eXRvci5jb20+?=
2016-02-17 12:35 ` [PATCH 02/13] efi/runtime-wrappers: Run UEFI Runtime Services with interrupts enabled Matt Fleming
2016-02-22 12:13   ` [tip:efi/core] " =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?=
2016-02-17 12:35 ` [PATCH 03/13] x86/mm/pageattr: Use _PAGE_GLOBAL bit for EFI page table mappings Matt Fleming
2016-02-22 12:13   ` [tip:efi/core] x86/mm/pat: " =?UTF-8?B?dGlwLWJvdCBmb3IgU2FpIFByYW5lZXRoIDx0aXBib3RAenl0b3IuY29tPg==?=
2016-02-23 17:47     ` Andy Lutomirski
2016-02-23 18:08       ` Linus Torvalds
2016-02-23 18:12         ` Borislav Petkov
2016-02-24  2:09         ` H. Peter Anvin
2016-02-24  2:13           ` H. Peter Anvin
2016-02-25  8:59         ` Ingo Molnar
2016-02-24  0:50       ` Sai Praneeth Prakhya
2016-02-24  2:43         ` Andy Lutomirski
2016-02-24 14:10           ` Matt Fleming
2016-02-24 16:20             ` Borislav Petkov
2016-02-24 16:36               ` Andy Lutomirski
2016-02-24 18:56                 ` Linus Torvalds
2016-02-24 19:45                   ` Matt Fleming
2016-02-24 19:50                     ` Andy Lutomirski
2016-02-29 10:56                     ` Sylvain Chouleur
2016-03-02 11:20                       ` Matt Fleming
2016-02-24 19:33                 ` Matt Fleming
2016-02-24 19:49                   ` Andy Lutomirski
2016-02-25  9:06                     ` Ingo Molnar
2016-02-25 15:27                     ` Matt Fleming
2016-02-24 16:41               ` Borislav Petkov
2016-02-24 16:47                 ` Andy Lutomirski
2016-02-24 16:39             ` Andy Lutomirski
2016-02-25 16:00               ` Matt Fleming
2016-02-17 12:35 ` [PATCH 04/13] efi/arm64: Drop __init annotation from handle_kernel_image() Matt Fleming
2016-02-22 12:14   ` [tip:efi/core] " =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?=
2016-02-17 12:35 ` [PATCH 05/13] arm64: vmlinux.lds.S: Handle .init.rodata.xxx and .init.bss sections Matt Fleming
2016-02-22 12:14   ` [tip:efi/core] arm64/vmlinux.lds.S: " =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?=
2016-02-17 12:35 ` [PATCH 06/13] efi/efistub: Prevent __init annotations from being used Matt Fleming
2016-02-22 12:14   ` [tip:efi/core] " =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?=
2016-02-17 12:36 ` [PATCH 07/13] efi/arm-init: Use read-only early mappings Matt Fleming
2016-02-22 12:15   ` [tip:efi/core] " =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?=
2016-02-17 12:36 ` [PATCH 08/13] efi/arm: Check for LPAE support before booting a LPAE kernel Matt Fleming
2016-02-22 12:15   ` [tip:efi/core] " =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?=
2016-02-17 12:36 ` [PATCH 09/13] efi/arm64: Check for h/w support before booting a >4 KB granule kernel Matt Fleming
2016-02-22 12:16   ` [tip:efi/core] efi/arm64: Check for h/w support before booting a >4 KB granular kernel =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?=
2016-03-06  3:35   ` [PATCH 09/13] efi/arm64: Check for h/w support before booting a >4 KB granule kernel Ard Biesheuvel
2016-03-07 11:02     ` Matt Fleming
2016-03-07 11:05       ` Ard Biesheuvel
2016-02-17 12:36 ` [PATCH 10/13] efi/arm*: Perform hardware compatibility check Matt Fleming
2016-02-22 12:16   ` [tip:efi/core] " =?UTF-8?B?dGlwLWJvdCBmb3IgQXJkIEJpZXNoZXV2ZWwgPHRpcGJvdEB6eXRvci5jb20+?=
2016-02-17 12:36 ` [PATCH 11/13] x86/mm/pageattr: Don't implicitly allow _PAGE_RW in kernel_map_pages_in_pgd() Matt Fleming
2016-02-22 12:16   ` [tip:efi/core] x86/mm/pat: " =?UTF-8?B?dGlwLWJvdCBmb3IgU2FpIFByYW5lZXRoIDx0aXBib3RAenl0b3IuY29tPg==?=
2016-02-17 12:36 ` [PATCH 12/13] x86/efi: Map EFI_MEMORY_{XP,RO} memory region bits to EFI page tables Matt Fleming
2016-02-22 12:17   ` [tip:efi/core] " =?UTF-8?B?dGlwLWJvdCBmb3IgU2FpIFByYW5lZXRoIDx0aXBib3RAenl0b3IuY29tPg==?=
2016-02-17 12:36 ` [PATCH 13/13] x86/efi: Only map kernel text for EFI mixed mode Matt Fleming
2016-02-22 12:17   ` [tip:efi/core] " =?UTF-8?B?dGlwLWJvdCBmb3IgU2FpIFByYW5lZXRoIDx0aXBib3RAenl0b3IuY29tPg==?=

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).