Message-ID: <c205297d-0f6e-4238-bd6a-3cab498c6a45@arm.com>
Date: Thu, 18 Dec 2025 14:55:23 +0000
From: Ben Horgan <ben.horgan@....com>
To: Jonathan Cameron <jonathan.cameron@...wei.com>,
 James Morse <james.morse@....com>
Cc: linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 D Scott Phillips OS <scott@...amperecomputing.com>,
 carl@...amperecomputing.com, lcherian@...vell.com,
 bobo.shaobowang@...wei.com, tan.shaopeng@...itsu.com,
 baolin.wang@...ux.alibaba.com, Jamie Iles <quic_jiles@...cinc.com>,
 Xin Hao <xhao@...ux.alibaba.com>, peternewman@...gle.com,
 dfustini@...libre.com, amitsinght@...vell.com,
 David Hildenbrand <david@...nel.org>, Dave Martin <dave.martin@....com>,
 Koba Ko <kobak@...dia.com>, Shanker Donthineni <sdonthineni@...dia.com>,
 fenghuay@...dia.com, baisheng.gao@...soc.com, Gavin Shan <gshan@...hat.com>,
 rohit.mathew@....com, reinette.chatre@...el.com,
 Punit Agrawal <punit.agrawal@....qualcomm.com>
Subject: Re: [RFC PATCH 01/38] arm64: mpam: Context switch the MPAM registers



On 12/18/25 14:52, Ben Horgan wrote:
> Hi Jonathan,
> 
> On 12/18/25 10:35, Jonathan Cameron wrote:
>> On Fri, 5 Dec 2025 21:58:24 +0000
>> James Morse <james.morse@....com> wrote:
>>
>>> MPAM allows traffic in the SoC to be labeled by the OS, these labels
>>> are used to apply policy in caches and bandwidth regulators, and to
>>> monitor traffic in the SoC. The label is made up of a PARTID and PMG
>>> value. The x86 equivalent calls these CLOSID and RMID, but they don't
>>> map precisely.
>>>
>>> MPAM has two CPU system registers that is used to hold the PARTID and PMG
>>> values that traffic generated at each exception level will use. These can be
>>> set per-task by the resctrl file system. (resctrl is the defacto interface
>>> for controlling this stuff).
>>>
>>> Add a helper to switch this.
>>>
>>> struct task_struct's separate CLOSID and RMID fields are insufficient
>>> to implement resctrl using MPAM, as resctrl can change the PARTID (CLOSID)
>>> and PMG (sort of like the RMID) separately. On x86, the rmid is an
>>> independent number, so a race that writes a mismatched  closid and rmid
>>> into hardware is benign. On arm64, the pmg bits extend the partid.
>>> (i.e. partid-5 has a pmg-0 that is not the same as partid-6's pmg-0).
>>> In this case, mismatching the values will 'dirty' a pmg value that
>>> resctrl believes is clean, and is not tracking with its 'limbo' code.
>>>
>>> To avoid this, the partid and pmg are always read and written as a pair.
>>> Instead of making struct task_struct's closid and rmid fields an
>>> endian-unsafe union, add the value to struct thread_info and always use
>>> READ_ONCE()/WRITE_ONCE() when accessing this field.
>>>
>>> Resctrl allows a per-cpu 'default' value to be set, this overrides the
>>> values when scheduling a task in the default control-group, which has
>>> PARTID 0. The way 'code data prioritisation' gets emulated means the
>>> register value for the default group needs to be a variable.
>>>
>>> The current system register value is kept in a per-cpu variable to
>>> avoid writing to the system register if the value isn't going to change.
>>> Writes to this register may reset the hardware state for regulating
>>> bandwidth.
>>>
>>> Finally, there is no reason to context switch these registers unless
>>> there is a driver changing the values in struct task_struct. Hide
>>> the whole thing behind a static key. This also allows the driver to
>>> disable MPAM in response to errors reported by hardware. Move the
>>> existing static key to belong to the arch code, as in the future
>>> the MPAM driver may become a loadable module.
>>>
>>> All this should depend on whether there is an MPAM driver, hide
>>> it behind CONFIG_MPAM.
>>>
>>> CC: Amit Singh Tomar <amitsinght@...vell.com>
>>> Signed-off-by: James Morse <james.morse@....com>
>>
>>> diff --git a/arch/arm64/include/asm/mpam.h b/arch/arm64/include/asm/mpam.h
>>> new file mode 100644
>>> index 000000000000..86a55176f884
>>> --- /dev/null
>>> +++ b/arch/arm64/include/asm/mpam.h
>>> @@ -0,0 +1,74 @@
>> ...
>>
>>> +/*
>>> + * The resctrl filesystem writes to the partid/pmg values for threads and CPUs,
>>> + * which may race with reads in __mpam_sched_in(). Ensure only one of the old
>>> + * or new values are used. Particular care should be taken with the pmg field
>>> + * as __mpam_sched_in() 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.
>>> + *
>>> + * A value in struct thread_info is used instead of struct task_struct as the
>>> + * cpu's u64 register format is used, but struct task_struct has two u32'.
>>
>> This comment probably wants to provide a little more info if it is to be useful,
>>
>> Is it a reference to the closid and rmid fields under CONFIG_X86_CPU_RESCTRL?
>> I'm not immediately understanding why that matters given you could slap
>> a union on it without (I think) resulting in anything else moving.
> 
> Yes, the fields referred to are those closid and rmid. As James writes
> in the commit message a union is an alternative, but it would be endian
> unsafe. Unlikely to matter but lets not break things.

Meant to say... I'll add clarification in this vein to the comment.

> 
> I'm replying for James as he is otherwise engaged. Thanks for the review
> of this series and all your review on the previous MPAM series.
> 
>>
>> Now having it in thread_info moves it into arch header territory so
>> might make sense for that reason.
>>
>>> + */
>>> +static inline u64 mpam_get_regval(struct task_struct *tsk)
>>> +{
>>> +#ifdef CONFIG_ARM64_MPAM
>>> +	return READ_ONCE(task_thread_info(tsk)->mpam_partid_pmg);
>>> +#else
>>> +	return 0;
>>> +#endif
>>> +}
>>
> 
> Thanks,
> 
> Ben
> 

-- 
Thanks,

Ben

Powered by blists - more mailing lists