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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 16 May 2023 16:49:10 +0200
From:   Peter Newman <peternewman@...gle.com>
To:     Reinette Chatre <reinette.chatre@...el.com>
Cc:     Fenghua Yu <fenghua.yu@...el.com>, Babu Moger <babu.moger@....com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
        "H. Peter Anvin" <hpa@...or.com>,
        Stephane Eranian <eranian@...gle.com>,
        James Morse <james.morse@....com>,
        linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH v1 7/9] x86/resctrl: Assign HW RMIDs to CPUs for soft RMID

Hi Reinette,

On Thu, May 11, 2023 at 11:40 PM Reinette Chatre
<reinette.chatre@...el.com> 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