Hi Reinette,
On 7/22/25 18:27, Reinette Chatre wrote:
> Hi Babu,
>
> On 7/22/25 8:51 AM, Moger, Babu wrote:
>> Hi Reinette,
>>
>> On 7/17/25 22:51, Reinette Chatre wrote:
>>> Hi Babu,
>>>
>>> On 7/8/25 3:17 PM, Babu Moger wrote:
>>>> System software can read resctrl event data for a particular resource by
>>>> writing the RMID and Event Identifier (EvtID) to the QM_EVTSEL register
>>>> and then reading the event data from the QM_CTR register.
>>>>
>>>> In ABMC mode, the event data of a specific counter ID can be read by
>>>> setting the following fields in QM_EVTSEL.ExtendedEvtID = 1,
>>>
>>> Seems easier to parse when "fields in" -> "fields:".
>>>
>>
>> Sure.
>>
>>
>>>> QM_EVTSEL.EvtID = L3CacheABMC (=1) and setting [RMID] to the desired
>>>> counter ID. Reading QM_CTR will then return the contents of the specified
>>>> counter ID. The E bit will be set if the counter configuration was invalid,
>>>
>>> Where is "the E bit" defined?
>>
>> QM_CTR MSRS bits
>>
>> Bits Mnemonic Description.
>> 63 E Error on access to counter
>> 62 U Count for this event is currently unavailable
>> 61-CNT_LEN _ Reserved, read as zero
>> CNT_LEN-1:0 CNT Count of monitored resource or event
>>
>>
>> The bit 63 is "E bit" -> RMID_VAL_ERROR
>> the bit 62 is "U bit" -> RMID_VAL_UNAVAIL
>>
>> How about?
>>
>> The RMID_VAL_ERROR bit will be set if the counter configuration
>> was invalid, or if an invalid counter ID was set in the QM_EVTSEL[RMID] field
>
> It is not clear to me why the comments only mention one of the error bits handled
> in the code.
> The motivation for hardware counters is that reading of RMID may return
> "Unavailable". Does this imply that reading a hardware counter should
> never return "Unavailable"? Why is this error handled as possible when
> reading a counter though?
Checked again here. Yes. RMID_VAL_UNAVAIL is also a possibility. I should
have added the text before.
>
>
>>>> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
>>>> index a230d98e9d73..026c2e2d19d3 100644
>>>> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
>>>> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
>>>> @@ -259,6 +259,76 @@ int resctrl_arch_rmid_read(struct rdt_resource *r, struct rdt_mon_domain *d,
>>>> return 0;
>>>> }
>>>>
>>>> +static int __cntr_id_read(u32 cntr_id, u64 *val)
>>>> +{
>>>> + u64 msr_val;
>>>> +
>>>> + /*
>>>> + * QM_EVTSEL Register definition:
>>>> + * =======================================================
>>>> + * Bits Mnemonic Description
>>>> + * =======================================================
>>>> + * 63:44 -- Reserved
>>>> + * 43:32 RMID Resource Monitoring Identifier
>>>> + * 31 ExtEvtID Extended Event Identifier
>>>> + * 30:8 -- Reserved
>>>> + * 7:0 EvtID Event Identifier
>>>> + * =======================================================
>>>> + * The contents of a specific counter can be read by setting the
>>>> + * following fields in QM_EVTSEL.ExtendedEvtID(=1) and
>>>> + * QM_EVTSEL.EvtID = L3CacheABMC (=1) and setting [RMID] to the
>>>> + * desired counter ID. Reading QM_CTR will then return the
>>>> + * contents of the specified counter. The E bit will be set if the
>>>> + * counter configuration was invalid, or if an invalid counter ID
>>>> + * was set in the QM_EVTSEL[RMID] field.
>>>
>>> (same comments as changelog)
>>
>> "The E bit" -> "The RMID_VAL_ERROR bit"
>
> For comments to be accurate, per the handling that follows, RMID_VAL_UNAVAIL
> is also a possibility, or is it not actually possible and just coded to match
> RMID reading?
>
Added the text now.
"The RMID_VAL_UNAVAIL bit will be set if the counter data is not currently
available."
--
Thanks
Babu Moger
