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

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

[Igor Druzhinin]

This MSR exists since Nehalem and is actively used by Linux, for instance,
to improve sampling efficiency.

Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
---
New patch in v4 as suggested by Andrew.
---
 xen/arch/x86/hvm/vmx/vmx.c      | 7 +++++--
 xen/include/asm-x86/msr-index.h | 6 +++++-
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 835b905..5a4ca35 100644
--- 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 },
     { MSR_GM_LASTBRANCH_TOS,        1 },
     { MSR_GM_LASTBRANCH_0_FROM_IP,  NUM_MSR_GM_LASTBRANCH_FROM_TO },
     { MSR_GM_LASTBRANCH_0_TO_IP,    NUM_MSR_GM_LASTBRANCH_FROM_TO },
diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
index 43d26ef..25c4308 100644
--- 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
 #define MSR_GM_LASTBRANCH_0_FROM_IP	0x00000680
 #define MSR_GM_LASTBRANCH_0_TO_IP	0x000006c0
-- 
2.7.4

[PATCH v4 2/2] x86/intel: insert Ice Lake-SP and Ice Lake-D model numbers

[Igor Druzhinin]

LBR, C-state MSRs should correspond to Ice Lake desktop according to
SDM rev. 74 for both models.

Ice Lake-SP is known to expose IF_PSCHANGE_MC_NO in IA32_ARCH_CAPABILITIES MSR
(as advisory tells and Whitley SDP confirms) which means the erratum is fixed
in hardware for that model and therefore it shouldn't be present in
has_if_pschange_mc list. Provisionally assume the same to be the case
for Ice Lake-D while advisory is not yet updated.

Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
---
Changes in v4:
- now based on SDM update
- new LBR (0x1e0)does not seem to be exposed in the docs

Changes in v3:
- Add Ice Lake-D model numbers
- Drop has_if_pschange_mc hunk following Tiger Lake related discussion
---
 xen/arch/x86/acpi/cpu_idle.c | 2 ++
 xen/arch/x86/hvm/vmx/vmx.c   | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/acpi/cpu_idle.c b/xen/arch/x86/acpi/cpu_idle.c
index c092086..d788c8b 100644
--- a/xen/arch/x86/acpi/cpu_idle.c
+++ b/xen/arch/x86/acpi/cpu_idle.c
@@ -181,6 +181,8 @@ static void do_get_hw_residencies(void *arg)
     case 0x55:
     case 0x5E:
     /* Ice Lake */
+    case 0x6A:
+    case 0x6C:
     case 0x7D:
     case 0x7E:
     /* Tiger Lake */
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 5a4ca35..52469ca 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2982,7 +2982,7 @@ static const struct lbr_info *last_branch_msr_get(void)
         /* Goldmont Plus */
         case 0x7a:
         /* Ice Lake */
-        case 0x7d: case 0x7e:
+        case 0x6a: case 0x6c: case 0x7d: case 0x7e:
         /* Tiger Lake */
         case 0x8c: case 0x8d:
         /* Tremont */
-- 
2.7.4

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

[Jan Beulich]

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?

> --- 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?

Jan

Re: [PATCH v4 2/2] x86/intel: insert Ice Lake-SP and Ice Lake-D model numbers

[Jan Beulich]

On 14.04.2021 06:40, Igor Druzhinin wrote:
> LBR, C-state MSRs should correspond to Ice Lake desktop according to
> SDM rev. 74 for both models.
> 
> Ice Lake-SP is known to expose IF_PSCHANGE_MC_NO in IA32_ARCH_CAPABILITIES MSR
> (as advisory tells and Whitley SDP confirms) which means the erratum is fixed
> in hardware for that model and therefore it shouldn't be present in
> has_if_pschange_mc list. Provisionally assume the same to be the case
> for Ice Lake-D while advisory is not yet updated.
> 
> Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

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

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

[Jan Beulich]

On 15.04.2021 01:13, Igor Druzhinin wrote:
> On 14/04/2021 12:41, Jan Beulich wrote:
>> On 14.04.2021 06:40, Igor Druzhinin wrote:
>>> --- 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. 

Hmm, yes - fair argument.

Jan