Re: [PATCH v1 7/9] x86/resctrl: Assign HW RMIDs to CPUs for soft RMID

[Reinette Chatre <reinette.chatre@intel.com>]

Hi Peter,

On 5/16/2023 7:49 AM, Peter Newman wrote:
> On Thu, May 11, 2023 at 11:40 PM Reinette Chatre
> <reinette.chatre@intel.com> wrote:
>> On 4/21/2023 7:17 AM, Peter Newman wrote:
>>> +     rmid = 0;
>>> +     for_each_cpu(i, &l3ci->shared_cpu_map) {
>>> +             if (i == cpu)
>>> +                     break;
>>> +             rmid++;
>>> +     }
>>> +
>>> +     return rmid;
>>> +}
>>
>> I do not see any impact to the (soft) RMIDs that can be assigned to monitor
>> groups, yet from what I understand a generic "RMID" is used as index to MBM state.
>> Is this correct? A hardware RMID and software RMID would thus share the
>> same MBM state. If this is correct I think we need to work on making
>> the boundaries between hard and soft RMID more clear.
> 
> The only RMID-indexed state used by soft RMIDs right now is
> mbm_state::soft_rmid_bytes. The other aspect of the boundary is
> ensuring that nothing will access the hard RMID-specific state for a
> soft RMID.
> 
> The remainder of the mbm_state is only accessed by the software
> controller, which you suggested that I disable.
> 
> The arch_mbm_state is accessed only through resctrl_arch_rmid_read()
> and resctrl_arch_reset_rmid(), which are called by __mon_event_count()
> or the limbo handler.
> 
> __mon_event_count() is aware of soft RMIDs, so I would just need to
> ensure the software controller is disabled and never put any RMIDs on
> the limbo list. To be safe, I can also add
> WARN_ON_ONCE(rdt_mon_soft_rmid) to the rmid-indexing of the mbm_state
> arrays in the software controller and before the
> resctrl_arch_rmid_read() call in the limbo handler to catch if they're
> ever using soft RMIDs.

I understand and trust that you can ensure that this implementation is
done safely. Please also consider how future changes to resctrl may stumble
if there are not clear boundaries. You may be able to "ensure the software
controller is disabled and never put any RMIDs on the limbo list", but
consider if these rules will be clear to somebody who comes along in a year
or more.

Documenting the data structures with these unique usages will help. 
Specific accessors can sometimes be useful to make it obvious in which state
the data is being accessed and what data can be accessed. Using WARN
as you suggest is a useful tool.

Reinette