linux-kernel - Re: [PATCH] arm_mpam: Try reading again if MPAM instance returns not ready

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] [day] [month] [year] [list]

Message-ID: <ed882938-41da-d7c5-8eb0-968f8d7de131@huawei.com>
Date: Sat, 20 Sep 2025 18:14:06 +0800
From: Zeng Heng <zengheng4@...wei.com>
To: James Morse <james.morse@....com>
CC: <amitsinght@...vell.com>, <baisheng.gao@...soc.com>,
	<baolin.wang@...ux.alibaba.com>, <bobo.shaobowang@...wei.com>,
	<carl@...amperecomputing.com>, <catalin.marinas@....com>, <dakr@...nel.org>,
	<dave.martin@....com>, <david@...hat.com>, <dfustini@...libre.com>,
	<fenghuay@...dia.com>, <gregkh@...uxfoundation.org>, <guohanjun@...wei.com>,
	<jonathan.cameron@...wei.com>, <kobak@...dia.com>, <lcherian@...vell.com>,
	<lenb@...nel.org>, <linux-acpi@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
	<lpieralisi@...nel.org>, <peternewman@...gle.com>, <quic_jiles@...cinc.com>,
	<rafael@...nel.org>, <robh@...nel.org>, <rohit.mathew@....com>,
	<scott@...amperecomputing.com>, <sdonthineni@...dia.com>,
	<sudeep.holla@....com>, <tan.shaopeng@...itsu.com>, <will@...nel.org>,
	<xhao@...ux.alibaba.com>
Subject: Re: [PATCH] arm_mpam: Try reading again if MPAM instance returns not
 ready



On 2025/9/20 0:11, James Morse wrote:
> Hi Zeng,
> 
> On 16/09/2025 14:17, Zeng Heng wrote:
>> After updating the monitor configuration, the first read of the monitoring
>> result requires waiting for the "not ready" duration before an effective
>> value can be obtained.
> 
> May need to wait - some platforms need to do this, some don't.
> Yours is the first I've heard of that does this!
> 

I'm afraid similar platforms do exist. As long as one component has more
than one MSC, after first updating the component’s monitor every MSC
instance needs to wait for MAX_NRDY_USEC us before reading the monitor
result.

In fact, most platforms don’t have nearly as many performance monitors
as PARTIDs, so the monitors often have to be time-shared, which making
the problem even more pronounced.

> 
>> Because a component consists of multiple MPAM instances, after updating the
>> configuration of each instance, should wait for the "not ready" period of
>> per single instance before the valid monitoring value can be obtained, not
>> just wait for once interval per component.
> 
> I'm really trying to avoid that ... if you have ~200 MSC pretending to be one thing, you'd
> wait 200x the maximum period. On systems with CMN, the number of MSC scales with the
> number of CPUs, so 200x isn't totally crazy.
> > I think the real problem here is the driver doesn't go on to 
reconfigure MSC-2 if MSC-1
> returned not-ready, meaning the "I'll only wait once" logic kicks in and returns not-ready
> to the user. (which is presumably what you're seeing?)

Yes, exactly.

> 
> Does this solve your problem?:
> -----------------%<-----------------
> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index 404bd4c1fd5e..2f39d0339349 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
> @@ -1395,7 +1395,7 @@ static void __ris_msmon_read(void *arg)
> 
>   static int _msmon_read(struct mpam_component *comp, struct mon_read *arg)
>   {
> -       int err, idx;
> +       int err, any_err = 0, idx;
>          struct mpam_msc *msc;
>          struct mpam_vmsc *vmsc;
>          struct mpam_msc_ris *ris;
> @@ -1412,15 +1412,19 @@ static int _msmon_read(struct mpam_component *comp, stru
> ct mon_read *arg)
>                                                      true);
>                          if (!err && arg->err)
>                                  err = arg->err;
> +
> +                       /*
> +                        * Save one error to be returned to the caller, but
> +                        * keep reading counters so that the get reprogrammed.
> +                        * On platforms with NRDY this lets us wait once.
> +                        */
>                          if (err)
> -                               break;
> +                               any_err = err;
>                  }
> -               if (err)
> -                       break;
>          }
>          srcu_read_unlock(&mpam_srcu, idx);
> 
> -       return err;
> +       return any_err;
>   }
> 
>   int mpam_msmon_read(struct mpam_component *comp, struct mon_cfg *ctx,
> -----------------%<-----------------
> 

I agree with this modification: Reconfigure all MSCs first, then if any
of them returns EBUSY, wait just once for MAX_NRDY_USEC and re-read
monitor result, this guarantees that the monitor result is valid.



Thanks,
Zeng Heng