[PATCH v3 23/29] arm_mpam: Add mpam_msmon_read() to read monitor value

[Ben Horgan]

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 at nvidia.com>
>> Signed-off-by: James Morse <james.morse at arm.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.

> 
> 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
> 

Thanks,

Ben