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

[Moger, Babu]

Hi Tony,


On 6/12/2026 10:40 AM, Luck, Tony wrote:
> > > 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).

Ack.

> 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]

Actually, I don’t need these changes anymore—they were carried over from 
a previous version. Thanks for making the updates, though.

Thanks
Babu