Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1206236 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [GIT PULL 0/2] EFI changes for v4.3 (part two) Matt Fleming <matt@codeblueprint.co.uk> - 2015-08-12 18:20 +0200
    [PATCH 2/2] acpi, apei: use appropriate pgprot_t to map GHES memory Matt Fleming <matt@codeblueprint.co.uk> - 2015-08-12 18:20 +0200
      Re: [PATCH 2/2] acpi, apei: use appropriate pgprot_t to map GHES  memory Ingo Molnar <mingo@kernel.org> - 2015-08-13 10:20 +0200
        Re: [PATCH 2/2] acpi, apei: use appropriate pgprot_t to map GHES  memory Matt Fleming <matt@codeblueprint.co.uk> - 2015-08-13 11:30 +0200
          Re: [PATCH 2/2] acpi, apei: use appropriate pgprot_t to map GHES  memory Will Deacon <will.deacon@arm.com> - 2015-08-13 13:20 +0200
            Re: [PATCH 2/2] acpi, apei: use appropriate pgprot_t to map GHES  memory "Zhang, Jonathan Zhixiong" <zjzhang@codeaurora.org> - 2015-08-14 21:10 +0200
    [PATCH 1/2] arm64: apei: implement arch_apei_get_mem_attributes() Matt Fleming <matt@codeblueprint.co.uk> - 2015-08-12 18:20 +0200
      Re: [PATCH 1/2] arm64: apei: implement arch_apei_get_mem_attributes() Ingo Molnar <mingo@kernel.org> - 2015-08-13 10:30 +0200
        Re: [PATCH 1/2] arm64: apei: implement arch_apei_get_mem_attributes() "Zhang, Jonathan Zhixiong" <zjzhang@codeaurora.org> - 2015-08-14 21:00 +0200
          Re: [PATCH 1/2] arm64: apei: implement arch_apei_get_mem_attributes() Matt Fleming <matt@codeblueprint.co.uk> - 2015-08-14 21:20 +0200

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

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

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

In this pull is the arm64 arch_apei_get_mem_attributes() implementation
with the changes requested by Ard. The final patch uses the new
arch_apei_get_mem_attributes() in the GHES driver, which means the
driver now maps the GHES region correctly on arm64.

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 tags/efi-next

for you to fetch changes up to 4cf8e8af8ac0a810f05ecc3fce440626b4dcb7e3:

  acpi, apei: use appropriate pgprot_t to map GHES memory (2015-08-12 14:44:46 +0100)

----------------------------------------------------------------
 * Additional patches for arm64 and to the ACPI GHES driver to allow
   architectures to specify which mapping attributes to use to ioremap
   the GHES region - Jonathan (Zhixiong) Zhang

----------------------------------------------------------------
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 | 30 ++++++++++++++++++++++++++++++
 drivers/acpi/apei/ghes.c      |  6 ++++--
 2 files changed, 34 insertions(+), 2 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1206237 — [PATCH 2/2] acpi, apei: use appropriate pgprot_t to map GHES memory

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. On an arm64
platform, firmware updates GHES memory region with uncached
access attribute, and then Linux reads stale data from cache.

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

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 | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 2bfd53cbfe80..0aa37c57acec 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -164,8 +164,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;
 }
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1206594 — Re: [PATCH 2/2] acpi, apei: use appropriate pgprot_t to map GHES memory

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

> 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. On an arm64
> platform, firmware updates GHES memory region with uncached
> access attribute, and then Linux reads stale data from cache.

So this paragraph does not parse for me ...

If it tries to explain a bug it falls very short of doing a proper job of that.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1206639 — Re: [PATCH 2/2] acpi, apei: use appropriate pgprot_t to map GHES memory

On Thu, 13 Aug, at 10:19:17AM, Ingo Molnar wrote:
> 
> * Matt Fleming <matt@codeblueprint.co.uk> wrote:
> 
> > 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. On an arm64
> > platform, firmware updates GHES memory region with uncached
> > access attribute, and then Linux reads stale data from cache.
> 
> So this paragraph does not parse for me ...
> 
> If it tries to explain a bug it falls very short of doing a proper job of that.

I'll let Jonathan provide more details but I understood the problem to
be a cache (in)coherency issue.

The kernel currently maps the the GHES memory region as cacheable
(PAGE_KERNEL) for all architectures. This memory region is used as a
communication buffer for reporting hardware errors from the firmware to
kernel. Essentially the firmware writes hardware error records there,
trigger an NMI/interrupt, and the GHES driver goes off and grabs the
error record from the GHES region.

Since the firmware gets first crack at inspecting the error this
mechanism is referred to as "firmware first" in the ACPI spec.

Now, there's a mismatch on arm64 platforms between how the kernel maps
the GHES region (PAGE_KERNEL) and how the firmware maps it
(EFI_MEMORY_UC, i.e. uncacheable), leading to the possibility of the
kernel GHES driver reading stale data from the cache when it receives
the NMI/interrupt.

As for exactly why the arm64 firmware uses an uncached mapping, I
presume it's to avoid relying on the kernel to get the necessary cache
flushing correct.

The proposed solution is to query the EFI memory map to ensure the
kernel uses a compatible mapping.

None of this should affect x86, it still uses PAGE_KERNEL because we're
yet to see any hardware that has an EFI memory map entry for the GHES
region that's incompatible with PAGE_KERNEL.

Jonathan, would you like to provide more details?

-- 
Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1206730 — Re: [PATCH 2/2] acpi, apei: use appropriate pgprot_t to map GHES memory

On Thu, Aug 13, 2015 at 10:24:32AM +0100, Matt Fleming wrote:
> On Thu, 13 Aug, at 10:19:17AM, Ingo Molnar wrote:
> > * Matt Fleming <matt@codeblueprint.co.uk> wrote:
> > 
> > > 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. On an arm64
> > > platform, firmware updates GHES memory region with uncached
> > > access attribute, and then Linux reads stale data from cache.
> > 
> > So this paragraph does not parse for me ...
> > 
> > If it tries to explain a bug it falls very short of doing a proper job of that.
> 
> I'll let Jonathan provide more details but I understood the problem to
> be a cache (in)coherency issue.
> 
> The kernel currently maps the the GHES memory region as cacheable
> (PAGE_KERNEL) for all architectures. This memory region is used as a
> communication buffer for reporting hardware errors from the firmware to
> kernel. Essentially the firmware writes hardware error records there,
> trigger an NMI/interrupt, and the GHES driver goes off and grabs the
> error record from the GHES region.
> 
> Since the firmware gets first crack at inspecting the error this
> mechanism is referred to as "firmware first" in the ACPI spec.
> 
> Now, there's a mismatch on arm64 platforms between how the kernel maps
> the GHES region (PAGE_KERNEL) and how the firmware maps it
> (EFI_MEMORY_UC, i.e. uncacheable), leading to the possibility of the
> kernel GHES driver reading stale data from the cache when it receives
> the NMI/interrupt.
> 
> As for exactly why the arm64 firmware uses an uncached mapping, I
> presume it's to avoid relying on the kernel to get the necessary cache
> flushing correct.
> 
> The proposed solution is to query the EFI memory map to ensure the
> kernel uses a compatible mapping.
> 
> None of this should affect x86, it still uses PAGE_KERNEL because we're
> yet to see any hardware that has an EFI memory map entry for the GHES
> region that's incompatible with PAGE_KERNEL.
> 
> Jonathan, would you like to provide more details?

FWIW, that matches my understanding of the problem too. The ARM architecture
refers to this situation as "mismatched memory attributes" and typically
requires some explicit cache maintenance to achieve portable behaviour in
this scenario.

It's much better to avoid the mismatch in the first place, if you can,
which is what this is all about.

Will
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1207862 — Re: [PATCH 2/2] acpi, apei: use appropriate pgprot_t to map GHES memory

On 8/13/2015 4:14 AM, Will Deacon wrote:
> On Thu, Aug 13, 2015 at 10:24:32AM +0100, Matt Fleming wrote:
>> On Thu, 13 Aug, at 10:19:17AM, Ingo Molnar wrote:
>>> * Matt Fleming <matt@codeblueprint.co.uk> wrote:
>>>
>>>> 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. On an arm64
>>>> platform, firmware updates GHES memory region with uncached
>>>> access attribute, and then Linux reads stale data from cache.
>>>
>>> So this paragraph does not parse for me ...
I will update the paragraph to explain that with current code,
PAGE_KERNEL is always used, this means that the kernel assumes
cache coherency to the memory region is maintained by firmware;
such assumption is not always correct.
>>>
>>> If it tries to explain a bug it falls very short of doing a proper job of that.
>>
>> I'll let Jonathan provide more details but I understood the problem to
>> be a cache (in)coherency issue.
>>
>> The kernel currently maps the the GHES memory region as cacheable
>> (PAGE_KERNEL) for all architectures. This memory region is used as a
>> communication buffer for reporting hardware errors from the firmware to
>> kernel. Essentially the firmware writes hardware error records there,
>> trigger an NMI/interrupt, and the GHES driver goes off and grabs the
>> error record from the GHES region.
>>
>> Since the firmware gets first crack at inspecting the error this
>> mechanism is referred to as "firmware first" in the ACPI spec.
>>
>> Now, there's a mismatch on arm64 platforms between how the kernel maps
>> the GHES region (PAGE_KERNEL) and how the firmware maps it
>> (EFI_MEMORY_UC, i.e. uncacheable), leading to the possibility of the
>> kernel GHES driver reading stale data from the cache when it receives
>> the NMI/interrupt.
>>
>> As for exactly why the arm64 firmware uses an uncached mapping, I
>> presume it's to avoid relying on the kernel to get the necessary cache
>> flushing correct.
>>
>> The proposed solution is to query the EFI memory map to ensure the
>> kernel uses a compatible mapping.
>>
>> None of this should affect x86, it still uses PAGE_KERNEL because we're
>> yet to see any hardware that has an EFI memory map entry for the GHES
>> region that's incompatible with PAGE_KERNEL.
>>
>> Jonathan, would you like to provide more details?
Not really. Matt and Will articulated it to the points.
>
> FWIW, that matches my understanding of the problem too. The ARM architecture
> refers to this situation as "mismatched memory attributes" and typically
> requires some explicit cache maintenance to achieve portable behaviour in
> this scenario.
>
> It's much better to avoid the mismatch in the first place, if you can,
> which is what this is all about.
>
> Will
> --
> To unsubscribe from this list: send the line "unsubscribe linux-efi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

-- 
Jonathan (Zhixiong) Zhang
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

* 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web