linux-kernel - Re: [PATCH v2 21/29] arm_mpam: Probe and reset the rest of the features

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <db5bff88-44d5-49ff-9460-f04e6e878bb1@arm.com>
Date: Fri, 3 Oct 2025 19:05:01 +0100
From: James Morse <james.morse@....com>
To: Jonathan Cameron <jonathan.cameron@...wei.com>
Cc: linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-acpi@...r.kernel.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@...hat.com>, Dave Martin <dave.martin@....com>,
 Koba Ko <kobak@...dia.com>, Shanker Donthineni <sdonthineni@...dia.com>,
 fenghuay@...dia.com, baisheng.gao@...soc.com, Rob Herring <robh@...nel.org>,
 Rohit Mathew <rohit.mathew@....com>, Rafael Wysocki <rafael@...nel.org>,
 Len Brown <lenb@...nel.org>, Lorenzo Pieralisi <lpieralisi@...nel.org>,
 Hanjun Guo <guohanjun@...wei.com>, Sudeep Holla <sudeep.holla@....com>,
 Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Danilo Krummrich <dakr@...nel.org>, Zeng Heng <zengheng4@...wei.com>
Subject: Re: [PATCH v2 21/29] arm_mpam: Probe and reset the rest of the
 features

Hi Jonathan,

On 12/09/2025 14:07, Jonathan Cameron wrote:
> On Wed, 10 Sep 2025 20:43:01 +0000
> James Morse <james.morse@....com> wrote:
> 
>> MPAM supports more features than are going to be exposed to resctrl.
>> For partid other than 0, the reset values of these controls isn't
>> known.
>>
>> Discover the rest of the features so they can be reset to avoid any
>> side effects when resctrl is in use.
>>
>> PARTID narrowing allows MSC/RIS to support less configuration space than
>> is usable. If this feature is found on a class of device we are likely
>> to use, then reduce the partid_max to make it usable. This allows us
>> to map a PARTID to itself.

> A few trivial things inline.
> 
> Reviewed-by: Jonathan Cameron <jonathan.cameron@...wei.com>

Thanks!


>> @@ -667,10 +676,35 @@ static void mpam_ris_hw_probe(struct mpam_msc_ris *ris)
>>  	struct mpam_msc *msc = ris->vmsc->msc;
>>  	struct device *dev = &msc->pdev->dev;
>>  	struct mpam_props *props = &ris->props;
>> +	struct mpam_class *class = ris->vmsc->comp->class;
>>  
>>  	lockdep_assert_held(&msc->probe_lock);
>>  	lockdep_assert_held(&msc->part_sel_lock);
>>  
>> +	/* Cache Capacity Partitioning */
>> +	if (FIELD_GET(MPAMF_IDR_HAS_CCAP_PART, ris->idr)) {
>> +		u32 ccap_features = mpam_read_partsel_reg(msc, CCAP_IDR);
>> +
>> +		props->cmax_wd = FIELD_GET(MPAMF_CCAP_IDR_CMAX_WD, ccap_features);
>> +		if (props->cmax_wd &&
>> +		    FIELD_GET(MPAMF_CCAP_IDR_HAS_CMAX_SOFTLIM, ccap_features))
>> +			mpam_set_feature(mpam_feat_cmax_softlim, props);
>> +
>> +		if (props->cmax_wd &&
>> +		    !FIELD_GET(MPAMF_CCAP_IDR_NO_CMAX, ccap_features))
>> +			mpam_set_feature(mpam_feat_cmax_cmax, props);
>> +
>> +		if (props->cmax_wd &&
>> +		    FIELD_GET(MPAMF_CCAP_IDR_HAS_CMIN, ccap_features))
>> +			mpam_set_feature(mpam_feat_cmax_cmin, props);
>> +
>> +		props->cassoc_wd = FIELD_GET(MPAMF_CCAP_IDR_CASSOC_WD, ccap_features);
>> +

> Trivial but blank line here feels inconsistent with local style. I'd drop it.

Sure,


>> +		if (props->cassoc_wd &&
>> +		    FIELD_GET(MPAMF_CCAP_IDR_HAS_CASSOC, ccap_features))
>> +			mpam_set_feature(mpam_feat_cmax_cassoc, props);
>> +	}
>> +
> 
>> diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h
>> index 17570d9aae9b..326ba9114d70 100644
>> --- a/drivers/resctrl/mpam_internal.h
>> +++ b/drivers/resctrl/mpam_internal.h
>> @@ -136,25 +136,34 @@ static inline void mpam_mon_sel_lock_init(struct mpam_msc *msc)
>>   * When we compact the supported features, we don't care what they are.
>>   * Storing them as a bitmap makes life easy.
>>   */
>> -typedef u16 mpam_features_t;
>> +typedef u32 mpam_features_t;

> This is strengthening my view that this should just be a DECLARE_BITMAP(MPAM_FEATURE_LAST)
> in the appropriate places.

I don't think this list is going to grow much. But sure.

Most stuff can be churned out, but this makes an utter mess of exposing the value to
debugfs.., I do need that exposed as "why didn't resctrl do what I wanted" is a very
common question - and being able to see what the hardware supports really helps.

Looks like exposing the first 'ulong' is the simplest - it kicks the can this creates
down the road. I experimented with u32_array - but its no better.


Thanks,

James