diff mbox series

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

Message ID 1618375222-9283-1-git-send-email-igor.druzhinin@citrix.com (mailing list archive)
State Superseded
Headers show
Series [v4,1/2] x86/vtx: add LBR_SELECT to the list of LBR MSRs | expand

Commit Message

Igor Druzhinin April 14, 2021, 4:40 a.m. UTC
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(-)

Comments

Jan Beulich April 14, 2021, 11:41 a.m. UTC | #1
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
Igor Druzhinin April 14, 2021, 11:13 p.m. UTC | #2
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
Jan Beulich April 15, 2021, 9:01 a.m. UTC | #3
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
diff mbox series

Patch

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