[GIT PULL 0/2] EFI changes for v4.3 (part two)

[GIT PULL 0/2] EFI changes for v4.3 (part two)

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

Folks, please pull the remaining parts of the EFI changes for v4.3.

Nothing much has changed in terms of code since the last version of
this part two pull request was sent on August 12th,

  https://lkml.kernel.org/r/1439396234-22863-1-git-send-email-matt@codeblueprint.co.uk

The main difference is the improved commit message in PATCH 2, which
now contains much more information as well as the user visible
side-effects of the issue that the patch fixes.

There were also some improvements to the comments inside PATCH 1 as
well as moving the function out of line at Ingo's suggestion.

The branch is based on tip/core/efi.

The following changes since commit 8d446c8647c9ab8fcb45a8fc7dbbafe1f83aa2f3:

  arm64/mm: Add PROT_DEVICE_nGnRnE and PROT_NORMAL_WT (2015-08-08 10:37:40 +0200)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git acpi/ghes

for you to fetch changes up to 404f03dbebede4d0ebeabd06e4b9120b481ae536:

  acpi, apei: Use appropriate pgprot_t to map GHES memory (2015-09-04 13:39:31 +0100)

----------------------------------------------------------------
Jonathan (Zhixiong) Zhang (2):
      arm64: apei: Implement arch_apei_get_mem_attributes()
      acpi, apei: Use appropriate pgprot_t to map GHES memory

 arch/arm64/include/asm/acpi.h |  5 +++++
 arch/arm64/kernel/acpi.c      | 29 +++++++++++++++++++++++++++++
 drivers/acpi/apei/ghes.c      | 10 +++++++---
 3 files changed, 41 insertions(+), 3 deletions(-)

[PATCH 1/2] arm64: apei: Implement arch_apei_get_mem_attributes()

[Matt Fleming]

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

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 according to the UEFI spec. Otherwise, return PAGE_KERNEL.

Reviewed-by: Matt Fleming <matt.fleming@intel.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/arm64/include/asm/acpi.h |  5 +++++
 arch/arm64/kernel/acpi.c      | 29 +++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
index 406485ed110a..5aa892a12a0d 100644
--- a/arch/arm64/include/asm/acpi.h
+++ b/arch/arm64/include/asm/acpi.h
@@ -92,4 +92,9 @@ static inline const char *acpi_get_enable_method(int cpu)
 {
 	return acpi_psci_present() ? "psci" : NULL;
 }
+
+#ifdef	CONFIG_ACPI_APEI
+pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr);
+#endif
+
 #endif /*_ASM_ACPI_H*/
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index 19de7537e7d3..9f083606e5bf 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -29,6 +29,11 @@
 #include <asm/cpu_ops.h>
 #include <asm/smp_plat.h>
 
+#ifdef CONFIG_ACPI_APEI
+#include <linux/efi.h>
+#include <asm/pgtable.h>
+#endif
+
 int acpi_noirq = 1;		/* skip ACPI IRQ initialization */
 int acpi_disabled = 1;
 EXPORT_SYMBOL(acpi_disabled);
@@ -230,3 +235,27 @@ void __init acpi_gic_init(void)
 
 	early_acpi_os_unmap_memory((char *)table, tbl_size);
 }
+
+#ifdef  CONFIG_ACPI_APEI
+pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
+{
+	/*
+	 * 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 a corresponding MAIR attribute encoding.
+	 * The EFI memory attribute advises all possible capabilities
+	 * of a memory region. We use the most efficient capability.
+	 */
+
+	u64 attr;
+
+	attr = efi_mem_attributes(addr);
+	if (attr & EFI_MEMORY_WB)
+		return PAGE_KERNEL;
+	if (attr & EFI_MEMORY_WT)
+		return __pgprot(PROT_NORMAL_WT);
+	if (attr & EFI_MEMORY_WC)
+		return __pgprot(PROT_NORMAL_NC);
+	return __pgprot(PROT_DEVICE_nGnRnE);
+}
+#endif
-- 
2.1.0

[PATCH 2/2] acpi, apei: Use appropriate pgprot_t to map GHES memory

[Matt Fleming]

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

If the ACPI APEI firmware handles hardware error first (called "firmware
first handling"), the firmware updates the GHES memory region with hardware
error record (called "generic hardware error record"). Essentially the
firmware writes hardware error records in the GHES memory region, triggers
an NMI/interrupt, then the GHES driver goes off and grabs the error record
from the GHES region.

The kernel currently maps the GHES memory region as cacheable
(PAGE_KERNEL) for all architectures. However, on some arm64 platforms,
there is a mismatch between how the kernel maps the GHES region
(PAGE_KERNEL) and how the firmware maps it (EFI_MEMORY_UC, ie.
uncacheable), leading to the possibility of the kernel GHES driver
reading stale data from the cache when it receives the interrupt.

With stale data being read, the kernel is unaware there is new hardware
error to be handled when there actually is; this may lead to further damage
in various scenarios, such as error propagation caused data corruption.
If uncorrected error (such as double bit ECC error) happened in memory
operation and if the kernel is unaware of such event happening, errorneous
data may be propagated to the disk.

Instead GHES memory region should be mapped with page protection type
according to what is returned from arch_apei_get_mem_attribute().

Reviewed-by: Matt Fleming <matt@codeblueprint.co.uk>
Acked-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 drivers/acpi/apei/ghes.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 2bfd53cbfe80..e661695cf123 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -161,11 +161,15 @@ static void __iomem *ghes_ioremap_pfn_nmi(u64 pfn)
 
 static void __iomem *ghes_ioremap_pfn_irq(u64 pfn)
 {
-	unsigned long vaddr;
+	unsigned long vaddr, paddr;
+	pgprot_t prot;
 
 	vaddr = (unsigned long)GHES_IOREMAP_IRQ_PAGE(ghes_ioremap_area->addr);
-	ioremap_page_range(vaddr, vaddr + PAGE_SIZE,
-			   pfn << PAGE_SHIFT, PAGE_KERNEL);
+
+	paddr = pfn << PAGE_SHIFT;
+	prot = arch_apei_get_mem_attribute(paddr);
+
+	ioremap_page_range(vaddr, vaddr + PAGE_SIZE, paddr, prot);
 
 	return (void __iomem *)vaddr;
 }
-- 
2.1.0

[PATCH 2/2] acpi, apei: Use appropriate pgprot_t to map GHES memory

[Matt Fleming]

From: "Jonathan (Zhixiong) Zhang" <zjzhang-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

If the ACPI APEI firmware handles hardware error first (called "firmware
first handling"), the firmware updates the GHES memory region with hardware
error record (called "generic hardware error record"). Essentially the
firmware writes hardware error records in the GHES memory region, triggers
an NMI/interrupt, then the GHES driver goes off and grabs the error record
from the GHES region.

The kernel currently maps the GHES memory region as cacheable
(PAGE_KERNEL) for all architectures. However, on some arm64 platforms,
there is a mismatch between how the kernel maps the GHES region
(PAGE_KERNEL) and how the firmware maps it (EFI_MEMORY_UC, ie.
uncacheable), leading to the possibility of the kernel GHES driver
reading stale data from the cache when it receives the interrupt.

With stale data being read, the kernel is unaware there is new hardware
error to be handled when there actually is; this may lead to further damage
in various scenarios, such as error propagation caused data corruption.
If uncorrected error (such as double bit ECC error) happened in memory
operation and if the kernel is unaware of such event happening, errorneous
data may be propagated to the disk.

Instead GHES memory region should be mapped with page protection type
according to what is returned from arch_apei_get_mem_attribute().

Reviewed-by: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
Acked-by: Borislav Petkov <bp-l3A5Bk7waGM@public.gmane.org>
Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Signed-off-by: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/acpi/apei/ghes.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 2bfd53cbfe80..e661695cf123 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -161,11 +161,15 @@ static void __iomem *ghes_ioremap_pfn_nmi(u64 pfn)
 
 static void __iomem *ghes_ioremap_pfn_irq(u64 pfn)
 {
-	unsigned long vaddr;
+	unsigned long vaddr, paddr;
+	pgprot_t prot;
 
 	vaddr = (unsigned long)GHES_IOREMAP_IRQ_PAGE(ghes_ioremap_area->addr);
-	ioremap_page_range(vaddr, vaddr + PAGE_SIZE,
-			   pfn << PAGE_SHIFT, PAGE_KERNEL);
+
+	paddr = pfn << PAGE_SHIFT;
+	prot = arch_apei_get_mem_attribute(paddr);
+
+	ioremap_page_range(vaddr, vaddr + PAGE_SIZE, paddr, prot);
 
 	return (void __iomem *)vaddr;
 }
-- 
2.1.0

Re: [GIT PULL 0/2] EFI changes for v4.3 (part two)

[Ingo Molnar]

* Matt Fleming <matt@codeblueprint.co.uk> wrote:

> From: Matt Fleming <matt.fleming@intel.com>
> 
> Folks, please pull the remaining parts of the EFI changes for v4.3.
> 
> Nothing much has changed in terms of code since the last version of
> this part two pull request was sent on August 12th,
> 
>   https://lkml.kernel.org/r/1439396234-22863-1-git-send-email-matt@codeblueprint.co.uk
> 
> The main difference is the improved commit message in PATCH 2, which
> now contains much more information as well as the user visible
> side-effects of the issue that the patch fixes.
> 
> There were also some improvements to the comments inside PATCH 1 as
> well as moving the function out of line at Ingo's suggestion.
> 
> The branch is based on tip/core/efi.
> 
> The following changes since commit 8d446c8647c9ab8fcb45a8fc7dbbafe1f83aa2f3:
> 
>   arm64/mm: Add PROT_DEVICE_nGnRnE and PROT_NORMAL_WT (2015-08-08 10:37:40 +0200)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git acpi/ghes
> 
> for you to fetch changes up to 404f03dbebede4d0ebeabd06e4b9120b481ae536:
> 
>   acpi, apei: Use appropriate pgprot_t to map GHES memory (2015-09-04 13:39:31 +0100)
> 
> ----------------------------------------------------------------
> Jonathan (Zhixiong) Zhang (2):
>       arm64: apei: Implement arch_apei_get_mem_attributes()
>       acpi, apei: Use appropriate pgprot_t to map GHES memory
> 
>  arch/arm64/include/asm/acpi.h |  5 +++++
>  arch/arm64/kernel/acpi.c      | 29 +++++++++++++++++++++++++++++
>  drivers/acpi/apei/ghes.c      | 10 +++++++---
>  3 files changed, 41 insertions(+), 3 deletions(-)

Applied to tip:core/efi, thanks Matt!

	Ingo

Re: [GIT PULL 0/2] EFI changes for v4.3 (part two)

[Ingo Molnar]

* Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org> wrote:

> From: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> Folks, please pull the remaining parts of the EFI changes for v4.3.
> 
> Nothing much has changed in terms of code since the last version of
> this part two pull request was sent on August 12th,
> 
>   https://lkml.kernel.org/r/1439396234-22863-1-git-send-email-matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org
> 
> The main difference is the improved commit message in PATCH 2, which
> now contains much more information as well as the user visible
> side-effects of the issue that the patch fixes.
> 
> There were also some improvements to the comments inside PATCH 1 as
> well as moving the function out of line at Ingo's suggestion.
> 
> The branch is based on tip/core/efi.
> 
> The following changes since commit 8d446c8647c9ab8fcb45a8fc7dbbafe1f83aa2f3:
> 
>   arm64/mm: Add PROT_DEVICE_nGnRnE and PROT_NORMAL_WT (2015-08-08 10:37:40 +0200)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git acpi/ghes
> 
> for you to fetch changes up to 404f03dbebede4d0ebeabd06e4b9120b481ae536:
> 
>   acpi, apei: Use appropriate pgprot_t to map GHES memory (2015-09-04 13:39:31 +0100)
> 
> ----------------------------------------------------------------
> Jonathan (Zhixiong) Zhang (2):
>       arm64: apei: Implement arch_apei_get_mem_attributes()
>       acpi, apei: Use appropriate pgprot_t to map GHES memory
> 
>  arch/arm64/include/asm/acpi.h |  5 +++++
>  arch/arm64/kernel/acpi.c      | 29 +++++++++++++++++++++++++++++
>  drivers/acpi/apei/ghes.c      | 10 +++++++---
>  3 files changed, 41 insertions(+), 3 deletions(-)

Applied to tip:core/efi, thanks Matt!

	Ingo

[tip:core/efi] arm64, acpi/apei: Implement arch_apei_get_mem_attributes()

[tip-bot for Jonathan (Zhixiong) Zhang]

Commit-ID:  89e44b51cc0db50ea4b5bbb5d582c4db88bbaed8
Gitweb:     http://git.kernel.org/tip/89e44b51cc0db50ea4b5bbb5d582c4db88bbaed8
Author:     Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
AuthorDate: Fri, 4 Sep 2015 14:11:41 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 14 Sep 2015 11:40:03 +0200

arm64, acpi/apei: Implement arch_apei_get_mem_attributes()

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 according to the UEFI spec. Otherwise, return
PAGE_KERNEL.

Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
[ Small stylistic tweaks. ]
Reviewed-by: Matt Fleming <matt.fleming@intel.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will.deacon@arm.com>
Link: http://lkml.kernel.org/r/1441372302-23242-2-git-send-email-matt@codeblueprint.co.uk
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/arm64/include/asm/acpi.h |  5 +++++
 arch/arm64/kernel/acpi.c      | 29 +++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
index 406485e..5aa892a 100644
--- a/arch/arm64/include/asm/acpi.h
+++ b/arch/arm64/include/asm/acpi.h
@@ -92,4 +92,9 @@ static inline const char *acpi_get_enable_method(int cpu)
 {
 	return acpi_psci_present() ? "psci" : NULL;
 }
+
+#ifdef	CONFIG_ACPI_APEI
+pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr);
+#endif
+
 #endif /*_ASM_ACPI_H*/
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index 19de753..137d537 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -29,6 +29,11 @@
 #include <asm/cpu_ops.h>
 #include <asm/smp_plat.h>
 
+#ifdef CONFIG_ACPI_APEI
+# include <linux/efi.h>
+# include <asm/pgtable.h>
+#endif
+
 int acpi_noirq = 1;		/* skip ACPI IRQ initialization */
 int acpi_disabled = 1;
 EXPORT_SYMBOL(acpi_disabled);
@@ -230,3 +235,27 @@ void __init acpi_gic_init(void)
 
 	early_acpi_os_unmap_memory((char *)table, tbl_size);
 }
+
+#ifdef CONFIG_ACPI_APEI
+pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
+{
+	/*
+	 * 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 a corresponding MAIR attribute encoding.
+	 * The EFI memory attribute advises all possible capabilities
+	 * of a memory region. We use the most efficient capability.
+	 */
+
+	u64 attr;
+
+	attr = efi_mem_attributes(addr);
+	if (attr & EFI_MEMORY_WB)
+		return PAGE_KERNEL;
+	if (attr & EFI_MEMORY_WT)
+		return __pgprot(PROT_NORMAL_WT);
+	if (attr & EFI_MEMORY_WC)
+		return __pgprot(PROT_NORMAL_NC);
+	return __pgprot(PROT_DEVICE_nGnRnE);
+}
+#endif

[tip:core/efi] acpi/apei: Use appropriate pgprot_t to map GHES memory

[tip-bot for Jonathan (Zhixiong) Zhang]

Commit-ID:  8ece249a811e93d3f60e3f1ebdc86c7e7a95bdbf
Gitweb:     http://git.kernel.org/tip/8ece249a811e93d3f60e3f1ebdc86c7e7a95bdbf
Author:     Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
AuthorDate: Fri, 4 Sep 2015 14:11:42 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 14 Sep 2015 11:40:04 +0200

acpi/apei: Use appropriate pgprot_t to map GHES memory

If the ACPI APEI firmware handles hardware error first (called
"firmware first handling"), the firmware updates the GHES memory
region with hardware error record (called "generic hardware
error record"). Essentially the firmware writes hardware error
records in the GHES memory region, triggers an NMI/interrupt,
then the GHES driver goes off and grabs the error record from
the GHES region.

The kernel currently maps the GHES memory region as cacheable
(PAGE_KERNEL) for all architectures. However, on some arm64
platforms, there is a mismatch between how the kernel maps the
GHES region (PAGE_KERNEL) and how the firmware maps it
(EFI_MEMORY_UC, ie. uncacheable), leading to the possibility of
the kernel GHES driver reading stale data from the cache when it
receives the interrupt.

With stale data being read, the kernel is unaware there is new
hardware error to be handled when there actually is; this may
lead to further damage in various scenarios, such as error
propagation caused data corruption. If uncorrected error (such
as double bit ECC error) happened in memory operation and if the
kernel is unaware of such an event happening, errorneous data may
be propagated to the disk.

Instead GHES memory region should be mapped with page protection
type according to what is returned from arch_apei_get_mem_attribute().

Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
[ Small stylistic tweaks. ]
Reviewed-by: Matt Fleming <matt@codeblueprint.co.uk>
Acked-by: Borislav Petkov <bp@suse.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/1441372302-23242-3-git-send-email-matt@codeblueprint.co.uk
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 drivers/acpi/apei/ghes.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 2bfd53c..e661695 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -161,11 +161,15 @@ static void __iomem *ghes_ioremap_pfn_nmi(u64 pfn)
 
 static void __iomem *ghes_ioremap_pfn_irq(u64 pfn)
 {
-	unsigned long vaddr;
+	unsigned long vaddr, paddr;
+	pgprot_t prot;
 
 	vaddr = (unsigned long)GHES_IOREMAP_IRQ_PAGE(ghes_ioremap_area->addr);
-	ioremap_page_range(vaddr, vaddr + PAGE_SIZE,
-			   pfn << PAGE_SHIFT, PAGE_KERNEL);
+
+	paddr = pfn << PAGE_SHIFT;
+	prot = arch_apei_get_mem_attribute(paddr);
+
+	ioremap_page_range(vaddr, vaddr + PAGE_SIZE, paddr, prot);
 
 	return (void __iomem *)vaddr;
 }

Re: [PATCH 1/2] arm64: apei: implement arch_apei_get_mem_attributes()

[Matt Fleming]

On Fri, 14 Aug, at 11:55:27AM, Zhang, Jonathan Zhixiong wrote:
> 
> >Also, this doesn't look like a small function - why is it inlined?
> The function grew a bit after its first version. Since it is on the
> fence of not being a small function, I did not change it to out of
> the line. I will move it to arch/arm64/kernel/acpi.c guarded by
> CONFIG_ACPI_APEI instead of having to create a very small new file,
> if there is no opposition.

Please keep Will in the loop (now Cc'd) since he originally suggested
moving this to a header file and marking it static inline.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH 1/2] arm64: apei: implement arch_apei_get_mem_attributes()

[Zhang, Jonathan Zhixiong]

On 8/13/2015 1:22 AM, Ingo Molnar wrote:
>
> * Matt Fleming <matt@codeblueprint.co.uk> wrote:
>
>> From: "Jonathan (Zhixiong) Zhang" <zjzhang@codeaurora.org>
>>
>> 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 according to the UEFI spec. Otherwise, return PAGE_KERNEL.
>>
>> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
>> Acked-by: Hanjun Guo <hanjun.guo@linaro.org>
>> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
>> Signed-off-by: Matt Fleming <matt.fleming@intel.com>
>> ---
>>   arch/arm64/include/asm/acpi.h | 30 ++++++++++++++++++++++++++++++
>>   1 file changed, 30 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
>> index 406485ed110a..1cbad43e561d 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)
>> @@ -92,4 +97,29 @@ static inline const char *acpi_get_enable_method(int cpu)
>>   {
>>   	return acpi_psci_present() ? "psci" : NULL;
>>   }
>> +
>> +#ifdef	CONFIG_ACPI_APEI
>> +static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
>> +{
>> +	/*
>> +	 * 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.
>
> s/mapped to corresponding/
>    mapped to a corresponding
Will do.
>
>> +	 * The EFI memory attribute advises all possible capabilities of a
>> +	 * memory region. We use the most efficient capability.
>> +	 */
>> +
>> +	u64 attr;
>> +
>> +	attr = efi_mem_attributes(addr);
>> +	if (attr & EFI_MEMORY_WB)
>> +		return PAGE_KERNEL;
>> +	if (attr & EFI_MEMORY_WT)
>> +		return __pgprot(PROT_NORMAL_WT);
>> +	if (attr & EFI_MEMORY_WC)
>> +		return __pgprot(PROT_NORMAL_NC);
>> +	return __pgprot(PROT_DEVICE_nGnRnE);
>> +}
>> +#endif
>
> Also, this doesn't look like a small function - why is it inlined?
The function grew a bit after its first version. Since it is on the
fence of not being a small function, I did not change it to out of
the line. I will move it to arch/arm64/kernel/acpi.c guarded by
CONFIG_ACPI_APEI instead of having to create a very small new file,
if there is no opposition.
>
> Moving it to .c would also slightly improve compilation times for every .c file
> that includes asm/acpi.h.
>
> Thanks,
>
> 	Ingo
>

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

Re: [PATCH 1/2] arm64: apei: implement arch_apei_get_mem_attributes()

[Zhang, Jonathan Zhixiong]

On 8/13/2015 1:22 AM, Ingo Molnar wrote:
>
> * Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org> wrote:
>
>> From: "Jonathan (Zhixiong) Zhang" <zjzhang-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>
>> 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 according to the UEFI spec. Otherwise, return PAGE_KERNEL.
>>
>> Reviewed-by: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>
>> Acked-by: Hanjun Guo <hanjun.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> Reviewed-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> Signed-off-by: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> ---
>>   arch/arm64/include/asm/acpi.h | 30 ++++++++++++++++++++++++++++++
>>   1 file changed, 30 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
>> index 406485ed110a..1cbad43e561d 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)
>> @@ -92,4 +97,29 @@ static inline const char *acpi_get_enable_method(int cpu)
>>   {
>>   	return acpi_psci_present() ? "psci" : NULL;
>>   }
>> +
>> +#ifdef	CONFIG_ACPI_APEI
>> +static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
>> +{
>> +	/*
>> +	 * 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.
>
> s/mapped to corresponding/
>    mapped to a corresponding
Will do.
>
>> +	 * The EFI memory attribute advises all possible capabilities of a
>> +	 * memory region. We use the most efficient capability.
>> +	 */
>> +
>> +	u64 attr;
>> +
>> +	attr = efi_mem_attributes(addr);
>> +	if (attr & EFI_MEMORY_WB)
>> +		return PAGE_KERNEL;
>> +	if (attr & EFI_MEMORY_WT)
>> +		return __pgprot(PROT_NORMAL_WT);
>> +	if (attr & EFI_MEMORY_WC)
>> +		return __pgprot(PROT_NORMAL_NC);
>> +	return __pgprot(PROT_DEVICE_nGnRnE);
>> +}
>> +#endif
>
> Also, this doesn't look like a small function - why is it inlined?
The function grew a bit after its first version. Since it is on the
fence of not being a small function, I did not change it to out of
the line. I will move it to arch/arm64/kernel/acpi.c guarded by
CONFIG_ACPI_APEI instead of having to create a very small new file,
if there is no opposition.
>
> Moving it to .c would also slightly improve compilation times for every .c file
> that includes asm/acpi.h.
>
> Thanks,
>
> 	Ingo
>

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

Re: [PATCH 1/2] arm64: apei: implement arch_apei_get_mem_attributes()

[Ingo Molnar]

* Matt Fleming <matt@codeblueprint.co.uk> wrote:

> From: "Jonathan (Zhixiong) Zhang" <zjzhang@codeaurora.org>
> 
> 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 according to the UEFI spec. Otherwise, return PAGE_KERNEL.
> 
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> Acked-by: Hanjun Guo <hanjun.guo@linaro.org>
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
> Signed-off-by: Matt Fleming <matt.fleming@intel.com>
> ---
>  arch/arm64/include/asm/acpi.h | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
> index 406485ed110a..1cbad43e561d 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)
> @@ -92,4 +97,29 @@ static inline const char *acpi_get_enable_method(int cpu)
>  {
>  	return acpi_psci_present() ? "psci" : NULL;
>  }
> +
> +#ifdef	CONFIG_ACPI_APEI
> +static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
> +{
> +	/*
> +	 * 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.

s/mapped to corresponding/
  mapped to a corresponding

> +	 * The EFI memory attribute advises all possible capabilities of a
> +	 * memory region. We use the most efficient capability.
> +	 */
> +
> +	u64 attr;
> +
> +	attr = efi_mem_attributes(addr);
> +	if (attr & EFI_MEMORY_WB)
> +		return PAGE_KERNEL;
> +	if (attr & EFI_MEMORY_WT)
> +		return __pgprot(PROT_NORMAL_WT);
> +	if (attr & EFI_MEMORY_WC)
> +		return __pgprot(PROT_NORMAL_NC);
> +	return __pgprot(PROT_DEVICE_nGnRnE);
> +}
> +#endif

Also, this doesn't look like a small function - why is it inlined?

Moving it to .c would also slightly improve compilation times for every .c file 
that includes asm/acpi.h.

Thanks,

	Ingo

[PATCH 1/2] arm64: apei: implement arch_apei_get_mem_attributes()

[Matt Fleming]

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

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 according to the UEFI spec. Otherwise, return PAGE_KERNEL.

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Hanjun Guo <hanjun.guo@linaro.org>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/arm64/include/asm/acpi.h | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
index 406485ed110a..1cbad43e561d 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)
@@ -92,4 +97,29 @@ static inline const char *acpi_get_enable_method(int cpu)
 {
 	return acpi_psci_present() ? "psci" : NULL;
 }
+
+#ifdef	CONFIG_ACPI_APEI
+static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
+{
+	/*
+	 * 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.
+	 * The EFI memory attribute advises all possible capabilities of a
+	 * memory region. We use the most efficient capability.
+	 */
+
+	u64 attr;
+
+	attr = efi_mem_attributes(addr);
+	if (attr & EFI_MEMORY_WB)
+		return PAGE_KERNEL;
+	if (attr & EFI_MEMORY_WT)
+		return __pgprot(PROT_NORMAL_WT);
+	if (attr & EFI_MEMORY_WC)
+		return __pgprot(PROT_NORMAL_NC);
+	return __pgprot(PROT_DEVICE_nGnRnE);
+}
+#endif
+
 #endif /*_ASM_ACPI_H*/
-- 
2.1.0