RE: [PATCH v3 02/12] x86/resctrl: Add data structures and definitions for PLZA configuration

[Luck, Tony]

> > diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> > index 9dc6b610e4e2..623628d3c643 100644
> > --- a/arch/x86/include/asm/msr-index.h
> > +++ b/arch/x86/include/asm/msr-index.h
> > @@ -1287,10 +1287,17 @@
> >  /* - AMD: */
> >  #define MSR_IA32_MBA_BW_BASE               0xc0000200
> >  #define MSR_IA32_SMBA_BW_BASE              0xc0000280
> > +#define MSR_IA32_PQR_PLZA_ASSOC            0xc00003fc
> >  #define MSR_IA32_L3_QOS_ABMC_CFG   0xc00003fd
> >  #define MSR_IA32_L3_QOS_EXT_CFG            0xc00003ff
> >  #define MSR_IA32_EVT_CFG_BASE              0xc0000400
> >
> > +/* Lower 32 bits of MSR_IA32_PQR_PLZA_ASSOC */
> > +#define RMID_EN                            BIT(31)
> > +/* Upper 32 bits of MSR_IA32_PQR_PLZA_ASSOC */
> > +#define CLOSID_EN                  BIT(15)
> > +#define PLZA_EN                            BIT(31)
> > +
>
> This is unexpected. So far resctrl has only defined the MSR numbers in this file, not
> the individual fields. This seems a legitimate use of msr-index.h but creates inconsistency
> with how the fields of the other resctrl registers are defined. This may be ok so I am
> looking past this for now. Since I am not familiar with this use I am looking at other
> patterns of this and it seems that the register fields are usually defined right after
> the register to make this relationship clear and also use more verbose naming to establish
> this relationship ... I do not think such cryptic names should be used without context
> in such a global scope. Please compare with how other fields are defined at this scope.

There's also patches in flight to treat MSRs as a single "u64" and move away from
the low level implementation detail that the RDMSR/WRMSR instructions split into
upper/lower halves.

All the kernel interfaces are moving to rdmsrq() and wrmsrq() (together with related
functions).

So maybe:

#define PQR_PLZA_RMID_EN        BIT_ULL(31)
#define PQR_PLZA_CLOSID_EN      BIT_ULL(47)
#define PQR_PLZA_PLZA_EN        BIT_ULL(63)

[modify with whatever addition prefix characters seem necessary]

-Tony