linux-kernel - RE: [PATCH v3 23/29] arm_mpam: Add mpam_msmon

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:
 <OSZPR01MB8798EEEF42B16AD8F35DE8EE8BC3A@OSZPR01MB8798.jpnprd01.prod.outlook.com>
Date: Fri, 7 Nov 2025 05:01:56 +0000
From: "Shaopeng Tan (Fujitsu)" <tan.shaopeng@...itsu.com>
To: 'Ben Horgan' <ben.horgan@....com>, 'James Morse' <james.morse@....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>
CC: 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>, 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>, Catalin Marinas
	<catalin.marinas@....com>, Will Deacon <will@...nel.org>, Greg Kroah-Hartman
	<gregkh@...uxfoundation.org>, Danilo Krummrich <dakr@...nel.org>, Jeremy
 Linton <jeremy.linton@....com>, Gavin Shan <gshan@...hat.com>
Subject: RE: [PATCH v3 23/29] arm_mpam: Add mpam_msmon_read() to read monitor
 value

Hi Ben,

> Hi Shaopeng,
> 
> On 11/5/25 08:32, Shaopeng Tan (Fujitsu) wrote:
> > Hello James,
> >
> >> Reading a monitor involves configuring what you want to monitor, and
> >> reading the value. Components made up of multiple MSC may need values
> >> from each MSC. MSCs may take time to configure, returning 'not ready'.
> >> The maximum 'not ready' time should have been provided by firmware.
> >>
> >> Add mpam_msmon_read() to hide all this. If (one of) the MSC returns
> >> not ready, then wait the full timeout value before trying again.
> >>
> >> CC: Shanker Donthineni <sdonthineni@...dia.com>
> >> Signed-off-by: James Morse <james.morse@....com>
> [...]
> >> +/* Call with MSC lock held */
> >> +static void __ris_msmon_read(void *arg) {
> >> +	u64 now;
> >> +	bool nrdy = false;
> >> +	struct mon_read *m = arg;
> >> +	struct mon_cfg *ctx = m->ctx;
> >> +	struct mpam_msc_ris *ris = m->ris;
> >> +	struct mpam_props *rprops = &ris->props;
> >> +	struct mpam_msc *msc = m->ris->vmsc->msc;
> >> +	u32 mon_sel, ctl_val, flt_val, cur_ctl, cur_flt;
> >> +
> >> +	if (!mpam_mon_sel_lock(msc)) {
> >> +		m->err = -EIO;
> >> +		return;
> >> +	}
> >> +	mon_sel = FIELD_PREP(MSMON_CFG_MON_SEL_MON_SEL,
> >> ctx->mon) |
> >> +		  FIELD_PREP(MSMON_CFG_MON_SEL_RIS, ris->ris_idx);
> >> +	mpam_write_monsel_reg(msc, CFG_MON_SEL, mon_sel);
> >> +
> >> +	/*
> >> +	 * Read the existing configuration to avoid re-writing the same values.
> >> +	 * This saves waiting for 'nrdy' on subsequent reads.
> >> +	 */
> >> +	read_msmon_ctl_flt_vals(m, &cur_ctl, &cur_flt);
> >> +	clean_msmon_ctl_val(&cur_ctl);
> >> +	gen_msmon_ctl_flt_vals(m, &ctl_val, &flt_val);
> >> +	if (cur_flt != flt_val || cur_ctl != (ctl_val | MSMON_CFG_x_CTL_EN))
> >> +		write_msmon_ctl_flt_vals(m, ctl_val, flt_val);
> >
> > When reading the CSU counter of a different control group, the counter is
> cleared to 0 by the write_msmon_ctl_flt_vals().
> >
> >> +	switch (m->type) {
> >> +	case mpam_feat_msmon_csu:
> >> +		now = mpam_read_monsel_reg(msc, CSU);
> >> +		if (mpam_has_feature(mpam_feat_msmon_csu_hw_nrdy,
> >> rprops))
> >> +			nrdy = now & MSMON___NRDY;
> >> +		break;
> >
> > The first time read the counter(MSMON_CSU), all bits except nrdy are 0.
> 
> 
> I'm trying to understand your problem. Isn't what you are describing the
> intended behaviour of the nrdy bit? It takes some time to get a count of the
> cache utilization so if it's not ready this is set and the driver retries.

I apologize for not explain it correctly.
The key point is that for some SOC chip, if `mpam_feat_msmon_csu_hw_nrdy` is false,
the NRDY bit value of counter(MSMON_CSU) cannot be set to 'nrdy'.
'nrdy' will keep its initial value(false).

> > This means when check the cache monitoring value the first time, cat
> > mon_data/mon_L3_0*/llc_occupancy the result will be 0.
> > From the second time, the result will return to normal.
> 
> This is not expected. On creating a new ctrl_mon group, generating some
> memory traffic and checking the llc_occumpancy I see a non-zero value on my
> setup. Not sure why you wouldn't.

Best regards,
Shaopeng TAN