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

[Michał Leszczyński <michal.leszczynski@xxxxxxx>]

----- 30 cze 2020 o 20:03, Tamas K Lengyel tamas.k.lengyel@xxxxxxxxx napisał(a):

> 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


+1 for keeping the unused #defines, this is a helpful piece of knowledge
which speeds up further patch development. It doesn't affect the compilation
nor runtime time and it doesn't occupy too much space in the code so I would
opt for keep it.

I will rebase this series onto latest master within patch v5. The remaining
patches in this series are not affected and still could be reviewed,
so I will wait a few days before posting the new version.


Best regards,
Michał Leszczyński
CERT Polska