Re: [PATCH v7 22/24] x86/resctrl: Update assignments on event configuration changes

[Reinette Chatre]

Hi Babu,

On 10/4/24 8:02 AM, Moger, Babu wrote:
> On 10/3/2024 9:17 PM, Reinette Chatre wrote:
>> On 10/3/24 5:51 PM, Moger, Babu wrote:
>>> On 10/2/2024 1:20 PM, Reinette Chatre wrote:
>>>> On 9/27/24 9:22 AM, Moger, Babu wrote:
>>>>> On 9/19/2024 12:45 PM, Reinette Chatre wrote:
>>>>>> On 9/4/24 3:21 PM, Babu Moger wrote:

>>>>> Using the domain bitmap we can figure out which of the counters are assigned in the domain. I can use the hardware help to update the assignment for each counter.  This has to be done via IPI.
>>>>> Something like this.
>>>>>
>>>>> static void rdtgroup_abmc_dom_cfg(void *info)
>>>>> {
>>>>>       union l3_qos_abmc_cfg *abmc_cfg = info;
>>>>>           u32 val = abmc_cfg->bw_type;
>>>>>
>>>>>           /* Get the counter configuration */
>>>>>       wrmsrl(MSR_IA32_L3_QOS_ABMC_CFG, *abmc_cfg);
>>>>>       rdmsrl(MGR_IA32_L3_QOS_ABMC_DSC, *abmc_cfg);
>>>>>
>>>>
>>>> This is not clear to me. I expected MSR_IA32_L3_QOS_ABMC_DSC
>>>> to return the bw_type that was just written to
>>>> MSR_IA32_L3_QOS_ABMC_CFG.
>>>>
>>>> It is also not clear to me how these registers can be
>>>> used without a valid counter ID. I seem to miss
>>>> the context of this call.
>>>
>>> Event configuration changes are domain specific. We have the domain data structure and we have the bitmap(mbm_cntr_map) in rdt_mon_domain. This bitmap tells us which of the counters in the domain are configured. So, we can get the  counter id from this bitmap. Using the counter id, we can query the hardware to get the current configuration by this sequence.
>>>
>>> /* Get the counter configuration */
>>> for (i=0; i< r->mon.num_mbm_cntrs; i++) {
>>>   if (test_bit(i, d->mbm_cntr_map)) {
>>>     abmc_cfg->cntr_id = i;
>>>     abmc_cfg.split.cfg_en = 0;
>>>     wrmsrl(MSR_IA32_L3_QOS_ABMC_CFG, *abmc_cfg);
>>>
>>>     /* Reading L3_QOS_ABMC_DSC returns the configuration of the
>>>      * counter id specified in L3_QOS_ABMC_CFG.cntr_id with RMID(bw_src)
>>>      * and event configuration(bw_type)  Get the counter configuration
>>>      */
>>>     rdmsrl(MGR_IA32_L3_QOS_ABMC_DSC, *abmc_cfg);
>>>
>>
>> Apologies but I do still have the same question as before ... wouldn't
>> MSR_IA32_L3_QOS_ABMC_DSC return the value that was just written to
>> MSR_IA32_L3_QOS_ABMC_CFG? If so, the previous wrmsrl() would set the
>> counter's bw_type to what is set in *abmc_cfg provided as parameter. It
>> thus still seems unclear why reading it back is necessary.
> 
> Yes. It is not clear in the spec.
> 
> QOS_ABMC_DSC is read-only MSR and used only to get the configured counter id information.
> 
> The configuration is only updated when QOS_ABMC_CFG.cfg_en = 1.
> 
> When you write QOS_ABMC_CFG with cntr_id = n, abmc_cfg.split.cfg_en
> = 0 and reading the QOS_ABMC_DSC back will return the configuration
> of cntr_id. Note that when abmc_cfg.split.cfg_en = 0, it will not
> change the counter id configuration. when you read QOS_ABMC_DSC back
> here, we will get bw_type (event config), bw_src (RMID) etc.

ah. I see now. Thank you very much for clarifying.

Reinette