| Message ID | 20200421142603.3894-22-catalin.marinas@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: Memory Tagging Extension user-space support | expand |
On Tue, Apr 21, 2020 at 03:26:01PM +0100, Catalin Marinas wrote: > Even if the ID_AA64PFR1_EL1 register advertises the presence of MTE, it > is not guaranteed that the memory system on the SoC supports the > feature. In the absence of system-wide MTE support, the behaviour is > undefined and the kernel should not enable the MTE memory type in > MAIR_EL1. > > For FDT, add an 'arm,armv8.5-memtag' property to the /memory nodes and > check for its presence during MTE probing. For example: > > memory@80000000 { > device_type = "memory"; > arm,armv8.5-memtag; > reg = <0x00000000 0x80000000 0 0x80000000>, > <0x00000008 0x80000000 0 0x80000000>; > }; > > If the /memory nodes are not present in DT or if at least one node does > not support MTE, the feature will be disabled. On EFI systems, it is > assumed that the memory description matches the EFI memory map (if not, > it is considered a firmware bug). > > MTE is not currently supported on ACPI systems. > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> > Cc: Rob Herring <Rob.Herring@arm.com> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Will Deacon <will@kernel.org> > Cc: Suzuki K Poulose <Suzuki.Poulose@arm.com> This patch turns out to be incomplete. While it does not expose the HWCAP2_MTE to user when the above DT property is not present, it still allows user access to the ID_AA64PFR1_EL1.MTE field (via MRS emulations) since it is marked as visible.
On Fri, Apr 24, 2020 at 02:57:36PM +0100, Catalin Marinas wrote: > On Tue, Apr 21, 2020 at 03:26:01PM +0100, Catalin Marinas wrote: > > Even if the ID_AA64PFR1_EL1 register advertises the presence of MTE, it > > is not guaranteed that the memory system on the SoC supports the > > feature. In the absence of system-wide MTE support, the behaviour is > > undefined and the kernel should not enable the MTE memory type in > > MAIR_EL1. > > > > For FDT, add an 'arm,armv8.5-memtag' property to the /memory nodes and > > check for its presence during MTE probing. For example: > > > > memory@80000000 { > > device_type = "memory"; > > arm,armv8.5-memtag; > > reg = <0x00000000 0x80000000 0 0x80000000>, > > <0x00000008 0x80000000 0 0x80000000>; > > }; > > > > If the /memory nodes are not present in DT or if at least one node does > > not support MTE, the feature will be disabled. On EFI systems, it is > > assumed that the memory description matches the EFI memory map (if not, > > it is considered a firmware bug). > > > > MTE is not currently supported on ACPI systems. > > > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> > > Cc: Rob Herring <Rob.Herring@arm.com> > > Cc: Mark Rutland <mark.rutland@arm.com> > > Cc: Will Deacon <will@kernel.org> > > Cc: Suzuki K Poulose <Suzuki.Poulose@arm.com> > > This patch turns out to be incomplete. While it does not expose the > HWCAP2_MTE to user when the above DT property is not present, it still > allows user access to the ID_AA64PFR1_EL1.MTE field (via MRS emulations) > since it is marked as visible. Attempt below at moving the check to the CPUID fields setup. This way we can avoid the original patch entirely since the sanitised id regs will have a zero MTE field if DT doesn't support it. diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h index afc315814563..0a24d36bf231 100644 --- a/arch/arm64/include/asm/cpufeature.h +++ b/arch/arm64/include/asm/cpufeature.h @@ -61,6 +61,7 @@ struct arm64_ftr_bits { u8 shift; u8 width; s64 safe_val; /* safe value for FTR_EXACT features */ + s64 (*filter)(const struct arm64_ftr_bits *, s64); }; /* @@ -542,7 +543,10 @@ cpuid_feature_extract_field(u64 features, int field, bool sign) static inline s64 arm64_ftr_value(const struct arm64_ftr_bits *ftrp, u64 val) { - return (s64)cpuid_feature_extract_field_width(val, ftrp->shift, ftrp->width, ftrp->sign); + s64 fval = (s64)cpuid_feature_extract_field_width(val, ftrp->shift, ftrp->width, ftrp->sign); + if (ftrp->filter) + fval = ftrp->filter(ftrp, fval); + return fval; } static inline bool id_aa64mmfr0_mixed_endian_el0(u64 mmfr0) diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index a32aad1d5b57..b0f37c77ec77 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -89,23 +89,28 @@ DEFINE_STATIC_KEY_ARRAY_FALSE(cpu_hwcap_keys, ARM64_NCAPS); EXPORT_SYMBOL(cpu_hwcap_keys); #define __ARM64_FTR_BITS(SIGNED, VISIBLE, STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL) \ - { \ .sign = SIGNED, \ .visible = VISIBLE, \ .strict = STRICT, \ .type = TYPE, \ .shift = SHIFT, \ .width = WIDTH, \ - .safe_val = SAFE_VAL, \ - } + .safe_val = SAFE_VAL /* Define a feature with unsigned values */ #define ARM64_FTR_BITS(VISIBLE, STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL) \ - __ARM64_FTR_BITS(FTR_UNSIGNED, VISIBLE, STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL) + { __ARM64_FTR_BITS(FTR_UNSIGNED, VISIBLE, STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL), } /* Define a feature with a signed value */ #define S_ARM64_FTR_BITS(VISIBLE, STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL) \ - __ARM64_FTR_BITS(FTR_SIGNED, VISIBLE, STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL) + { __ARM64_FTR_BITS(FTR_SIGNED, VISIBLE, STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL), } + +/* Define a feature with a filter function to process the field value */ +#define FILTERED_ARM64_FTR_BITS(SIGNED, VISIBLE, STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL, filter_fn) \ + { \ + __ARM64_FTR_BITS(SIGNED, VISIBLE, STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL), \ + .filter = filter_fn, \ + } #define ARM64_FTR_END \ { \ @@ -120,6 +125,42 @@ static void cpu_enable_cnp(struct arm64_cpu_capabilities const *cap); static bool __system_matches_cap(unsigned int n); +#ifdef CONFIG_ARM64_MTE +s64 mte_ftr_filter(const struct arm64_ftr_bits *ftrp, s64 val) +{ + struct device_node *np; + static bool memory_checked = false; + static bool mte_capable = true; + + /* EL0-only MTE is not supported by Linux, don't expose it */ + if (val < ID_AA64PFR1_MTE) + return ID_AA64PFR1_MTE_NI; + + if (memory_checked) + return mte_capable ? val : ID_AA64PFR1_MTE_NI; + + if (!acpi_disabled) { + pr_warn("MTE not supported on ACPI systems\n"); + return ID_AA64PFR1_MTE_NI; + } + + /* check the DT "memory" nodes for MTE support */ + for_each_node_by_type(np, "memory") { + memory_checked = true; + mte_capable &= of_property_read_bool(np, "arm,armv8.5-memtag"); + } + + if (!memory_checked || !mte_capable) { + pr_warn("System memory is not MTE-capable\n"); + memory_checked = true; + mte_capable = false; + return ID_AA64PFR1_MTE_NI; + } + + return val; +} +#endif + /* * NOTE: Any changes to the visibility of features should be kept in * sync with the documentation of the CPU feature register ABI. @@ -184,8 +225,10 @@ static const struct arm64_ftr_bits ftr_id_aa64pfr0[] = { static const struct arm64_ftr_bits ftr_id_aa64pfr1[] = { ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR1_SSBS_SHIFT, 4, ID_AA64PFR1_SSBS_PSTATE_NI), - ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_MTE), - FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR1_MTE_SHIFT, 4, ID_AA64PFR1_MTE_NI), +#ifdef CONFIG_ARM64_MTE + FILTERED_ARM64_FTR_BITS(FTR_UNSIGNED, FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, + ID_AA64PFR1_MTE_SHIFT, 4, ID_AA64PFR1_MTE_NI, mte_ftr_filter), +#endif ARM64_FTR_END, };
Hi Catalin, On 04/24/2020 05:17 PM, Catalin Marinas wrote: > On Fri, Apr 24, 2020 at 02:57:36PM +0100, Catalin Marinas wrote: >> On Tue, Apr 21, 2020 at 03:26:01PM +0100, Catalin Marinas wrote: >>> Even if the ID_AA64PFR1_EL1 register advertises the presence of MTE, it >>> is not guaranteed that the memory system on the SoC supports the >>> feature. In the absence of system-wide MTE support, the behaviour is >>> undefined and the kernel should not enable the MTE memory type in >>> MAIR_EL1. >>> >>> For FDT, add an 'arm,armv8.5-memtag' property to the /memory nodes and >>> check for its presence during MTE probing. For example: >>> >>> memory@80000000 { >>> device_type = "memory"; >>> arm,armv8.5-memtag; >>> reg = <0x00000000 0x80000000 0 0x80000000>, >>> <0x00000008 0x80000000 0 0x80000000>; >>> }; >>> >>> If the /memory nodes are not present in DT or if at least one node does >>> not support MTE, the feature will be disabled. On EFI systems, it is >>> assumed that the memory description matches the EFI memory map (if not, >>> it is considered a firmware bug). >>> >>> MTE is not currently supported on ACPI systems. >>> >>> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> >>> Cc: Rob Herring <Rob.Herring@arm.com> >>> Cc: Mark Rutland <mark.rutland@arm.com> >>> Cc: Will Deacon <will@kernel.org> >>> Cc: Suzuki K Poulose <Suzuki.Poulose@arm.com> >> >> This patch turns out to be incomplete. While it does not expose the >> HWCAP2_MTE to user when the above DT property is not present, it still >> allows user access to the ID_AA64PFR1_EL1.MTE field (via MRS emulations) >> since it is marked as visible. > > Attempt below at moving the check to the CPUID fields setup. This way we > can avoid the original patch entirely since the sanitised id regs will > have a zero MTE field if DT doesn't support it. > > diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h > index afc315814563..0a24d36bf231 100644 > --- a/arch/arm64/include/asm/cpufeature.h > +++ b/arch/arm64/include/asm/cpufeature.h > @@ -61,6 +61,7 @@ struct arm64_ftr_bits { > u8 shift; > u8 width; > s64 safe_val; /* safe value for FTR_EXACT features */ > + s64 (*filter)(const struct arm64_ftr_bits *, s64); > }; > > /* > @@ -542,7 +543,10 @@ cpuid_feature_extract_field(u64 features, int field, bool sign) > > static inline s64 arm64_ftr_value(const struct arm64_ftr_bits *ftrp, u64 val) > { > - return (s64)cpuid_feature_extract_field_width(val, ftrp->shift, ftrp->width, ftrp->sign); > + s64 fval = (s64)cpuid_feature_extract_field_width(val, ftrp->shift, ftrp->width, ftrp->sign); > + if (ftrp->filter) > + fval = ftrp->filter(ftrp, fval); > + return fval; > } > This change makes sure that the sanitised infrastructure is initialised with masked value and all consumers see a "sanitised" value, including KVM (unless they emulate it directly on the local CPU) > > +#ifdef CONFIG_ARM64_MTE > +s64 mte_ftr_filter(const struct arm64_ftr_bits *ftrp, s64 val) > +{ > + struct device_node *np; > + static bool memory_checked = false; > + static bool mte_capable = true; > + > + /* EL0-only MTE is not supported by Linux, don't expose it */ > + if (val < ID_AA64PFR1_MTE) > + return ID_AA64PFR1_MTE_NI; > + > + if (memory_checked) > + return mte_capable ? val : ID_AA64PFR1_MTE_NI; > + > + if (!acpi_disabled) { > + pr_warn("MTE not supported on ACPI systems\n"); > + return ID_AA64PFR1_MTE_NI; > + } > + > + /* check the DT "memory" nodes for MTE support */ > + for_each_node_by_type(np, "memory") { > + memory_checked = true; > + mte_capable &= of_property_read_bool(np, "arm,armv8.5-memtag"); > + } > + > + if (!memory_checked || !mte_capable) { > + pr_warn("System memory is not MTE-capable\n"); > + memory_checked = true; > + mte_capable = false; > + return ID_AA64PFR1_MTE_NI; > + } > + > + return val; > +} > +#endif > + > /* > * NOTE: Any changes to the visibility of features should be kept in > * sync with the documentation of the CPU feature register ABI. > @@ -184,8 +225,10 @@ static const struct arm64_ftr_bits ftr_id_aa64pfr0[] = { > > static const struct arm64_ftr_bits ftr_id_aa64pfr1[] = { > ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR1_SSBS_SHIFT, 4, ID_AA64PFR1_SSBS_PSTATE_NI), > - ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_MTE), > - FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR1_MTE_SHIFT, 4, ID_AA64PFR1_MTE_NI), > +#ifdef CONFIG_ARM64_MTE > + FILTERED_ARM64_FTR_BITS(FTR_UNSIGNED, FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, > + ID_AA64PFR1_MTE_SHIFT, 4, ID_AA64PFR1_MTE_NI, mte_ftr_filter), > +#endif > ARM64_FTR_END, > }; > Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
diff --git a/arch/arm64/boot/dts/arm/fvp-base-revc.dts b/arch/arm64/boot/dts/arm/fvp-base-revc.dts index 66381d89c1ce..c620a289f15e 100644 --- a/arch/arm64/boot/dts/arm/fvp-base-revc.dts +++ b/arch/arm64/boot/dts/arm/fvp-base-revc.dts @@ -94,6 +94,7 @@ memory@80000000 { device_type = "memory"; + arm,armv8.5-memtag; reg = <0x00000000 0x80000000 0 0x80000000>, <0x00000008 0x80000000 0 0x80000000>; }; diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index d2fe8ff72324..a32aad1d5b57 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -7,6 +7,7 @@ #define pr_fmt(fmt) "CPU features: " fmt +#include <linux/acpi.h> #include <linux/bsearch.h> #include <linux/cpumask.h> #include <linux/crash_dump.h> @@ -14,6 +15,7 @@ #include <linux/stop_machine.h> #include <linux/types.h> #include <linux/mm.h> +#include <linux/of.h> #include <linux/cpu.h> #include <asm/cpu.h> #include <asm/cpufeature.h> @@ -1412,6 +1414,51 @@ static bool can_use_gic_priorities(const struct arm64_cpu_capabilities *entry, #endif #ifdef CONFIG_ARM64_MTE +static bool has_usable_mte(const struct arm64_cpu_capabilities *entry, + int scope) +{ + struct device_node *np; + bool memory_checked = false; + bool mte_capable = true; + + if (!has_cpuid_feature(entry, scope)) + return false; + + /* + * If !SCOPE_SYSTEM, return true as per the above CPUID check (late + * CPU bring-up/hotplug). Otherwise, perform addtional checks on the + * system memory MTE support. + */ + if (scope != SCOPE_SYSTEM) + return true; + + if (!acpi_disabled) { + pr_warn("MTE not supported on ACPI systems\n"); + return false; + } + + /* check the "memory" nodes for MTE support */ + for_each_node_by_type(np, "memory") { + memory_checked = true; + mte_capable &= of_property_read_bool(np, "arm,armv8.5-memtag"); + } + + if (!memory_checked || !mte_capable) { + pr_warn("System memory is not MTE-capable\n"); + return false; + } + + return true; +} + +static bool has_hwcap_mte(const struct arm64_cpu_capabilities *entry, + int scope) +{ + if (scope == SCOPE_SYSTEM) + return system_supports_mte(); + return this_cpu_has_cap(ARM64_MTE); +} + static void cpu_enable_mte(struct arm64_cpu_capabilities const *cap) { u64 mair; @@ -1828,7 +1875,7 @@ static const struct arm64_cpu_capabilities arm64_features[] = { .desc = "Memory Tagging Extension", .capability = ARM64_MTE, .type = ARM64_CPUCAP_SYSTEM_FEATURE, - .matches = has_cpuid_feature, + .matches = has_usable_mte, .sys_reg = SYS_ID_AA64PFR1_EL1, .field_pos = ID_AA64PFR1_MTE_SHIFT, .min_field_value = ID_AA64PFR1_MTE, @@ -1950,7 +1997,7 @@ static const struct arm64_cpu_capabilities arm64_elf_hwcaps[] = { HWCAP_MULTI_CAP(ptr_auth_hwcap_gen_matches, CAP_HWCAP, KERNEL_HWCAP_PACG), #endif #ifdef CONFIG_ARM64_MTE - HWCAP_CAP(SYS_ID_AA64PFR1_EL1, ID_AA64PFR1_MTE_SHIFT, FTR_UNSIGNED, ID_AA64PFR1_MTE, CAP_HWCAP, KERNEL_HWCAP_MTE), + HWCAP_CAP_MATCH(has_hwcap_mte, CAP_HWCAP, KERNEL_HWCAP_MTE), #endif /* CONFIG_ARM64_MTE */ {}, };
Even if the ID_AA64PFR1_EL1 register advertises the presence of MTE, it is not guaranteed that the memory system on the SoC supports the feature. In the absence of system-wide MTE support, the behaviour is undefined and the kernel should not enable the MTE memory type in MAIR_EL1. For FDT, add an 'arm,armv8.5-memtag' property to the /memory nodes and check for its presence during MTE probing. For example: memory@80000000 { device_type = "memory"; arm,armv8.5-memtag; reg = <0x00000000 0x80000000 0 0x80000000>, <0x00000008 0x80000000 0 0x80000000>; }; If the /memory nodes are not present in DT or if at least one node does not support MTE, the feature will be disabled. On EFI systems, it is assumed that the memory description matches the EFI memory map (if not, it is considered a firmware bug). MTE is not currently supported on ACPI systems. Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> Cc: Rob Herring <Rob.Herring@arm.com> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Will Deacon <will@kernel.org> Cc: Suzuki K Poulose <Suzuki.Poulose@arm.com> --- Notes: New in v3. Ongoing (internal) discussions on whether this is the right approach. The issue needs to be solved similarly for ACPI systems. arch/arm64/boot/dts/arm/fvp-base-revc.dts | 1 + arch/arm64/kernel/cpufeature.c | 51 ++++++++++++++++++++++- 2 files changed, 50 insertions(+), 2 deletions(-)