* Re: [PATCH v2 1/2] arm64: cpufeature: Add support for the MPAM v0.1 architecture version
[not found] ` <20260203095406.6437-2-zengheng4@huawei.com>
@ 2026-05-07 14:09 ` James Morse
2026-05-08 6:04 ` Zeng Heng
0 siblings, 1 reply; 8+ messages in thread
From: James Morse @ 2026-05-07 14:09 UTC (permalink / raw)
To: Zeng Heng, xry111, catalin.marinas, maz, ardb, yang, ryan.roberts,
kevin.brodsky, reinette.chatre, miko.lenczewski, will,
suzuki.poulose, thuth, ben.horgan, james.clark, lpieralisi,
broonie, oupton, anshuman.khandual, yeoreum.yun, leo.yan,
mrigendra.chaubey, fenghuay, ahmed.genidi, mark.rutland
Cc: linux-kernel, linux-arm-kernel, wangkefeng.wang
Hi Zeng,
(sorry for the delay - arm silently blocked your original mail. I've added you
to the whitelist, we'll see if that actually does anything!)
On 03/02/2026 09:54, Zeng Heng wrote:
> According to the MPAM spec [1], the supported architecture versions are
> v1.0, v1.1 and v0.1. MPAM versions v0.1 and v1.1 are functionally
> identical, but v0.1 additionally supports the FORCE_NS feature.
>
> ID_AA64PR | ID_AA64PR | MPAM Extension | Notes
> F0_EL1. | F1_EL1. | Architecture |
> MPAM | MPAM_frac | version |
> ---------------------------------------------------------------------------
> 0b0000 | 0b0001 | v0.1 | MPAM v0.1 is implemented.
> | | | MPAM v0.1 is the same as MPAM v1.1
> | | | with FORCE_NS which is
> | | | incompatible with MPAM v1.0.
> ---------------------------------------------------------------------------
> 0b0001 | 0b0000 | v1.0 | MPAM v1.0 is implemented.
> ---------------------------------------------------------------------------
> 0b0001 | 0b0001 | v1.1 | MPAM v1.1 is implemented.
> | | | MPAM v1.1 includes all features of
> | | | MPAM v1.0.
> | | | It must not include FORCE_NS.
>
> FORCE_NS is a feature that operates in EL3 mode. Consequently, the current
> Linux MPAM driver is also compatible with MPAM v0.1. To support v0.1, the
> existing driver which only checks ID_AA64PFR0_EL1.MPAM for the major
> version needs to examine ID_AA64PFR1_EL1.MPAM_frac for the minor version
> as well.
>
> [1] https://developer.arm.com/documentation/ddi0598/db/?lang=en
> diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h
> index cacd20df1786..606cf14e4044 100644
> --- a/arch/arm64/include/asm/el2_setup.h
> +++ b/arch/arm64/include/asm/el2_setup.h
> @@ -501,7 +501,9 @@
> #endif
>
> .macro finalise_el2_state
> - check_override id_aa64pfr0, ID_AA64PFR0_EL1_MPAM_SHIFT, .Linit_mpam_\@, .Lskip_mpam_\@, x1, x2
> + check_override id_aa64pfr0, ID_AA64PFR0_EL1_MPAM_SHIFT, .Linit_mpam_\@, .Lmpam_minor_\@, x1, x2
> +.Lmpam_minor_\@:
> + check_override id_aa64pfr1, ID_AA64PFR1_EL1_MPAM_frac_SHIFT, .Linit_mpam_\@, .Lskip_mpam_\@, x1, x2
Heh, I see the 'frac' field already has an override.
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index c840a93b9ef9..e8fa6f7cf2a2 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -1159,6 +1159,14 @@ static __init void detect_system_supports_pseudo_nmi(void)
> static inline void detect_system_supports_pseudo_nmi(void) { }
> #endif
>
> +static bool detect_ftr_has_mpam(void)
> +{
> + u64 pfr0 = read_sanitised_ftr_reg(SYS_ID_AA64PFR0_EL1);
> + u64 pfr1 = read_sanitised_ftr_reg(SYS_ID_AA64PFR1_EL1);
> +
> + return id_aa64pfr0_mpam(pfr0) || id_aa64pfr1_mpamfrac(pfr1);
> +}
> +
> void __init init_cpu_features(struct cpuinfo_arm64 *info)
> {
> /* Before we start using the tables, make sure it is sorted */
> @@ -2473,7 +2481,7 @@ cpucap_panic_on_conflict(const struct arm64_cpu_capabilities *cap)
> static bool
> test_has_mpam(const struct arm64_cpu_capabilities *entry, int scope)
> {
> - if (!has_cpuid_feature(entry, scope))
> + if (!detect_ftr_has_mpam())
> return false;
>
> /* Check firmware actually enabled MPAM on this cpu. */
> @@ -3078,7 +3086,6 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
> .capability = ARM64_MPAM,
> .matches = test_has_mpam,
> .cpu_enable = cpu_enable_mpam,
> - ARM64_CPUID_FIELDS(ID_AA64PFR0_EL1, MPAM, 1)
> },
> {
> .desc = "Memory Partitioning And Monitoring Virtualisation",
I think this is preferable to a 'metacap' as the pointer auth code does - we never need
to expose the difference between MPAM v0.1 and v1.x to anything.
Reviewed-by: James Morse <james.morse@arm.com>
Thanks,
James
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] arm_mpam: Update architecture version check for MPAM MSC
[not found] ` <20260203095406.6437-3-zengheng4@huawei.com>
@ 2026-05-07 16:03 ` James Morse
2026-05-08 2:26 ` Zeng Heng
0 siblings, 1 reply; 8+ messages in thread
From: James Morse @ 2026-05-07 16:03 UTC (permalink / raw)
To: Zeng Heng, xry111, catalin.marinas, maz, ardb, yang, ryan.roberts,
kevin.brodsky, reinette.chatre, miko.lenczewski, will,
suzuki.poulose, thuth, ben.horgan, james.clark, lpieralisi,
broonie, oupton, anshuman.khandual, yeoreum.yun, leo.yan,
mrigendra.chaubey, fenghuay, ahmed.genidi, mark.rutland
Cc: linux-kernel, linux-arm-kernel, wangkefeng.wang
Hi Zeng,
On 03/02/2026 09:54, Zeng Heng wrote:
> In addition to updating the CPU MPAM version check, the MPAM MSC version
> check also need to be updated. mpam_msc_check_aidr() is added to check
> the MSC AIDR register, ensuring that both the major and minor version
> numbers fall within the supported range of the MPAM architecture version.
> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index 744df3a6a078..a58031f0a280 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
> @@ -202,6 +202,17 @@ static inline void _mpam_write_monsel_reg(struct mpam_msc *msc, u16 reg, u32 val
>
> #define mpam_write_monsel_reg(msc, reg, val) _mpam_write_monsel_reg(msc, MSMON_##reg, val)
>
> +static bool mpam_msc_check_aidr(struct mpam_msc *msc)
> +{
> + u32 rev;
> +
> + rev = __mpam_read_reg(msc, MPAMF_AIDR) & MPAMF_AIDR_ARCH_REV;
> +
> + return rev == MPAM_ARCHITECTURE_V0_1 ||
> + rev == MPAM_ARCHITECTURE_V1_0 ||
> + rev == MPAM_ARCHITECTURE_V1_1;
> +}
> +
> static u64 mpam_msc_read_idr(struct mpam_msc *msc)
> {
> u64 idr_high = 0, idr_low;
> @@ -842,9 +853,8 @@ static int mpam_msc_hw_probe(struct mpam_msc *msc)
>
> lockdep_assert_held(&msc->probe_lock);
>
> - idr = __mpam_read_reg(msc, MPAMF_AIDR);
> - if ((idr & MPAMF_AIDR_ARCH_MAJOR_REV) != MPAM_ARCHITECTURE_V1) {
> - dev_err_once(dev, "MSC does not match MPAM architecture v1.x\n");
This deliberately only checks the major number. (The MSC architecture always had a
major and minor number). Before your change, MPAM v1.2 is supported, after we'd
get an error for a MPAM v1.2 MSC. (if such a thing exists)
I think its simpler to rule out the unsupported combinations, something like:
| static bool mpam_msc_check_aidr(struct mpam_msc *msc)
| {
| u32 rev;
|
| rev = __mpam_read_reg(msc, MPAMF_AIDR) & MPAMF_AIDR_ARCH_REV;
|
| /*
| * v0.0 and >v2.x aren't supported, but anything else should be backward
| * compatible to v0.1 or v1.0.
| */
| if (!rev)
| return false;
| if (rev & MPAMF_AIDR_ARCH_MAJOR_REV > MPAM_ARCHITECTURE_V1)
| return false;
|
| return true;
| }
> + if (!mpam_msc_check_aidr(msc)) {
> + dev_err_once(dev, "MSC does not match MPAM architecture\n");
> return -EIO;
> }
I'd like to keep the 'v1.x' in this message - this should help folk with old stable
kernels running on new hardware work out why the feature isn't available.
(assuming they have some documentation that says v2.0 in it!)
I've rebased this with the above changes, which I'll post shortly for fixes.
Thanks,
James
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] arm_mpam: Update architecture version check for MPAM MSC
2026-05-07 16:03 ` [PATCH v2 2/2] arm_mpam: Update architecture version check for MPAM MSC James Morse
@ 2026-05-08 2:26 ` Zeng Heng
2026-05-08 3:47 ` Zeng Heng
0 siblings, 1 reply; 8+ messages in thread
From: Zeng Heng @ 2026-05-08 2:26 UTC (permalink / raw)
To: James Morse, ben.horgan, xry111, catalin.marinas, maz, ardb, yang,
ryan.roberts, kevin.brodsky, reinette.chatre, miko.lenczewski,
will, suzuki.poulose, thuth, james.clark, lpieralisi, broonie,
oupton, anshuman.khandual, yeoreum.yun, leo.yan,
mrigendra.chaubey, fenghuay, ahmed.genidi, mark.rutland
Cc: linux-kernel, linux-arm-kernel, wangkefeng.wang
Hi James,
On 2026/5/8 0:03, James Morse wrote:
> Hi Zeng,
>
> On 03/02/2026 09:54, Zeng Heng wrote:
>> In addition to updating the CPU MPAM version check, the MPAM MSC version
>> check also need to be updated. mpam_msc_check_aidr() is added to check
>> the MSC AIDR register, ensuring that both the major and minor version
>> numbers fall within the supported range of the MPAM architecture version.
>
>> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
>> index 744df3a6a078..a58031f0a280 100644
>> --- a/drivers/resctrl/mpam_devices.c
>> +++ b/drivers/resctrl/mpam_devices.c
>> @@ -202,6 +202,17 @@ static inline void _mpam_write_monsel_reg(struct mpam_msc *msc, u16 reg, u32 val
>>
>> #define mpam_write_monsel_reg(msc, reg, val) _mpam_write_monsel_reg(msc, MSMON_##reg, val)
>>
>> +static bool mpam_msc_check_aidr(struct mpam_msc *msc)
>> +{
>> + u32 rev;
>> +
>> + rev = __mpam_read_reg(msc, MPAMF_AIDR) & MPAMF_AIDR_ARCH_REV;
>> +
>> + return rev == MPAM_ARCHITECTURE_V0_1 ||
>> + rev == MPAM_ARCHITECTURE_V1_0 ||
>> + rev == MPAM_ARCHITECTURE_V1_1;
>> +}
>> +
>> static u64 mpam_msc_read_idr(struct mpam_msc *msc)
>> {
>> u64 idr_high = 0, idr_low;
>> @@ -842,9 +853,8 @@ static int mpam_msc_hw_probe(struct mpam_msc *msc)
>>
>> lockdep_assert_held(&msc->probe_lock);
>>
>> - idr = __mpam_read_reg(msc, MPAMF_AIDR);
>> - if ((idr & MPAMF_AIDR_ARCH_MAJOR_REV) != MPAM_ARCHITECTURE_V1) {
>> - dev_err_once(dev, "MSC does not match MPAM architecture v1.x\n");
>
> This deliberately only checks the major number. (The MSC architecture always had a
> major and minor number). Before your change, MPAM v1.2 is supported, after we'd
> get an error for a MPAM v1.2 MSC. (if such a thing exists)
>
> I think its simpler to rule out the unsupported combinations, something like:
> | static bool mpam_msc_check_aidr(struct mpam_msc *msc)
> | {
> | u32 rev;
> |
> | rev = __mpam_read_reg(msc, MPAMF_AIDR) & MPAMF_AIDR_ARCH_REV;
> |
> | /*
> | * v0.0 and >v2.x aren't supported, but anything else should be backward
> | * compatible to v0.1 or v1.0.
> | */
> | if (!rev)
> | return false;
> | if (rev & MPAMF_AIDR_ARCH_MAJOR_REV > MPAM_ARCHITECTURE_V1)
> | return false;
> |
> | return true;
> | }
>
>> + if (!mpam_msc_check_aidr(msc)) {
>> + dev_err_once(dev, "MSC does not match MPAM architecture\n");
>> return -EIO;
>> }
>
> I'd like to keep the 'v1.x' in this message - this should help folk with old stable
> kernels running on new hardware work out why the feature isn't available.
> (assuming they have some documentation that says v2.0 in it!)
>
> I've rebased this with the above changes, which I'll post shortly for fixes.
>
>
Agreed. Keep the backward compatibility extension for versions
(compatible with v0.x(x>0) and 1.x), and remove the redundant
MPAM_ARCHITECTURE_Vx_x macro definitions.
I've verified locally that everything works fine.
Best regards,
Zeng Heng
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] arm_mpam: Update architecture version check for MPAM MSC
2026-05-08 2:26 ` Zeng Heng
@ 2026-05-08 3:47 ` Zeng Heng
2026-05-08 9:37 ` Ben Horgan
0 siblings, 1 reply; 8+ messages in thread
From: Zeng Heng @ 2026-05-08 3:47 UTC (permalink / raw)
To: James Morse, ben.horgan, xry111, catalin.marinas, maz, ardb, yang,
ryan.roberts, kevin.brodsky, reinette.chatre, miko.lenczewski,
will, suzuki.poulose, thuth, james.clark, lpieralisi, broonie,
oupton, anshuman.khandual, yeoreum.yun, leo.yan,
mrigendra.chaubey, fenghuay, ahmed.genidi, mark.rutland
Cc: linux-kernel, linux-arm-kernel, wangkefeng.wang
Hi James,
On 2026/5/8 10:26, Zeng Heng wrote:
>> I think its simpler to rule out the unsupported combinations,
>> something like:
>> | static bool mpam_msc_check_aidr(struct mpam_msc *msc)
>> | {
>> | u32 rev;
>> |
>> | rev = __mpam_read_reg(msc, MPAMF_AIDR) & MPAMF_AIDR_ARCH_REV;
>> |
>> | /*
>> | * v0.0 and >v2.x aren't supported, but anything else should be
>> backward
>> | * compatible to v0.1 or v1.0.
>> | */
>> | if (!rev)
>> | return false;
>> | if (rev & MPAMF_AIDR_ARCH_MAJOR_REV > MPAM_ARCHITECTURE_V1)
>> | return false;
>> |
Oops, after more complete version number testing, I found there's an
operator precedence issue here. The correct fix is:
if ((rev & MPAMF_AIDR_ARCH_MAJOR_REV) > MPAM_ARCHITECTURE_V1)
return false;
Note that '>' has higher precedence than '&'.
With this fix included:
Tested-by: Zeng Heng <zengheng4@huawei.com>
>> | return true;
>> | }
>>
>>> + if (!mpam_msc_check_aidr(msc)) {
>>> + dev_err_once(dev, "MSC does not match MPAM architecture\n");
>>> return -EIO;
>>> }
>>
>> I'd like to keep the 'v1.x' in this message - this should help folk
>> with old stable
>> kernels running on new hardware work out why the feature isn't available.
>> (assuming they have some documentation that says v2.0 in it!)
>>
>> I've rebased this with the above changes, which I'll post shortly for
>> fixes.
>>
>>
>
> Agreed. Keep the backward compatibility extension for versions
> (compatible with v0.x(x>0) and 1.x), and remove the redundant
> MPAM_ARCHITECTURE_Vx_x macro definitions.
>
> I've verified locally that everything works fine.
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] arm64: cpufeature: Add support for the MPAM v0.1 architecture version
2026-05-07 14:09 ` [PATCH v2 1/2] arm64: cpufeature: Add support for the MPAM v0.1 architecture version James Morse
@ 2026-05-08 6:04 ` Zeng Heng
0 siblings, 0 replies; 8+ messages in thread
From: Zeng Heng @ 2026-05-08 6:04 UTC (permalink / raw)
To: James Morse, ben.horgan, xry111, catalin.marinas, maz, ardb, yang,
ryan.roberts, kevin.brodsky, reinette.chatre, miko.lenczewski,
will, suzuki.poulose, thuth, james.clark, lpieralisi, broonie,
oupton, anshuman.khandual, yeoreum.yun, leo.yan,
mrigendra.chaubey, fenghuay, ahmed.genidi, mark.rutland
Cc: linux-kernel, linux-arm-kernel, wangkefeng.wang, sunnanyong
Hi James,
On 2026/5/7 22:09, James Morse wrote:
> Hi Zeng,
>
> (sorry for the delay - arm silently blocked your original mail. I've added you
> to the whitelist, we'll see if that actually does anything!)
>
Thank you for the notification and for restoring my email address, as
well as for your patience in reviewing them.
As I'm still concerned that some patch series may have been missed and
not yet reviewed, please allow me to supplement the links here:
1. Introduce Narrow PARTID feature support:
https://lore.kernel.org/all/20260413085405.1166412-1-zengheng4@huawei.com/
2. Several MPAM-SMMU bugfix submissions for the MPAM extras branch:
https://lore.kernel.org/all/20260414032610.1523958-1-zengheng4@huawei.com/
Best regards,
Zeng Heng
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] arm_mpam: Update architecture version check for MPAM MSC
2026-05-08 3:47 ` Zeng Heng
@ 2026-05-08 9:37 ` Ben Horgan
2026-05-08 9:56 ` Ben Horgan
0 siblings, 1 reply; 8+ messages in thread
From: Ben Horgan @ 2026-05-08 9:37 UTC (permalink / raw)
To: Zeng Heng, James Morse, xry111, catalin.marinas, maz, ardb, yang,
ryan.roberts, kevin.brodsky, reinette.chatre, miko.lenczewski,
will, suzuki.poulose, thuth, james.clark, lpieralisi, broonie,
oupton, anshuman.khandual, yeoreum.yun, leo.yan,
mrigendra.chaubey, fenghuay, ahmed.genidi, mark.rutland
Cc: linux-kernel, linux-arm-kernel, wangkefeng.wang
Hi James and Zeng,
On 5/8/26 04:47, Zeng Heng wrote:
> Hi James,
>
> On 2026/5/8 10:26, Zeng Heng wrote:
>
>>> I think its simpler to rule out the unsupported combinations, something like:
>>> | static bool mpam_msc_check_aidr(struct mpam_msc *msc)
>>> | {
>>> | u32 rev;
>>> |
>>> | rev = __mpam_read_reg(msc, MPAMF_AIDR) & MPAMF_AIDR_ARCH_REV;
>>> |
>>> | /*
>>> | * v0.0 and >v2.x aren't supported, but anything else should be backward
>>> | * compatible to v0.1 or v1.0.
>>> | */
>>> | if (!rev)
>>> | return false;
>>> | if (rev & MPAMF_AIDR_ARCH_MAJOR_REV > MPAM_ARCHITECTURE_V1)
>>> | return false;
>>> |
>
> Oops, after more complete version number testing, I found there's an
> operator precedence issue here. The correct fix is:
>
> if ((rev & MPAMF_AIDR_ARCH_MAJOR_REV) > MPAM_ARCHITECTURE_V1)
> return false;
>
> Note that '>' has higher precedence than '&'.
Isn't it better to use FIELD_GET()? We could also avoid creating MPAMF_AIDR_ARCH_REV and use MPAMF_AIDR_ARCH_MAJOR_REV
and MPAMF_AIDR_ARCH_MINOR_REV directly to stop splitting the logic over two files. mpam_msc_check_aidr() could become:
static bool mpam_msc_check_aidr(struct mpam_msc *msc)
{
u32 aidr = __mpam_read_reg(msc, MPAMF_AIDR);
u32 major = FIELD_GET(MPAMF_AIDR_ARCH_MAJOR_REV, aidr);
u32 minor = FIELD_GET(MPAMF_AIDR_ARCH_MINOR_REV, aidr);
/*
* v0.0 and >v2.x aren't supported, but anything else should be backward
* compatible to v0.1 or v1.0.
*/
if (!major && !minor)
return false;
if (major > MPAM_ARCHITECTURE_V1)
return false;
return true;
}
Thanks,
Ben
>
>
> With this fix included:
> Tested-by: Zeng Heng <zengheng4@huawei.com>
>
>
>>> | return true;
>>> | }
>>>
>>>> + if (!mpam_msc_check_aidr(msc)) {
>>>> + dev_err_once(dev, "MSC does not match MPAM architecture\n");
>>>> return -EIO;
>>>> }
>>>
>>> I'd like to keep the 'v1.x' in this message - this should help folk with old stable
>>> kernels running on new hardware work out why the feature isn't available.
>>> (assuming they have some documentation that says v2.0 in it!)
>>>
>>> I've rebased this with the above changes, which I'll post shortly for fixes.
>>>
>>>
>>
>> Agreed. Keep the backward compatibility extension for versions
>> (compatible with v0.x(x>0) and 1.x), and remove the redundant
>> MPAM_ARCHITECTURE_Vx_x macro definitions.
>>
>> I've verified locally that everything works fine.
>>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] arm_mpam: Update architecture version check for MPAM MSC
2026-05-08 9:37 ` Ben Horgan
@ 2026-05-08 9:56 ` Ben Horgan
2026-05-08 16:07 ` James Morse
0 siblings, 1 reply; 8+ messages in thread
From: Ben Horgan @ 2026-05-08 9:56 UTC (permalink / raw)
To: Zeng Heng, James Morse, xry111, catalin.marinas, maz, ardb, yang,
ryan.roberts, kevin.brodsky, reinette.chatre, miko.lenczewski,
will, suzuki.poulose, thuth, james.clark, lpieralisi, broonie,
oupton, anshuman.khandual, yeoreum.yun, leo.yan,
mrigendra.chaubey, fenghuay, ahmed.genidi, mark.rutland
Cc: linux-kernel, linux-arm-kernel, wangkefeng.wang
Hi again James and Zeng,
On 5/8/26 10:37, Ben Horgan wrote:
> Hi James and Zeng,
>
> On 5/8/26 04:47, Zeng Heng wrote:
>> Hi James,
>>
>> On 2026/5/8 10:26, Zeng Heng wrote:
>>
>>>> I think its simpler to rule out the unsupported combinations, something like:
>>>> | static bool mpam_msc_check_aidr(struct mpam_msc *msc)
>>>> | {
>>>> | u32 rev;
>>>> |
>>>> | rev = __mpam_read_reg(msc, MPAMF_AIDR) & MPAMF_AIDR_ARCH_REV;
>>>> |
>>>> | /*
>>>> | * v0.0 and >v2.x aren't supported, but anything else should be backward
>>>> | * compatible to v0.1 or v1.0.
>>>> | */
>>>> | if (!rev)
>>>> | return false;
>>>> | if (rev & MPAMF_AIDR_ARCH_MAJOR_REV > MPAM_ARCHITECTURE_V1)
>>>> | return false;
>>>> |
>>
>> Oops, after more complete version number testing, I found there's an
>> operator precedence issue here. The correct fix is:
>>
>> if ((rev & MPAMF_AIDR_ARCH_MAJOR_REV) > MPAM_ARCHITECTURE_V1)
>> return false;
>>
>> Note that '>' has higher precedence than '&'.
>
> Isn't it better to use FIELD_GET()? We could also avoid creating MPAMF_AIDR_ARCH_REV and use MPAMF_AIDR_ARCH_MAJOR_REV
> and MPAMF_AIDR_ARCH_MINOR_REV directly to stop splitting the logic over two files. mpam_msc_check_aidr() could become:
>
> static bool mpam_msc_check_aidr(struct mpam_msc *msc)
> {
> u32 aidr = __mpam_read_reg(msc, MPAMF_AIDR);
> u32 major = FIELD_GET(MPAMF_AIDR_ARCH_MAJOR_REV, aidr);
> u32 minor = FIELD_GET(MPAMF_AIDR_ARCH_MINOR_REV, aidr);
>
> /*
> * v0.0 and >v2.x aren't supported, but anything else should be backward
> * compatible to v0.1 or v1.0.
> */
> if (!major && !minor)
> return false;
> if (major > MPAM_ARCHITECTURE_V1)
This isn't correct. I missed that MPAM_ARCHITECTURE_V1 is 0x10 (which matches MPAMF_AIDR_ARCH_REV) and not 1.
Thanks,
Ben
> return false;
>
> return true;
> }
>
>
> Thanks,
>
> Ben
>
>>
>>
>> With this fix included:
>> Tested-by: Zeng Heng <zengheng4@huawei.com>
>>
>>
>>>> | return true;
>>>> | }
>>>>
>>>>> + if (!mpam_msc_check_aidr(msc)) {
>>>>> + dev_err_once(dev, "MSC does not match MPAM architecture\n");
>>>>> return -EIO;
>>>>> }
>>>>
>>>> I'd like to keep the 'v1.x' in this message - this should help folk with old stable
>>>> kernels running on new hardware work out why the feature isn't available.
>>>> (assuming they have some documentation that says v2.0 in it!)
>>>>
>>>> I've rebased this with the above changes, which I'll post shortly for fixes.
>>>>
>>>>
>>>
>>> Agreed. Keep the backward compatibility extension for versions
>>> (compatible with v0.x(x>0) and 1.x), and remove the redundant
>>> MPAM_ARCHITECTURE_Vx_x macro definitions.
>>>
>>> I've verified locally that everything works fine.
>>>
>>
>>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] arm_mpam: Update architecture version check for MPAM MSC
2026-05-08 9:56 ` Ben Horgan
@ 2026-05-08 16:07 ` James Morse
0 siblings, 0 replies; 8+ messages in thread
From: James Morse @ 2026-05-08 16:07 UTC (permalink / raw)
To: Ben Horgan, Zeng Heng, xry111, catalin.marinas, maz, ardb, yang,
ryan.roberts, kevin.brodsky, reinette.chatre, miko.lenczewski,
will, suzuki.poulose, thuth, james.clark, lpieralisi, broonie,
oupton, anshuman.khandual, yeoreum.yun, leo.yan,
mrigendra.chaubey, fenghuay, ahmed.genidi, mark.rutland
Cc: linux-kernel, linux-arm-kernel, wangkefeng.wang
Hi Ben,
On 08/05/2026 10:56, Ben Horgan wrote:
> On 5/8/26 10:37, Ben Horgan wrote:
>> On 5/8/26 04:47, Zeng Heng wrote:
>>> On 2026/5/8 10:26, Zeng Heng wrote:
>>>
>>>>> I think its simpler to rule out the unsupported combinations, something like:
>>>>> | static bool mpam_msc_check_aidr(struct mpam_msc *msc)
>>>>> | {
>>>>> | u32 rev;
>>>>> |
>>>>> | rev = __mpam_read_reg(msc, MPAMF_AIDR) & MPAMF_AIDR_ARCH_REV;
>>>>> |
>>>>> | /*
>>>>> | * v0.0 and >v2.x aren't supported, but anything else should be backward
>>>>> | * compatible to v0.1 or v1.0.
>>>>> | */
>>>>> | if (!rev)
>>>>> | return false;
>>>>> | if (rev & MPAMF_AIDR_ARCH_MAJOR_REV > MPAM_ARCHITECTURE_V1)
>>>>> | return false;
>>>>> |
>>>
>>> Oops, after more complete version number testing, I found there's an
>>> operator precedence issue here. The correct fix is:
>>>
>>> if ((rev & MPAMF_AIDR_ARCH_MAJOR_REV) > MPAM_ARCHITECTURE_V1)
>>> return false;
>>>
>>> Note that '>' has higher precedence than '&'.
(yeah, the compiler winged at me. I should have said I didn't even build the hunk above
before suggesting it!)
>> Isn't it better to use FIELD_GET()?
Always!
>> We could also avoid creating MPAMF_AIDR_ARCH_REV and use MPAMF_AIDR_ARCH_MAJOR_REV
>> and MPAMF_AIDR_ARCH_MINOR_REV directly to stop splitting the logic over two files. mpam_msc_check_aidr() could become:
>>
>> static bool mpam_msc_check_aidr(struct mpam_msc *msc)
>> {
>> u32 aidr = __mpam_read_reg(msc, MPAMF_AIDR);
>> u32 major = FIELD_GET(MPAMF_AIDR_ARCH_MAJOR_REV, aidr);
>> u32 minor = FIELD_GET(MPAMF_AIDR_ARCH_MINOR_REV, aidr);
>>
>> /*
>> * v0.0 and >v2.x aren't supported, but anything else should be backward
>> * compatible to v0.1 or v1.0.
>> */
>> if (!major && !minor)
>> return false;
>> if (major > MPAM_ARCHITECTURE_V1)
>
> This isn't correct. I missed that MPAM_ARCHITECTURE_V1 is 0x10 (which matches MPAMF_AIDR_ARCH_REV) and not 1.
Done locally. Thanks!
I also ran over two bugs with v0.1 MSC causing mpam_disable() to be called before
mpam_enable().
Thanks,
James
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-05-08 16:07 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260203095406.6437-1-zengheng4@huawei.com>
[not found] ` <20260203095406.6437-2-zengheng4@huawei.com>
2026-05-07 14:09 ` [PATCH v2 1/2] arm64: cpufeature: Add support for the MPAM v0.1 architecture version James Morse
2026-05-08 6:04 ` Zeng Heng
[not found] ` <20260203095406.6437-3-zengheng4@huawei.com>
2026-05-07 16:03 ` [PATCH v2 2/2] arm_mpam: Update architecture version check for MPAM MSC James Morse
2026-05-08 2:26 ` Zeng Heng
2026-05-08 3:47 ` Zeng Heng
2026-05-08 9:37 ` Ben Horgan
2026-05-08 9:56 ` Ben Horgan
2026-05-08 16:07 ` James Morse
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox