Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1336382 > unrolled thread

[GIT PULL 00/13] EFI changes for v4.6 part 2

Back to article view | Back to linux.kernel

Contents

  [GIT PULL 00/13] EFI changes for v4.6 part 2 Matt Fleming <matt@codeblueprint.co.uk> - 2016-02-17 13:40 +0100
    [PATCH 08/13] efi/arm: Check for LPAE support before booting a LPAE kernel Matt Fleming <matt@codeblueprint.co.uk> - 2016-02-17 13:40 +0100
      [tip:efi/core] efi/arm: Check for LPAE support before booting a LPAE kernel tip-bot for Ard Biesheuvel <tipbot@zytor.com>@zytor.com - 2016-02-23 10:30 +0100
    [PATCH 11/13] x86/mm/pageattr: Don't implicitly allow _PAGE_RW in kernel_map_pages_in_pgd() Matt Fleming <matt@codeblueprint.co.uk> - 2016-02-17 13:40 +0100
      [tip:efi/core] x86/mm/pat: Don't implicitly allow _PAGE_RW in kernel_map_pages_in_pgd() tip-bot for Sai Praneeth <tipbot@zytor.com>@zytor.com - 2016-02-23 10:20 +0100
    [PATCH 05/13] arm64: vmlinux.lds.S: Handle .init.rodata.xxx and .init.bss sections Matt Fleming <matt@codeblueprint.co.uk> - 2016-02-17 13:40 +0100
      [tip:efi/core] arm64/vmlinux.lds.S: Handle .init.rodata.xxx and .init.bss sections tip-bot for Ard Biesheuvel <tipbot@zytor.com>@zytor.com - 2016-02-23 10:30 +0100
    [PATCH 03/13] x86/mm/pageattr: Use _PAGE_GLOBAL bit for EFI page table mappings Matt Fleming <matt@codeblueprint.co.uk> - 2016-02-17 13:40 +0100
      [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings tip-bot for Sai Praneeth <tipbot@zytor.com>@zytor.com - 2016-02-23 10:20 +0100
        Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page  table mappings Andy Lutomirski <luto@amacapital.net> - 2016-02-23 18:50 +0100
          Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page  table mappings Linus Torvalds <torvalds@linux-foundation.org> - 2016-02-23 19:10 +0100
            Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page  table mappings Borislav Petkov <bp@alien8.de> - 2016-02-23 19:20 +0100
            Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings "H. Peter Anvin" <hpa@zytor.com> - 2016-02-24 03:20 +0100
            Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings "H. Peter Anvin" <hpa@zytor.com> - 2016-02-24 03:20 +0100
            Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page  table mappings Ingo Molnar <mingo@kernel.org> - 2016-02-25 10:00 +0100
          Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page  table mappings Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com> - 2016-02-24 02:00 +0100
            Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page  table mappings Andy Lutomirski <luto@amacapital.net> - 2016-02-24 03:50 +0100
              Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page  table mappings Matt Fleming <matt@codeblueprint.co.uk> - 2016-02-24 15:20 +0100
                Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page  table mappings Borislav Petkov <bp@alien8.de> - 2016-02-24 17:30 +0100
                  Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page  table mappings Andy Lutomirski <luto@amacapital.net> - 2016-02-24 17:40 +0100
                    Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page  table mappings Linus Torvalds <torvalds@linux-foundation.org> - 2016-02-24 20:00 +0100
                      Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page  table mappings Matt Fleming <matt@codeblueprint.co.uk> - 2016-02-24 20:50 +0100
                        Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page  table mappings Andy Lutomirski <luto@amacapital.net> - 2016-02-24 21:00 +0100
                        Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page  table mappings Sylvain Chouleur <sylvain.chouleur@gmail.com> - 2016-02-29 12:00 +0100
                          Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page  table mappings Matt Fleming <matt@codeblueprint.co.uk> - 2016-03-02 12:30 +0100
                    Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page  table mappings Matt Fleming <matt@codeblueprint.co.uk> - 2016-02-24 20:40 +0100
                      Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page  table mappings Andy Lutomirski <luto@amacapital.net> - 2016-02-24 21:00 +0100
                        Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page  table mappings Ingo Molnar <mingo@kernel.org> - 2016-02-25 10:10 +0100
                        Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page  table mappings Matt Fleming <matt@codeblueprint.co.uk> - 2016-02-25 16:30 +0100
                  Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page  table mappings Andy Lutomirski <luto@amacapital.net> - 2016-02-24 17:50 +0100
                  Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page  table mappings Borislav Petkov <bp@alien8.de> - 2016-02-24 17:50 +0100
                Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page  table mappings Andy Lutomirski <luto@amacapital.net> - 2016-02-24 17:50 +0100
                  Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page  table mappings Matt Fleming <matt@codeblueprint.co.uk> - 2016-02-25 17:10 +0100
    [PATCH 07/13] efi/arm-init: Use read-only early mappings Matt Fleming <matt@codeblueprint.co.uk> - 2016-02-17 13:40 +0100
      [tip:efi/core] efi/arm-init: Use read-only early mappings tip-bot for Ard Biesheuvel <tipbot@zytor.com>@zytor.com - 2016-02-23 10:30 +0100
    [PATCH 06/13] efi/efistub: Prevent __init annotations from being used Matt Fleming <matt@codeblueprint.co.uk> - 2016-02-17 13:40 +0100
      [tip:efi/core] efi/efistub: Prevent __init annotations from being used tip-bot for Ard Biesheuvel <tipbot@zytor.com>@zytor.com - 2016-02-23 10:10 +0100
    [PATCH 02/13] efi/runtime-wrappers: Run UEFI Runtime Services with interrupts enabled Matt Fleming <matt@codeblueprint.co.uk> - 2016-02-17 13:40 +0100
      [tip:efi/core] efi/runtime-wrappers: Run UEFI Runtime Services with interrupts enabled tip-bot for Ard Biesheuvel <tipbot@zytor.com>@zytor.com - 2016-02-23 10:30 +0100
    [PATCH 13/13] x86/efi: Only map kernel text for EFI mixed mode Matt Fleming <matt@codeblueprint.co.uk> - 2016-02-17 13:40 +0100
      [tip:efi/core] x86/efi: Only map kernel text for EFI mixed mode tip-bot for Sai Praneeth <tipbot@zytor.com>@zytor.com - 2016-02-23 10:20 +0100
    [PATCH 12/13] x86/efi: Map EFI_MEMORY_{XP,RO} memory region bits to EFI page tables Matt Fleming <matt@codeblueprint.co.uk> - 2016-02-17 13:40 +0100
      [tip:efi/core] x86/efi: Map EFI_MEMORY_{XP,RO} memory region bits to EFI page tables tip-bot for Sai Praneeth <tipbot@zytor.com>@zytor.com - 2016-02-23 10:20 +0100
    [PATCH 10/13] efi/arm*: Perform hardware compatibility check Matt Fleming <matt@codeblueprint.co.uk> - 2016-02-17 13:40 +0100
      [tip:efi/core] efi/arm*: Perform hardware compatibility check tip-bot for Ard Biesheuvel <tipbot@zytor.com>@zytor.com - 2016-02-23 10:30 +0100
    [PATCH 09/13] efi/arm64: Check for h/w support before booting a >4 KB granule kernel Matt Fleming <matt@codeblueprint.co.uk> - 2016-02-17 13:40 +0100
      [tip:efi/core] efi/arm64: Check for h/w support before booting a >4 KB granular kernel tip-bot for Ard Biesheuvel <tipbot@zytor.com>@zytor.com - 2016-02-23 10:30 +0100
    [PATCH 01/13] efi: Reformat GUID tables to follow the format in UEFI spec Matt Fleming <matt@codeblueprint.co.uk> - 2016-02-17 13:50 +0100
      [tip:efi/core] efi: Reformat GUID tables to follow the format in UEFI spec tip-bot for Peter Jones <tipbot@zytor.com>@zytor.com - 2016-02-23 10:30 +0100

Page 1 of 3  [1] 2 3  Next page →

#1336382 — [GIT PULL 00/13] EFI changes for v4.6 part 2

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(-)

[toc] | [next] | [standalone]

#1336383 — [PATCH 08/13] efi/arm: Check for LPAE support before booting a LPAE kernel

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

[toc] | [prev] | [next] | [standalone]

#1340463 — [tip:efi/core] efi/arm: Check for LPAE support before booting a LPAE kernel

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,

[toc] | [prev] | [next] | [standalone]

#1336384 — [PATCH 11/13] x86/mm/pageattr: Don't implicitly allow _PAGE_RW in kernel_map_pages_in_pgd()

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

[toc] | [prev] | [next] | [standalone]

#1340448 — [tip:efi/core] x86/mm/pat: Don't implicitly allow _PAGE_RW in kernel_map_pages_in_pgd()

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;
 

[toc] | [prev] | [next] | [standalone]

#1336385 — [PATCH 05/13] arm64: vmlinux.lds.S: Handle .init.rodata.xxx and .init.bss sections

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

[toc] | [prev] | [next] | [standalone]

#1340468 — [tip:efi/core] arm64/vmlinux.lds.S: Handle .init.rodata.xxx and .init.bss sections

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)

[toc] | [prev] | [next] | [standalone]

#1336386 — [PATCH 03/13] x86/mm/pageattr: Use _PAGE_GLOBAL bit for EFI page table mappings

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

[toc] | [prev] | [next] | [standalone]

#1340438 — [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings

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));
 

[toc] | [prev] | [next] | [standalone]

#1340904 — Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings

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

[toc] | [prev] | [next] | [standalone]

#1340921 — Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings

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

[toc] | [prev] | [next] | [standalone]

#1340924 — Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings

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.

[toc] | [prev] | [next] | [standalone]

#1341250 — Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings

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.

[toc] | [prev] | [next] | [standalone]

#1341252 — Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings

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.

[toc] | [prev] | [next] | [standalone]

#1342987 — Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings

* 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

[toc] | [prev] | [next] | [standalone]

#1341212 — Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings

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

[toc] | [prev] | [next] | [standalone]

#1341262 — Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings

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

[toc] | [prev] | [next] | [standalone]

#1342054 — Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings

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.

[toc] | [prev] | [next] | [standalone]

#1342177 — Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings

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.

[toc] | [prev] | [next] | [standalone]

#1342187 — Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings

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

[toc] | [prev] | [next] | [standalone]

Page 1 of 3  [1] 2 3  Next page →

Back to top | Article view | linux.kernel

csiph-web