Re: [PATCH v3 06/47] arm64: mpam: Context switch the MPAM registers

[Catalin Marinas <catalin.marinas@arm.com>]

On Mon, Jan 12, 2026 at 04:58:33PM +0000, Ben Horgan wrote:
>  menu "ARMv8.5 architectural features"
> diff --git a/arch/arm64/include/asm/mpam.h b/arch/arm64/include/asm/mpam.h
> new file mode 100644
> index 000000000000..14011e5970ce
> --- /dev/null
> +++ b/arch/arm64/include/asm/mpam.h
> @@ -0,0 +1,67 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2025 Arm Ltd. */
> +
> +#ifndef __ASM__MPAM_H
> +#define __ASM__MPAM_H
> +
> +#include <linux/jump_label.h>
> +#include <linux/percpu.h>
> +#include <linux/sched.h>
> +
> +#include <asm/sysreg.h>
> +
> +DECLARE_STATIC_KEY_FALSE(mpam_enabled);
> +DECLARE_PER_CPU(u64, arm64_mpam_default);
> +DECLARE_PER_CPU(u64, arm64_mpam_current);
> +
> +/*
> + * The value of the MPAM0_EL1 sysreg when a task is in resctrl's default group.
> + * This is used by the context switch code to use the resctrl CPU property
> + * instead. The value is modified when CDP is enabled/disabled by mounting
> + * the resctrl filesystem.
> + */
> +extern u64 arm64_mpam_global_default;
> +
> +/*
> + * The resctrl filesystem writes to the partid/pmg values for threads and CPUs,
> + * which may race with reads in mpam_thread_switch(). Ensure only one of the old
> + * or new values are used. Particular care should be taken with the pmg field as
> + * mpam_thread_switch() may read a partid and pmg that don't match, causing this
> + * value to be stored with cache allocations, despite being considered 'free' by
> + * resctrl.
> + */
> +#ifdef CONFIG_ARM64_MPAM
> +static inline u64 mpam_get_regval(struct task_struct *tsk)
> +{
> +	return READ_ONCE(task_thread_info(tsk)->mpam_partid_pmg);
> +}
> +
> +static inline void mpam_thread_switch(struct task_struct *tsk)
> +{
> +	u64 oldregval;
> +	int cpu = smp_processor_id();
> +	u64 regval = mpam_get_regval(tsk);
> +
> +	if (!static_branch_likely(&mpam_enabled))
> +		return;
> +
> +	if (regval == READ_ONCE(arm64_mpam_global_default))
> +		regval = READ_ONCE(per_cpu(arm64_mpam_default, cpu));
> +
> +	oldregval = READ_ONCE(per_cpu(arm64_mpam_current, cpu));
> +	if (oldregval == regval)
> +		return;
> +
> +	write_sysreg_s(regval, SYS_MPAM1_EL1);
> +	isb();
> +
> +	/* Synchronising the EL0 write is left until the ERET to EL0 */
> +	write_sysreg_s(regval, SYS_MPAM0_EL1);

Since we have an isb() already, does it make any difference if we write
MPAM0 before the barrier? Similar question for other places where we
write these two registers.

At some point, we should go through __switch_to() and coalesce the isbs
into fewer as we keep accumulating them (e.g. all those switch function
setting some sync variable if needed).

> +
> +	WRITE_ONCE(per_cpu(arm64_mpam_current, cpu), regval);

Is it too expensive to read the MPAM sysregs and avoid carrying around
another per-CPU state? You use it for pm restoring but we could just
save it in cpu_do_suspend() like other sysregs. Not a big issue, it just
feels like this function got unnecessarily complicated (it took me a bit
to figure out what it all does).

A related question - is resctrl_arch_set_cdp_enabled() always called in
non-preemptible contexts? We potentially have a race between setting
current->mpam_partid_msg and arm64_mpam_global_default, so the check in
mpam_thread_switch() can get confused.

And I couldn't figure out where the MPAMx_EL1 registers are written. If
any global/per-cpu/per-task value is changed, does the kernel wait until
the next thread switch to write the sysreg? The only places I can found
touching these sysregs are the thread switch, pm notifiers and KVM.

-- 
Catalin