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

[Tamas K Lengyel <tamas.k.lengyel@xxxxxxxxx>]

On Tue, Jun 30, 2020 at 11:39 AM Andrew Cooper
<andrew.cooper3@xxxxxxxxxx> wrote:
>
> 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.

In the past I found it very valuable when this type of plumbing was
already present in Xen instead of me having to go into the SDM to digg
out the magic numbers. So while some of the bits might not be used
right now I also don't see any downside in having them, just in case.

Tamas