[PATCH] arm64: acpi: arch_apei_get_mem_attributes() should use memblock

[PATCH] arm64: acpi: arch_apei_get_mem_attributes() should use memblock

[James Morse]

arm64's arch_apei_get_mem_attributes() tries to use
efi_mem_attributes() to read the memory attributes from the efi
memory map.

drivers/firmware/efi/arm-init.c:efi_init() calls efi_memmap_unmap(),
which clears the EFI_MEMMAP bit from efi.flags once efi_init() has
finished with the memory map. This causes efi_mem_attributes() to
return 0 meaning PROT_DEVICE_nGnRnE is the chosen memory type for
all ACPI APEI mappings.

APEI may call this from NMI context, so we can't re-map the EFI
memory map as this may need to allocate virtual address space.
'ghes_ioremap_area' is APEIs cache of virtual address space to
access the buffer once we tell it the memory attributes.

Do as acpi_os_ioremap() does, and consult memblock to learn if
the provided address is memory, or not.

Signed-off-by: James Morse <james.morse@arm.com>
Cc: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
Fixes: 89e44b51cc0d ("arm64, acpi/apei: Implement arch_apei_get_mem_attributes()")

This doesn't code even get built on mainline as HAVE_ACPI_APEI isn't
defined, until https://lkml.org/lkml/2016/8/10/231 gets merged.
I don't think this should go to stable.

I also took the opportunity to remove some unnecessarily ifdef'd
includes.

 arch/arm64/kernel/acpi.c | 28 ++++++++--------------------
 1 file changed, 8 insertions(+), 20 deletions(-)

diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index 252a6d9c1da5..985f721f3bdd 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -18,6 +18,7 @@
 #include <linux/acpi.h>
 #include <linux/bootmem.h>
 #include <linux/cpumask.h>
+#include <linux/efi.h>
 #include <linux/init.h>
 #include <linux/irq.h>
 #include <linux/irqdomain.h>
@@ -28,13 +29,9 @@
 
 #include <asm/cputype.h>
 #include <asm/cpu_ops.h>
+#include <asm/pgtable.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);
@@ -241,22 +238,13 @@ void __init acpi_boot_table_init(void)
 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.
+	 * The EFI memmap isn't mapped, instead read the version exported
+	 * into memblock. EFI's reserve_regions() call adds memory with the
+	 * WB attribute to memblock via early_init_dt_add_memory_arch().
 	 */
+	if (!memblock_is_memory(addr))
+		return __pgprot(PROT_DEVICE_nGnRnE);
 
-	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);
+	return PAGE_KERNEL;
 }
 #endif
-- 
2.10.1

[PATCH] arm64: acpi: arch_apei_get_mem_attributes() should use memblock

[Baicar, Tyler]

On 11/8/2016 3:27 AM, James Morse wrote:
> arm64's arch_apei_get_mem_attributes() tries to use
> efi_mem_attributes() to read the memory attributes from the efi
> memory map.
>
> drivers/firmware/efi/arm-init.c:efi_init() calls efi_memmap_unmap(),
> which clears the EFI_MEMMAP bit from efi.flags once efi_init() has
> finished with the memory map. This causes efi_mem_attributes() to
> return 0 meaning PROT_DEVICE_nGnRnE is the chosen memory type for
> all ACPI APEI mappings.
>
> APEI may call this from NMI context, so we can't re-map the EFI
> memory map as this may need to allocate virtual address space.
> 'ghes_ioremap_area' is APEIs cache of virtual address space to
> access the buffer once we tell it the memory attributes.
>
> Do as acpi_os_ioremap() does, and consult memblock to learn if
> the provided address is memory, or not.
>
> Signed-off-by: James Morse <james.morse@arm.com>
> Cc: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Tested-by: Tyler Baicar <tbaicar@codeaurora.org>

Hello James,

This patch seems fine, APEI/GHES functionality still works properly for me.
I tested on a 4.8 kernel with the patch you mention below from Jonathan and
my patchset https://lkml.org/lkml/2016/10/21/746

My only question is when you say that this may be called from an NMI 
context.
arch_apei_get_mem_attributes() only gets called from ghes_ioremap_pfn_irq()
which only appears to get called if we are not in an NMI context.

 From drivers/acpi/apei/ghes.c:

312                 if (in_nmi) {
313 raw_spin_lock(&ghes_ioremap_lock_nmi);
314                         vaddr = ghes_ioremap_pfn_nmi(paddr >> 
PAGE_SHIFT);
315                 } else {
316 spin_lock_irqsave(&ghes_ioremap_lock_irq, flags);
317                         vaddr = ghes_ioremap_pfn_irq(paddr >> 
PAGE_SHIFT);
318                 }

So can this really be called from an NMI context?

Thanks,
Tyler
> ---
> Fixes: 89e44b51cc0d ("arm64, acpi/apei: Implement arch_apei_get_mem_attributes()")
>
> This doesn't code even get built on mainline as HAVE_ACPI_APEI isn't
> defined, until https://lkml.org/lkml/2016/8/10/231 gets merged.
> I don't think this should go to stable.
>
> I also took the opportunity to remove some unnecessarily ifdef'd
> includes.
>
>   arch/arm64/kernel/acpi.c | 28 ++++++++--------------------
>   1 file changed, 8 insertions(+), 20 deletions(-)
>
> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index 252a6d9c1da5..985f721f3bdd 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -18,6 +18,7 @@
>   #include <linux/acpi.h>
>   #include <linux/bootmem.h>
>   #include <linux/cpumask.h>
> +#include <linux/efi.h>
>   #include <linux/init.h>
>   #include <linux/irq.h>
>   #include <linux/irqdomain.h>
> @@ -28,13 +29,9 @@
>   
>   #include <asm/cputype.h>
>   #include <asm/cpu_ops.h>
> +#include <asm/pgtable.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);
> @@ -241,22 +238,13 @@ void __init acpi_boot_table_init(void)
>   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.
> +	 * The EFI memmap isn't mapped, instead read the version exported
> +	 * into memblock. EFI's reserve_regions() call adds memory with the
> +	 * WB attribute to memblock via early_init_dt_add_memory_arch().
>   	 */
> +	if (!memblock_is_memory(addr))
> +		return __pgprot(PROT_DEVICE_nGnRnE);
>   
> -	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);
> +	return PAGE_KERNEL;
>   }
>   #endif

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

[PATCH] arm64: acpi: arch_apei_get_mem_attributes() should use memblock

[James Morse]

Hi Tyler,

On 08/11/16 18:41, Baicar, Tyler wrote:
> This patch seems fine, APEI/GHES functionality still works properly for me.
> I tested on a 4.8 kernel with the patch you mention below from Jonathan and
> my patchset https://lkml.org/lkml/2016/10/21/746

What are the memory attributes of the region your firmware writes the error
status data to?

I guess its attributes in the UEFI memory map must be 'NC' or <empty>. In which
case the current default of Device_nGnRnE isn't so wrong. Otherwise your
firmware must be doing some cache cleaning, which shouldn't be necessary.


> My only question is when you say that this may be called from an NMI context.
> arch_apei_get_mem_attributes() only gets called from ghes_ioremap_pfn_irq()
> which only appears to get called if we are not in an NMI context.

Yes, that's broken too.
ghes_ioremap_pfn_nmi() assumes PAGE_KERNEL. If firmware writes the error data
via a non-cachable mapping, it will tell us this via the UEFI memory map, which
we currently misread.

I have patches to fix this too, but it isn't needed until someone turns on
CONFIG_HAVE_ACPI_APEI_NMI on arm64 (or ia64...).


> So can this really be called from an NMI context?

Not today, but it may be in the future. Re-mapping the EFI memory map to read it
is possible in the short term, but generates more work if we ever want to
support CONFIG_HAVE_ACPI_APEI_NMI. I implied all this into the word 'may', I
will try to be clearer next time!


Thanks,

James

[PATCH] arm64: acpi: arch_apei_get_mem_attributes() should use memblock

[Ard Biesheuvel]

Hi James,

On 8 November 2016 at 10:27, James Morse <james.morse@arm.com> wrote:
> arm64's arch_apei_get_mem_attributes() tries to use
> efi_mem_attributes() to read the memory attributes from the efi
> memory map.
>
> drivers/firmware/efi/arm-init.c:efi_init() calls efi_memmap_unmap(),
> which clears the EFI_MEMMAP bit from efi.flags once efi_init() has
> finished with the memory map. This causes efi_mem_attributes() to
> return 0 meaning PROT_DEVICE_nGnRnE is the chosen memory type for
> all ACPI APEI mappings.
>
> APEI may call this from NMI context, so we can't re-map the EFI
> memory map as this may need to allocate virtual address space.
> 'ghes_ioremap_area' is APEIs cache of virtual address space to
> access the buffer once we tell it the memory attributes.
>
> Do as acpi_os_ioremap() does, and consult memblock to learn if
> the provided address is memory, or not.
>
> Signed-off-by: James Morse <james.morse@arm.com>
> Cc: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> Fixes: 89e44b51cc0d ("arm64, acpi/apei: Implement arch_apei_get_mem_attributes()")
>
> This doesn't code even get built on mainline as HAVE_ACPI_APEI isn't
> defined, until https://lkml.org/lkml/2016/8/10/231 gets merged.
> I don't think this should go to stable.
>
> I also took the opportunity to remove some unnecessarily ifdef'd
> includes.
>
>  arch/arm64/kernel/acpi.c | 28 ++++++++--------------------
>  1 file changed, 8 insertions(+), 20 deletions(-)
>
> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index 252a6d9c1da5..985f721f3bdd 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -18,6 +18,7 @@
>  #include <linux/acpi.h>
>  #include <linux/bootmem.h>
>  #include <linux/cpumask.h>
> +#include <linux/efi.h>
>  #include <linux/init.h>
>  #include <linux/irq.h>
>  #include <linux/irqdomain.h>
> @@ -28,13 +29,9 @@
>
>  #include <asm/cputype.h>
>  #include <asm/cpu_ops.h>
> +#include <asm/pgtable.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);
> @@ -241,22 +238,13 @@ void __init acpi_boot_table_init(void)
>  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.
> +        * The EFI memmap isn't mapped, instead read the version exported
> +        * into memblock. EFI's reserve_regions() call adds memory with the
> +        * WB attribute to memblock via early_init_dt_add_memory_arch().
>          */
> +       if (!memblock_is_memory(addr))
> +               return __pgprot(PROT_DEVICE_nGnRnE);
>
> -       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);
> +       return PAGE_KERNEL;

As you know, we recently applied [0] to ensure that memory regions
that are marked by UEFI was write-through cacheable (WT) or
write-combining (WC) are not misidentified by the kernel as having
strongly ordered semantics.

This means that in this case, you may be returning PAGE_KERNEL for
regions that are lacking the EFI_MEMORY_WB attribute, which kind of
defeats the purpose of this function (AIUI), to align the kernel's
view of the region's attributes with the view of the firmware. I would
expect that, for use cases like this one, the burden is on the
firmware to ensure that only a single EFI_MEMORY_xx attribute is set
if any kind of coherency between UEFI and the kernel is expected. If
that single attribute is WT or WC, PAGE_KERNEL is most certainly
wrong.


[0] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=cb82cce7035

[PATCH] arm64: acpi: arch_apei_get_mem_attributes() should use memblock

[James Morse]

Hi Ard,

On 09/11/16 12:12, Ard Biesheuvel wrote:
> On 8 November 2016 at 10:27, James Morse <james.morse@arm.com> wrote:
>> arm64's arch_apei_get_mem_attributes() tries to use
>> efi_mem_attributes() to read the memory attributes from the efi
>> memory map.
>>
>> drivers/firmware/efi/arm-init.c:efi_init() calls efi_memmap_unmap(),
>> which clears the EFI_MEMMAP bit from efi.flags once efi_init() has
>> finished with the memory map. This causes efi_mem_attributes() to
>> return 0 meaning PROT_DEVICE_nGnRnE is the chosen memory type for
>> all ACPI APEI mappings.
>>
>> APEI may call this from NMI context, so we can't re-map the EFI
>> memory map as this may need to allocate virtual address space.
>> 'ghes_ioremap_area' is APEIs cache of virtual address space to
>> access the buffer once we tell it the memory attributes.
>>
>> Do as acpi_os_ioremap() does, and consult memblock to learn if
>> the provided address is memory, or not.

>> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c

>> @@ -241,22 +238,13 @@ void __init acpi_boot_table_init(void)
>>  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.
>> +        * The EFI memmap isn't mapped, instead read the version exported
>> +        * into memblock. EFI's reserve_regions() call adds memory with the
>> +        * WB attribute to memblock via early_init_dt_add_memory_arch().
>>          */
>> +       if (!memblock_is_memory(addr))
>> +               return __pgprot(PROT_DEVICE_nGnRnE);
>>
>> -       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);
>> +       return PAGE_KERNEL;
> 
> As you know, we recently applied [0] to ensure that memory regions
> that are marked by UEFI was write-through cacheable (WT) or
> write-combining (WC) are not misidentified by the kernel as having
> strongly ordered semantics.

Ah, I'd forgotten all about that...


> This means that in this case, you may be returning PAGE_KERNEL for
> regions that are lacking the EFI_MEMORY_WB attribute, which kind of
> defeats the purpose of this function (AIUI), to align the kernel's
> view of the region's attributes with the view of the firmware. I would
> expect that, for use cases like this one, the burden is on the
> firmware to ensure that only a single EFI_MEMORY_xx attribute is set
> if any kind of coherency between UEFI and the kernel is expected. If
> that single attribute is WT or WC, PAGE_KERNEL is most certainly
> wrong.

I've been trying to test some of the APEI code using some hacks on top of
kvmtool. (actually, quite a lot of hacks).

When I trigger an event, I see efi_mem_attributes() always return 0 because the
EFI memory map isn't mapped. This turns out to be because I have 'efi=noruntime'
on the command line. I stopped digging when I found this previously-dead code,
but should have gone further.

If this is an expected interaction, we can ignore this as a bug in my test
setup. (and I have to work out how to get efi runtime services going...)

If not, I can try always mapping the EFI memory map in
arm_enable_runtime_services() if we booted via ACPI, as in this case runtime
services isn't the only user of the memory map.


My intention here was to just mirror acpi_os_ioremap(), which will call
ioremap_cache() for WB/WC/WT regions, which (also) ends up using PROT_NORMAL. We
may get away with this, but you're right, we are less likely to here.



Thanks,

James

[PATCH] arm64: acpi: arch_apei_get_mem_attributes() should use memblock

[Will Deacon]

On Wed, Nov 09, 2016 at 06:14:21PM +0000, James Morse wrote:
> On 09/11/16 12:12, Ard Biesheuvel wrote:
> > This means that in this case, you may be returning PAGE_KERNEL for
> > regions that are lacking the EFI_MEMORY_WB attribute, which kind of
> > defeats the purpose of this function (AIUI), to align the kernel's
> > view of the region's attributes with the view of the firmware. I would
> > expect that, for use cases like this one, the burden is on the
> > firmware to ensure that only a single EFI_MEMORY_xx attribute is set
> > if any kind of coherency between UEFI and the kernel is expected. If
> > that single attribute is WT or WC, PAGE_KERNEL is most certainly
> > wrong.
> 
> I've been trying to test some of the APEI code using some hacks on top of
> kvmtool. (actually, quite a lot of hacks).
> 
> When I trigger an event, I see efi_mem_attributes() always return 0 because the
> EFI memory map isn't mapped. This turns out to be because I have 'efi=noruntime'
> on the command line. I stopped digging when I found this previously-dead code,
> but should have gone further.
> 
> If this is an expected interaction, we can ignore this as a bug in my test
> setup. (and I have to work out how to get efi runtime services going...)
> 
> If not, I can try always mapping the EFI memory map in
> arm_enable_runtime_services() if we booted via ACPI, as in this case runtime
> services isn't the only user of the memory map.
> 
> 
> My intention here was to just mirror acpi_os_ioremap(), which will call
> ioremap_cache() for WB/WC/WT regions, which (also) ends up using PROT_NORMAL. We
> may get away with this, but you're right, we are less likely to here.

I'd certainly be interested in opinions as to what acpi_os_ioremap is
supposed to do, since it has fingers in the NOMAP pie and if we change the
polarity of pfn_valid() for NOMAP mappings (as suggested by Robert Richter
to fix his NUMA issue), then acpi_os_ioremap will actually *fail* for
these WB/WC regions.

Will

[PATCH] arm64: acpi: arch_apei_get_mem_attributes() should use memblock

[Ard Biesheuvel]

On 9 November 2016 at 18:14, James Morse <james.morse@arm.com> wrote:
> Hi Ard,
>
> On 09/11/16 12:12, Ard Biesheuvel wrote:
>> On 8 November 2016 at 10:27, James Morse <james.morse@arm.com> wrote:
[...]
>>> @@ -241,22 +238,13 @@ void __init acpi_boot_table_init(void)
>>>  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.
>>> +        * The EFI memmap isn't mapped, instead read the version exported
>>> +        * into memblock. EFI's reserve_regions() call adds memory with the
>>> +        * WB attribute to memblock via early_init_dt_add_memory_arch().
>>>          */
>>> +       if (!memblock_is_memory(addr))
>>> +               return __pgprot(PROT_DEVICE_nGnRnE);
>>>
>>> -       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);
>>> +       return PAGE_KERNEL;
>>
>> As you know, we recently applied [0] to ensure that memory regions
>> that are marked by UEFI was write-through cacheable (WT) or
>> write-combining (WC) are not misidentified by the kernel as having
>> strongly ordered semantics.
>
> Ah, I'd forgotten all about that...
>
>
>> This means that in this case, you may be returning PAGE_KERNEL for
>> regions that are lacking the EFI_MEMORY_WB attribute, which kind of
>> defeats the purpose of this function (AIUI), to align the kernel's
>> view of the region's attributes with the view of the firmware. I would
>> expect that, for use cases like this one, the burden is on the
>> firmware to ensure that only a single EFI_MEMORY_xx attribute is set
>> if any kind of coherency between UEFI and the kernel is expected. If
>> that single attribute is WT or WC, PAGE_KERNEL is most certainly
>> wrong.
>
> I've been trying to test some of the APEI code using some hacks on top of
> kvmtool. (actually, quite a lot of hacks).
>
> When I trigger an event, I see efi_mem_attributes() always return 0 because the
> EFI memory map isn't mapped. This turns out to be because I have 'efi=noruntime'
> on the command line. I stopped digging when I found this previously-dead code,
> but should have gone further.
>

Indeed. Ordinarily, the UEFI memory map should always be mapped at
runtime, and so this code should always be able to rely on it.
However, it does beg the question whether efi=noruntime should
propagate across the ACPI subsystem as well.

> If this is an expected interaction, we can ignore this as a bug in my test
> setup. (and I have to work out how to get efi runtime services going...)
>
> If not, I can try always mapping the EFI memory map in
> arm_enable_runtime_services() if we booted via ACPI, as in this case runtime
> services isn't the only user of the memory map.
>

efi=noruntime is intended to avoid UEFI runtime services calls into
known buggy firmware, and so it would be perfectly reasonable, also
given the above, to always map the UEFI memory map, even if
efi=noruntime is passed. We simply didn't bother at the time when
nothing was relying on the UEFI memory map other than those UEFI
runtime services.

> My intention here was to just mirror acpi_os_ioremap(), which will call
> ioremap_cache() for WB/WC/WT regions, which (also) ends up using PROT_NORMAL. We
> may get away with this, but you're right, we are less likely to here.
>

Yeah, that is another wart we have to do something about sooner rather
than later ...