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

[Peter Newman]

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