From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id A8E132D2384 for ; Tue, 9 Dec 2025 15:08:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1765292931; cv=none; b=VEJtWneDg7jC1x0AubxKt/HCFG5gBp+GMrs+DEh2T17RMjX6DGe1TiTujHTmJJFQUgUpViKJi5byGN2vkA+orp0LFA6iFER5mD6NwDviJGTrlbZumiR8nyI1nfGXnN4pBnLgz8Cc6vmAT3mY4XCmFVWx+ZoShIF/ryP8kD2AyBc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1765292931; c=relaxed/simple; bh=pvuhNQp94JDtj7u8SGqY8UR2TBjYdjnAbB6BAU6yAj4=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=cNRdJ05WCni4iMIo454j7sXg7bnQVzTU3WNpyG7W8ws0w48+Qb7JGSeiBM6KZYM788HqaYpoyBq2uNnE1W++G9/TsEgkO/YWb7MbuqITi0GE2ecRh+lRSi+9gpv/Lv43NmBhZmyjVyC1rbZ75frKXDIF7hVkg6BrNpQIegylt4U= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 98589175D; Tue, 9 Dec 2025 07:08:40 -0800 (PST) Received: from [10.1.196.46] (e134344.arm.com [10.1.196.46]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 8CB683F762; Tue, 9 Dec 2025 07:08:44 -0800 (PST) Message-ID: <47b3b953-1ab9-4b85-9cb4-e1ad8eb4a0f6@arm.com> Date: Tue, 9 Dec 2025 15:08:43 +0000 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RFC PATCH 01/38] arm64: mpam: Context switch the MPAM registers To: Fenghua Yu , James Morse , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Cc: D Scott Phillips OS , carl@os.amperecomputing.com, lcherian@marvell.com, bobo.shaobowang@huawei.com, tan.shaopeng@fujitsu.com, baolin.wang@linux.alibaba.com, Jamie Iles , Xin Hao , peternewman@google.com, dfustini@baylibre.com, amitsinght@marvell.com, David Hildenbrand , Dave Martin , Koba Ko , Shanker Donthineni , baisheng.gao@unisoc.com, Jonathan Cameron , Gavin Shan , rohit.mathew@arm.com, reinette.chatre@intel.com, Punit Agrawal References: <20251205215901.17772-1-james.morse@arm.com> <20251205215901.17772-2-james.morse@arm.com> From: Ben Horgan Content-Language: en-US In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hi Fenghua, On 12/5/25 23:53, Fenghua Yu wrote: > Hi, James, > > On 12/5/25 13:58, James Morse wrote: [...] >> >> 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. >> [...] >> 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 [...] >> +/* >> + * 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_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'. >> + */ >> +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 >> +} >> + >> +static inline void mpam_thread_switch(struct task_struct *tsk) >> +{ >> +    u64 oldregval; >> +    int cpu = smp_processor_id(); >> +    u64 regval = mpam_get_regval(tsk); >> + >> +    if (!IS_ENABLED(CONFIG_ARM64_MPAM) || >> +        !static_branch_likely(&mpam_enabled)) >> +        return; >> + >> +    if (regval == READ_ONCE(arm64_mpam_global_default)) > Why is this check needed? We need to read arm64_mpam_default in any > case, right? If a task is in the default resctrl group then the per-cpu resctrl configuration determines the mpam configuration. The way this is dealt with here is that for a task in the default group mpam_partid_pmg is set to arm64_mpam_global_default. When mpam_partid_pmg is arm64_mpam_global_default we know the task is in the default group, otherwise it would have a different partid, and so we consider the per-cpu configuration, arm64_mpam_default. When the task is not in the default resctrl group then we can just consider the per-task configuration. As mentioned in the commit message, this is complicated by code data prioritisation (cdp) and the value of arm64_mpam_global_default, depends on whether this is enabled or not. See resctrl_arch_set_cdp_enabled() added in a later patch. I'm not sure of a way to make this clearer in the code while keeping the arch and drivers/resctrl patches separate. > +        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); >> + >> +    WRITE_ONCE(per_cpu(arm64_mpam_current, cpu), regval); >> +} >> +#endif /* __ASM__MPAM_H */ > > [SNIP] > Thanks. > > -Fenghua > Thanks, Ben