[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8d6e9ff2-2279-475a-8135-304dac6c0920@arm.com>
Date: Wed, 10 Sep 2025 20:32:49 +0100
From: James Morse <james.morse@....com>
To: "Shaopeng Tan (Fujitsu)" <tan.shaopeng@...itsu.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Cc: "shameerali.kolothum.thodi@...wei.com"
<shameerali.kolothum.thodi@...wei.com>,
D Scott Phillips OS <scott@...amperecomputing.com>,
"carl@...amperecomputing.com" <carl@...amperecomputing.com>,
"lcherian@...vell.com" <lcherian@...vell.com>,
"bobo.shaobowang@...wei.com" <bobo.shaobowang@...wei.com>,
"baolin.wang@...ux.alibaba.com" <baolin.wang@...ux.alibaba.com>,
Jamie Iles <quic_jiles@...cinc.com>, Xin Hao <xhao@...ux.alibaba.com>,
"peternewman@...gle.com" <peternewman@...gle.com>,
"dfustini@...libre.com" <dfustini@...libre.com>,
"amitsinght@...vell.com" <amitsinght@...vell.com>,
David Hildenbrand <david@...hat.com>, Rex Nie <rex.nie@...uarmicro.com>,
Dave Martin <dave.martin@....com>, Koba Ko <kobak@...dia.com>,
Shanker Donthineni <sdonthineni@...dia.com>,
"fenghuay@...dia.com" <fenghuay@...dia.com>,
"baisheng.gao@...soc.com" <baisheng.gao@...soc.com>,
Jonathan Cameron <jonathan.cameron@...wei.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>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Danilo Krummrich <dakr@...nel.org>, Ben Horgan <ben.horgan@....com>
Subject: Re: [PATCH 12/33] arm_mpam: Add the class and component structures
for ris firmware described
Hi Shaopeng,
On 09/09/2025 08:30, Shaopeng Tan (Fujitsu) wrote:
>> An MSC is a container of resources, each identified by their RIS index.
>> Some RIS are described by firmware to provide their position in the system.
>> Others are discovered when the driver probes the hardware.
>>
>> To configure a resource it needs to be found by its class, e.g. 'L2'.
>> There are two kinds of grouping, a class is a set of components, which are
>> visible to user-space as there are likely to be multiple instances of the L2 cache.
>> (e.g. one per cluster or package)
>>
>> struct mpam_components are a set of struct mpam_vmsc. A vMSC groups the
>> RIS in an MSC that control the same logical piece of hardware. (e.g. L2).
>> This is to allow hardware implementations where two controls are presented as
>> different RIS. Re-combining these RIS allows their feature bits to be or-ed. This
>> structure is not visible outside mpam_devices.c
>>
>> struct mpam_vmsc are then a set of struct mpam_msc_ris, which are not
>> visible as each L2 cache may be composed of individual slices which need to
>> be configured the same as the hardware is not able to distribute the
>> configuration.
>>
>> Add support for creating and destroying these structures.
>>
>> A gfp is passed as the structures may need creating when a new RIS entry is
>> discovered when probing the MSC.
>> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
>> index 71a1fb1a9c75..5baf2a8786fb 100644
>> --- a/drivers/resctrl/mpam_devices.c
>> +++ b/drivers/resctrl/mpam_devices.c
>> @@ -35,11 +34,483 @@
>> +static struct mpam_vmsc *mpam_vmsc_get(struct mpam_component
>> *comp,
>> + struct mpam_msc *msc, bool alloc,
>> + gfp_t gfp)
>> +{
>> + struct mpam_vmsc *vmsc;
>> +
>> + lockdep_assert_held(&mpam_list_lock);
>> +
>> + list_for_each_entry(vmsc, &comp->vmsc, comp_list) {
>> + if (vmsc->msc->id == msc->id)
>> + return vmsc;
>> + }
>> +
>> + if (!alloc)
>> + return ERR_PTR(-ENOENT);
>
> It seems like it is always false here.
> If necessary, why not do this at the beginning of the function?
Because the VMSC may exist - in which case the 'get' function should return it
regardless of whether the caller wants to allocate it if its missing.
I'd anticipated callers like resctrl would want to grab components by things like cache-
id, without allocating them by accident. But that coded ended up just searching the lists
instead. I'll rip this out.
Thanks,
James
Powered by blists - more mailing lists