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

[Moger, Babu]

Hi Reinette,

On 6/12/2026 12:32 PM, Moger, Babu wrote:
> Hi Reinette,
>
> On 6/11/2026 6:40 PM, Reinette Chatre wrote:
> > Hi Babu,
> >
> > On 4/30/26 4:24 PM, Babu Moger wrote:
> > > Privilege Level Zero Association (PLZA) is configured per logical 
> > > processor
> > > via MSR_IA32_PQR_PLZA_ASSOC (0xc00003fc). Software must program RMID and
> > > CLOSID association fields and their enable bits using the layout defined
> > > for the MSR.
> > >
> > > Define MSR_IA32_PQR_PLZA_ASSOC and the RMID_EN, CLOSID_EN, and 
> > > PLZA_EN bit
> > > masks in asm/msr-index.h. Add union msr_pqr_plza_assoc in arch resctrl
> > > internal.h
> >
> > Above paragraph captures what can be seen from the patch. Please check 
> > entire
> > series for this since many changelogs in this series verbatim 
> > describes the code
> > changes in patch without helping reader understand why those changes 
> > are made.
>
> Sure. Will rewrite the changelog. And will check other patches also.
>
> > > Signed-off-by: Babu Moger <babu.moger@xxxxxxx>
> > > ---
> >
> > > 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.
>
> Sure. Will use the names tony suggested.
> https://lore.kernel.org/lkml/ 
> SJ1PR11MB6083C069F99FAB8A0BEB8518FC182@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
>
> Also will moving the register "MSR_IA32_PQR_PLZA_ASSOC" together with 
> BIT definition. It will break the sorting order. Hope that is not a 
> problem.

Never mind. I don't need the bit definitions anymore. I don't need to 
move the register.

Thanks
Babu