On Wed, Apr 29, 2020 at 03:07:09PM -0700, Yu-cheng Yu wrote:
> diff --git a/arch/x86/include/asm/msr-index.h 
> b/arch/x86/include/asm/msr-index.h
> index 12c9684d59ba..47f603729543 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -885,4 +885,22 @@
>  #define MSR_VM_IGNNE                    0xc0010115
>  #define MSR_VM_HSAVE_PA                 0xc0010117
>  
> +/* Control-flow Enforcement Technology MSRs */
> +#define MSR_IA32_U_CET               0x6a0 /* user mode cet setting */
> +#define MSR_IA32_S_CET               0x6a2 /* kernel mode cet setting */
> +#define MSR_IA32_PL0_SSP     0x6a4 /* kernel shstk pointer */
> +#define MSR_IA32_PL1_SSP     0x6a5 /* ring-1 shstk pointer */
> +#define MSR_IA32_PL2_SSP     0x6a6 /* ring-2 shstk pointer */
> +#define MSR_IA32_PL3_SSP     0x6a7 /* user shstk pointer */
> +#define MSR_IA32_INT_SSP_TAB 0x6a8 /* exception shstk table */
> +
> +/* MSR_IA32_U_CET and MSR_IA32_S_CET bits */
> +#define MSR_IA32_CET_SHSTK_EN                0x0000000000000001ULL

Can we drop the MSR_IA32 prefix for the individual bits?  Mostly to yield
shorter line lengths, but also because it's more or less redundant info,
and in some ways unhelpful as it's hard to quickly differentiate between
"this is an MSR index" and "this is a bit/mask for an MSR".

My vote would also be to use BIT() or BIT_ULL().  The SDM defines the flags
by their (decimal) bit number.  Manually converting the bits to masks makes
it difficult to check for correctness.

E.g.

#define CET_SHSTK_EN            BIT(0)
#define CET_WRSS_EN             BIT(1)
#define CET_ENDBR_EN            BIT(2)
#define CET_LEG_IW_EN           BIT(3)
#define CET_NO_TRACK_EN         BIT(4)
#define CET_WAIT_ENDBR          BIT(5)

> +#define MSR_IA32_CET_WRSS_EN         0x0000000000000002ULL
> +#define MSR_IA32_CET_ENDBR_EN                0x0000000000000004ULL
> +#define MSR_IA32_CET_LEG_IW_EN               0x0000000000000008ULL
> +#define MSR_IA32_CET_NO_TRACK_EN     0x0000000000000010ULL
> +#define MSR_IA32_CET_WAIT_ENDBR      0x00000000000000800UL
> +#define MSR_IA32_CET_BITMAP_MASK     0xfffffffffffff000ULL

This particular define, the so called BITMAP_MASK, is no longer used in the
IBT series.  IMO it'd be better off dropping this mask as it's not clear
from the name that this is really nothing more than a mask for a virtual
address, e.g. at first glance (for someone without CET knowledge) it looks
like bits 63:12 hold a bitmap as opposed to holding a pointer to a bitmap.

Reply via email to