Re: [PATCH v4 1/2] x86/vtx: add LBR_SELECT to the list of LBR MSRs

[Igor Druzhinin <igor.druzhinin@xxxxxxxxxx>]

On 14/04/2021 12:41, Jan Beulich wrote:
> On 14.04.2021 06:40, Igor Druzhinin wrote:
> > --- a/xen/arch/x86/hvm/vmx/vmx.c
> > +++ b/xen/arch/x86/hvm/vmx/vmx.c
> > @@ -2915,14 +2915,16 @@ static const struct lbr_info {
> >   }, nh_lbr[] = {
> >       { MSR_IA32_LASTINTFROMIP,       1 },
> >       { MSR_IA32_LASTINTTOIP,         1 },
> > -    { MSR_C2_LASTBRANCH_TOS,        1 },
> > +    { MSR_NHL_LBR_SELECT,           1 },
> > +    { MSR_NHL_LASTBRANCH_TOS,       1 },
> >       { MSR_P4_LASTBRANCH_0_FROM_LIP, NUM_MSR_P4_LASTBRANCH_FROM_TO },
> >       { MSR_P4_LASTBRANCH_0_TO_LIP,   NUM_MSR_P4_LASTBRANCH_FROM_TO },
> >       { 0, 0 }
> >   }, sk_lbr[] = {
> >       { MSR_IA32_LASTINTFROMIP,       1 },
> >       { MSR_IA32_LASTINTTOIP,         1 },
> > -    { MSR_SKL_LASTBRANCH_TOS,       1 },
> > +    { MSR_NHL_LBR_SELECT,           1 },
> > +    { MSR_NHL_LASTBRANCH_TOS,       1 },
> >       { MSR_SKL_LASTBRANCH_0_FROM_IP, NUM_MSR_SKL_LASTBRANCH },
> >       { MSR_SKL_LASTBRANCH_0_TO_IP,   NUM_MSR_SKL_LASTBRANCH },
> >       { MSR_SKL_LASTBRANCH_0_INFO,    NUM_MSR_SKL_LASTBRANCH },
> > @@ -2937,6 +2939,7 @@ static const struct lbr_info {
> >   }, gm_lbr[] = {
> >       { MSR_IA32_LASTINTFROMIP,       1 },
> >       { MSR_IA32_LASTINTTOIP,         1 },
> > +    { MSR_GM_LBR_SELECT,            1 },
> What about Xeon Phi, Silvermont, and Airmont?
Yes, you're right - forgot about those. Will need to shuffle arrays a 
little.
> > --- a/xen/include/asm-x86/msr-index.h
> > +++ b/xen/include/asm-x86/msr-index.h
> > @@ -606,14 +606,18 @@
> >   #define NUM_MSR_C2_LASTBRANCH_FROM_TO 4
> >   #define NUM_MSR_ATOM_LASTBRANCH_FROM_TO       8
> >   
> > +/* Nehalem (and newer) last-branch recording */
> > +#define MSR_NHL_LBR_SELECT             0x000001c8
> > +#define MSR_NHL_LASTBRANCH_TOS         0x000001c9
> > +
> >   /* Skylake (and newer) last-branch recording */
> > -#define MSR_SKL_LASTBRANCH_TOS         0x000001c9
> >   #define MSR_SKL_LASTBRANCH_0_FROM_IP  0x00000680
> >   #define MSR_SKL_LASTBRANCH_0_TO_IP    0x000006c0
> >   #define MSR_SKL_LASTBRANCH_0_INFO     0x00000dc0
> >   #define NUM_MSR_SKL_LASTBRANCH                32
> >   
> >   /* Goldmont last-branch recording */
> > +#define MSR_GM_LBR_SELECT              0x000001c8
> >   #define MSR_GM_LASTBRANCH_TOS         0x000001c9
> Wouldn't it make sense to also re-use the NHL constants, like you
> do for Skylake?
I didn't really see GM to be derived from NHL so decided to split those. 
Looks cleaner to me that way otherwise might be a little confusing to 
use NHL constants in GM definitions. Given the change above - I will 
have to reshuffle those anyway in v5.
Igor