Thank you Will for the feedback. Pls. see comments below.
On 7/16/2015 10:18 AM, Will Deacon wrote:
On Wed, Jul 15, 2015 at 01:31:55AM +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
arch/arm64/kernel/Makefile | 1 + arch/arm64/kernel/apei.c | 27 +++++++++++++++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 arch/arm64/kernel/apei.c
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile index 426d0763c81b..c135f1e4e3b8 100644 --- a/arch/arm64/kernel/Makefile +++ b/arch/arm64/kernel/Makefile @@ -36,6 +36,7 @@ arm64-obj-$(CONFIG_EFI) += efi.o efi-stub.o efi-entry.o arm64-obj-$(CONFIG_PCI) += pci.o arm64-obj-$(CONFIG_ARMV8_DEPRECATED) += armv8_deprecated.o arm64-obj-$(CONFIG_ACPI) += acpi.o +arm64-obj-$(CONFIG_ACPI_APEI) += apei.o
obj-y += $(arm64-obj-y) vdso/ obj-m += $(arm64-obj-m) diff --git a/arch/arm64/kernel/apei.c b/arch/arm64/kernel/apei.c new file mode 100644 index 000000000000..9c6b62f40ad7 --- /dev/null +++ b/arch/arm64/kernel/apei.c @@ -0,0 +1,27 @@ +/*
- Arch-specific APEI-related functions.
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License as published by
- the Free Software Foundation; either version 2 of the License, or
- (at your option) any later version.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- */
+#include <acpi/apei.h>
+#include <linux/efi.h>
+#include <asm/pgtable.h>
+pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr) +{
- if (efi_mem_attributes(addr) & EFI_MEMORY_UC)
return PROT_DEVICE_nGnRE;- else
return PAGE_KERNEL;+}
Do we really need a new file and out-of-line call for this?
We have a choice of either adding this function to arch/arm64/kernel/acpi.c, or creating arch/arm64/kernel/apei.c. As we continue to work on firmware first HW error handling for arm64, more arm64 specific APEI related functions may need to be implemented, thus I think it would be good to create arch/arm64/kernel/apei.c. That being said, to date we have found the needs to have only two arm64 specific APEI related functions. The other one can be found in LEG kernel, through this commit: aa2d69c88b27 ACPI, APEI, ARM64: APEI initial support for aarch64 My understanding is that Linaro will work on to upstream that commit. I do not strongly prefer either choice.
When APEI ghes driver maps the memory region that has error record updated by firmware, it executes in IRQ, timer or SEA handler. Since ioremap() can not be used in atomic context, so APEI implements a special version of atomic ioremap function calling ioremap_page_range(). On the other hand, x86 and ARM64 have different ways to define pgprot_t for page that needs to be accessed with uncached property. x86 defines PAGE_KERNEL_NOCACHE, while arm64 defines PROT_DEVICE_nGnRE. Therefore arch specific implementation is needed. There are other ways to achieve such needs. V3 of this patch set tried another way [1]. I think the current way makes the most sense, since it made generic APEI code to stay generic (no knowledge about EFI, no arch dependent ifdefs).
There are discussions centering around making this memory attribute retrieval generic. As Matt said in [2]: APEI case is special because irrespective of what the kernel says we want to be compatible with the firmware's memory map.
[1] http://fa.linux.kernel.narkive.com/YYXPJIjs/patch-v3-0-4-map-ghes-memory-reg... [2] http://fa.linux.kernel.narkive.com/YYXPJIjs/patch-v3-0-4-map-ghes-memory-reg...
Will