Re: [PATCH] x86/resctrl: Move MSR defines into msr-index.h

[Reinette Chatre <reinette.chatre@intel.com>]

Hi Boris,

On 11/6/2022 1:29 PM, Borislav Petkov wrote:
> From: Borislav Petkov <bp@suse.de>
> 
> msr-index.h should contain all MSRs for easier grepping for MSR numbers
> when dealing with unchecked MSR access warnings, for example.
> 
> Move the resctrl ones. Prefix IA32_PQR_ASSOC with "MSR_" while at it.
> 
> No functional changes.
> 
> Signed-off-by: Borislav Petkov <bp@suse.de>
> ---
>  arch/x86/include/asm/msr-index.h          | 22 ++++++++++++++++------
>  arch/x86/include/asm/resctrl.h            |  8 +++-----
>  arch/x86/kernel/cpu/resctrl/core.c        |  2 +-
>  arch/x86/kernel/cpu/resctrl/internal.h    | 10 +---------
>  arch/x86/kernel/cpu/resctrl/pseudo_lock.c |  4 ++--
>  5 files changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index a3eb4d3e70b8..c29cf6d1935b 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -4,12 +4,7 @@
>  
>  #include <linux/bits.h>
>  
> -/*
> - * CPU model specific register (MSR) numbers.
> - *
> - * Do not add new entries to this file unless the definitions are shared
> - * between multiple compilation units.
> - */
> +/* CPU model specific register (MSR) numbers. */
>  

Not an objection, just an observation: the above is a significant
change for a patch with the x86/resctrl prefix.

>  /* x86-64 specific MSRs */
>  #define MSR_EFER		0xc0000080 /* extended feature register */
> @@ -1051,6 +1046,21 @@
>  #define VMX_BASIC_MEM_TYPE_WB	6LLU
>  #define VMX_BASIC_INOUT		0x0040000000000000LLU
>  
> +/* Resctrl MSRs: */
> +/* - Intel: */
> +#define MSR_IA32_L3_QOS_CFG		0xc81
> +#define MSR_IA32_L2_QOS_CFG		0xc82
> +#define MSR_IA32_QM_EVTSEL		0xc8d
> +#define MSR_IA32_QM_CTR			0xc8e
> +#define MSR_IA32_PQR_ASSOC		0xc8f
> +#define MSR_IA32_L3_CBM_BASE		0xc90
> +#define MSR_IA32_L2_CBM_BASE		0xd10
> +#define MSR_IA32_MBA_THRTL_BASE		0xd50
> +
> +
> +/* - AMD: */
> +#define MSR_IA32_MBA_BW_BASE		0xc0000200
> +

Is it important to distinguish between Intel and AMD?
All but one (MSR_IA32_MBA_THRTL_BASE) of the register values
under the "Intel" heading are also used by AMD.

(sidenote: multiple blank lines above)

> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index 5f7128686cfd..4f43da46243d 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -8,15 +8,7 @@
>  #include <linux/fs_context.h>
>  #include <linux/jump_label.h>
>  
> -#define MSR_IA32_L3_QOS_CFG		0xc81
> -#define MSR_IA32_L2_QOS_CFG		0xc82
> -#define MSR_IA32_L3_CBM_BASE		0xc90
> -#define MSR_IA32_L2_CBM_BASE		0xd10
> -#define MSR_IA32_MBA_THRTL_BASE		0xd50
> -#define MSR_IA32_MBA_BW_BASE		0xc0000200
> -
> -#define MSR_IA32_QM_CTR			0x0c8e
> -#define MSR_IA32_QM_EVTSEL		0x0c8d
> +
>  
>  #define L3_QOS_CDP_ENABLE		0x01ULL

This new empty line results in a multiple blank lines
warning in this area.
  
Reinette