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

[Igor Druzhinin]

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