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

Groups > linux.kernel > #1201741 > unrolled thread

[PATCH V10 0/5] map GHES memory region according to EFI memory map

Back to article view | Back to linux.kernel

Contents

  [PATCH V10 0/5] map GHES memory region according to EFI memory map "Jonathan (Zhixiong) Zhang" <zjzhang@codeaurora.org> - 2015-08-06 15:50 +0200
    [PATCH V10 1/5] efi: x86: rearrange efi_mem_attributes() "Jonathan (Zhixiong) Zhang" <zjzhang@codeaurora.org> - 2015-08-06 15:50 +0200
    [PATCH V10 4/5] arm64: apei: implement arch_apei_get_mem_attributes() "Jonathan (Zhixiong) Zhang" <zjzhang@codeaurora.org> - 2015-08-06 15:50 +0200
      Re: [PATCH V10 4/5] arm64: apei: implement arch_apei_get_mem_attributes() Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2015-08-07 11:10 +0200
        Re: [PATCH V10 4/5] arm64: apei: implement  arch_apei_get_mem_attributes() Matt Fleming <matt@codeblueprint.co.uk> - 2015-08-07 11:40 +0200
          Re: [PATCH V10 4/5] arm64: apei: implement arch_apei_get_mem_attributes() Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2015-08-07 12:00 +0200
            Re: [PATCH V10 4/5] arm64: apei: implement  arch_apei_get_mem_attributes() "Zhang, Jonathan Zhixiong" <zjzhang@codeaurora.org> - 2015-08-07 20:20 +0200
              Re: [PATCH V10 4/5] arm64: apei: implement  arch_apei_get_mem_attributes() Matt Fleming <matt@codeblueprint.co.uk> - 2015-08-07 21:00 +0200
                Re: [PATCH V10 4/5] arm64: apei: implement arch_apei_get_mem_attributes() Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2015-08-08 10:20 +0200
                  Re: [PATCH V10 4/5] arm64: apei: implement  arch_apei_get_mem_attributes() "Zhang, Jonathan Zhixiong" <zjzhang@codeaurora.org> - 2015-08-08 18:40 +0200
        Re: [PATCH V10 4/5] arm64: apei: implement  arch_apei_get_mem_attributes() "Zhang, Jonathan Zhixiong" <zjzhang@codeaurora.org> - 2015-08-07 19:50 +0200
          Re: [PATCH V10 4/5] arm64: apei: implement  arch_apei_get_mem_attributes() Matt Fleming <matt@codeblueprint.co.uk> - 2015-08-07 21:00 +0200

#1201741 — [PATCH V10 0/5] map GHES memory region according to EFI memory map

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.

V10:
1. Fixed compilation issue on x86.
V9:
1. Rebased to arm64-upstream-14543 of arm64/master.
2. Match strict MM type in arch_apei_get_mem_attribute().
V8:
1. For x86, always return PAGE_KERNEL for arch_apei_get_mem_attribute().
   The rational is explained in comment.
2. Rebased to arm64-upstream-14201 of arm64/master,
   next-20150724 of linux-next/master.
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    | 27 +++++++++++++++++++++++++++
 arch/arm64/include/asm/memory.h  |  1 +
 arch/arm64/include/asm/pgtable.h |  2 ++
 arch/arm64/mm/proc.S             |  4 +++-
 arch/x86/include/asm/acpi.h      | 24 ++++++++++++++++++++++++
 arch/x86/platform/efi/efi.c      | 18 ------------------
 drivers/acpi/apei/ghes.c         |  6 ++++--
 drivers/firmware/efi/efi.c       | 31 +++++++++++++++++++++++++++++++
 8 files changed, 92 insertions(+), 21 deletions(-)

-- 
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] | [next] | [standalone]

#1201744 — [PATCH V10 1/5] efi: x86: rearrange efi_mem_attributes()

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.

Reviewed-by: Matt Fleming <matt.fleming@intel.com>
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 cfba30f27392..1ea9cb05163b 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -952,24 +952,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 46eb8a6e9393..713f92f6b254 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -602,3 +602,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

--
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]

#1201745 — [PATCH V10 4/5] 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>
Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
---
This patch as is does not build on current tip of next branch of
git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git, a small
tweak is needed. It builds on tip of linux-next/master of linus'
repo and origin/master of arm64 repo.
---
 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..1025d0401016 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)
@@ -91,4 +95,26 @@ 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)
+{
+	u64 attr;
+
+	attr = efi_mem_attributes(addr);
+	if (attr & EFI_MEMORY_UC)
+		return __pgprot(PROT_DEVICE_nGnRnE);
+	if (attr & EFI_MEMORY_WC)
+		return __pgprot(PROT_NORMAL_NC);
+	if (attr & EFI_MEMORY_WT)
+		return __pgprot(PROT_NORMAL_WT);
+	return __pgprot(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

--
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]

#1202470 — Re: [PATCH V10 4/5] arm64: apei: implement arch_apei_get_mem_attributes()

On 6 August 2015 at 15:40, Jonathan (Zhixiong) Zhang
<zjzhang@codeaurora.org> 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>
> Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
> ---
> This patch as is does not build on current tip of next branch of
> git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git, a small
> tweak is needed. It builds on tip of linux-next/master of linus'
> repo and origin/master of arm64 repo.
> ---
>  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..1025d0401016 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)
> @@ -91,4 +95,26 @@ 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)
> +{
> +       u64 attr;
> +
> +       attr = efi_mem_attributes(addr);
> +       if (attr & EFI_MEMORY_UC)
> +               return __pgprot(PROT_DEVICE_nGnRnE);
> +       if (attr & EFI_MEMORY_WC)
> +               return __pgprot(PROT_NORMAL_NC);
> +       if (attr & EFI_MEMORY_WT)
> +               return __pgprot(PROT_NORMAL_WT);
> +       return __pgprot(PAGE_KERNEL);

The EFI memory types are not exclusive, and so many regions will have
all of the above set. The UEFI spec does not define how to interpret
these superimposed attributes, it is up to the OS to decide on a
consistent approach.

For instance, this region (captured from a arm64 boot log with
uefi_debug set on the command line)

[Runtime Data       |RUN|  |  |  |   |WB|WT|WC|UC]

would be mapped uncached when following the above logic, while it
makes more sense to map using PAGE_KERNEL in this case.

From the spec:

"""
EFI_MEMORY_UC: The memory region supports being configured as not cacheable.
EFI_MEMORY_WC: The memory region supports being configured as write combining.
EFI_MEMORY_WT: The memory region supports being configured as
cacheable with a “write through” policy. Writes that hit in the cache
will also be written to main memory.
EFI_MEMORY_WB: The memory region supports being configured as
cacheable with a “write back” policy. Reads and writes that hit in the
cache do not propagate to main memory. Dirty data is written back to
main memory when a new cache line is allocated.
"""

Also, the final __pgprot() is redundant here, it is already present in
the definition of PAGE_KERNEL.

-- 
Ard.
--
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]

#1202515 — Re: [PATCH V10 4/5] arm64: apei: implement arch_apei_get_mem_attributes()

On Fri, 07 Aug, at 11:00:17AM, Ard Biesheuvel wrote:
> 
> The EFI memory types are not exclusive, and so many regions will have
> all of the above set. The UEFI spec does not define how to interpret
> these superimposed attributes, it is up to the OS to decide on a
> consistent approach.
> 
> For instance, this region (captured from a arm64 boot log with
> uefi_debug set on the command line)
> 
> [Runtime Data       |RUN|  |  |  |   |WB|WT|WC|UC]
> 
> would be mapped uncached when following the above logic, while it
> makes more sense to map using PAGE_KERNEL in this case.

Urgh... good point Ard. Right now this is limited to the GHES driver, so
it's unclear whether this patch is buggy in practice or not.

Does it *ever* make sense to map a region as cacheable (WB/WT/WC) on
arm64 for the APEI/GHES case? Does the firmware handle the necessary
cache flushing?

> From the spec:
> 
> """
> EFI_MEMORY_UC: The memory region supports being configured as not cacheable.
> EFI_MEMORY_WC: The memory region supports being configured as write combining.
> EFI_MEMORY_WT: The memory region supports being configured as
> cacheable with a “write through” policy. Writes that hit in the cache
> will also be written to main memory.
> EFI_MEMORY_WB: The memory region supports being configured as
> cacheable with a “write back” policy. Reads and writes that hit in the
> cache do not propagate to main memory. Dirty data is written back to
> main memory when a new cache line is allocated.
> """
 
Jonathan, can you please provide the EFI memory map region attributes
for the GHES region that requires this series?

-- 
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]

#1202527 — Re: [PATCH V10 4/5] arm64: apei: implement arch_apei_get_mem_attributes()

On 7 August 2015 at 11:37, Matt Fleming <matt@codeblueprint.co.uk> wrote:
> On Fri, 07 Aug, at 11:00:17AM, Ard Biesheuvel wrote:
>>
>> The EFI memory types are not exclusive, and so many regions will have
>> all of the above set. The UEFI spec does not define how to interpret
>> these superimposed attributes, it is up to the OS to decide on a
>> consistent approach.
>>
>> For instance, this region (captured from a arm64 boot log with
>> uefi_debug set on the command line)
>>
>> [Runtime Data       |RUN|  |  |  |   |WB|WT|WC|UC]
>>
>> would be mapped uncached when following the above logic, while it
>> makes more sense to map using PAGE_KERNEL in this case.
>
> Urgh... good point Ard. Right now this is limited to the GHES driver, so
> it's unclear whether this patch is buggy in practice or not.
>
> Does it *ever* make sense to map a region as cacheable (WB/WT/WC) on
> arm64 for the APEI/GHES case? Does the firmware handle the necessary
> cache flushing?
>

No it does not. Currently, we only consider EFI_MEMORY_WB when
discovering system RAM from the UEFI memory map, so the direct linear
mapping should have a hole where the APEI/GHES regions lives if it
doesn't have the WB attribute set. This means we can map it WT/WC/UC
without violating architectural rules regarding mismatches attributes,
but it requires an explicit ioremap()

>> From the spec:
>>
>> """
>> EFI_MEMORY_UC: The memory region supports being configured as not cacheable.
>> EFI_MEMORY_WC: The memory region supports being configured as write combining.
>> EFI_MEMORY_WT: The memory region supports being configured as
>> cacheable with a “write through” policy. Writes that hit in the cache
>> will also be written to main memory.
>> EFI_MEMORY_WB: The memory region supports being configured as
>> cacheable with a “write back” policy. Reads and writes that hit in the
>> cache do not propagate to main memory. Dirty data is written back to
>> main memory when a new cache line is allocated.
>> """
>
> Jonathan, can you please provide the EFI memory map region attributes
> for the GHES region that requires this series?
>
> --
> 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]

#1202985 — Re: [PATCH V10 4/5] arm64: apei: implement arch_apei_get_mem_attributes()

On 8/7/2015 2:50 AM, Ard Biesheuvel wrote:
> On 7 August 2015 at 11:37, Matt Fleming <matt@codeblueprint.co.uk> wrote:
>> On Fri, 07 Aug, at 11:00:17AM, Ard Biesheuvel wrote:
>>>
>>> The EFI memory types are not exclusive, and so many regions will have
>>> all of the above set. The UEFI spec does not define how to interpret
>>> these superimposed attributes, it is up to the OS to decide on a
>>> consistent approach.
>>>
>>> For instance, this region (captured from a arm64 boot log with
>>> uefi_debug set on the command line)
>>>
>>> [Runtime Data       |RUN|  |  |  |   |WB|WT|WC|UC]
>>>
>>> would be mapped uncached when following the above logic, while it
>>> makes more sense to map using PAGE_KERNEL in this case.
>>
>> Urgh... good point Ard. Right now this is limited to the GHES driver, so
>> it's unclear whether this patch is buggy in practice or not.
>>
>> Does it *ever* make sense to map a region as cacheable (WB/WT/WC) on
>> arm64 for the APEI/GHES case? Does the firmware handle the necessary
>> cache flushing?
>>
>
> No it does not. Currently, we only consider EFI_MEMORY_WB when
> discovering system RAM from the UEFI memory map, so the direct linear
> mapping should have a hole where the APEI/GHES regions lives if it
> doesn't have the WB attribute set. This means we can map it WT/WC/UC
> without violating architectural rules regarding mismatches attributes,
> but it requires an explicit ioremap()
On some (future) arm64 platforms, APEI/GHES region may have full
coherent access by platform. In such case, the APEI/GHES region have
the same memory attributes as the rest of system RAM, such region
do not need to be advised by UEFI as separate entry, but as part of
system RAM memory region.
That being said, for arm64 platforms that do not have WB capability
for APEI/GHES region, such region should be mapped accordingly.
>
>>>  From the spec:
>>>
>>> """
>>> EFI_MEMORY_UC: The memory region supports being configured as not cacheable.
>>> EFI_MEMORY_WC: The memory region supports being configured as write combining.
>>> EFI_MEMORY_WT: The memory region supports being configured as
>>> cacheable with a “write through” policy. Writes that hit in the cache
>>> will also be written to main memory.
>>> EFI_MEMORY_WB: The memory region supports being configured as
>>> cacheable with a “write back” policy. Reads and writes that hit in the
>>> cache do not propagate to main memory. Dirty data is written back to
>>> main memory when a new cache line is allocated.
>>> """
>>
>> Jonathan, can you please provide the EFI memory map region attributes
>> for the GHES region that requires this series?
[Reserved           |   |  |  |  |   |  |  |  |UC]

>>
>> --
>> Matt Fleming, Intel Open Source Technology Center

-- 
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]

#1202997 — Re: [PATCH V10 4/5] arm64: apei: implement arch_apei_get_mem_attributes()

On Fri, 07 Aug, at 11:16:03AM, Zhang, Jonathan Zhixiong wrote:
> 
> On some (future) arm64 platforms, APEI/GHES region may have full
> coherent access by platform. In such case, the APEI/GHES region have
> the same memory attributes as the rest of system RAM, such region
> do not need to be advised by UEFI as separate entry, but as part of
> system RAM memory region.
> That being said, for arm64 platforms that do not have WB capability
> for APEI/GHES region, such region should be mapped accordingly.

OK, so what I need to know right now is whether I need to drop this
entire series from my pull request or whether you can send a patch on
top of the existing ones in the EFI 'next' branch to address the mapping
heuristic in arch_apei_get_mem_attributes().

> >>Jonathan, can you please provide the EFI memory map region attributes
> >>for the GHES region that requires this series?
> [Reserved           |   |  |  |  |   |  |  |  |UC]

Assuming this memmap entry is indicative of most GHES region on arm64
right now, I think it's worth taking this patch as-is and addressing the
issue Ard raised as a separate patch.

Does that work?

-- 
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]

#1203199 — Re: [PATCH V10 4/5] arm64: apei: implement arch_apei_get_mem_attributes()

On 7 August 2015 at 20:57, Matt Fleming <matt@codeblueprint.co.uk> wrote:
> On Fri, 07 Aug, at 11:16:03AM, Zhang, Jonathan Zhixiong wrote:
>>
>> On some (future) arm64 platforms, APEI/GHES region may have full
>> coherent access by platform. In such case, the APEI/GHES region have
>> the same memory attributes as the rest of system RAM, such region
>> do not need to be advised by UEFI as separate entry, but as part of
>> system RAM memory region.
>> That being said, for arm64 platforms that do not have WB capability
>> for APEI/GHES region, such region should be mapped accordingly.
>
> OK, so what I need to know right now is whether I need to drop this
> entire series from my pull request or whether you can send a patch on
> top of the existing ones in the EFI 'next' branch to address the mapping
> heuristic in arch_apei_get_mem_attributes().
>
>> >>Jonathan, can you please provide the EFI memory map region attributes
>> >>for the GHES region that requires this series?
>> [Reserved           |   |  |  |  |   |  |  |  |UC]
>
> Assuming this memmap entry is indicative of most GHES region on arm64
> right now, I think it's worth taking this patch as-is and addressing the
> issue Ard raised as a separate patch.
>
> Does that work?
>

I think that is fine.

So we'll expect two patches on top of Matt's -next branch:
- one that removes the redundant __pgprot
- one that inverts the order in which the memory attributes are tested

It would be good to have these in the same release so that the
behavior does not change between releases.

Thanks,
Ard.
--
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]

#1203319 — Re: [PATCH V10 4/5] arm64: apei: implement arch_apei_get_mem_attributes()

On 8/8/2015 1:11 AM, Ard Biesheuvel wrote:
> On 7 August 2015 at 20:57, Matt Fleming <matt@codeblueprint.co.uk> wrote:
>> On Fri, 07 Aug, at 11:16:03AM, Zhang, Jonathan Zhixiong wrote:
>>>
>>> On some (future) arm64 platforms, APEI/GHES region may have full
>>> coherent access by platform. In such case, the APEI/GHES region have
>>> the same memory attributes as the rest of system RAM, such region
>>> do not need to be advised by UEFI as separate entry, but as part of
>>> system RAM memory region.
>>> That being said, for arm64 platforms that do not have WB capability
>>> for APEI/GHES region, such region should be mapped accordingly.
>>
>> OK, so what I need to know right now is whether I need to drop this
>> entire series from my pull request or whether you can send a patch on
>> top of the existing ones in the EFI 'next' branch to address the mapping
>> heuristic in arch_apei_get_mem_attributes().
>>
>>>>> Jonathan, can you please provide the EFI memory map region attributes
>>>>> for the GHES region that requires this series?
>>> [Reserved           |   |  |  |  |   |  |  |  |UC]
>>
>> Assuming this memmap entry is indicative of most GHES region on arm64
>> right now, I think it's worth taking this patch as-is and addressing the
>> issue Ard raised as a separate patch.
>>
>> Does that work?
>>
>
> I think that is fine.
>
> So we'll expect two patches on top of Matt's -next branch:
> - one that removes the redundant __pgprot
> - one that inverts the order in which the memory attributes are tested
>
> It would be good to have these in the same release so that the
> behavior does not change between releases.
I have above mentioned two changes ready to go. I will send V11 of the
patch set out in the next hour.
>
> Thanks,
> Ard.
>

-- 
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]

#1202955 — Re: [PATCH V10 4/5] arm64: apei: implement arch_apei_get_mem_attributes()

Thanks for the review, Ard. Comments inline below.

On 8/7/2015 2:00 AM, Ard Biesheuvel wrote:
> On 6 August 2015 at 15:40, Jonathan (Zhixiong) Zhang
> <zjzhang@codeaurora.org> 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>
>> Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
>> ---
>> This patch as is does not build on current tip of next branch of
>> git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git, a small
>> tweak is needed. It builds on tip of linux-next/master of linus'
>> repo and origin/master of arm64 repo.
>> ---
>>   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..1025d0401016 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)
>> @@ -91,4 +95,26 @@ 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)
>> +{
>> +       u64 attr;
>> +
>> +       attr = efi_mem_attributes(addr);
>> +       if (attr & EFI_MEMORY_UC)
>> +               return __pgprot(PROT_DEVICE_nGnRnE);
>> +       if (attr & EFI_MEMORY_WC)
>> +               return __pgprot(PROT_NORMAL_NC);
>> +       if (attr & EFI_MEMORY_WT)
>> +               return __pgprot(PROT_NORMAL_WT);
>> +       return __pgprot(PAGE_KERNEL);
>
> The EFI memory types are not exclusive, and so many regions will have
> all of the above set. The UEFI spec does not define how to interpret
> these superimposed attributes, it is up to the OS to decide on a
> consistent approach.
>
> For instance, this region (captured from a arm64 boot log with
> uefi_debug set on the command line)
>
> [Runtime Data       |RUN|  |  |  |   |WB|WT|WC|UC]
>
> would be mapped uncached when following the above logic, while it
> makes more sense to map using PAGE_KERNEL in this case.
>
>  From the spec:
>
> """
> EFI_MEMORY_UC: The memory region supports being configured as not cacheable.
> EFI_MEMORY_WC: The memory region supports being configured as write combining.
> EFI_MEMORY_WT: The memory region supports being configured as
> cacheable with a “write through” policy. Writes that hit in the cache
> will also be written to main memory.
> EFI_MEMORY_WB: The memory region supports being configured as
> cacheable with a “write back” policy. Reads and writes that hit in the
> cache do not propagate to main memory. Dirty data is written back to
> main memory when a new cache line is allocated.
> """
EFI memory map advises all possible (memory type) capabilities of a
memory region. As you said, Linux should decide on a consistent
approach. What about us using the most efficient capability when
the attributes are superimposed? In such case, we will simply revert
the sequence in the above code.
>
> Also, the final __pgprot() is redundant here, it is already present in
> the definition of PAGE_KERNEL.
Yes, will fix it.

-- 
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]

#1202998 — Re: [PATCH V10 4/5] arm64: apei: implement arch_apei_get_mem_attributes()

On Fri, 07 Aug, at 10:40:20AM, Zhang, Jonathan Zhixiong wrote:
> >
> >Also, the final __pgprot() is redundant here, it is already present in
> >the definition of PAGE_KERNEL.
> Yes, will fix it.

I've already picked up this series and sent it as a pull request. Please
write the patch that cleans up __pgprot() ontop of the 'next' branch at,

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

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