linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] UEFI: EFI_MEMORY_ATTRIBUTES_TABLE support for x86
@ 2016-12-06 19:15 Sai Praneeth Prakhya
  2016-12-06 19:16 ` [PATCH 1/4] efi: Make EFI_MEMORY_ATTRIBUTES_TABLE initialization common across all architectures Sai Praneeth Prakhya
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Sai Praneeth Prakhya @ 2016-12-06 19:15 UTC (permalink / raw)
  To: linux-efi, linux-kernel
  Cc: jlee, bp, ricardo.neri, matt, ard.biesheuvel, ravi.v.shankar,
	fenghua.yu, Sai Praneeth

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

UEFI v2.6 introduces EFI_MEMORY_ATTRIBUTES_TABLE which describes memory 
protections that may be applied to EFI Runtime code and data regions by 
kernel. This helps kernel to map efi runtime regions more strictly and 
hence allowing only appropriate accesses to these regions. Please refer 
to "EFI_MEMORY_ATTRIBUTES_TABLE" in section 4.6 of UEFI specification 
v2.6 for more information on this table.

This patch set relies on commit a604af075a32 ("efi: Add support for the 
EFI_MEMORY_ATTRIBUTES_TABLE config table"), commit 10f0d2f57705 ("efi: 
Implement generic support for the Memory Attributes table") and hence 
implements support for only x86.

Since the above commits have already implemented early discovery and 
validation of table, the following patches implement a call back 
function for x86 which is called only when EFI_MEMORY_ATTRIBUTES_TABLE 
is detected.

Patch #1 makes the efi_memory_attributes table detection code generic 
across all architectures

Patch #2 adds EFI_MEM_ATTR bit to keep track of this feature

Patch #3 Implements call back function that does stricter mappings based 
on this table

Patch #4 Skip parsing of EFI_PROPERTIES_TABLE if EFI_MEMORY_ATTRIBUTES_TABLE 
is detected

Sai Praneeth (4):
  efi: Make EFI_MEMORY_ATTRIBUTES_TABLE initialization common across all
    architectures
  efi: Introduce EFI_MEM_ATTR bit and set it from memory attributes
    table
  x86/efi: Add support for EFI_MEMORY_ATTRIBUTES_TABLE
  efi: Skip parsing of EFI_PROPERTIES_TABLE if
    EFI_MEMORY_ATTRIBUTES_TABLE is detected

 arch/x86/platform/efi/efi_64.c  | 64 ++++++++++++++++++++++++++++++++++-------
 drivers/firmware/efi/arm-init.c |  1 -
 drivers/firmware/efi/efi.c      | 13 +++++++++
 drivers/firmware/efi/memattr.c  |  6 +++-
 include/linux/efi.h             |  1 +
 5 files changed, 73 insertions(+), 12 deletions(-)

-- 
2.1.4

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

* [PATCH 1/4] efi: Make EFI_MEMORY_ATTRIBUTES_TABLE initialization common across all architectures
  2016-12-06 19:15 [PATCH 0/4] UEFI: EFI_MEMORY_ATTRIBUTES_TABLE support for x86 Sai Praneeth Prakhya
@ 2016-12-06 19:16 ` Sai Praneeth Prakhya
  2016-12-07 13:28   ` Matt Fleming
  2016-12-06 19:16 ` [PATCH 2/4] efi: Introduce EFI_MEM_ATTR bit and set it from memory attributes table Sai Praneeth Prakhya
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Sai Praneeth Prakhya @ 2016-12-06 19:16 UTC (permalink / raw)
  To: linux-efi, linux-kernel
  Cc: jlee, bp, ricardo.neri, matt, ard.biesheuvel, ravi.v.shankar,
	fenghua.yu, Sai Praneeth

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

Since EFI_PROPERTIES_TABLE and EFI_MEMORY_ATTRIBUTES_TABLE deal with
updating memory region attributes, it makes sense to call
EFI_MEMORY_ATTRIBUTES_TABLE initialization function from the same place
as EFI_PROPERTIES_TABLE. This also moves the EFI_MEMORY_ATTRIBUTES_TABLE
initialization code to a more generic efi initialization path rather
than ARM specific efi initialization. This is important because
EFI_MEMORY_ATTRIBUTES_TABLE will be supported by x86 as well.

Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Cc: Lee, Chun-Yi <jlee@suse.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Ricardo Neri <ricardo.neri@intel.com>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Ravi Shankar <ravi.v.shankar@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
---
 drivers/firmware/efi/arm-init.c | 1 -
 drivers/firmware/efi/efi.c      | 2 ++
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
index f853ad2c4ca0..1027d7b44358 100644
--- a/drivers/firmware/efi/arm-init.c
+++ b/drivers/firmware/efi/arm-init.c
@@ -250,7 +250,6 @@ void __init efi_init(void)
 	}
 
 	reserve_regions();
-	efi_memattr_init();
 	efi_esrt_init();
 	efi_memmap_unmap();
 
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 92914801e388..e7d404059b73 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -529,6 +529,8 @@ int __init efi_config_parse_tables(void *config_tables, int count, int sz,
 		}
 	}
 
+	efi_memattr_init();
+
 	/* Parse the EFI Properties table if it exists */
 	if (efi.properties_table != EFI_INVALID_TABLE_ADDR) {
 		efi_properties_table_t *tbl;
-- 
2.1.4

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

* [PATCH 2/4] efi: Introduce EFI_MEM_ATTR bit and set it from memory attributes table
  2016-12-06 19:15 [PATCH 0/4] UEFI: EFI_MEMORY_ATTRIBUTES_TABLE support for x86 Sai Praneeth Prakhya
  2016-12-06 19:16 ` [PATCH 1/4] efi: Make EFI_MEMORY_ATTRIBUTES_TABLE initialization common across all architectures Sai Praneeth Prakhya
@ 2016-12-06 19:16 ` Sai Praneeth Prakhya
  2016-12-06 19:16 ` [PATCH 3/4] x86/efi: Add support for EFI_MEMORY_ATTRIBUTES_TABLE Sai Praneeth Prakhya
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Sai Praneeth Prakhya @ 2016-12-06 19:16 UTC (permalink / raw)
  To: linux-efi, linux-kernel
  Cc: jlee, bp, ricardo.neri, matt, ard.biesheuvel, ravi.v.shankar,
	fenghua.yu, Sai Praneeth

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

UEFI v2.6 introduces a configuration table called
EFI_MEMORY_ATTRIBUTES_TABLE which provides additional information about
efi runtime regions. Currently this table describes memory protections
that may be applied to EFI Runtime code and data regions by kernel.
Allocate a EFI_XXX bit to keep track of whether this feature is
published by firmware or not.

Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Cc: Lee, Chun-Yi <jlee@suse.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Ricardo Neri <ricardo.neri@intel.com>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Ravi Shankar <ravi.v.shankar@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
---
 drivers/firmware/efi/memattr.c | 1 +
 include/linux/efi.h            | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/firmware/efi/memattr.c b/drivers/firmware/efi/memattr.c
index 236004b9a50d..402197460507 100644
--- a/drivers/firmware/efi/memattr.c
+++ b/drivers/firmware/efi/memattr.c
@@ -43,6 +43,7 @@ int __init efi_memattr_init(void)
 
 	tbl_size = sizeof(*tbl) + tbl->num_entries * tbl->desc_size;
 	memblock_reserve(efi.mem_attr_table, tbl_size);
+	set_bit(EFI_MEM_ATTR, &efi.flags);
 
 unmap:
 	early_memunmap(tbl, sizeof(*tbl));
diff --git a/include/linux/efi.h b/include/linux/efi.h
index a07a476178cd..41047ea555e3 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1063,6 +1063,7 @@ extern int __init efi_setup_pcdp_console(char *);
 #define EFI_ARCH_1		7	/* First arch-specific bit */
 #define EFI_DBG			8	/* Print additional debug info at runtime */
 #define EFI_NX_PE_DATA		9	/* Can runtime data regions be mapped non-executable? */
+#define EFI_MEM_ATTR		10	/* Did firmware publish EFI_MEMORY_ATTRIBUTES table? */
 
 #ifdef CONFIG_EFI
 /*
-- 
2.1.4

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

* [PATCH 3/4] x86/efi: Add support for EFI_MEMORY_ATTRIBUTES_TABLE
  2016-12-06 19:15 [PATCH 0/4] UEFI: EFI_MEMORY_ATTRIBUTES_TABLE support for x86 Sai Praneeth Prakhya
  2016-12-06 19:16 ` [PATCH 1/4] efi: Make EFI_MEMORY_ATTRIBUTES_TABLE initialization common across all architectures Sai Praneeth Prakhya
  2016-12-06 19:16 ` [PATCH 2/4] efi: Introduce EFI_MEM_ATTR bit and set it from memory attributes table Sai Praneeth Prakhya
@ 2016-12-06 19:16 ` Sai Praneeth Prakhya
  2016-12-06 19:16 ` [PATCH 4/4] efi: Skip parsing of EFI_PROPERTIES_TABLE if EFI_MEMORY_ATTRIBUTES_TABLE is detected Sai Praneeth Prakhya
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Sai Praneeth Prakhya @ 2016-12-06 19:16 UTC (permalink / raw)
  To: linux-efi, linux-kernel
  Cc: jlee, bp, ricardo.neri, matt, ard.biesheuvel, ravi.v.shankar,
	fenghua.yu, Sai Praneeth

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

UEFI v2.6 introduces EFI_MEMORY_ATTRIBUTES_TABLE which describes memory
protections that may be applied to EFI Runtime code and data regions by
kernel. This enables kernel to map these regions more strictly thereby
increasing security. Presently, the only valid bits for attribute field
of a memory descriptor are EFI_MEMORY_RO and EFI_MEMORY_XP, hence use
these bits to update mappings in efi_pgd.

UEFI specification recommends to use this feature instead of
EFI_PROPERTIES_TABLE and hence while updating efi mappings we first
check for EFI_MEMORY_ATTRIBUTES_TABLE and if it's present we update
mappings according to this table and hence disregarding
EFI_PROPERTIES_TABLE even if it's published by firmware. We consider
EFI_PROPERTIES_TABLE only when EFI_MEMORY_ATTRIBUTES_TABLE is absent.

Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Cc: Lee, Chun-Yi <jlee@suse.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Ricardo Neri <ricardo.neri@intel.com>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Ravi Shankar <ravi.v.shankar@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/platform/efi/efi_64.c | 64 +++++++++++++++++++++++++++++++++++-------
 drivers/firmware/efi/memattr.c |  5 +++-
 2 files changed, 58 insertions(+), 11 deletions(-)

diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
index 58b0f801f66f..de12d9f5cfc3 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -368,10 +368,44 @@ void __init parse_efi_setup(u64 phys_addr, u32 data_len)
 	efi_setup = phys_addr + sizeof(struct setup_data);
 }
 
-void __init efi_runtime_update_mappings(void)
+static int __init efi_update_mappings(efi_memory_desc_t *md, unsigned long pf)
 {
 	unsigned long pfn;
 	pgd_t *pgd = efi_pgd;
+	int err1, err2;
+
+	/* Update the 1:1 mapping */
+	pfn = md->phys_addr >> PAGE_SHIFT;
+	err1 = kernel_map_pages_in_pgd(pgd, pfn, md->phys_addr, md->num_pages, pf);
+	if (err1) {
+		pr_err("Error while updating 1:1 mapping PA 0x%llx -> VA 0x%llx!\n",
+			   md->phys_addr, md->virt_addr);
+	}
+
+	err2 = kernel_map_pages_in_pgd(pgd, pfn, md->virt_addr, md->num_pages, pf);
+	if (err2) {
+		pr_err("Error while updating VA mapping PA 0x%llx -> VA 0x%llx!\n",
+			   md->phys_addr, md->virt_addr);
+	}
+
+	return err1 || err2;
+}
+
+static int __init efi_update_mem_attr(struct mm_struct *mm, efi_memory_desc_t *md)
+{
+	unsigned long pf = 0;
+
+	if (md->attribute & EFI_MEMORY_XP)
+		pf |= _PAGE_NX;
+
+	if (!(md->attribute & EFI_MEMORY_RO))
+		pf |= _PAGE_RW;
+
+	return efi_update_mappings(md, pf);
+}
+
+void __init efi_runtime_update_mappings(void)
+{
 	efi_memory_desc_t *md;
 
 	if (efi_enabled(EFI_OLD_MEMMAP)) {
@@ -380,6 +414,24 @@ void __init efi_runtime_update_mappings(void)
 		return;
 	}
 
+	/*
+	 * Use EFI Memory Attribute Table for mapping permissions if it
+	 * exists, since it is intended to supersede EFI_PROPERTIES_TABLE.
+	 */
+	if (efi_enabled(EFI_MEM_ATTR)) {
+		efi_memattr_apply_permissions(NULL, efi_update_mem_attr);
+		return;
+	}
+
+	/*
+	 * EFI_MEMORY_ATTRIBUTES_TABLE is intended to replace
+	 * EFI_PROPERTIES_TABLE. So, use EFI_PROPERTIES_TABLE to update
+	 * permissions only if EFI_MEMORY_ATTRIBUTES_TABLE is not
+	 * published by firmware. Even if we find a buggy implementation of
+	 * EFI_MEMORY_ATTRIBUTES_TABLE don't fall back to
+	 * EFI_PROPERTIES_TABLE because of the same above mentioned reason.
+	 */
+
 	if (!efi_enabled(EFI_NX_PE_DATA))
 		return;
 
@@ -400,15 +452,7 @@ void __init efi_runtime_update_mappings(void)
 			(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);
+		efi_update_mappings(md, pf);
 	}
 }
 
diff --git a/drivers/firmware/efi/memattr.c b/drivers/firmware/efi/memattr.c
index 402197460507..8986757eafaf 100644
--- a/drivers/firmware/efi/memattr.c
+++ b/drivers/firmware/efi/memattr.c
@@ -175,8 +175,11 @@ int __init efi_memattr_apply_permissions(struct mm_struct *mm,
 				md.phys_addr + size - 1,
 				efi_md_typeattr_format(buf, sizeof(buf), &md));
 
-		if (valid)
+		if (valid) {
 			ret = fn(mm, &md);
+			if (ret)
+				pr_err("Error updating mappings, skipping subsequent md's\n");
+		}
 	}
 	memunmap(tbl);
 	return ret;
-- 
2.1.4

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

* [PATCH 4/4] efi: Skip parsing of EFI_PROPERTIES_TABLE if EFI_MEMORY_ATTRIBUTES_TABLE is detected
  2016-12-06 19:15 [PATCH 0/4] UEFI: EFI_MEMORY_ATTRIBUTES_TABLE support for x86 Sai Praneeth Prakhya
                   ` (2 preceding siblings ...)
  2016-12-06 19:16 ` [PATCH 3/4] x86/efi: Add support for EFI_MEMORY_ATTRIBUTES_TABLE Sai Praneeth Prakhya
@ 2016-12-06 19:16 ` Sai Praneeth Prakhya
  2016-12-07 13:36   ` Matt Fleming
  2016-12-07 13:26 ` [PATCH 0/4] UEFI: EFI_MEMORY_ATTRIBUTES_TABLE support for x86 Matt Fleming
  2016-12-07 13:56 ` Matt Fleming
  5 siblings, 1 reply; 13+ messages in thread
From: Sai Praneeth Prakhya @ 2016-12-06 19:16 UTC (permalink / raw)
  To: linux-efi, linux-kernel
  Cc: jlee, bp, ricardo.neri, matt, ard.biesheuvel, ravi.v.shankar,
	fenghua.yu, Sai Praneeth

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

UEFI specification v2.6 recommends not to use
"EFI_PROPERTIES_RUNTIME_MEMORY_PROTECTION_NON_EXECUTABLE_PE_DATA"
attribute of EFI_PROPERTIES_TABLE. Presently, this is the *only* bit
defined in EFI_PROPERTIES_TABLE. This bit implies that EFI Runtime code
and data regions of an executable image are separate and are aligned as
specified in spec. Please refer to "EFI_PROPERTIES_TABLE" in section 4.6
of UEFI specification v2.6 for more information on this table.

UEFI v2.6 introduces EFI_MEMORY_ATTRIBUTES_TABLE and is intended to
replace EFI_PROPERTIES_TABLE. If EFI_MEMORY_ATTRIBUTES_TABLE is found we
skip updating of efi runtime region mappings based on
EFI_PROPERTIES_TABLE, so let's also skip parsing of EFI_PROPERTIES_TABLE
if we find EFI_MEMORY_ATTRIBUTES_TABLE because we are not using this
table anyways. The only caveat here is, if further versions of UEFI spec
adds some more bits (hence some more attributes) to EFI_PROPERTIES_TABLE
then we might need to parse it again, otherwise there is no good in
doing that. We can also expect that the same attributes might be reflected in
EFI_MEMORY_ATTRIBUTES_TABLE and hence saving us from parsing
EFI_PROPERTIES_TABLE again.

Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Cc: Lee, Chun-Yi <jlee@suse.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Ricardo Neri <ricardo.neri@intel.com>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Ravi Shankar <ravi.v.shankar@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
---
 drivers/firmware/efi/efi.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index e7d404059b73..e6c6feaa4d78 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -531,6 +531,17 @@ int __init efi_config_parse_tables(void *config_tables, int count, int sz,
 
 	efi_memattr_init();
 
+	/*
+	 * Since EFI_MEMORY_ATTRIBUTES_TABLE is intended to replace
+	 * EFI_PROPERTIES_TABLE, let's skip parsing of EFI_PROPERTIES_TABLE
+	 * if we find EFI_MEMORY_ATTRIBUTES_TABLE.
+	 * Note: We might need to *re-enable* parsing of EFI_PROPERTIES_TABLE
+	 * if it defines some bits that are not defined in
+	 * EFI_MEMORY_ATTRIBUTES_TABLE.
+	 */
+	if (efi_enabled(EFI_MEM_ATTR))
+		return 0;
+
 	/* Parse the EFI Properties table if it exists */
 	if (efi.properties_table != EFI_INVALID_TABLE_ADDR) {
 		efi_properties_table_t *tbl;
-- 
2.1.4

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

* Re: [PATCH 0/4] UEFI: EFI_MEMORY_ATTRIBUTES_TABLE support for x86
  2016-12-06 19:15 [PATCH 0/4] UEFI: EFI_MEMORY_ATTRIBUTES_TABLE support for x86 Sai Praneeth Prakhya
                   ` (3 preceding siblings ...)
  2016-12-06 19:16 ` [PATCH 4/4] efi: Skip parsing of EFI_PROPERTIES_TABLE if EFI_MEMORY_ATTRIBUTES_TABLE is detected Sai Praneeth Prakhya
@ 2016-12-07 13:26 ` Matt Fleming
  2016-12-07 13:56 ` Matt Fleming
  5 siblings, 0 replies; 13+ messages in thread
From: Matt Fleming @ 2016-12-07 13:26 UTC (permalink / raw)
  To: Sai Praneeth Prakhya
  Cc: linux-efi, linux-kernel, jlee, bp, ricardo.neri, ard.biesheuvel,
	ravi.v.shankar, fenghua.yu

On Tue, 06 Dec, at 11:15:59AM, Sai Praneeth Prakhya wrote:
> From: Sai Praneeth <sai.praneeth.prakhya@intel.com>
> 
> UEFI v2.6 introduces EFI_MEMORY_ATTRIBUTES_TABLE which describes memory 
> protections that may be applied to EFI Runtime code and data regions by 
> kernel. This helps kernel to map efi runtime regions more strictly and 
> hence allowing only appropriate accesses to these regions. Please refer 
> to "EFI_MEMORY_ATTRIBUTES_TABLE" in section 4.6 of UEFI specification 
> v2.6 for more information on this table.
 
I guess this is v3 of your patch series right? Please be sure to
include a version tag in future like you did for you v2. 

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

* Re: [PATCH 1/4] efi: Make EFI_MEMORY_ATTRIBUTES_TABLE initialization common across all architectures
  2016-12-06 19:16 ` [PATCH 1/4] efi: Make EFI_MEMORY_ATTRIBUTES_TABLE initialization common across all architectures Sai Praneeth Prakhya
@ 2016-12-07 13:28   ` Matt Fleming
  0 siblings, 0 replies; 13+ messages in thread
From: Matt Fleming @ 2016-12-07 13:28 UTC (permalink / raw)
  To: Sai Praneeth Prakhya
  Cc: linux-efi, linux-kernel, jlee, bp, ricardo.neri, ard.biesheuvel,
	ravi.v.shankar, fenghua.yu

On Tue, 06 Dec, at 11:16:00AM, Sai Praneeth Prakhya wrote:
> From: Sai Praneeth <sai.praneeth.prakhya@intel.com>
> 
> Since EFI_PROPERTIES_TABLE and EFI_MEMORY_ATTRIBUTES_TABLE deal with
> updating memory region attributes, it makes sense to call
> EFI_MEMORY_ATTRIBUTES_TABLE initialization function from the same place
> as EFI_PROPERTIES_TABLE. This also moves the EFI_MEMORY_ATTRIBUTES_TABLE
> initialization code to a more generic efi initialization path rather
> than ARM specific efi initialization. This is important because
> EFI_MEMORY_ATTRIBUTES_TABLE will be supported by x86 as well.
> 
> Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
> Cc: Lee, Chun-Yi <jlee@suse.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Ricardo Neri <ricardo.neri@intel.com>
> Cc: Matt Fleming <matt@codeblueprint.co.uk>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Ravi Shankar <ravi.v.shankar@intel.com>
> Cc: Fenghua Yu <fenghua.yu@intel.com>
> ---
>  drivers/firmware/efi/arm-init.c | 1 -
>  drivers/firmware/efi/efi.c      | 2 ++
>  2 files changed, 2 insertions(+), 1 deletion(-)

You should have applied Joey's Reviewed-by tag when sending out this
version so that it doesn't get lost.

It's not a big deal, just something to watch out for with future
submissions.

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

* Re: [PATCH 4/4] efi: Skip parsing of EFI_PROPERTIES_TABLE if EFI_MEMORY_ATTRIBUTES_TABLE is detected
  2016-12-06 19:16 ` [PATCH 4/4] efi: Skip parsing of EFI_PROPERTIES_TABLE if EFI_MEMORY_ATTRIBUTES_TABLE is detected Sai Praneeth Prakhya
@ 2016-12-07 13:36   ` Matt Fleming
  0 siblings, 0 replies; 13+ messages in thread
From: Matt Fleming @ 2016-12-07 13:36 UTC (permalink / raw)
  To: Sai Praneeth Prakhya
  Cc: linux-efi, linux-kernel, jlee, bp, ricardo.neri, ard.biesheuvel,
	ravi.v.shankar, fenghua.yu

On Tue, 06 Dec, at 11:16:03AM, Sai Praneeth Prakhya wrote:
> From: Sai Praneeth <sai.praneeth.prakhya@intel.com>
> 
> UEFI specification v2.6 recommends not to use
> "EFI_PROPERTIES_RUNTIME_MEMORY_PROTECTION_NON_EXECUTABLE_PE_DATA"
> attribute of EFI_PROPERTIES_TABLE. Presently, this is the *only* bit
> defined in EFI_PROPERTIES_TABLE. This bit implies that EFI Runtime code
> and data regions of an executable image are separate and are aligned as
> specified in spec. Please refer to "EFI_PROPERTIES_TABLE" in section 4.6
> of UEFI specification v2.6 for more information on this table.
> 
> UEFI v2.6 introduces EFI_MEMORY_ATTRIBUTES_TABLE and is intended to
> replace EFI_PROPERTIES_TABLE. If EFI_MEMORY_ATTRIBUTES_TABLE is found we
> skip updating of efi runtime region mappings based on
> EFI_PROPERTIES_TABLE, so let's also skip parsing of EFI_PROPERTIES_TABLE
> if we find EFI_MEMORY_ATTRIBUTES_TABLE because we are not using this
> table anyways. The only caveat here is, if further versions of UEFI spec
> adds some more bits (hence some more attributes) to EFI_PROPERTIES_TABLE
> then we might need to parse it again, otherwise there is no good in
> doing that. We can also expect that the same attributes might be reflected in
> EFI_MEMORY_ATTRIBUTES_TABLE and hence saving us from parsing
> EFI_PROPERTIES_TABLE again.
> 
> Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
> Cc: Lee, Chun-Yi <jlee@suse.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Ricardo Neri <ricardo.neri@intel.com>
> Cc: Matt Fleming <matt@codeblueprint.co.uk>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Ravi Shankar <ravi.v.shankar@intel.com>
> Cc: Fenghua Yu <fenghua.yu@intel.com>
> ---
>  drivers/firmware/efi/efi.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)

I see where you're coming from with this patch, but I think it's
unnecessary. Turning on/off parsing of Table A based on existence of
Table B just seems like extra work.

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

* Re: [PATCH 0/4] UEFI: EFI_MEMORY_ATTRIBUTES_TABLE support for x86
  2016-12-06 19:15 [PATCH 0/4] UEFI: EFI_MEMORY_ATTRIBUTES_TABLE support for x86 Sai Praneeth Prakhya
                   ` (4 preceding siblings ...)
  2016-12-07 13:26 ` [PATCH 0/4] UEFI: EFI_MEMORY_ATTRIBUTES_TABLE support for x86 Matt Fleming
@ 2016-12-07 13:56 ` Matt Fleming
  2016-12-07 19:01   ` Sai Praneeth Prakhya
  5 siblings, 1 reply; 13+ messages in thread
From: Matt Fleming @ 2016-12-07 13:56 UTC (permalink / raw)
  To: Sai Praneeth Prakhya
  Cc: linux-efi, linux-kernel, jlee, bp, ricardo.neri, ard.biesheuvel,
	ravi.v.shankar, fenghua.yu

On Tue, 06 Dec, at 11:15:59AM, Sai Praneeth Prakhya wrote:
> From: Sai Praneeth <sai.praneeth.prakhya@intel.com>
> 
> UEFI v2.6 introduces EFI_MEMORY_ATTRIBUTES_TABLE which describes memory 
> protections that may be applied to EFI Runtime code and data regions by 
> kernel. This helps kernel to map efi runtime regions more strictly and 
> hence allowing only appropriate accesses to these regions. Please refer 
> to "EFI_MEMORY_ATTRIBUTES_TABLE" in section 4.6 of UEFI specification 
> v2.6 for more information on this table.
> 
> This patch set relies on commit a604af075a32 ("efi: Add support for the 
> EFI_MEMORY_ATTRIBUTES_TABLE config table"), commit 10f0d2f57705 ("efi: 
> Implement generic support for the Memory Attributes table") and hence 
> implements support for only x86.
> 
> Since the above commits have already implemented early discovery and 
> validation of table, the following patches implement a call back 
> function for x86 which is called only when EFI_MEMORY_ATTRIBUTES_TABLE 
> is detected.
> 
> Patch #1 makes the efi_memory_attributes table detection code generic 
> across all architectures
> 
> Patch #2 adds EFI_MEM_ATTR bit to keep track of this feature
> 
> Patch #3 Implements call back function that does stricter mappings based 
> on this table
> 
> Patch #4 Skip parsing of EFI_PROPERTIES_TABLE if EFI_MEMORY_ATTRIBUTES_TABLE 
> is detected
> 
> Sai Praneeth (4):
>   efi: Make EFI_MEMORY_ATTRIBUTES_TABLE initialization common across all
>     architectures
>   efi: Introduce EFI_MEM_ATTR bit and set it from memory attributes
>     table
>   x86/efi: Add support for EFI_MEMORY_ATTRIBUTES_TABLE
>   efi: Skip parsing of EFI_PROPERTIES_TABLE if
>     EFI_MEMORY_ATTRIBUTES_TABLE is detected
> 
>  arch/x86/platform/efi/efi_64.c  | 64 ++++++++++++++++++++++++++++++++++-------
>  drivers/firmware/efi/arm-init.c |  1 -
>  drivers/firmware/efi/efi.c      | 13 +++++++++
>  drivers/firmware/efi/memattr.c  |  6 +++-
>  include/linux/efi.h             |  1 +
>  5 files changed, 73 insertions(+), 12 deletions(-)

Thanks Sai, I've queued this up for v4.11.

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

* Re: [PATCH 0/4] UEFI: EFI_MEMORY_ATTRIBUTES_TABLE support for x86
  2016-12-07 13:56 ` Matt Fleming
@ 2016-12-07 19:01   ` Sai Praneeth Prakhya
  2016-12-07 20:04     ` Matt Fleming
  0 siblings, 1 reply; 13+ messages in thread
From: Sai Praneeth Prakhya @ 2016-12-07 19:01 UTC (permalink / raw)
  To: Matt Fleming
  Cc: linux-efi, linux-kernel, jlee, bp, ricardo.neri, ard.biesheuvel,
	ravi.v.shankar, fenghua.yu

On Wed, 2016-12-07 at 13:56 +0000, Matt Fleming wrote:
> On Tue, 06 Dec, at 11:15:59AM, Sai Praneeth Prakhya wrote:
> > From: Sai Praneeth <sai.praneeth.prakhya@intel.com>
> > 
> > UEFI v2.6 introduces EFI_MEMORY_ATTRIBUTES_TABLE which describes memory 
> > protections that may be applied to EFI Runtime code and data regions by 
> > kernel. This helps kernel to map efi runtime regions more strictly and 
> > hence allowing only appropriate accesses to these regions. Please refer 
> > to "EFI_MEMORY_ATTRIBUTES_TABLE" in section 4.6 of UEFI specification 
> > v2.6 for more information on this table.
> > 
> > This patch set relies on commit a604af075a32 ("efi: Add support for the 
> > EFI_MEMORY_ATTRIBUTES_TABLE config table"), commit 10f0d2f57705 ("efi: 
> > Implement generic support for the Memory Attributes table") and hence 
> > implements support for only x86.
> > 
> > Since the above commits have already implemented early discovery and 
> > validation of table, the following patches implement a call back 
> > function for x86 which is called only when EFI_MEMORY_ATTRIBUTES_TABLE 
> > is detected.
> > 
> > Patch #1 makes the efi_memory_attributes table detection code generic 
> > across all architectures
> > 
> > Patch #2 adds EFI_MEM_ATTR bit to keep track of this feature
> > 
> > Patch #3 Implements call back function that does stricter mappings based 
> > on this table
> > 
> > Patch #4 Skip parsing of EFI_PROPERTIES_TABLE if EFI_MEMORY_ATTRIBUTES_TABLE 
> > is detected
> > 
> > Sai Praneeth (4):
> >   efi: Make EFI_MEMORY_ATTRIBUTES_TABLE initialization common across all
> >     architectures
> >   efi: Introduce EFI_MEM_ATTR bit and set it from memory attributes
> >     table
> >   x86/efi: Add support for EFI_MEMORY_ATTRIBUTES_TABLE
> >   efi: Skip parsing of EFI_PROPERTIES_TABLE if
> >     EFI_MEMORY_ATTRIBUTES_TABLE is detected
> > 
> >  arch/x86/platform/efi/efi_64.c  | 64 ++++++++++++++++++++++++++++++++++-------
> >  drivers/firmware/efi/arm-init.c |  1 -
> >  drivers/firmware/efi/efi.c      | 13 +++++++++
> >  drivers/firmware/efi/memattr.c  |  6 +++-
> >  include/linux/efi.h             |  1 +
> >  5 files changed, 73 insertions(+), 12 deletions(-)
> 
> Thanks Sai, I've queued this up for v4.11.

Thanks Matt!

Would you like to see a new version of these patch series addressing
your comments? Like
1. Dropping of patch #4
2. Adding Reviewed-by tag of Joey (Sorry for that)
3. This time with correct version number

Regards,
Sai

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

* Re: [PATCH 0/4] UEFI: EFI_MEMORY_ATTRIBUTES_TABLE support for x86
  2016-12-07 19:01   ` Sai Praneeth Prakhya
@ 2016-12-07 20:04     ` Matt Fleming
  2016-12-07 20:13       ` Sai Praneeth Prakhya
  0 siblings, 1 reply; 13+ messages in thread
From: Matt Fleming @ 2016-12-07 20:04 UTC (permalink / raw)
  To: Sai Praneeth Prakhya
  Cc: linux-efi, linux-kernel, jlee, bp, ricardo.neri, ard.biesheuvel,
	ravi.v.shankar, fenghua.yu

On Wed, 07 Dec, at 11:01:06AM, Sai Praneeth Prakhya wrote:
> 
> Thanks Matt!
> 
> Would you like to see a new version of these patch series addressing
> your comments? Like
> 1. Dropping of patch #4
> 2. Adding Reviewed-by tag of Joey (Sorry for that)
> 3. This time with correct version number

No need, they're already in the 'next' branch of the EFI tree.

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

* Re: [PATCH 0/4] UEFI: EFI_MEMORY_ATTRIBUTES_TABLE support for x86
  2016-12-07 20:04     ` Matt Fleming
@ 2016-12-07 20:13       ` Sai Praneeth Prakhya
  2016-12-13  6:47         ` joeyli
  0 siblings, 1 reply; 13+ messages in thread
From: Sai Praneeth Prakhya @ 2016-12-07 20:13 UTC (permalink / raw)
  To: Matt Fleming
  Cc: linux-efi, linux-kernel, jlee, bp, ricardo.neri, ard.biesheuvel,
	ravi.v.shankar, fenghua.yu

On Wed, 2016-12-07 at 20:04 +0000, Matt Fleming wrote:
> On Wed, 07 Dec, at 11:01:06AM, Sai Praneeth Prakhya wrote:
> > 
> > Thanks Matt!
> > 
> > Would you like to see a new version of these patch series addressing
> > your comments? Like
> > 1. Dropping of patch #4
> > 2. Adding Reviewed-by tag of Joey (Sorry for that)
> > 3. This time with correct version number
> 
> No need, they're already in the 'next' branch of the EFI tree.

Great! Just saw the patches in 'next' branch.
Thanks again Matt.

Regards,
Sai

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

* Re: [PATCH 0/4] UEFI: EFI_MEMORY_ATTRIBUTES_TABLE support for x86
  2016-12-07 20:13       ` Sai Praneeth Prakhya
@ 2016-12-13  6:47         ` joeyli
  0 siblings, 0 replies; 13+ messages in thread
From: joeyli @ 2016-12-13  6:47 UTC (permalink / raw)
  To: Sai Praneeth Prakhya
  Cc: Matt Fleming, linux-efi, linux-kernel, bp, ricardo.neri,
	ard.biesheuvel, ravi.v.shankar, fenghua.yu

Hi Sai,

On Wed, Dec 07, 2016 at 12:13:28PM -0800, Sai Praneeth Prakhya wrote:
> On Wed, 2016-12-07 at 20:04 +0000, Matt Fleming wrote:
> > On Wed, 07 Dec, at 11:01:06AM, Sai Praneeth Prakhya wrote:
> > > 
> > > Thanks Matt!
> > > 
> > > Would you like to see a new version of these patch series addressing
> > > your comments? Like
> > > 1. Dropping of patch #4
> > > 2. Adding Reviewed-by tag of Joey (Sorry for that)
> > > 3. This time with correct version number
> > 
> > No need, they're already in the 'next' branch of the EFI tree.
> 
> Great! Just saw the patches in 'next' branch.
> Thanks again Matt.
> 
> Regards,
> Sai
> 
>

Thanks for your patches to enable this function for x86.

Joey Lee 

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

end of thread, other threads:[~2016-12-13  6:47 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-06 19:15 [PATCH 0/4] UEFI: EFI_MEMORY_ATTRIBUTES_TABLE support for x86 Sai Praneeth Prakhya
2016-12-06 19:16 ` [PATCH 1/4] efi: Make EFI_MEMORY_ATTRIBUTES_TABLE initialization common across all architectures Sai Praneeth Prakhya
2016-12-07 13:28   ` Matt Fleming
2016-12-06 19:16 ` [PATCH 2/4] efi: Introduce EFI_MEM_ATTR bit and set it from memory attributes table Sai Praneeth Prakhya
2016-12-06 19:16 ` [PATCH 3/4] x86/efi: Add support for EFI_MEMORY_ATTRIBUTES_TABLE Sai Praneeth Prakhya
2016-12-06 19:16 ` [PATCH 4/4] efi: Skip parsing of EFI_PROPERTIES_TABLE if EFI_MEMORY_ATTRIBUTES_TABLE is detected Sai Praneeth Prakhya
2016-12-07 13:36   ` Matt Fleming
2016-12-07 13:26 ` [PATCH 0/4] UEFI: EFI_MEMORY_ATTRIBUTES_TABLE support for x86 Matt Fleming
2016-12-07 13:56 ` Matt Fleming
2016-12-07 19:01   ` Sai Praneeth Prakhya
2016-12-07 20:04     ` Matt Fleming
2016-12-07 20:13       ` Sai Praneeth Prakhya
2016-12-13  6:47         ` joeyli

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