linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V7 0/5] map GHES memory region according to EFI memory map
@ 2015-07-21 21:59 Jonathan (Zhixiong) Zhang
  2015-07-21 21:59 ` [PATCH V7 1/5] efi: x86: rearrange efi_mem_attributes() Jonathan (Zhixiong) Zhang
                   ` (4 more replies)
  0 siblings, 5 replies; 22+ messages in thread
From: Jonathan (Zhixiong) Zhang @ 2015-07-21 21:59 UTC (permalink / raw)
  To: Matt Fleming, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86,
	tony.luck, fu.wei, al.stone, bp, rjw, lenb, ying.huang,
	catalin.marinas, will.deacon
  Cc: Jonathan (Zhixiong) Zhang, linux-efi, linux-kernel, linux-acpi,
	linux-arm-msm, linaro-acpi, vgandhi

From: "Jonathan (Zhixiong) Zhang" <zjzhang@codeaurora.org>

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. 

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             |  6 ++++--
 arch/x86/kernel/acpi/apei.c      | 10 ++++++++++
 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, 79 insertions(+), 22 deletions(-)

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


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

* [PATCH V7 1/5] efi: x86: rearrange efi_mem_attributes()
  2015-07-21 21:59 [PATCH V7 0/5] map GHES memory region according to EFI memory map Jonathan (Zhixiong) Zhang
@ 2015-07-21 21:59 ` Jonathan (Zhixiong) Zhang
  2015-07-21 21:59 ` [PATCH V7 2/5] x86: acpi: implement arch_apei_get_mem_attributes() Jonathan (Zhixiong) Zhang
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 22+ messages in thread
From: Jonathan (Zhixiong) Zhang @ 2015-07-21 21:59 UTC (permalink / raw)
  To: Matt Fleming, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86,
	ying.huang, fu.wei, al.stone, bp, rjw, catalin.marinas,
	will.deacon
  Cc: Jonathan (Zhixiong) Zhang, linux-efi, linux-kernel, linaro-acpi

From: "Jonathan (Zhixiong) Zhang" <zjzhang@codeaurora.org>

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.

Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
---
 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


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

* [PATCH V7 2/5] x86: acpi: implement arch_apei_get_mem_attributes()
  2015-07-21 21:59 [PATCH V7 0/5] map GHES memory region according to EFI memory map Jonathan (Zhixiong) Zhang
  2015-07-21 21:59 ` [PATCH V7 1/5] efi: x86: rearrange efi_mem_attributes() Jonathan (Zhixiong) Zhang
@ 2015-07-21 21:59 ` Jonathan (Zhixiong) Zhang
  2015-07-21 21:59 ` [PATCH V7 3/5] arm64: mm: add PROT_DEVICE_nGnRnE and PROT_NORMAL_WT Jonathan (Zhixiong) Zhang
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 22+ messages in thread
From: Jonathan (Zhixiong) Zhang @ 2015-07-21 21:59 UTC (permalink / raw)
  To: Matt Fleming, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86,
	ying.huang, fu.wei, al.stone, bp, rjw, lenb, catalin.marinas,
	will.deacon
  Cc: Jonathan (Zhixiong) Zhang, linux-efi, linux-kernel, linux-acpi,
	linaro-acpi

From: "Jonathan (Zhixiong) Zhang" <zjzhang@codeaurora.org>

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

If the physical address has memory attributes defined by EFI
memmap as EFI_MEMORY_UC, the page protection type is
PAGE_KENERL_NOCACHE. Otherwise, the page protection type is
PAGE_KERNEL.

Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
---
 arch/x86/kernel/acpi/apei.c | 10 ++++++++++
 include/acpi/apei.h         |  1 +
 2 files changed, 11 insertions(+)

diff --git a/arch/x86/kernel/acpi/apei.c b/arch/x86/kernel/acpi/apei.c
index c280df6b2aa2..9c6b3c8d81e4 100644
--- a/arch/x86/kernel/acpi/apei.c
+++ b/arch/x86/kernel/acpi/apei.c
@@ -14,6 +14,8 @@
 
 #include <acpi/apei.h>
 
+#include <linux/efi.h>
+
 #include <asm/mce.h>
 #include <asm/tlbflush.h>
 
@@ -60,3 +62,11 @@ void arch_apei_flush_tlb_one(unsigned long addr)
 {
 	__flush_tlb_one(addr);
 }
+
+pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
+{
+	if (efi_mem_attributes(addr) & EFI_MEMORY_UC)
+		return PAGE_KERNEL_NOCACHE;
+
+	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


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

* [PATCH V7 3/5] arm64: mm: add PROT_DEVICE_nGnRnE and PROT_NORMAL_WT
  2015-07-21 21:59 [PATCH V7 0/5] map GHES memory region according to EFI memory map Jonathan (Zhixiong) Zhang
  2015-07-21 21:59 ` [PATCH V7 1/5] efi: x86: rearrange efi_mem_attributes() Jonathan (Zhixiong) Zhang
  2015-07-21 21:59 ` [PATCH V7 2/5] x86: acpi: implement arch_apei_get_mem_attributes() Jonathan (Zhixiong) Zhang
@ 2015-07-21 21:59 ` Jonathan (Zhixiong) Zhang
  2015-07-24  2:52   ` Hanjun Guo
  2015-07-24 16:15   ` Catalin Marinas
  2015-07-21 21:59 ` [PATCH V7 4/5] arm64: apei: implement arch_apei_get_mem_attributes() Jonathan (Zhixiong) Zhang
  2015-07-21 21:59 ` [PATCH V7 5/5] acpi, apei: use appropriate pgprot_t to map GHES memory Jonathan (Zhixiong) Zhang
  4 siblings, 2 replies; 22+ messages in thread
From: Jonathan (Zhixiong) Zhang @ 2015-07-21 21:59 UTC (permalink / raw)
  To: catalin.marinas, will.deacon, fu.wei, al.stone,
	bp @ alien8 . de Matt Fleming, rjw
  Cc: Jonathan (Zhixiong) Zhang, linux-kernel, linaro-acpi

From: "Jonathan (Zhixiong) Zhang" <zjzhang@codeaurora.org>

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.

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 <zjzhang@codeaurora.org>
---
 arch/arm64/include/asm/memory.h  | 1 +
 arch/arm64/include/asm/pgtable.h | 2 ++
 arch/arm64/mm/proc.S             | 6 ++++--
 3 files changed, 7 insertions(+), 2 deletions(-)

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..23e265d732f8 100644
--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S
@@ -294,7 +294,7 @@ ENTRY(__cpu_setup)
 	msr	cpacr_el1, x0			// Enable FP/ASIMD
 	msr	mdscr_el1, xzr			// Reset mdscr_el1
 	/*
-	 * Memory region attributes for LPAE:
+	 * Memory region attributes for LPAE and EFI:
 	 *
 	 *   n = AttrIndx[2:0]
 	 *			n	MAIR
@@ -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


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

* [PATCH V7 4/5] arm64: apei: implement arch_apei_get_mem_attributes()
  2015-07-21 21:59 [PATCH V7 0/5] map GHES memory region according to EFI memory map Jonathan (Zhixiong) Zhang
                   ` (2 preceding siblings ...)
  2015-07-21 21:59 ` [PATCH V7 3/5] arm64: mm: add PROT_DEVICE_nGnRnE and PROT_NORMAL_WT Jonathan (Zhixiong) Zhang
@ 2015-07-21 21:59 ` Jonathan (Zhixiong) Zhang
  2015-07-24  2:59   ` Hanjun Guo
  2015-07-24 14:57   ` Will Deacon
  2015-07-21 21:59 ` [PATCH V7 5/5] acpi, apei: use appropriate pgprot_t to map GHES memory Jonathan (Zhixiong) Zhang
  4 siblings, 2 replies; 22+ messages in thread
From: Jonathan (Zhixiong) Zhang @ 2015-07-21 21:59 UTC (permalink / raw)
  To: catalin.marinas, will.deacon, fu.wei, al.stone,
	bp @ alien8 . de Matt Fleming, rjw
  Cc: Jonathan (Zhixiong) Zhang, linux-kernel, linaro-acpi

From: "Jonathan (Zhixiong) Zhang" <zjzhang@codeaurora.org>

If the physical address has memory attributes defined by EFI
memmap as EFI_MEMORY_UC, the page protection type is
PROT_DEVICE_nGnRE. Otherwise, the page protection type is
PAGE_KERNEL.

Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
---
This patch applies cleanly to efi-next-14364 of efi/next and
arm64-upstream-13521 of arm64/master, but needed slight change
to apply to next-20150720 of linux-next/master and
pm+acpi-4.2-rc3 of linux-pm/master. The later two branches
has newer arch/arm64/include/asm/acpi.h with following patch:
  b6cfb277378e ACPI / ARM64: add BAD_MADT_GICC_ENTRY() macro
---
 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..41c7623d5b84 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
+
 /* Basic configuration for ACPI */
 #ifdef	CONFIG_ACPI
 /* ACPI table mapping after acpi_gbl_permanent_mmap is set */
@@ -84,4 +89,25 @@ 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)
+{
+	pgprot_t prot;
+
+	prot = efi_mem_attributes(addr);
+	if (prot & EFI_MEMORY_UC)
+		return PROT_DEVICE_nGnRnE;
+	if (prot & EFI_MEMORY_WC)
+		return PROT_NORMAL_NC;
+	if (prot & 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


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

* [PATCH V7 5/5] acpi, apei: use appropriate pgprot_t to map GHES memory
  2015-07-21 21:59 [PATCH V7 0/5] map GHES memory region according to EFI memory map Jonathan (Zhixiong) Zhang
                   ` (3 preceding siblings ...)
  2015-07-21 21:59 ` [PATCH V7 4/5] arm64: apei: implement arch_apei_get_mem_attributes() Jonathan (Zhixiong) Zhang
@ 2015-07-21 21:59 ` Jonathan (Zhixiong) Zhang
  4 siblings, 0 replies; 22+ messages in thread
From: Jonathan (Zhixiong) Zhang @ 2015-07-21 21:59 UTC (permalink / raw)
  To: Matt Fleming, Thomas Gleixner, fu.wei, al.stone, bp, tony.luck,
	rjw, lenb, ying.huang, catalin.marinas, will.deacon
  Cc: Jonathan (Zhixiong) Zhang, linux-acpi, linux-kernel, linaro-acpi

From: "Jonathan (Zhixiong) Zhang" <zjzhang@codeaurora.org>

With ACPI APEI firmware first handling, generic hardware error
record is updated by firmware in GHES memory region. When firmware
updated GHES memory region with uncached access attribute, 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).

Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
---
 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


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

* Re: [PATCH V7 3/5] arm64: mm: add PROT_DEVICE_nGnRnE and PROT_NORMAL_WT
  2015-07-21 21:59 ` [PATCH V7 3/5] arm64: mm: add PROT_DEVICE_nGnRnE and PROT_NORMAL_WT Jonathan (Zhixiong) Zhang
@ 2015-07-24  2:52   ` Hanjun Guo
  2015-07-24 16:15   ` Catalin Marinas
  1 sibling, 0 replies; 22+ messages in thread
From: Hanjun Guo @ 2015-07-24  2:52 UTC (permalink / raw)
  To: Jonathan (Zhixiong) Zhang, catalin.marinas, will.deacon, fu.wei,
	al.stone, bp @ alien8 . de Matt Fleming, rjw
  Cc: linux-kernel, linaro-acpi, Ard Biesheuvel, Leif Lindholm

+Cc Ard and Leif.

On 07/22/2015 05:59 AM, Jonathan (Zhixiong) Zhang wrote:
> From: "Jonathan (Zhixiong) Zhang" <zjzhang@codeaurora.org>
>
> 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.
>
> 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 <zjzhang@codeaurora.org>
> ---
>   arch/arm64/include/asm/memory.h  | 1 +
>   arch/arm64/include/asm/pgtable.h | 2 ++
>   arch/arm64/mm/proc.S             | 6 ++++--
>   3 files changed, 7 insertions(+), 2 deletions(-)
>
> 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..23e265d732f8 100644
> --- a/arch/arm64/mm/proc.S
> +++ b/arch/arm64/mm/proc.S
> @@ -294,7 +294,7 @@ ENTRY(__cpu_setup)
>   	msr	cpacr_el1, x0			// Enable FP/ASIMD
>   	msr	mdscr_el1, xzr			// Reset mdscr_el1
>   	/*
> -	 * Memory region attributes for LPAE:
> +	 * Memory region attributes for LPAE and EFI:
>   	 *
>   	 *   n = AttrIndx[2:0]
>   	 *			n	MAIR
> @@ -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
>

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

* Re: [PATCH V7 4/5] arm64: apei: implement arch_apei_get_mem_attributes()
  2015-07-21 21:59 ` [PATCH V7 4/5] arm64: apei: implement arch_apei_get_mem_attributes() Jonathan (Zhixiong) Zhang
@ 2015-07-24  2:59   ` Hanjun Guo
  2015-07-24 19:25     ` Zhang, Jonathan Zhixiong
  2015-07-24 14:57   ` Will Deacon
  1 sibling, 1 reply; 22+ messages in thread
From: Hanjun Guo @ 2015-07-24  2:59 UTC (permalink / raw)
  To: Jonathan (Zhixiong) Zhang, catalin.marinas, will.deacon, fu.wei,
	al.stone, bp @ alien8 . de Matt Fleming, rjw
  Cc: linux-kernel, linaro-acpi

On 07/22/2015 05:59 AM, Jonathan (Zhixiong) Zhang wrote:
> From: "Jonathan (Zhixiong) Zhang" <zjzhang@codeaurora.org>
>
> If the physical address has memory attributes defined by EFI
> memmap as EFI_MEMORY_UC, the page protection type is
> PROT_DEVICE_nGnRE. Otherwise, the page protection type is
> PAGE_KERNEL.

...

[...]
> +#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)
> +{
> +	pgprot_t prot;
> +
> +	prot = efi_mem_attributes(addr);
> +	if (prot & EFI_MEMORY_UC)
> +		return PROT_DEVICE_nGnRnE;
> +	if (prot & EFI_MEMORY_WC)
> +		return PROT_NORMAL_NC;
> +	if (prot & EFI_MEMORY_WT)
> +		return PROT_NORMAL_WT;
> +	return PAGE_KERNEL;
> +}

The change log is not matching the code here, and the PROT_DEVICE_nGnRE
in the change log is wrong, if you fix that,

Acked-by: Hanjun Guo <hanjun.guo@linaro.org>

Thanks
Hanjun


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

* Re: [PATCH V7 4/5] arm64: apei: implement arch_apei_get_mem_attributes()
  2015-07-21 21:59 ` [PATCH V7 4/5] arm64: apei: implement arch_apei_get_mem_attributes() Jonathan (Zhixiong) Zhang
  2015-07-24  2:59   ` Hanjun Guo
@ 2015-07-24 14:57   ` Will Deacon
  2015-07-24 16:21     ` Catalin Marinas
  1 sibling, 1 reply; 22+ messages in thread
From: Will Deacon @ 2015-07-24 14:57 UTC (permalink / raw)
  To: Jonathan (Zhixiong) Zhang
  Cc: Catalin Marinas, fu.wei, al.stone, bp @ alien8 . de Matt Fleming,
	rjw, linux-kernel, linaro-acpi

On Tue, Jul 21, 2015 at 10:59:19PM +0100, Jonathan (Zhixiong) Zhang wrote:
> From: "Jonathan (Zhixiong) Zhang" <zjzhang@codeaurora.org>
> 
> If the physical address has memory attributes defined by EFI
> memmap as EFI_MEMORY_UC, the page protection type is
> PROT_DEVICE_nGnRE. Otherwise, the page protection type is
> PAGE_KERNEL.
> 
> Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
> ---
> This patch applies cleanly to efi-next-14364 of efi/next and
> arm64-upstream-13521 of arm64/master, but needed slight change
> to apply to next-20150720 of linux-next/master and
> pm+acpi-4.2-rc3 of linux-pm/master. The later two branches
> has newer arch/arm64/include/asm/acpi.h with following patch:
>   b6cfb277378e ACPI / ARM64: add BAD_MADT_GICC_ENTRY() macro
> ---
>  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
> +static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
> +{
> +	pgprot_t prot;
> +
> +	prot = efi_mem_attributes(addr);
> +	if (prot & EFI_MEMORY_UC)
> +		return PROT_DEVICE_nGnRnE;
> +	if (prot & EFI_MEMORY_WC)
> +		return PROT_NORMAL_NC;

Can we not use pgprot_noncached and pgprot_writecombine for these two?

> +	if (prot & EFI_MEMORY_WT)
> +		return PROT_NORMAL_WT;

Then you could add pgprot_writethrough for this guy.

Will

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

* Re: [PATCH V7 3/5] arm64: mm: add PROT_DEVICE_nGnRnE and PROT_NORMAL_WT
  2015-07-21 21:59 ` [PATCH V7 3/5] arm64: mm: add PROT_DEVICE_nGnRnE and PROT_NORMAL_WT Jonathan (Zhixiong) Zhang
  2015-07-24  2:52   ` Hanjun Guo
@ 2015-07-24 16:15   ` Catalin Marinas
  2015-07-24 16:26     ` Will Deacon
  2015-07-24 19:02     ` Zhang, Jonathan Zhixiong
  1 sibling, 2 replies; 22+ messages in thread
From: Catalin Marinas @ 2015-07-24 16:15 UTC (permalink / raw)
  To: Jonathan (Zhixiong) Zhang
  Cc: Will Deacon, fu.wei, al.stone, bp @ alien8 . de Matt Fleming,
	rjw, linux-kernel, linaro-acpi

On Tue, Jul 21, 2015 at 10:59:18PM +0100, Jonathan (Zhixiong) Zhang wrote:
> From: "Jonathan (Zhixiong) Zhang" <zjzhang@codeaurora.org>
> 
> 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.
> 
> 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 <zjzhang@codeaurora.org>

I'll let the EFI guys to comment on whether these are needed, what for
(we could probably live just fine only with Device nGnRE and Normal NC).

>  #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..23e265d732f8 100644
> --- a/arch/arm64/mm/proc.S
> +++ b/arch/arm64/mm/proc.S
> @@ -294,7 +294,7 @@ ENTRY(__cpu_setup)
>  	msr	cpacr_el1, x0			// Enable FP/ASIMD
>  	msr	mdscr_el1, xzr			// Reset mdscr_el1
>  	/*
> -	 * Memory region attributes for LPAE:
> +	 * Memory region attributes for LPAE and EFI:

LPAE refers to the page table format, the comment probably inherited
from arch/arm. EFI means a completely different thing, so please remove
it.

>  	 *
>  	 *   n = AttrIndx[2:0]
>  	 *			n	MAIR
> @@ -303,12 +303,14 @@ ENTRY(__cpu_setup)
>  	 *   DEVICE_GRE		010	00001100
>  	 *   NORMAL_NC		011	01000100
>  	 *   NORMAL		100	11111111
> +	 *   NORMAL_WT		101	10111011

I don't think the write allocation hint makes much sense for WT memory
(though it shouldn't cause issues either). Just use 10101010.

-- 
Catalin


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

* Re: [PATCH V7 4/5] arm64: apei: implement arch_apei_get_mem_attributes()
  2015-07-24 14:57   ` Will Deacon
@ 2015-07-24 16:21     ` Catalin Marinas
  2015-07-24 16:26       ` Will Deacon
  0 siblings, 1 reply; 22+ messages in thread
From: Catalin Marinas @ 2015-07-24 16:21 UTC (permalink / raw)
  To: Will Deacon
  Cc: Jonathan (Zhixiong) Zhang, fu.wei, al.stone,
	bp @ alien8 . de Matt Fleming, rjw, linux-kernel, linaro-acpi

On Fri, Jul 24, 2015 at 03:57:08PM +0100, Will Deacon wrote:
> On Tue, Jul 21, 2015 at 10:59:19PM +0100, Jonathan (Zhixiong) Zhang wrote:
> > diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
> > +static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
> > +{
> > +	pgprot_t prot;
> > +
> > +	prot = efi_mem_attributes(addr);
> > +	if (prot & EFI_MEMORY_UC)
> > +		return PROT_DEVICE_nGnRnE;
> > +	if (prot & EFI_MEMORY_WC)
> > +		return PROT_NORMAL_NC;
> 
> Can we not use pgprot_noncached and pgprot_writecombine for these two?

Actually, why do we even use pgprot_t for prot here? EFI_MEMORY_* don't
have anything to do with the arch-specific pgprot_t.

-- 
Catalin


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

* Re: [PATCH V7 4/5] arm64: apei: implement arch_apei_get_mem_attributes()
  2015-07-24 16:21     ` Catalin Marinas
@ 2015-07-24 16:26       ` Will Deacon
  2015-07-24 18:46         ` Zhang, Jonathan Zhixiong
  2015-07-27  9:38         ` Matt Fleming
  0 siblings, 2 replies; 22+ messages in thread
From: Will Deacon @ 2015-07-24 16:26 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Jonathan (Zhixiong) Zhang, fu.wei, al.stone,
	bp @ alien8 . de Matt Fleming, rjw, linux-kernel, linaro-acpi

On Fri, Jul 24, 2015 at 05:21:49PM +0100, Catalin Marinas wrote:
> On Fri, Jul 24, 2015 at 03:57:08PM +0100, Will Deacon wrote:
> > On Tue, Jul 21, 2015 at 10:59:19PM +0100, Jonathan (Zhixiong) Zhang wrote:
> > > diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
> > > +static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
> > > +{
> > > +	pgprot_t prot;
> > > +
> > > +	prot = efi_mem_attributes(addr);
> > > +	if (prot & EFI_MEMORY_UC)
> > > +		return PROT_DEVICE_nGnRnE;
> > > +	if (prot & EFI_MEMORY_WC)
> > > +		return PROT_NORMAL_NC;
> > 
> > Can we not use pgprot_noncached and pgprot_writecombine for these two?
> 
> Actually, why do we even use pgprot_t for prot here? EFI_MEMORY_* don't
> have anything to do with the arch-specific pgprot_t.

Good point; the pgprot_t confused me, so my suggestion is much use after
ll. We're better off with a u64 to avoid further confusion.

Will

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

* Re: [PATCH V7 3/5] arm64: mm: add PROT_DEVICE_nGnRnE and PROT_NORMAL_WT
  2015-07-24 16:15   ` Catalin Marinas
@ 2015-07-24 16:26     ` Will Deacon
  2015-07-24 16:34       ` Catalin Marinas
  2015-07-24 19:02     ` Zhang, Jonathan Zhixiong
  1 sibling, 1 reply; 22+ messages in thread
From: Will Deacon @ 2015-07-24 16:26 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Jonathan (Zhixiong) Zhang, fu.wei, al.stone,
	bp @ alien8 . de Matt Fleming, rjw, linux-kernel, linaro-acpi

On Fri, Jul 24, 2015 at 05:15:00PM +0100, Catalin Marinas wrote:
> On Tue, Jul 21, 2015 at 10:59:18PM +0100, Jonathan (Zhixiong) Zhang wrote:
> >  	 *
> >  	 *   n = AttrIndx[2:0]
> >  	 *			n	MAIR
> > @@ -303,12 +303,14 @@ ENTRY(__cpu_setup)
> >  	 *   DEVICE_GRE		010	00001100
> >  	 *   NORMAL_NC		011	01000100
> >  	 *   NORMAL		100	11111111
> > +	 *   NORMAL_WT		101	10111011
> 
> I don't think the write allocation hint makes much sense for WT memory
> (though it shouldn't cause issues either). Just use 10101010.

The EFI spec actually specifies the 1011 1011 encoding, so I think we should
follow that.

Will

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

* Re: [PATCH V7 3/5] arm64: mm: add PROT_DEVICE_nGnRnE and PROT_NORMAL_WT
  2015-07-24 16:26     ` Will Deacon
@ 2015-07-24 16:34       ` Catalin Marinas
  0 siblings, 0 replies; 22+ messages in thread
From: Catalin Marinas @ 2015-07-24 16:34 UTC (permalink / raw)
  To: Will Deacon
  Cc: Jonathan (Zhixiong) Zhang, fu.wei, al.stone,
	bp @ alien8 . de Matt Fleming, rjw, linux-kernel, linaro-acpi

On Fri, Jul 24, 2015 at 05:26:54PM +0100, Will Deacon wrote:
> On Fri, Jul 24, 2015 at 05:15:00PM +0100, Catalin Marinas wrote:
> > On Tue, Jul 21, 2015 at 10:59:18PM +0100, Jonathan (Zhixiong) Zhang wrote:
> > >  	 *
> > >  	 *   n = AttrIndx[2:0]
> > >  	 *			n	MAIR
> > > @@ -303,12 +303,14 @@ ENTRY(__cpu_setup)
> > >  	 *   DEVICE_GRE		010	00001100
> > >  	 *   NORMAL_NC		011	01000100
> > >  	 *   NORMAL		100	11111111
> > > +	 *   NORMAL_WT		101	10111011
> > 
> > I don't think the write allocation hint makes much sense for WT memory
> > (though it shouldn't cause issues either). Just use 10101010.
> 
> The EFI spec actually specifies the 1011 1011 encoding, so I think we should
> follow that.

OK (I didn't bother reading the spec).

-- 
Catalin


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

* Re: [PATCH V7 4/5] arm64: apei: implement arch_apei_get_mem_attributes()
  2015-07-24 16:26       ` Will Deacon
@ 2015-07-24 18:46         ` Zhang, Jonathan Zhixiong
  2015-07-27  9:38         ` Matt Fleming
  1 sibling, 0 replies; 22+ messages in thread
From: Zhang, Jonathan Zhixiong @ 2015-07-24 18:46 UTC (permalink / raw)
  To: Will Deacon, Catalin Marinas
  Cc: fu.wei, al.stone, bp @ alien8 . de Matt Fleming, rjw,
	linux-kernel, linaro-acpi



On 7/24/2015 9:26 AM, Will Deacon wrote:
> On Fri, Jul 24, 2015 at 05:21:49PM +0100, Catalin Marinas wrote:
>> On Fri, Jul 24, 2015 at 03:57:08PM +0100, Will Deacon wrote:
>>> On Tue, Jul 21, 2015 at 10:59:19PM +0100, Jonathan (Zhixiong) Zhang wrote:
>>>> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
>>>> +static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
>>>> +{
>>>> +	pgprot_t prot;
>>>> +
>>>> +	prot = efi_mem_attributes(addr);
>>>> +	if (prot & EFI_MEMORY_UC)
>>>> +		return PROT_DEVICE_nGnRnE;
>>>> +	if (prot & EFI_MEMORY_WC)
>>>> +		return PROT_NORMAL_NC;
>>>
>>> Can we not use pgprot_noncached and pgprot_writecombine for these two?
>>
>> Actually, why do we even use pgprot_t for prot here? EFI_MEMORY_* don't
>> have anything to do with the arch-specific pgprot_t.
>
> Good point; the pgprot_t confused me, so my suggestion is much use after
> ll. We're better off with a u64 to avoid further confusion.
Got it. Thanks for the catch, Will/Catalin.

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

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

* Re: [PATCH V7 3/5] arm64: mm: add PROT_DEVICE_nGnRnE and PROT_NORMAL_WT
  2015-07-24 16:15   ` Catalin Marinas
  2015-07-24 16:26     ` Will Deacon
@ 2015-07-24 19:02     ` Zhang, Jonathan Zhixiong
  2015-07-27 10:22       ` Catalin Marinas
  1 sibling, 1 reply; 22+ messages in thread
From: Zhang, Jonathan Zhixiong @ 2015-07-24 19:02 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Will Deacon, fu.wei, al.stone, bp @ alien8 . de Matt Fleming,
	rjw, linux-kernel, linaro-acpi

On 7/24/2015 9:15 AM, Catalin Marinas wrote:
> On Tue, Jul 21, 2015 at 10:59:18PM +0100, Jonathan (Zhixiong) Zhang wrote:
>> From: "Jonathan (Zhixiong) Zhang" <zjzhang@codeaurora.org>
>>
>> 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.
>>
>> 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 <zjzhang@codeaurora.org>
>
> I'll let the EFI guys to comment on whether these are needed, what for
> (we could probably live just fine only with Device nGnRE and Normal NC).
>
>>   #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..23e265d732f8 100644
>> --- a/arch/arm64/mm/proc.S
>> +++ b/arch/arm64/mm/proc.S
>> @@ -294,7 +294,7 @@ ENTRY(__cpu_setup)
>>   	msr	cpacr_el1, x0			// Enable FP/ASIMD
>>   	msr	mdscr_el1, xzr			// Reset mdscr_el1
>>   	/*
>> -	 * Memory region attributes for LPAE:
>> +	 * Memory region attributes for LPAE and EFI:
>
> LPAE refers to the page table format, the comment probably inherited
> from arch/arm. EFI means a completely different thing, so please remove
> it.
Yes, it was inherited from arch/arm/mm/proc-v7-3level.S. I'll remove
the change. That being said, the original statement is not totally
accurate I think, so if you prefer, I could change it like following.
-	 * Memory region attributes for LPAE:
+	 * Set MAIR with memory region attributes:

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

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

* Re: [PATCH V7 4/5] arm64: apei: implement arch_apei_get_mem_attributes()
  2015-07-24  2:59   ` Hanjun Guo
@ 2015-07-24 19:25     ` Zhang, Jonathan Zhixiong
  0 siblings, 0 replies; 22+ messages in thread
From: Zhang, Jonathan Zhixiong @ 2015-07-24 19:25 UTC (permalink / raw)
  To: Hanjun Guo, catalin.marinas, will.deacon, fu.wei, al.stone,
	bp @ alien8 . de Matt Fleming, rjw
  Cc: linux-kernel, linaro-acpi

On 7/23/2015 7:59 PM, Hanjun Guo wrote:
> On 07/22/2015 05:59 AM, Jonathan (Zhixiong) Zhang wrote:
>> From: "Jonathan (Zhixiong) Zhang" <zjzhang@codeaurora.org>
>>
>> If the physical address has memory attributes defined by EFI
>> memmap as EFI_MEMORY_UC, the page protection type is
>> PROT_DEVICE_nGnRE. Otherwise, the page protection type is
>> PAGE_KERNEL.
>
> ...
>
> [...]
>> +#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)
>> +{
>> +    pgprot_t prot;
>> +
>> +    prot = efi_mem_attributes(addr);
>> +    if (prot & EFI_MEMORY_UC)
>> +        return PROT_DEVICE_nGnRnE;
>> +    if (prot & EFI_MEMORY_WC)
>> +        return PROT_NORMAL_NC;
>> +    if (prot & EFI_MEMORY_WT)
>> +        return PROT_NORMAL_WT;
>> +    return PAGE_KERNEL;
>> +}
>
> The change log is not matching the code here, and the PROT_DEVICE_nGnRE
> in the change log is wrong, if you fix that,
>
> Acked-by: Hanjun Guo <hanjun.guo@linaro.org>
Thank you Hanjun for the catch and the ack.

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

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

* Re: [PATCH V7 4/5] arm64: apei: implement arch_apei_get_mem_attributes()
  2015-07-24 16:26       ` Will Deacon
  2015-07-24 18:46         ` Zhang, Jonathan Zhixiong
@ 2015-07-27  9:38         ` Matt Fleming
  2015-07-27  9:45           ` Will Deacon
  1 sibling, 1 reply; 22+ messages in thread
From: Matt Fleming @ 2015-07-27  9:38 UTC (permalink / raw)
  To: Will Deacon
  Cc: Catalin Marinas, Jonathan (Zhixiong) Zhang, fu.wei, al.stone,
	rjw, linux-kernel, linaro-acpi

On Fri, 2015-07-24 at 17:26 +0100, Will Deacon wrote:
> On Fri, Jul 24, 2015 at 05:21:49PM +0100, Catalin Marinas wrote:
> > On Fri, Jul 24, 2015 at 03:57:08PM +0100, Will Deacon wrote:
> > > On Tue, Jul 21, 2015 at 10:59:19PM +0100, Jonathan (Zhixiong) Zhang wrote:
> > > > diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
> > > > +static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
> > > > +{
> > > > +	pgprot_t prot;
> > > > +
> > > > +	prot = efi_mem_attributes(addr);
> > > > +	if (prot & EFI_MEMORY_UC)
> > > > +		return PROT_DEVICE_nGnRnE;
> > > > +	if (prot & EFI_MEMORY_WC)
> > > > +		return PROT_NORMAL_NC;
> > > 
> > > Can we not use pgprot_noncached and pgprot_writecombine for these two?
> > 
> > Actually, why do we even use pgprot_t for prot here? EFI_MEMORY_* don't
> > have anything to do with the arch-specific pgprot_t.
> 
> Good point; the pgprot_t confused me, so my suggestion is much use after
> ll. We're better off with a u64 to avoid further confusion.

Isn't the whole point of arch_apei_get_mem_attribute() to turn an
arch-independent memory attribute (EFI_MEMORY_*) into an arch-specific
value to pass to ioremap_page_range()?

I don't see how you can do that any other way than by using pgprot_t.

Really, the problem here is that ioremap_page_caller() has no notion of
"map this range in a firmware-compatible manner". If we could do, for
example,

	ioremap_page_range(vaddr, vend, paddr, PAGE_FW_COMPAT);

that would allow the innards of the arch-ioremap to figure out exactly
how to map this range so that the firmware could access it coherently.

I suggested this previously but it didn't gain any traction.


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

* Re: [PATCH V7 4/5] arm64: apei: implement arch_apei_get_mem_attributes()
  2015-07-27  9:38         ` Matt Fleming
@ 2015-07-27  9:45           ` Will Deacon
  2015-07-27  9:54             ` Matt Fleming
  0 siblings, 1 reply; 22+ messages in thread
From: Will Deacon @ 2015-07-27  9:45 UTC (permalink / raw)
  To: Matt Fleming
  Cc: Catalin Marinas, Jonathan (Zhixiong) Zhang, fu.wei, al.stone,
	rjw, linux-kernel, linaro-acpi

On Mon, Jul 27, 2015 at 10:38:11AM +0100, Matt Fleming wrote:
> On Fri, 2015-07-24 at 17:26 +0100, Will Deacon wrote:
> > On Fri, Jul 24, 2015 at 05:21:49PM +0100, Catalin Marinas wrote:
> > > On Fri, Jul 24, 2015 at 03:57:08PM +0100, Will Deacon wrote:
> > > > On Tue, Jul 21, 2015 at 10:59:19PM +0100, Jonathan (Zhixiong) Zhang wrote:
> > > > > diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
> > > > > +static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
> > > > > +{
> > > > > +	pgprot_t prot;
> > > > > +
> > > > > +	prot = efi_mem_attributes(addr);
> > > > > +	if (prot & EFI_MEMORY_UC)
> > > > > +		return PROT_DEVICE_nGnRnE;
> > > > > +	if (prot & EFI_MEMORY_WC)
> > > > > +		return PROT_NORMAL_NC;
> > > > 
> > > > Can we not use pgprot_noncached and pgprot_writecombine for these two?
> > > 
> > > Actually, why do we even use pgprot_t for prot here? EFI_MEMORY_* don't
> > > have anything to do with the arch-specific pgprot_t.
> > 
> > Good point; the pgprot_t confused me, so my suggestion is much use after
> > ll. We're better off with a u64 to avoid further confusion.
> 
> Isn't the whole point of arch_apei_get_mem_attribute() to turn an
> arch-independent memory attribute (EFI_MEMORY_*) into an arch-specific
> value to pass to ioremap_page_range()?

That bit's fine. The weird bit is:

  pgprot_t prot;

  prot = efi_mem_attributes(addr);

Since that's putting the arch-independent format into the pg_prot.

> I don't see how you can do that any other way than by using pgprot_t.
> 
> Really, the problem here is that ioremap_page_caller() has no notion of
> "map this range in a firmware-compatible manner". If we could do, for
> example,
> 
> 	ioremap_page_range(vaddr, vend, paddr, PAGE_FW_COMPAT);
> 
> that would allow the innards of the arch-ioremap to figure out exactly
> how to map this range so that the firmware could access it coherently.
> 
> I suggested this previously but it didn't gain any traction.

Yeah, or just ioremap_efi.

</me runs away>

Will

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

* Re: [PATCH V7 4/5] arm64: apei: implement arch_apei_get_mem_attributes()
  2015-07-27  9:45           ` Will Deacon
@ 2015-07-27  9:54             ` Matt Fleming
  2015-07-29  0:58               ` Zhang, Jonathan Zhixiong
  0 siblings, 1 reply; 22+ messages in thread
From: Matt Fleming @ 2015-07-27  9:54 UTC (permalink / raw)
  To: Will Deacon
  Cc: Catalin Marinas, Jonathan (Zhixiong) Zhang, fu.wei, al.stone,
	rjw, linux-kernel, linaro-acpi

On Mon, 2015-07-27 at 10:45 +0100, Will Deacon wrote:
> 
> That bit's fine. The weird bit is:
> 
>   pgprot_t prot;
> 
>   prot = efi_mem_attributes(addr);
> 
> Since that's putting the arch-independent format into the pg_prot.

Oops, missed that. Yeah that's funky.

> > I don't see how you can do that any other way than by using pgprot_t.
> > 
> > Really, the problem here is that ioremap_page_caller() has no notion of
> > "map this range in a firmware-compatible manner". If we could do, for
> > example,
> > 
> > 	ioremap_page_range(vaddr, vend, paddr, PAGE_FW_COMPAT);
> > 
> > that would allow the innards of the arch-ioremap to figure out exactly
> > how to map this range so that the firmware could access it coherently.
> > 
> > I suggested this previously but it didn't gain any traction.
> 
> Yeah, or just ioremap_efi.
> 
> </me runs away>

Someone beat you to it ;-)

arch/x86/include/asm/efi.h:#define efi_ioremap(addr, size, type, attr)	ioremap_cache(addr, size)
arch/x86/include/asm/efi.h:extern void __iomem *__init efi_ioremap(unsigned long addr, unsigned long size,
arch/x86/platform/efi/efi.c:		va = efi_ioremap(md->phys_addr, size,
arch/x86/platform/efi/efi_64.c:void __iomem *__init efi_ioremap(unsigned long phys_addr, unsigned long size,
arch/x86/platform/efi/efi_64.c:		efi_ioremap(top, size - (top - phys_addr), type, attribute);



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

* Re: [PATCH V7 3/5] arm64: mm: add PROT_DEVICE_nGnRnE and PROT_NORMAL_WT
  2015-07-24 19:02     ` Zhang, Jonathan Zhixiong
@ 2015-07-27 10:22       ` Catalin Marinas
  0 siblings, 0 replies; 22+ messages in thread
From: Catalin Marinas @ 2015-07-27 10:22 UTC (permalink / raw)
  To: Zhang, Jonathan Zhixiong
  Cc: Will Deacon, fu.wei, al.stone, bp @ alien8 . de Matt Fleming,
	rjw, linux-kernel, linaro-acpi

On Fri, Jul 24, 2015 at 08:02:44PM +0100, Zhang, Jonathan Zhixiong wrote:
> On 7/24/2015 9:15 AM, Catalin Marinas wrote:
> > On Tue, Jul 21, 2015 at 10:59:18PM +0100, Jonathan (Zhixiong) Zhang wrote:
> >>   #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..23e265d732f8 100644
> >> --- a/arch/arm64/mm/proc.S
> >> +++ b/arch/arm64/mm/proc.S
> >> @@ -294,7 +294,7 @@ ENTRY(__cpu_setup)
> >>   	msr	cpacr_el1, x0			// Enable FP/ASIMD
> >>   	msr	mdscr_el1, xzr			// Reset mdscr_el1
> >>   	/*
> >> -	 * Memory region attributes for LPAE:
> >> +	 * Memory region attributes for LPAE and EFI:
> >
> > LPAE refers to the page table format, the comment probably inherited
> > from arch/arm. EFI means a completely different thing, so please remove
> > it.
> Yes, it was inherited from arch/arm/mm/proc-v7-3level.S. I'll remove
> the change. That being said, the original statement is not totally
> accurate I think, so if you prefer, I could change it like following.
> -	 * Memory region attributes for LPAE:
> +	 * Set MAIR with memory region attributes:

The LPAE is not inaccurate, just superfluous (since that's the only
format supported by AArch64).

-- 
Catalin


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

* Re: [PATCH V7 4/5] arm64: apei: implement arch_apei_get_mem_attributes()
  2015-07-27  9:54             ` Matt Fleming
@ 2015-07-29  0:58               ` Zhang, Jonathan Zhixiong
  0 siblings, 0 replies; 22+ messages in thread
From: Zhang, Jonathan Zhixiong @ 2015-07-29  0:58 UTC (permalink / raw)
  To: Matt Fleming, Will Deacon
  Cc: Catalin Marinas, fu.wei, al.stone, rjw, linux-kernel, linaro-acpi

On 7/27/2015 2:54 AM, Matt Fleming wrote:
> On Mon, 2015-07-27 at 10:45 +0100, Will Deacon wrote:
>>
>> That bit's fine. The weird bit is:
>>
>>    pgprot_t prot;
>>
>>    prot = efi_mem_attributes(addr);
>>
>> Since that's putting the arch-independent format into the pg_prot.
>
> Oops, missed that. Yeah that's funky.
Sorry, that was my mistake. Corrected at V8.
>>> I don't see how you can do that any other way than by using pgprot_t.
>>>
>>> Really, the problem here is that ioremap_page_caller() has no notion of
>>> "map this range in a firmware-compatible manner". If we could do, for
>>> example,
>>>
>>> 	ioremap_page_range(vaddr, vend, paddr, PAGE_FW_COMPAT);
>>>
>>> that would allow the innards of the arch-ioremap to figure out exactly
>>> how to map this range so that the firmware could access it coherently.
With this patch set, arch_apei_get_mem_attribute() is defined for above
mentioned PAGE_FW_COMPAT. If in future there are additional use case for
mapping page in atomic context according to UEFI memory map, the
function name/definition can be generalized.
>>>
>>> I suggested this previously but it didn't gain any traction.
>>
>> Yeah, or just ioremap_efi.
>>
>> </me runs away>
>
> Someone beat you to it ;-)
>
> arch/x86/include/asm/efi.h:#define efi_ioremap(addr, size, type, attr)	ioremap_cache(addr, size)
> arch/x86/include/asm/efi.h:extern void __iomem *__init efi_ioremap(unsigned long addr, unsigned long size,
> arch/x86/platform/efi/efi.c:		va = efi_ioremap(md->phys_addr, size,
> arch/x86/platform/efi/efi_64.c:void __iomem *__init efi_ioremap(unsigned long phys_addr, unsigned long size,
> arch/x86/platform/efi/efi_64.c:		efi_ioremap(top, size - (top - phys_addr), type, attribute);
x86's efi_ioremap() is intended to run at init time only. For the
purpose of this patch set, we would need to define something new
for both archs. We may want to keep it simple at this time, how do you
prefer?

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

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

end of thread, other threads:[~2015-07-29  0:58 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-21 21:59 [PATCH V7 0/5] map GHES memory region according to EFI memory map Jonathan (Zhixiong) Zhang
2015-07-21 21:59 ` [PATCH V7 1/5] efi: x86: rearrange efi_mem_attributes() Jonathan (Zhixiong) Zhang
2015-07-21 21:59 ` [PATCH V7 2/5] x86: acpi: implement arch_apei_get_mem_attributes() Jonathan (Zhixiong) Zhang
2015-07-21 21:59 ` [PATCH V7 3/5] arm64: mm: add PROT_DEVICE_nGnRnE and PROT_NORMAL_WT Jonathan (Zhixiong) Zhang
2015-07-24  2:52   ` Hanjun Guo
2015-07-24 16:15   ` Catalin Marinas
2015-07-24 16:26     ` Will Deacon
2015-07-24 16:34       ` Catalin Marinas
2015-07-24 19:02     ` Zhang, Jonathan Zhixiong
2015-07-27 10:22       ` Catalin Marinas
2015-07-21 21:59 ` [PATCH V7 4/5] arm64: apei: implement arch_apei_get_mem_attributes() Jonathan (Zhixiong) Zhang
2015-07-24  2:59   ` Hanjun Guo
2015-07-24 19:25     ` Zhang, Jonathan Zhixiong
2015-07-24 14:57   ` Will Deacon
2015-07-24 16:21     ` Catalin Marinas
2015-07-24 16:26       ` Will Deacon
2015-07-24 18:46         ` Zhang, Jonathan Zhixiong
2015-07-27  9:38         ` Matt Fleming
2015-07-27  9:45           ` Will Deacon
2015-07-27  9:54             ` Matt Fleming
2015-07-29  0:58               ` Zhang, Jonathan Zhixiong
2015-07-21 21:59 ` [PATCH V7 5/5] acpi, apei: use appropriate pgprot_t to map GHES memory Jonathan (Zhixiong) Zhang

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