2015-07-24 23:51:21

by Zhang, Jonathan Zhixiong

[permalink] [raw]
Subject: [PATCH V8 0/5] map GHES memory region according to EFI memory map

From: "Jonathan (Zhixiong) Zhang" <[email protected]>

On a platform with APEI (ACPI Platform Error Interface) enabled, firmware
updates a memory region with hardware error record using nocache
attribute. When OS reads the region, since it maps the region with
cacahed attribute even though EFI memory map defines this region as
uncached, OS gets stale data and errorneously reports there is no new
HW error.

When ghes driver maps the memory region, it uses the cache attribute
according to EFI memory map, if EFI memory map feature is enabled
at runtime.

Since both arch/x86 and arch/ia64 implemented architecture agnostic EFI
memory map attribue lookup function efi_memattributes(), the code is
moved from arch/x86 into EFI subsystem and is declared as __weak; archs
other than ia64 should not override the default implementation.

V8:
1. For x86, always return PAGE_KERNEL for arch_apei_get_mem_attribute().
The rational is explained in comment.
2. Rebased to arm64-upstream-14201 of arm64/master,
next-20150724 of linux-next/master.
V7:
1. Added PROT_DEVICE_nGnRnE and PROT_NORMAL_WT to support all
possible UEFI memory types for arm64.
V6:
1. Implemented arch_apei_get_mem_attributes() for arm64 as inline
function.
2. Rebased to efi-next-14364 of efi/next, pm+acpi-4.2-rc3 of
linux-pm/master, arm64-upstream-13521 of arm64/master,
next-20150720 of linux-next/master.
V5:
1. Rebased to next-20150713 of linux-next/master, efi-next-14359 of
efi/next, pm+acpi-4.2-rc2 of linux-pm/master,
arm64-fixes-1215 of arm64/master.
2. Added comment for efi_mem_attributes(), explained why it is marked
as __weak at the function definition site.
V4:
1. Introduced arch_apei_get_mem_attributes() to allow arch specific
implementation of getting pgprot_t appropriate for a physical
address.
2. Implemented arch_apei_get_mem_attributes() for x86 and for arm64.
V3:
1. Rebased to v4.1-rc7.
2. Moved efi_mem_attributes() from arch/x86 to drivers/firmware/efi
and declared it as __weak.
3. Introduced ARCH_APEI_PAGE_KERNEL_UC to allow arch specific page
protection type for UC.
4. Removed efi_ioremap(). It can not be used for GHES memory region
mapping purpose since ioremap can not be used in atomic context.
V2:
1. Rebased to v4.1-rc5.
2. Split removal of efi_mem_attributes() and creation of efi_ioremap()
into two patches.

Jonathan (Zhixiong) Zhang (5):
efi: x86: rearrange efi_mem_attributes()
x86: acpi: implement arch_apei_get_mem_attributes()
arm64: mm: add PROT_DEVICE_nGnRnE and PROT_NORMAL_WT
arm64: apei: implement arch_apei_get_mem_attributes()
acpi, apei: use appropriate pgprot_t to map GHES memory

arch/arm64/include/asm/acpi.h | 26 ++++++++++++++++++++++++++
arch/arm64/include/asm/memory.h | 1 +
arch/arm64/include/asm/pgtable.h | 2 ++
arch/arm64/mm/proc.S | 4 +++-
arch/x86/kernel/acpi/apei.c | 19 +++++++++++++++++++
arch/x86/platform/efi/efi.c | 18 ------------------
drivers/acpi/apei/ghes.c | 6 ++++--
drivers/firmware/efi/efi.c | 31 +++++++++++++++++++++++++++++++
include/acpi/apei.h | 1 +
9 files changed, 87 insertions(+), 21 deletions(-)

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


2015-07-24 23:51:27

by Zhang, Jonathan Zhixiong

[permalink] [raw]
Subject: [PATCH V8 1/5] efi: x86: rearrange efi_mem_attributes()

From: "Jonathan (Zhixiong) Zhang" <[email protected]>

x86 and ia64 implement efi_mem_attributes() differently. This
function needs to be available for other arch (such as arm64)
as well, such as for the purpose of ACPI/APEI.

ia64 efi does not setup memmap variable and does not set
EFI_MEMMAP flag, so it needs to have its unique implementation
of efi_mem_attributes().

Move efi_mem_attributes() implementation from x86 to efi, and
declare it with __weak. It is recommended that other archs
should not override the default implementation.

Change-Id: I73e5fd173dd68ae6f0fe6ef53b365b274627bec1
Reviewed-by: Matt Fleming <[email protected]>
Signed-off-by: Jonathan (Zhixiong) Zhang <[email protected]>
---
arch/x86/platform/efi/efi.c | 18 ------------------
drivers/firmware/efi/efi.c | 31 +++++++++++++++++++++++++++++++
2 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index dbc8627a5cdf..88b3ebaeb72f 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -917,24 +917,6 @@ u32 efi_mem_type(unsigned long phys_addr)
return 0;
}

-u64 efi_mem_attributes(unsigned long phys_addr)
-{
- efi_memory_desc_t *md;
- void *p;
-
- if (!efi_enabled(EFI_MEMMAP))
- return 0;
-
- for (p = memmap.map; p < memmap.map_end; p += memmap.desc_size) {
- md = p;
- if ((md->phys_addr <= phys_addr) &&
- (phys_addr < (md->phys_addr +
- (md->num_pages << EFI_PAGE_SHIFT))))
- return md->attribute;
- }
- return 0;
-}
-
static int __init arch_parse_efi_cmdline(char *str)
{
if (parse_option_str(str, "old_map"))
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 3061bb8629dc..bf4190a4f3f5 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -517,3 +517,34 @@ char * __init efi_md_typeattr_format(char *buf, size_t size,
attr & EFI_MEMORY_UC ? "UC" : "");
return buf;
}
+
+/*
+ * efi_mem_attributes - lookup memmap attributes for physical address
+ * @phys_addr: the physical address to lookup
+ *
+ * Search in the EFI memory map for the region covering
+ * @phys_addr. Returns the EFI memory attributes if the region
+ * was found in the memory map, 0 otherwise.
+ *
+ * Despite being marked __weak, most architectures should *not*
+ * override this function. It is __weak solely for the benefit
+ * of ia64 which has a funky EFI memory map that doesn't work
+ * the same way as other architectures.
+ */
+u64 __weak efi_mem_attributes(unsigned long phys_addr)
+{
+ efi_memory_desc_t *md;
+ void *p;
+
+ if (!efi_enabled(EFI_MEMMAP))
+ return 0;
+
+ for (p = memmap.map; p < memmap.map_end; p += memmap.desc_size) {
+ md = p;
+ if ((md->phys_addr <= phys_addr) &&
+ (phys_addr < (md->phys_addr +
+ (md->num_pages << EFI_PAGE_SHIFT))))
+ return md->attribute;
+ }
+ return 0;
+}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2015-07-24 23:51:33

by Zhang, Jonathan Zhixiong

[permalink] [raw]
Subject: [PATCH V8 2/5] x86: acpi: implement arch_apei_get_mem_attributes()

From: "Jonathan (Zhixiong) Zhang" <[email protected]>

... to allow arch specific implementation of getting page
protection type associated with a physical address.

On x86, we currently have no way to lookup the EFI memory map
attributes for a region in a consistent way because the
memmap is discarded after efi_free_boot_services(). So if
you call efi_mem_attributes() during boot and at runtime,
you could theoretically see different attributes.

Since we are yet to see any x86 platforms that require
anything other than PAGE_KERNEL (some arm64 platforms
require the equivalent of PAGE_KERNEL_NOCACHE), return that
until we know differently.

Signed-off-by: Jonathan (Zhixiong) Zhang <[email protected]>
---
arch/x86/kernel/acpi/apei.c | 19 +++++++++++++++++++
include/acpi/apei.h | 1 +
2 files changed, 20 insertions(+)

diff --git a/arch/x86/kernel/acpi/apei.c b/arch/x86/kernel/acpi/apei.c
index c280df6b2aa2..675bd46c4e17 100644
--- a/arch/x86/kernel/acpi/apei.c
+++ b/arch/x86/kernel/acpi/apei.c
@@ -60,3 +60,22 @@ void arch_apei_flush_tlb_one(unsigned long addr)
{
__flush_tlb_one(addr);
}
+
+static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
+{
+
+ /*
+ * We currently have no way to lookup the EFI memory map
+ * attributes for a region in a consistent way because the
+ * memmap is discarded after efi_free_boot_services(). So if
+ * you call efi_mem_attributes() during boot and at runtime,
+ * you could theoretically see different attributes.
+ *
+ * Since we are yet to see any x86 platforms that require
+ * anything other than PAGE_KERNEL (some arm64 platforms
+ * require the equivalent of PAGE_KERNEL_NOCACHE), return that
+ * until we know differently.
+ */
+
+ return PAGE_KERNEL;
+}
diff --git a/include/acpi/apei.h b/include/acpi/apei.h
index 284801ac7042..64a12ce9880b 100644
--- a/include/acpi/apei.h
+++ b/include/acpi/apei.h
@@ -46,6 +46,7 @@ int erst_clear(u64 record_id);
int arch_apei_enable_cmcff(struct acpi_hest_header *hest_hdr, void *data);
void arch_apei_report_mem_error(int sev, struct cper_sec_mem_err *mem_err);
void arch_apei_flush_tlb_one(unsigned long addr);
+pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr);

#endif
#endif
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2015-07-24 23:51:38

by Zhang, Jonathan Zhixiong

[permalink] [raw]
Subject: [PATCH V8 3/5] arm64: mm: add PROT_DEVICE_nGnRnE and PROT_NORMAL_WT

From: "Jonathan (Zhixiong) Zhang" <[email protected]>

UEFI spec 2.5 section 2.3.6.1 defines that EFI_MEMORY_[UC|WC|WT|WB] are
possible EFI memory types for AArch64. Each of those EFI memory types
is mapped to a corresponding AArch64 memory type. So we need to define
PROT_DEVICE_nGnRnE and PROT_NORMWL_WT additionaly.

MT_NORMAL_WT is defined, and its encoding is added to MAIR_EL1 when
initializing cpu.

Change-Id: I20ac71ddf74c17e41769ecbb5f8c60eeefbb398a
Signed-off-by: Jonathan (Zhixiong) Zhang <[email protected]>
---
arch/arm64/include/asm/memory.h | 1 +
arch/arm64/include/asm/pgtable.h | 2 ++
arch/arm64/mm/proc.S | 4 +++-
3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index f800d45ea226..4112b3d7468e 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -100,6 +100,7 @@
#define MT_DEVICE_GRE 2
#define MT_NORMAL_NC 3
#define MT_NORMAL 4
+#define MT_NORMAL_WT 5

/*
* Memory types for Stage-2 translation
diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index 800ec0e87ed9..5c108ad13558 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -61,8 +61,10 @@ extern void __pgd_error(const char *file, int line, unsigned long val);
#define PROT_SECT_DEFAULT (PMD_TYPE_SECT | PMD_SECT_AF)
#endif

+#define PROT_DEVICE_nGnRnE (PROT_DEFAULT | PTE_PXN | PTE_UXN | PTE_ATTRINDX(MT_DEVICE_nGnRnE))
#define PROT_DEVICE_nGnRE (PROT_DEFAULT | PTE_PXN | PTE_UXN | PTE_ATTRINDX(MT_DEVICE_nGnRE))
#define PROT_NORMAL_NC (PROT_DEFAULT | PTE_PXN | PTE_UXN | PTE_ATTRINDX(MT_NORMAL_NC))
+#define PROT_NORMAL_WT (PROT_DEFAULT | PTE_PXN | PTE_UXN | PTE_ATTRINDX(MT_NORMAL_WT))
#define PROT_NORMAL (PROT_DEFAULT | PTE_PXN | PTE_UXN | PTE_ATTRINDX(MT_NORMAL))

#define PROT_SECT_DEVICE_nGnRE (PROT_SECT_DEFAULT | PMD_SECT_PXN | PMD_SECT_UXN | PMD_ATTRINDX(MT_DEVICE_nGnRE))
diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
index 838266f5b056..dfcc05804665 100644
--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S
@@ -303,12 +303,14 @@ ENTRY(__cpu_setup)
* DEVICE_GRE 010 00001100
* NORMAL_NC 011 01000100
* NORMAL 100 11111111
+ * NORMAL_WT 101 10111011
*/
ldr x5, =MAIR(0x00, MT_DEVICE_nGnRnE) | \
MAIR(0x04, MT_DEVICE_nGnRE) | \
MAIR(0x0c, MT_DEVICE_GRE) | \
MAIR(0x44, MT_NORMAL_NC) | \
- MAIR(0xff, MT_NORMAL)
+ MAIR(0xff, MT_NORMAL) | \
+ MAIR(0xbb, MT_NORMAL_WT)
msr mair_el1, x5
/*
* Prepare SCTLR
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2015-07-24 23:51:44

by Zhang, Jonathan Zhixiong

[permalink] [raw]
Subject: [PATCH V8 4/5] arm64: apei: implement arch_apei_get_mem_attributes()

From: "Jonathan (Zhixiong) Zhang" <[email protected]>

Table 8 of UEFI 2.5 section 2.3.6.1 defines mappings from EFI
memory types to MAIR attribute encodings for arm64.

If the physical address has memory attributes defined by EFI
memmap as EFI_MEMORY_[UC|WC|WT], return approprate page protection
type. Otherwise, return PAGE_KERNEL.

Change-Id: I7f836cae0fb3e74a34e27ac695c2637ffc4cddc6
Acked-by: Hanjun Guo <[email protected]>
Signed-off-by: Jonathan (Zhixiong) Zhang <[email protected]>
---
arch/arm64/include/asm/acpi.h | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)

diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
index 1ff9e6eb5e02..04720f819f0a 100644
--- a/arch/arm64/include/asm/acpi.h
+++ b/arch/arm64/include/asm/acpi.h
@@ -19,6 +19,11 @@
#include <asm/psci.h>
#include <asm/smp_plat.h>

+#ifdef CONFIG_ACPI_APEI
+#include <linux/efi.h>
+#include <asm/pgtable.h>
+#endif
+
/* Macros for consistency checks of the GICC subtable of MADT */
#define ACPI_MADT_GICC_LENGTH \
(acpi_gbl_FADT.header.revision < 6 ? 76 : 80)
@@ -91,4 +95,26 @@ static inline const char *acpi_get_enable_method(int cpu)
{
return acpi_psci_present() ? "psci" : NULL;
}
+
+#ifdef CONFIG_ACPI_APEI
+/*
+ * According to "Table 8 Map: EFI memory types to AArch64 memory types"
+ * of UEFI 2.5 section 2.3.6.1, each EFI memory type is mapped to
+ * corresponding MAIR attribute encoding.
+ */
+static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
+{
+ u64 attr;
+
+ attr = efi_mem_attributes(addr);
+ if (attr & EFI_MEMORY_UC)
+ return PROT_DEVICE_nGnRnE;
+ if (attr & EFI_MEMORY_WC)
+ return PROT_NORMAL_NC;
+ if (attr & EFI_MEMORY_WT)
+ return PROT_NORMAL_WT;
+ return PAGE_KERNEL;
+}
+#endif
+
#endif /*_ASM_ACPI_H*/
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2015-07-24 23:51:52

by Zhang, Jonathan Zhixiong

[permalink] [raw]
Subject: [PATCH V8 5/5] acpi, apei: use appropriate pgprot_t to map GHES memory

From: "Jonathan (Zhixiong) Zhang" <[email protected]>

With ACPI APEI firmware first handling, generic hardware error
record is updated by firmware in GHES memory region. On an arm64
platform, firmware updates GHES memory region with uncached
access attribute, and then Linux reads stale data from cache.

GHES memory region should be mapped with page protection type
according to what is returned from arch_apei_get_mem_attribute(),
instead of always with PAGE_KERNEL (eg. cached attribute).

Change-Id: Ie63440381e0f68100d4387e268058a2e2f9d63c6
Signed-off-by: Jonathan (Zhixiong) Zhang <[email protected]>
---
drivers/acpi/apei/ghes.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index b979b5dbe5bc..98609b404dae 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -173,8 +173,10 @@ static void __iomem *ghes_ioremap_pfn_irq(u64 pfn)
unsigned long vaddr;

vaddr = (unsigned long)GHES_IOREMAP_IRQ_PAGE(ghes_ioremap_area->addr);
- ioremap_page_range(vaddr, vaddr + PAGE_SIZE,
- pfn << PAGE_SHIFT, PAGE_KERNEL);
+ ioremap_page_range(vaddr,
+ vaddr + PAGE_SIZE,
+ pfn << PAGE_SHIFT,
+ arch_apei_get_mem_attribute(pfn << PAGE_SHIFT));

return (void __iomem *)vaddr;
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2015-07-27 09:19:11

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH V8 3/5] arm64: mm: add PROT_DEVICE_nGnRnE and PROT_NORMAL_WT

On Sat, Jul 25, 2015 at 12:51:07AM +0100, Jonathan (Zhixiong) Zhang wrote:
> From: "Jonathan (Zhixiong) Zhang" <[email protected]>
>
> UEFI spec 2.5 section 2.3.6.1 defines that EFI_MEMORY_[UC|WC|WT|WB] are
> possible EFI memory types for AArch64. Each of those EFI memory types
> is mapped to a corresponding AArch64 memory type. So we need to define
> PROT_DEVICE_nGnRnE and PROT_NORMWL_WT additionaly.
>
> MT_NORMAL_WT is defined, and its encoding is added to MAIR_EL1 when
> initializing cpu.
>
> Change-Id: I20ac71ddf74c17e41769ecbb5f8c60eeefbb398a
> Signed-off-by: Jonathan (Zhixiong) Zhang <[email protected]>

Please drop this Change-Id, it doesn't have any relevance to us.

Otherwise:

Reviewed-by: Catalin Marinas <[email protected]>

2015-07-27 10:04:03

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH V8 4/5] arm64: apei: implement arch_apei_get_mem_attributes()

On Sat, Jul 25, 2015 at 12:51:08AM +0100, Jonathan (Zhixiong) Zhang wrote:
> From: "Jonathan (Zhixiong) Zhang" <[email protected]>
>
> Table 8 of UEFI 2.5 section 2.3.6.1 defines mappings from EFI
> memory types to MAIR attribute encodings for arm64.
>
> If the physical address has memory attributes defined by EFI
> memmap as EFI_MEMORY_[UC|WC|WT], return approprate page protection
> type. Otherwise, return PAGE_KERNEL.
>
> Change-Id: I7f836cae0fb3e74a34e27ac695c2637ffc4cddc6
> Acked-by: Hanjun Guo <[email protected]>
> Signed-off-by: Jonathan (Zhixiong) Zhang <[email protected]>

Same comment about the Change-Id.

> ---
> arch/arm64/include/asm/acpi.h | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
> index 1ff9e6eb5e02..04720f819f0a 100644
> --- a/arch/arm64/include/asm/acpi.h
> +++ b/arch/arm64/include/asm/acpi.h
> @@ -19,6 +19,11 @@
> #include <asm/psci.h>
> #include <asm/smp_plat.h>
>
> +#ifdef CONFIG_ACPI_APEI
> +#include <linux/efi.h>
> +#include <asm/pgtable.h>
> +#endif
> +
> /* Macros for consistency checks of the GICC subtable of MADT */
> #define ACPI_MADT_GICC_LENGTH \
> (acpi_gbl_FADT.header.revision < 6 ? 76 : 80)
> @@ -91,4 +95,26 @@ static inline const char *acpi_get_enable_method(int cpu)
> {
> return acpi_psci_present() ? "psci" : NULL;
> }
> +
> +#ifdef CONFIG_ACPI_APEI
> +/*
> + * According to "Table 8 Map: EFI memory types to AArch64 memory types"
> + * of UEFI 2.5 section 2.3.6.1, each EFI memory type is mapped to
> + * corresponding MAIR attribute encoding.
> + */
> +static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
> +{
> + u64 attr;
> +
> + attr = efi_mem_attributes(addr);
> + if (attr & EFI_MEMORY_UC)
> + return PROT_DEVICE_nGnRnE;
> + if (attr & EFI_MEMORY_WC)
> + return PROT_NORMAL_NC;
> + if (attr & EFI_MEMORY_WT)
> + return PROT_NORMAL_WT;

Just to match the strict mm types, please add __pgprot(PROT_*) here.

> + return PAGE_KERNEL;

Otherwise:

Reviewed-by: Catalin Marinas <[email protected]>

2015-07-29 00:20:26

by Zhang, Jonathan Zhixiong

[permalink] [raw]
Subject: Re: [PATCH V8 3/5] arm64: mm: add PROT_DEVICE_nGnRnE and PROT_NORMAL_WT



On 7/27/2015 2:19 AM, Catalin Marinas wrote:
> On Sat, Jul 25, 2015 at 12:51:07AM +0100, Jonathan (Zhixiong) Zhang wrote:
>> From: "Jonathan (Zhixiong) Zhang" <[email protected]>
>>
>> UEFI spec 2.5 section 2.3.6.1 defines that EFI_MEMORY_[UC|WC|WT|WB] are
>> possible EFI memory types for AArch64. Each of those EFI memory types
>> is mapped to a corresponding AArch64 memory type. So we need to define
>> PROT_DEVICE_nGnRnE and PROT_NORMWL_WT additionaly.
>>
>> MT_NORMAL_WT is defined, and its encoding is added to MAIR_EL1 when
>> initializing cpu.
>>
>> Change-Id: I20ac71ddf74c17e41769ecbb5f8c60eeefbb398a
>> Signed-off-by: Jonathan (Zhixiong) Zhang <[email protected]>
>
> Please drop this Change-Id, it doesn't have any relevance to us.
Yes, will do.
> Otherwise:
>
> Reviewed-by: Catalin Marinas <[email protected]>
Thanks, appreciate it.

--
Jonathan (Zhixiong) Zhang
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2015-07-29 00:24:16

by Zhang, Jonathan Zhixiong

[permalink] [raw]
Subject: Re: [PATCH V8 4/5] arm64: apei: implement arch_apei_get_mem_attributes()

On 7/27/2015 3:03 AM, Catalin Marinas wrote:
> On Sat, Jul 25, 2015 at 12:51:08AM +0100, Jonathan (Zhixiong) Zhang wrote:
>> From: "Jonathan (Zhixiong) Zhang" <[email protected]>
>>
>> Table 8 of UEFI 2.5 section 2.3.6.1 defines mappings from EFI
>> memory types to MAIR attribute encodings for arm64.
>>
>> If the physical address has memory attributes defined by EFI
>> memmap as EFI_MEMORY_[UC|WC|WT], return approprate page protection
>> type. Otherwise, return PAGE_KERNEL.
>>
>> Change-Id: I7f836cae0fb3e74a34e27ac695c2637ffc4cddc6
>> Acked-by: Hanjun Guo <[email protected]>
>> Signed-off-by: Jonathan (Zhixiong) Zhang <[email protected]>
>
> Same comment about the Change-Id.
Yes.
>
>> ---
>> arch/arm64/include/asm/acpi.h | 26 ++++++++++++++++++++++++++
>> 1 file changed, 26 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
>> index 1ff9e6eb5e02..04720f819f0a 100644
>> --- a/arch/arm64/include/asm/acpi.h
>> +++ b/arch/arm64/include/asm/acpi.h
>> @@ -19,6 +19,11 @@
>> #include <asm/psci.h>
>> #include <asm/smp_plat.h>
>>
>> +#ifdef CONFIG_ACPI_APEI
>> +#include <linux/efi.h>
>> +#include <asm/pgtable.h>
>> +#endif
>> +
>> /* Macros for consistency checks of the GICC subtable of MADT */
>> #define ACPI_MADT_GICC_LENGTH \
>> (acpi_gbl_FADT.header.revision < 6 ? 76 : 80)
>> @@ -91,4 +95,26 @@ static inline const char *acpi_get_enable_method(int cpu)
>> {
>> return acpi_psci_present() ? "psci" : NULL;
>> }
>> +
>> +#ifdef CONFIG_ACPI_APEI
>> +/*
>> + * According to "Table 8 Map: EFI memory types to AArch64 memory types"
>> + * of UEFI 2.5 section 2.3.6.1, each EFI memory type is mapped to
>> + * corresponding MAIR attribute encoding.
>> + */
>> +static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
>> +{
>> + u64 attr;
>> +
>> + attr = efi_mem_attributes(addr);
>> + if (attr & EFI_MEMORY_UC)
>> + return PROT_DEVICE_nGnRnE;
>> + if (attr & EFI_MEMORY_WC)
>> + return PROT_NORMAL_NC;
>> + if (attr & EFI_MEMORY_WT)
>> + return PROT_NORMAL_WT;
>
> Just to match the strict mm types, please add __pgprot(PROT_*) here.
>
>> + return PAGE_KERNEL;
Will do.
>
> Otherwise:
>
> Reviewed-by: Catalin Marinas <[email protected]>
Thanks.

--
Jonathan (Zhixiong) Zhang
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project