Re: [PATCH v4 01/10] x86/vmx: add Intel PT MSR definitions

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 30/06/2020 13:33, Michał Leszczyński wrote:
> diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
> index b328a47ed8..0203029be9 100644
> --- a/xen/include/asm-x86/msr-index.h
> +++ b/xen/include/asm-x86/msr-index.h
> @@ -69,6 +69,43 @@
>  #define MSR_MCU_OPT_CTRL                    0x00000123
>  #define  MCU_OPT_CTRL_RNGDS_MITG_DIS        (_AC(1, ULL) <<  0)
>  
> +/* Intel PT MSRs */
> +#define MSR_RTIT_OUTPUT_BASE                0x00000560
> +
> +#define MSR_RTIT_OUTPUT_MASK                0x00000561
> +
> +#define MSR_RTIT_CTL                        0x00000570
> +#define  RTIT_CTL_TRACEEN                    (_AC(1, ULL) <<  0)
> +#define  RTIT_CTL_CYCEN                      (_AC(1, ULL) <<  1)

In addition to what Jan has said, please can we be consistent with an
underscore (or not) before EN.  Preferably with, so these would become
TRACE_EN and CYC_EN.

That said, there are a lot of bit definitions which aren't used at all. 
IMO, it would be better to introduce defines when you use them.

Thanks,

~Andrew