Hi Reinette,
On Thu, May 11, 2023 at 11:40 PM Reinette Chatre
<reinette.chatre@xxxxxxxxx> wrote:
> On 4/21/2023 7:17 AM, Peter Newman wrote:
> > + /* Locate the cacheinfo for this CPU's L3 cache. */
> > + for (i = 0; i < ci->num_leaves; i++) {
> > + if (ci->info_list[i].level == 3 &&
> > + (ci->info_list[i].attributes & CACHE_ID)) {
> > + l3ci = &ci->info_list[i];
> > + break;
> > + }
> > + }
> > + WARN_ON(!l3ci);
> > +
> > + if (!l3ci)
> > + return 0;
>
> You can use "if (WARN_ON(..))"
Thanks, I'll look for the other changes in the series which would
benefit from this.
> > + 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.
-Peter
>
> > +
> > static void clear_closid_rmid(int cpu)
> > {
> > struct resctrl_pqr_state *state = this_cpu_ptr(&pqr_state);
> > @@ -604,7 +636,12 @@ static void clear_closid_rmid(int cpu)
> > state->default_rmid = 0;
> > state->cur_closid = 0;
> > state->cur_rmid = 0;
> > - wrmsr(MSR_IA32_PQR_ASSOC, 0, 0);
> > + state->hw_rmid = 0;
> > +
> > + if (static_branch_likely(&rdt_soft_rmid_enable_key))
> > + state->hw_rmid = determine_hw_rmid_for_cpu(cpu);
> > +
> > + wrmsr(MSR_IA32_PQR_ASSOC, state->hw_rmid, 0);
> > }
> >
> > static int resctrl_online_cpu(unsigned int cpu)
>
> Reinette
