The current code sets up the memory attribute for Normal Tagged memory
in MAIR_EL1 whenever MTE is supported according to AA64PFR1.MTE without
taking arm64.nomte command line option into account.
This breaks when tag pages are reused as regular memory, as direct
access to such pages through the linear map may create an invalid DRAM
address (tags-of-tags).
This change delays MTE MAIR_EL1 setup until feature overrides can be
processed.
Signed-off-by: Evgenii Stepanov <[email protected]>
---
arch/arm64/include/asm/mte.h | 5 +++++
arch/arm64/include/asm/proc-fns.h | 4 ++++
arch/arm64/kernel/mte.c | 22 ++++++++++++++++++++++
arch/arm64/kernel/smp.c | 3 +++
arch/arm64/mm/mmu.c | 3 +++
arch/arm64/mm/proc.S | 28 +++++++++++++++++++---------
6 files changed, 56 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
index aa523591a44e5..bcabafe010d8c 100644
--- a/arch/arm64/include/asm/mte.h
+++ b/arch/arm64/include/asm/mte.h
@@ -48,6 +48,7 @@ long get_mte_ctrl(struct task_struct *task);
int mte_ptrace_copy_tags(struct task_struct *child, long request,
unsigned long addr, unsigned long data);
size_t mte_probe_user_range(const char __user *uaddr, size_t size);
+void mte_update_mair(void);
#else /* CONFIG_ARM64_MTE */
@@ -87,6 +88,10 @@ static inline int mte_ptrace_copy_tags(struct task_struct *child,
return -EIO;
}
+static inline void mte_update_mair(void)
+{
+}
+
#endif /* CONFIG_ARM64_MTE */
static inline void mte_disable_tco_entry(struct task_struct *task)
diff --git a/arch/arm64/include/asm/proc-fns.h b/arch/arm64/include/asm/proc-fns.h
index 0d5d1f0525eb3..34d9c25960343 100644
--- a/arch/arm64/include/asm/proc-fns.h
+++ b/arch/arm64/include/asm/proc-fns.h
@@ -19,6 +19,10 @@ extern void cpu_do_idle(void);
extern void cpu_do_suspend(struct cpu_suspend_ctx *ptr);
extern u64 cpu_do_resume(phys_addr_t ptr, u64 idmap_ttbr);
+#ifdef CONFIG_ARM64_MTE
+extern void cpu_mte_update_mair(void);
+#endif
+
#include <asm/memory.h>
#endif /* __ASSEMBLY__ */
diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
index b2b730233274b..4882c259febb4 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -21,6 +21,7 @@
#include <asm/barrier.h>
#include <asm/cpufeature.h>
#include <asm/mte.h>
+#include <asm/proc-fns.h>
#include <asm/ptrace.h>
#include <asm/sysreg.h>
@@ -574,3 +575,24 @@ size_t mte_probe_user_range(const char __user *uaddr, size_t size)
return 0;
}
+
+/*
+ * Open coded check for MTE before CPU features are set up.
+ */
+static bool arm64_early_this_cpu_has_mte(void)
+{
+ u64 pfr1;
+
+ if (!IS_ENABLED(CONFIG_ARM64_MTE))
+ return false;
+
+ pfr1 = __read_sysreg_by_encoding(SYS_ID_AA64PFR1_EL1);
+ return cpuid_feature_extract_unsigned_field(pfr1,
+ ID_AA64PFR1_MTE_SHIFT);
+}
+
+void mte_update_mair(void)
+{
+ if (arm64_early_this_cpu_has_mte())
+ cpu_mte_update_mair();
+}
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 62ed361a4376b..6725f978d6e50 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -43,6 +43,7 @@
#include <asm/daifflags.h>
#include <asm/kvm_mmu.h>
#include <asm/mmu_context.h>
+#include <asm/mte.h>
#include <asm/numa.h>
#include <asm/processor.h>
#include <asm/smp_plat.h>
@@ -226,6 +227,8 @@ asmlinkage notrace void secondary_start_kernel(void)
*/
check_local_cpu_capabilities();
+ mte_update_mair();
+
ops = get_cpu_ops(cpu);
if (ops->cpu_postboot)
ops->cpu_postboot();
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index db7c4e6ae57bb..37835686369ea 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -35,6 +35,7 @@
#include <linux/sizes.h>
#include <asm/tlb.h>
#include <asm/mmu_context.h>
+#include <asm/mte.h>
#include <asm/ptdump.h>
#include <asm/tlbflush.h>
#include <asm/pgalloc.h>
@@ -812,6 +813,8 @@ void __init paging_init(void)
idmap_t0sz = 63UL - __fls(__pa_symbol(_end) | GENMASK(VA_BITS_MIN - 1, 0));
+ mte_update_mair();
+
map_kernel(pgdp);
map_mem(pgdp);
diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
index 7837a69524c53..dc740db298dea 100644
--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S
@@ -393,6 +393,22 @@ SYM_FUNC_END(idmap_kpti_install_ng_mappings)
.popsection
#endif
+#ifdef CONFIG_ARM64_MTE
+/*
+ * cpu_mte_update_mair
+ *
+ * Setup Normal Tagged memory type at the corresponding MAIR index
+ */
+SYM_FUNC_START(cpu_mte_update_mair)
+ mov_q x9, MAIR_EL1_SET
+ mov x10, #MAIR_ATTR_NORMAL_TAGGED
+ bfi x9, x10, #(8 * MT_NORMAL_TAGGED), #8
+ msr mair_el1, x9
+ tlbi vmalle1 // Invalidate local TLB
+ dsb nsh
+ ret
+SYM_FUNC_END(cpu_mte_update_mair)
+#endif
/*
* __cpu_setup
*
@@ -421,16 +437,14 @@ SYM_FUNC_START(__cpu_setup)
* Default values for VMSA control registers. These will be adjusted
* below depending on detected CPU features.
*/
- mair .req x17
tcr .req x16
- mov_q mair, MAIR_EL1_SET
mov_q tcr, TCR_TxSZ(VA_BITS) | TCR_CACHE_FLAGS | TCR_SMP_FLAGS | \
TCR_TG_FLAGS | TCR_KASLR_FLAGS | TCR_ASID16 | \
TCR_TBI0 | TCR_A1 | TCR_KASAN_SW_FLAGS
#ifdef CONFIG_ARM64_MTE
/*
- * Update MAIR_EL1, GCR_EL1 and TFSR*_EL1 if MTE is supported
+ * Update GCR_EL1 and TFSR*_EL1 if MTE is supported
* (ID_AA64PFR1_EL1[11:8] > 1).
*/
mrs x10, ID_AA64PFR1_EL1
@@ -438,10 +452,6 @@ SYM_FUNC_START(__cpu_setup)
cmp x10, #ID_AA64PFR1_MTE
b.lt 1f
- /* Normal Tagged memory type at the corresponding MAIR index */
- mov x10, #MAIR_ATTR_NORMAL_TAGGED
- bfi mair, x10, #(8 * MT_NORMAL_TAGGED), #8
-
mov x10, #KERNEL_GCR_EL1
msr_s SYS_GCR_EL1, x10
@@ -493,7 +503,8 @@ SYM_FUNC_START(__cpu_setup)
orr tcr, tcr, #TCR_HA // hardware Access flag update
1:
#endif /* CONFIG_ARM64_HW_AFDBM */
- msr mair_el1, mair
+ mov_q x9, MAIR_EL1_SET
+ msr mair_el1, x9
msr tcr_el1, tcr
/*
* Prepare SCTLR
@@ -501,6 +512,5 @@ SYM_FUNC_START(__cpu_setup)
mov_q x0, INIT_SCTLR_EL1_MMU_ON
ret // return to head.S
- .unreq mair
.unreq tcr
SYM_FUNC_END(__cpu_setup)
--
2.37.1.559.g78731f0fdb-goog
On Fri, 05 Aug 2022 22:47:34 +0100,
Evgenii Stepanov <[email protected]> wrote:
>
> The current code sets up the memory attribute for Normal Tagged memory
> in MAIR_EL1 whenever MTE is supported according to AA64PFR1.MTE without
> taking arm64.nomte command line option into account.
>
> This breaks when tag pages are reused as regular memory, as direct
> access to such pages through the linear map may create an invalid DRAM
> address (tags-of-tags).
How comes such memory is being used? How comes it is in the linear
map?
arm64.nomte is affecting the use of MTE feature on the platform. It
doesn't guard the use of a MTE carve-out, and doesn't allow it to be
used in any shape or form.
To use this memory, you should remove the MTE configuration
altogether, as you cannot infer what the CPU is doing with it.
M.
--
Without deviation from the norm, progress is not possible.
On Tue, Aug 9, 2022 at 1:50 AM Marc Zyngier <[email protected]> wrote:
> How comes such memory is being used? How comes it is in the linear
> map?
>
> arm64.nomte is affecting the use of MTE feature on the platform. It
> doesn't guard the use of a MTE carve-out, and doesn't allow it to be
> used in any shape or form.
>
> To use this memory, you should remove the MTE configuration
> altogether, as you cannot infer what the CPU is doing with it.
This can be used to enable MTE in TZ but not in the NS memory.
On Tue, 09 Aug 2022 17:41:28 +0100,
Evgenii Stepanov <[email protected]> wrote:
>
> On Tue, Aug 9, 2022 at 1:50 AM Marc Zyngier <[email protected]> wrote:
> > How comes such memory is being used? How comes it is in the linear
> > map?
> >
> > arm64.nomte is affecting the use of MTE feature on the platform. It
> > doesn't guard the use of a MTE carve-out, and doesn't allow it to be
> > used in any shape or form.
> >
> > To use this memory, you should remove the MTE configuration
> > altogether, as you cannot infer what the CPU is doing with it.
>
> This can be used to enable MTE in TZ but not in the NS memory.
In which case what is the tag memory doing in the linear map?
Shouldn't it be marked as reserved, not mapped, and in general
completely ignored by the NS OS?
M.
--
Without deviation from the norm, progress is not possible.
On Tue, Aug 9, 2022 at 9:49 AM Marc Zyngier <[email protected]> wrote:
>
> In which case what is the tag memory doing in the linear map?
> Shouldn't it be marked as reserved, not mapped, and in general
> completely ignored by the NS OS?
That would be wasteful. The idea is to only reserve the parts of the
tag memory that correspond to the TZ carveout and release the rest to
the NS OS.
On Tue, Aug 9, 2022 at 10:29 AM Evgenii Stepanov <[email protected]> wrote:
>
> On Tue, Aug 9, 2022 at 9:49 AM Marc Zyngier <[email protected]> wrote:
> >
> > In which case what is the tag memory doing in the linear map?
> > Shouldn't it be marked as reserved, not mapped, and in general
> > completely ignored by the NS OS?
>
> That would be wasteful. The idea is to only reserve the parts of the
> tag memory that correspond to the TZ carveout and release the rest to
> the NS OS.
More generally, one can imagine a system where *any* tagged memory
transaction can result in an SError because the MTE implementation was
not configured by an earlier bootloader phase, e.g. because the
bootloader was configured to disable MTE at runtime. On such systems,
the kernel must refrain from causing tagged memory transactions to be
issued via the linear map, and that's exactly what this patch does.
Peter
On Tue, Aug 09, 2022 at 06:24:23PM -0700, Peter Collingbourne wrote:
> On Tue, Aug 9, 2022 at 10:29 AM Evgenii Stepanov <[email protected]> wrote:
> > On Tue, Aug 9, 2022 at 9:49 AM Marc Zyngier <[email protected]> wrote:
> > > In which case what is the tag memory doing in the linear map?
> > > Shouldn't it be marked as reserved, not mapped, and in general
> > > completely ignored by the NS OS?
> >
> > That would be wasteful. The idea is to only reserve the parts of the
> > tag memory that correspond to the TZ carveout and release the rest to
> > the NS OS.
>
> More generally, one can imagine a system where *any* tagged memory
> transaction can result in an SError because the MTE implementation was
> not configured by an earlier bootloader phase, e.g. because the
> bootloader was configured to disable MTE at runtime. On such systems,
> the kernel must refrain from causing tagged memory transactions to be
> issued via the linear map, and that's exactly what this patch does.
The problem is that it doesn't. The 8.5 architecture allows any Normal
Cacheable (even non-tagged) mapping to fetch tags. It may happen that on
certain implementations setting MAIR to non-tagged works but that's not
guaranteed and with the Linux kernel we tend to stick to the architected
behaviour (with a few exceptions like PMU counters and errata).
There is an ongoing discussion with the architects and partners on
whether we can tighten the architecture as not to cause visible
side-effects like SError but not sure whether that has been closed yet
(just back from holiday).
Until that's sorted, tag storage cannot be reused in an arm64-generic
way in the kernel.
--
Catalin
On Tue, Aug 16, 2022 at 12:51 AM Catalin Marinas
<[email protected]> wrote:
>
> On Tue, Aug 09, 2022 at 06:24:23PM -0700, Peter Collingbourne wrote:
> > On Tue, Aug 9, 2022 at 10:29 AM Evgenii Stepanov <[email protected]> wrote:
> > > On Tue, Aug 9, 2022 at 9:49 AM Marc Zyngier <[email protected]> wrote:
> > > > In which case what is the tag memory doing in the linear map?
> > > > Shouldn't it be marked as reserved, not mapped, and in general
> > > > completely ignored by the NS OS?
> > >
> > > That would be wasteful. The idea is to only reserve the parts of the
> > > tag memory that correspond to the TZ carveout and release the rest to
> > > the NS OS.
> >
> > More generally, one can imagine a system where *any* tagged memory
> > transaction can result in an SError because the MTE implementation was
> > not configured by an earlier bootloader phase, e.g. because the
> > bootloader was configured to disable MTE at runtime. On such systems,
> > the kernel must refrain from causing tagged memory transactions to be
> > issued via the linear map, and that's exactly what this patch does.
>
> The problem is that it doesn't. The 8.5 architecture allows any Normal
> Cacheable (even non-tagged) mapping to fetch tags. It may happen that on
> certain implementations setting MAIR to non-tagged works but that's not
> guaranteed and with the Linux kernel we tend to stick to the architected
> behaviour (with a few exceptions like PMU counters and errata).
>
> There is an ongoing discussion with the architects and partners on
> whether we can tighten the architecture as not to cause visible
> side-effects like SError but not sure whether that has been closed yet
> (just back from holiday).
>
> Until that's sorted, tag storage cannot be reused in an arm64-generic
> way in the kernel.
We can see how that discussion turns out, but let me take a shot at
persuading you that this is the right thing to do in any case.
Again, this isn't necessarily about reusing tag storage. It's about
whether the accesses via the linear map are expected to work at all.
As defined, the architecture gives EL2 the role of controlling whether
the system is deemed to implement, among other features, FEAT_MTE2, as
there is no capability for EL3 to trap accesses to the relevant ID
register. On the other hand, EL3 does to a large extent control
whether FEAT_MTE2 is implemented on a particular system, regardless of
whether the CPUs are capable of supporting it. Therefore, the kernel
has pragmatically defined arm64.nomte, together with other command
line arguments like arm64.nopauth, arm64.nobti and arm64.nosve, as
non-architectured means of overriding ID register bits. If the
relevant ID register bits for MTE as filtered through the command line
arguments are 0, this implies that FEAT_MTE2 is not implemented.
At this point we rejoin what is architected. Among the features
implied by FEAT_MTE2 is the ability to assign the Tagged Normal
attribute to pages via the MAIR. If the kernel were to use the Tagged
Normal attribute anyway, the behavior is defined to be UNPREDICTABLE.
Therefore, the kernel must not use this attribute in order to avoid
UNPREDICTABLE behavior. It is simply a bug that we are failing to
respect arm64.nomte when computing the MAIR.
Peter
On Tue, Aug 16, 2022 at 10:38:34PM -0700, Peter Collingbourne wrote:
> On Tue, Aug 16, 2022 at 12:51 AM Catalin Marinas
> <[email protected]> wrote:
> >
> > On Tue, Aug 09, 2022 at 06:24:23PM -0700, Peter Collingbourne wrote:
> > > On Tue, Aug 9, 2022 at 10:29 AM Evgenii Stepanov <[email protected]> wrote:
> > > > On Tue, Aug 9, 2022 at 9:49 AM Marc Zyngier <[email protected]> wrote:
> > > > > In which case what is the tag memory doing in the linear map?
> > > > > Shouldn't it be marked as reserved, not mapped, and in general
> > > > > completely ignored by the NS OS?
> > > >
> > > > That would be wasteful. The idea is to only reserve the parts of the
> > > > tag memory that correspond to the TZ carveout and release the rest to
> > > > the NS OS.
> > >
> > > More generally, one can imagine a system where *any* tagged memory
> > > transaction can result in an SError because the MTE implementation was
> > > not configured by an earlier bootloader phase, e.g. because the
> > > bootloader was configured to disable MTE at runtime. On such systems,
> > > the kernel must refrain from causing tagged memory transactions to be
> > > issued via the linear map, and that's exactly what this patch does.
> >
> > The problem is that it doesn't. The 8.5 architecture allows any Normal
> > Cacheable (even non-tagged) mapping to fetch tags. It may happen that on
> > certain implementations setting MAIR to non-tagged works but that's not
> > guaranteed and with the Linux kernel we tend to stick to the architected
> > behaviour (with a few exceptions like PMU counters and errata).
> >
> > There is an ongoing discussion with the architects and partners on
> > whether we can tighten the architecture as not to cause visible
> > side-effects like SError but not sure whether that has been closed yet
> > (just back from holiday).
> >
> > Until that's sorted, tag storage cannot be reused in an arm64-generic
> > way in the kernel.
>
> We can see how that discussion turns out, but let me take a shot at
> persuading you that this is the right thing to do in any case.
The argument for MAIR change in the patch description was to prevent the
kernel from accessing the tag storage. In some interpretation of the
architecture (and the proposed change), setting ATA to 0 should be
sufficient as not to generate SErrors, so no need to tweak MAIR. But if
tweaking MAIR is sufficient (and maybe necessary) on all current CPUs,
we might as well tighten the architecture. I'll be away from tomorrow
for another week but please raise it with the Arm architects when you
get a chance (otherwise I can raise it when I'm back).
> Again, this isn't necessarily about reusing tag storage. It's about
> whether the accesses via the linear map are expected to work at all.
Maybe but if it happens to work on some SoC, I don't want to give the
impression that reusing tag storage is purely a kernel problem. That's
pretty much the implication at the beginning of this thread.
In a SoC with MTE enabled by firmware, you are not expected to map the
tag allocation storage in the linear map. If such tag storage can be
reused as normal DRAM, the firmware should somehow disable MTE, probably
in an implementation-specific way at the memory controller level. If it
can't, then such memory cannot be used for anything else other than tag
storage.
> As defined, the architecture gives EL2 the role of controlling whether
> the system is deemed to implement, among other features, FEAT_MTE2, as
> there is no capability for EL3 to trap accesses to the relevant ID
> register. On the other hand, EL3 does to a large extent control
> whether FEAT_MTE2 is implemented on a particular system, regardless of
> whether the CPUs are capable of supporting it. Therefore, the kernel
> has pragmatically defined arm64.nomte, together with other command
> line arguments like arm64.nopauth, arm64.nobti and arm64.nosve, as
> non-architectured means of overriding ID register bits. If the
> relevant ID register bits for MTE as filtered through the command line
> arguments are 0, this implies that FEAT_MTE2 is not implemented.
It's more like telling the kernel that it should not attempt to use it
rather than completely hiding the feature. The hypervisor can do this
via ID reg trapping.
> At this point we rejoin what is architected. Among the features
> implied by FEAT_MTE2 is the ability to assign the Tagged Normal
> attribute to pages via the MAIR. If the kernel were to use the Tagged
> Normal attribute anyway, the behavior is defined to be UNPREDICTABLE.
> Therefore, the kernel must not use this attribute in order to avoid
> UNPREDICTABLE behavior. It is simply a bug that we are failing to
> respect arm64.nomte when computing the MAIR.
As I said above, it depends on how you interpret the command line
option. It's not unpredictable behaviour if the hardware does support
MTE, as per the raw ID register.
Now, the reason I did the MAIR setting early in proc.S is that the
values may be cached in the TLB. When a secondary CPU is brought up, it
starts with the corresponding MAIR entry untagged but once it turns on
CnP (if also present), it would pollute the TLB of the peer CPUs with
the wrong attribute. So there is a small window before the local TLBI
where the peer CPUs may fail to write tags.
Evgenii's patch also looks too complicated with some assembly function.
We can do everything in C as I did in some early versions of the MTE
patchset. See the cpu_enable_mte() hunk here:
https://lore.kernel.org/all/[email protected]/
and you also need this hack (which I didn't particularly like, hence
moving it to proc.S):
https://lore.kernel.org/all/[email protected]/
I'm about to go on holiday and don't have time to prepare a patch but
feel free to pick the corresponding hunks from the above patches and
post a new one. We just need some clarification in the architecture as I
don't want people to start believing that arm64.nomte solves the
carveout reuse problem.
--
Catalin