[PATCH v3 0/2] x86: KVM: Add missing AMD features

[PATCH v3 0/2] x86: KVM: Add missing AMD features

[Maksim Davydov]

This series adds definition of some missing AMD features in
0x80000008_EBX and 0x80000021_EAX functions. It also gives an opportunity
to expose these features to userspace. 

Related discussion in QEMU:
https://lore.kernel.org/kvm/24462567-e486-4b7f-b869-a1fab48d739c@yandex-team.ru/

v3 -> v2:
* rebased onto the newest master
* AMD_IBPB_RET was removed because all work had been done in 71dd5d5300d2
* renamed "IBRS provides same mode protection" bit

v2 -> v1:
* fixed the bug in the FSRC definition

v2:
https://lore.kernel.org/kvm/20241126094424.943192-1-davydov-max@yandex-team.ru/

v1:
https://lore.kernel.org/kvm/20241113133042.702340-1-davydov-max@yandex-team.ru/


Maksim Davydov (2):
  x86: KVM: Advertise FSRS and FSRC on AMD to userspace
  x86: KVM: Advertise AMD's speculation control features

 arch/x86/include/asm/cpufeatures.h | 5 +++++
 arch/x86/kvm/cpuid.c               | 9 +++++----
 2 files changed, 10 insertions(+), 4 deletions(-)

-- 
2.34.1

[PATCH v3 1/2] x86: KVM: Advertise FSRS and FSRC on AMD to userspace

[Maksim Davydov]

Fast short REP STOSB and fast short CMPSB support on AMD processors are
provided in other CPUID function in comparison with Intel processors:
* FSRS: 10 bit in 0x80000021_EAX
* FSRC: 11 bit in 0x80000021_EAX

AMD bit numbers differ from existing definition of FSRC and
FSRS. So, the new appropriate values have to be added with new names.

It's safe to advertise these features to userspace because they are a part
of CPU model definition and they can't be disabled (as existing Intel
features).

Fixes: 2a4209d6a9cb ("KVM: x86: Advertise fast REP string features inherent to the CPU")
Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
---
 arch/x86/include/asm/cpufeatures.h | 2 ++
 arch/x86/kvm/cpuid.c               | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 17b6590748c0..45f87a026bba 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -460,6 +460,8 @@
 #define X86_FEATURE_NULL_SEL_CLR_BASE	(20*32+ 6) /* Null Selector Clears Base */
 #define X86_FEATURE_AUTOIBRS		(20*32+ 8) /* Automatic IBRS */
 #define X86_FEATURE_NO_SMM_CTL_MSR	(20*32+ 9) /* SMM_CTL MSR is not present */
+#define X86_FEATURE_AMD_FSRS	        (20*32+10) /* AMD Fast short REP STOSB supported */
+#define X86_FEATURE_AMD_FSRC		(20*32+11) /* AMD Fast short REP CMPSB supported */
 
 #define X86_FEATURE_SBPB		(20*32+27) /* Selective Branch Prediction Barrier */
 #define X86_FEATURE_IBPB_BRTYPE		(20*32+28) /* MSR_PRED_CMD[IBPB] flushes all branch type predictions */
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 097bdc022d0f..7bc095add8ee 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -799,8 +799,8 @@ void kvm_set_cpu_caps(void)
 
 	kvm_cpu_cap_mask(CPUID_8000_0021_EAX,
 		F(NO_NESTED_DATA_BP) | F(LFENCE_RDTSC) | 0 /* SmmPgCfgLock */ |
-		F(NULL_SEL_CLR_BASE) | F(AUTOIBRS) | 0 /* PrefetchCtlMsr */ |
-		F(WRMSR_XX_BASE_NS)
+		F(NULL_SEL_CLR_BASE) | F(AUTOIBRS) | F(AMD_FSRS) |
+		F(AMD_FSRC) | 0 /* PrefetchCtlMsr */ | F(WRMSR_XX_BASE_NS)
 	);
 
 	kvm_cpu_cap_check_and_set(X86_FEATURE_SBPB);
-- 
2.34.1

[PATCH v3 2/2] x86: KVM: Advertise AMD's speculation control features

[Maksim Davydov]

It seems helpful to expose to userspace some speculation control features
from 0x80000008_EBX function:
* 16 bit. IBRS always on. Indicates whether processor prefers that
  IBRS is always on. It simplifies speculation managing.
* 18 bit. IBRS is preferred over software solution. Indicates that
  software mitigations can be replaced with more performant IBRS.
* 19 bit. IBRS provides Same Mode Protection. Indicates that when IBRS
  is set indirect branch predictions are not influenced by any prior
  indirect branches.
* 29 bit. BTC_NO. Indicates that processor isn't affected by branch type
  confusion. It's used during mitigations setting up.

Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
Reviewed-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/include/asm/cpufeatures.h | 3 +++
 arch/x86/kvm/cpuid.c               | 5 +++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 45f87a026bba..0a709d03ee5c 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -343,7 +343,10 @@
 #define X86_FEATURE_AMD_IBPB		(13*32+12) /* Indirect Branch Prediction Barrier */
 #define X86_FEATURE_AMD_IBRS		(13*32+14) /* Indirect Branch Restricted Speculation */
 #define X86_FEATURE_AMD_STIBP		(13*32+15) /* Single Thread Indirect Branch Predictors */
+#define X86_FEATURE_AMD_IBRS_ALWAYS_ON	(13*32+16) /* Indirect Branch Restricted Speculation always-on preferred */
 #define X86_FEATURE_AMD_STIBP_ALWAYS_ON	(13*32+17) /* Single Thread Indirect Branch Predictors always-on preferred */
+#define X86_FEATURE_AMD_IBRS_PREFERRED	(13*32+18) /* Indirect Branch Restricted Speculation is preferred over SW solution */
+#define X86_FEATURE_AMD_IBRS_SAME_MODE	(13*32+19) /* Indirect Branch Restricted Speculation provides Same Mode protection */
 #define X86_FEATURE_AMD_PPIN		(13*32+23) /* "amd_ppin" Protected Processor Inventory Number */
 #define X86_FEATURE_AMD_SSBD		(13*32+24) /* Speculative Store Bypass Disable */
 #define X86_FEATURE_VIRT_SSBD		(13*32+25) /* "virt_ssbd" Virtualized Speculative Store Bypass Disable */
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 7bc095add8ee..204192425e2c 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -756,8 +756,9 @@ void kvm_set_cpu_caps(void)
 	kvm_cpu_cap_mask(CPUID_8000_0008_EBX,
 		F(CLZERO) | F(XSAVEERPTR) |
 		F(WBNOINVD) | F(AMD_IBPB) | F(AMD_IBRS) | F(AMD_SSBD) | F(VIRT_SSBD) |
-		F(AMD_SSB_NO) | F(AMD_STIBP) | F(AMD_STIBP_ALWAYS_ON) |
-		F(AMD_PSFD) | F(AMD_IBPB_RET)
+		F(AMD_SSB_NO) | F(AMD_STIBP) | F(AMD_IBRS_ALWAYS_ON) |
+		F(AMD_STIBP_ALWAYS_ON) | F(AMD_IBRS_PREFERRED) |
+		F(AMD_IBRS_SAME_MODE) | F(AMD_PSFD) | F(BTC_NO) | F(AMD_IBPB_RET)
 	);
 
 	/*
-- 
2.34.1

Re: [PATCH v3 1/2] x86: KVM: Advertise FSRS and FSRC on AMD to userspace

[Jim Mattson]

On Wed, Dec 4, 2024 at 5:43 AM Maksim Davydov
<davydov-max@yandex-team.ru> wrote:
>
> Fast short REP STOSB and fast short CMPSB support on AMD processors are
> provided in other CPUID function in comparison with Intel processors:
> * FSRS: 10 bit in 0x80000021_EAX
> * FSRC: 11 bit in 0x80000021_EAX

I have to wonder why these bits aren't documented in the APM. I assume
you pulled them out of some PPR? I would be hesitant to include CPUID
bit definitions that may be microarchitecture-specific rather than
architectural.

Perhaps someone from AMD should at least ACK this change?

> AMD bit numbers differ from existing definition of FSRC and
> FSRS. So, the new appropriate values have to be added with new names.
>
> It's safe to advertise these features to userspace because they are a part
> of CPU model definition and they can't be disabled (as existing Intel
> features).
>
> Fixes: 2a4209d6a9cb ("KVM: x86: Advertise fast REP string features inherent to the CPU")
> Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
> ---
>  arch/x86/include/asm/cpufeatures.h | 2 ++
>  arch/x86/kvm/cpuid.c               | 4 ++--
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index 17b6590748c0..45f87a026bba 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -460,6 +460,8 @@
>  #define X86_FEATURE_NULL_SEL_CLR_BASE  (20*32+ 6) /* Null Selector Clears Base */
>  #define X86_FEATURE_AUTOIBRS           (20*32+ 8) /* Automatic IBRS */
>  #define X86_FEATURE_NO_SMM_CTL_MSR     (20*32+ 9) /* SMM_CTL MSR is not present */
> +#define X86_FEATURE_AMD_FSRS           (20*32+10) /* AMD Fast short REP STOSB supported */
> +#define X86_FEATURE_AMD_FSRC           (20*32+11) /* AMD Fast short REP CMPSB supported */
>
>  #define X86_FEATURE_SBPB               (20*32+27) /* Selective Branch Prediction Barrier */
>  #define X86_FEATURE_IBPB_BRTYPE                (20*32+28) /* MSR_PRED_CMD[IBPB] flushes all branch type predictions */
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 097bdc022d0f..7bc095add8ee 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -799,8 +799,8 @@ void kvm_set_cpu_caps(void)
>
>         kvm_cpu_cap_mask(CPUID_8000_0021_EAX,
>                 F(NO_NESTED_DATA_BP) | F(LFENCE_RDTSC) | 0 /* SmmPgCfgLock */ |
> -               F(NULL_SEL_CLR_BASE) | F(AUTOIBRS) | 0 /* PrefetchCtlMsr */ |
> -               F(WRMSR_XX_BASE_NS)
> +               F(NULL_SEL_CLR_BASE) | F(AUTOIBRS) | F(AMD_FSRS) |
> +               F(AMD_FSRC) | 0 /* PrefetchCtlMsr */ | F(WRMSR_XX_BASE_NS)
>         );
>
>         kvm_cpu_cap_check_and_set(X86_FEATURE_SBPB);
> --
> 2.34.1
>

Re: [PATCH v3 1/2] x86: KVM: Advertise FSRS and FSRC on AMD to userspace

[Maksim Davydov]

On 12/6/24 21:11, Moger, Babu wrote:
> 
> On 12/4/2024 10:57 AM, Jim Mattson wrote:
>> On Wed, Dec 4, 2024 at 5:43 AM Maksim Davydov
>> <davydov-max@yandex-team.ru>  wrote:
>>> Fast short REP STOSB and fast short CMPSB support on AMD processors are
>>> provided in other CPUID function in comparison with Intel processors:
>>> * FSRS: 10 bit in 0x80000021_EAX
>>> * FSRC: 11 bit in 0x80000021_EAX
>> I have to wonder why these bits aren't documented in the APM. I assume
>> you pulled them out of some PPR? I would be hesitant to include CPUID
>> bit definitions that may be microarchitecture-specific rather than
>> architectural.
>>
>> Perhaps someone from AMD should at least ACK this change?
> 
> APM updates are in progress right now, but haven’t been able to get an ETA.
> 
> Will confirm once APM is released.
>

Thanks a lot!
It means that this series should be sent as 2 independent parts:
1. FSRS and FSRC will wait for updated APM
2. Speculation control bits will be sent as a separate patch

>>> AMD bit numbers differ from existing definition of FSRC and
>>> FSRS. So, the new appropriate values have to be added with new names.
>>>
>>> It's safe to advertise these features to userspace because they are a part
>>> of CPU model definition and they can't be disabled (as existing Intel
>>> features).
>>>
>>> Fixes: 2a4209d6a9cb ("KVM: x86: Advertise fast REP string features inherent to the CPU")
>>> Signed-off-by: Maksim Davydov<davydov-max@yandex-team.ru>
>>> ---
>>>   arch/x86/include/asm/cpufeatures.h | 2 ++
>>>   arch/x86/kvm/cpuid.c               | 4 ++--
>>>   2 files changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
>>> index 17b6590748c0..45f87a026bba 100644
>>> --- a/arch/x86/include/asm/cpufeatures.h
>>> +++ b/arch/x86/include/asm/cpufeatures.h
>>> @@ -460,6 +460,8 @@
>>>   #define X86_FEATURE_NULL_SEL_CLR_BASE  (20*32+ 6) /* Null Selector Clears Base */
>>>   #define X86_FEATURE_AUTOIBRS           (20*32+ 8) /* Automatic IBRS */
>>>   #define X86_FEATURE_NO_SMM_CTL_MSR     (20*32+ 9) /* SMM_CTL MSR is not present */
>>> +#define X86_FEATURE_AMD_FSRS           (20*32+10) /* AMD Fast short REP STOSB supported */
>>> +#define X86_FEATURE_AMD_FSRC           (20*32+11) /* AMD Fast short REP CMPSB supported */
>>>
>>>   #define X86_FEATURE_SBPB               (20*32+27) /* Selective Branch Prediction Barrier */
>>>   #define X86_FEATURE_IBPB_BRTYPE                (20*32+28) /* MSR_PRED_CMD[IBPB] flushes all branch type predictions */
>>> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
>>> index 097bdc022d0f..7bc095add8ee 100644
>>> --- a/arch/x86/kvm/cpuid.c
>>> +++ b/arch/x86/kvm/cpuid.c
>>> @@ -799,8 +799,8 @@ void kvm_set_cpu_caps(void)
>>>
>>>          kvm_cpu_cap_mask(CPUID_8000_0021_EAX,
>>>                  F(NO_NESTED_DATA_BP) | F(LFENCE_RDTSC) | 0 /* SmmPgCfgLock */ |
>>> -               F(NULL_SEL_CLR_BASE) | F(AUTOIBRS) | 0 /* PrefetchCtlMsr */ |
>>> -               F(WRMSR_XX_BASE_NS)
>>> +               F(NULL_SEL_CLR_BASE) | F(AUTOIBRS) | F(AMD_FSRS) |
>>> +               F(AMD_FSRC) | 0 /* PrefetchCtlMsr */ | F(WRMSR_XX_BASE_NS)
>>>          );
>>>
>>>          kvm_cpu_cap_check_and_set(X86_FEATURE_SBPB);
>>> --
>>> 2.34.1
>>>

-- 
Best regards,
Maksim Davydov

Re: [PATCH v3 1/2] x86: KVM: Advertise FSRS and FSRC on AMD to userspace

[Moger, Babu]

On 12/9/24 06:11, Maksim Davydov wrote:
> 
> 
> On 12/6/24 21:11, Moger, Babu wrote:
>>
>> On 12/4/2024 10:57 AM, Jim Mattson wrote:
>>> On Wed, Dec 4, 2024 at 5:43 AM Maksim Davydov
>>> <davydov-max@yandex-team.ru>  wrote:
>>>> Fast short REP STOSB and fast short CMPSB support on AMD processors are
>>>> provided in other CPUID function in comparison with Intel processors:
>>>> * FSRS: 10 bit in 0x80000021_EAX
>>>> * FSRC: 11 bit in 0x80000021_EAX
>>> I have to wonder why these bits aren't documented in the APM. I assume
>>> you pulled them out of some PPR? I would be hesitant to include CPUID
>>> bit definitions that may be microarchitecture-specific rather than
>>> architectural.
>>>
>>> Perhaps someone from AMD should at least ACK this change?
>>
>> APM updates are in progress right now, but haven’t been able to get an ETA.
>>
>> Will confirm once APM is released.
>>
> 
> Thanks a lot!
> It means that this series should be sent as 2 independent parts:
> 1. FSRS and FSRC will wait for updated APM

Yes. We can wait on this. APM is being updated right now. We can add FSRS
and FSRC support when APM is publicly available.

> 2. Speculation control bits will be sent as a separate patch

Sure.
> 
>>>> AMD bit numbers differ from existing definition of FSRC and
>>>> FSRS. So, the new appropriate values have to be added with new names.
>>>>
>>>> It's safe to advertise these features to userspace because they are a
>>>> part
>>>> of CPU model definition and they can't be disabled (as existing Intel
>>>> features).
>>>>
>>>> Fixes: 2a4209d6a9cb ("KVM: x86: Advertise fast REP string features
>>>> inherent to the CPU")
>>>> Signed-off-by: Maksim Davydov<davydov-max@yandex-team.ru>
>>>> ---
>>>>   arch/x86/include/asm/cpufeatures.h | 2 ++
>>>>   arch/x86/kvm/cpuid.c               | 4 ++--
>>>>   2 files changed, 4 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/arch/x86/include/asm/cpufeatures.h
>>>> b/arch/x86/include/asm/cpufeatures.h
>>>> index 17b6590748c0..45f87a026bba 100644
>>>> --- a/arch/x86/include/asm/cpufeatures.h
>>>> +++ b/arch/x86/include/asm/cpufeatures.h
>>>> @@ -460,6 +460,8 @@
>>>>   #define X86_FEATURE_NULL_SEL_CLR_BASE  (20*32+ 6) /* Null Selector
>>>> Clears Base */
>>>>   #define X86_FEATURE_AUTOIBRS           (20*32+ 8) /* Automatic IBRS */
>>>>   #define X86_FEATURE_NO_SMM_CTL_MSR     (20*32+ 9) /* SMM_CTL MSR is
>>>> not present */
>>>> +#define X86_FEATURE_AMD_FSRS           (20*32+10) /* AMD Fast short
>>>> REP STOSB supported */
>>>> +#define X86_FEATURE_AMD_FSRC           (20*32+11) /* AMD Fast short
>>>> REP CMPSB supported */
>>>>
>>>>   #define X86_FEATURE_SBPB               (20*32+27) /* Selective
>>>> Branch Prediction Barrier */
>>>>   #define X86_FEATURE_IBPB_BRTYPE                (20*32+28) /*
>>>> MSR_PRED_CMD[IBPB] flushes all branch type predictions */
>>>> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
>>>> index 097bdc022d0f..7bc095add8ee 100644
>>>> --- a/arch/x86/kvm/cpuid.c
>>>> +++ b/arch/x86/kvm/cpuid.c
>>>> @@ -799,8 +799,8 @@ void kvm_set_cpu_caps(void)
>>>>
>>>>          kvm_cpu_cap_mask(CPUID_8000_0021_EAX,
>>>>                  F(NO_NESTED_DATA_BP) | F(LFENCE_RDTSC) | 0 /*
>>>> SmmPgCfgLock */ |
>>>> -               F(NULL_SEL_CLR_BASE) | F(AUTOIBRS) | 0 /*
>>>> PrefetchCtlMsr */ |
>>>> -               F(WRMSR_XX_BASE_NS)
>>>> +               F(NULL_SEL_CLR_BASE) | F(AUTOIBRS) | F(AMD_FSRS) |
>>>> +               F(AMD_FSRC) | 0 /* PrefetchCtlMsr */ |
>>>> F(WRMSR_XX_BASE_NS)
>>>>          );
>>>>
>>>>          kvm_cpu_cap_check_and_set(X86_FEATURE_SBPB);
>>>> -- 
>>>> 2.34.1
>>>>
> 

-- 
Thanks
Babu Moger

Re: [PATCH v3 1/2] x86: KVM: Advertise FSRS and FSRC on AMD to userspace

[Borislav Petkov]

On Wed, Dec 04, 2024 at 04:43:44PM +0300, Maksim Davydov wrote:
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index 17b6590748c0..45f87a026bba 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -460,6 +460,8 @@
>  #define X86_FEATURE_NULL_SEL_CLR_BASE	(20*32+ 6) /* Null Selector Clears Base */
>  #define X86_FEATURE_AUTOIBRS		(20*32+ 8) /* Automatic IBRS */
>  #define X86_FEATURE_NO_SMM_CTL_MSR	(20*32+ 9) /* SMM_CTL MSR is not present */
> +#define X86_FEATURE_AMD_FSRS	        (20*32+10) /* AMD Fast short REP STOSB supported */
> +#define X86_FEATURE_AMD_FSRC		(20*32+11) /* AMD Fast short REP CMPSB supported */

Since Intel has the same flags, you should do

	if (cpu_has(c, X86_FEATURE_AMD_FSRS))
		set_cpu_cap(c, X86_FEATURE_FSRS);

and the other one too. Probably in init_amd() so that guest userspace doesn't
need to differentiate between the two and you don't have to do...

>  #define X86_FEATURE_SBPB		(20*32+27) /* Selective Branch Prediction Barrier */
>  #define X86_FEATURE_IBPB_BRTYPE		(20*32+28) /* MSR_PRED_CMD[IBPB] flushes all branch type predictions */
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 097bdc022d0f..7bc095add8ee 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -799,8 +799,8 @@ void kvm_set_cpu_caps(void)
>  
>  	kvm_cpu_cap_mask(CPUID_8000_0021_EAX,
>  		F(NO_NESTED_DATA_BP) | F(LFENCE_RDTSC) | 0 /* SmmPgCfgLock */ |
> -		F(NULL_SEL_CLR_BASE) | F(AUTOIBRS) | 0 /* PrefetchCtlMsr */ |
> -		F(WRMSR_XX_BASE_NS)
> +		F(NULL_SEL_CLR_BASE) | F(AUTOIBRS) | F(AMD_FSRS) |
> +		F(AMD_FSRC) | 0 /* PrefetchCtlMsr */ | F(WRMSR_XX_BASE_NS)

... this.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v3 1/2] x86: KVM: Advertise FSRS and FSRC on AMD to userspace

[Borislav Petkov]

On Wed, Dec 04, 2024 at 04:43:44PM +0300, Maksim Davydov wrote:
> Fast short REP STOSB and fast short CMPSB support on AMD processors are
> provided in other CPUID function in comparison with Intel processors:
> * FSRS: 10 bit in 0x80000021_EAX
> * FSRC: 11 bit in 0x80000021_EAX
> 
> AMD bit numbers differ from existing definition of FSRC and
> FSRS. So, the new appropriate values have to be added with new names.
> 
> It's safe to advertise these features to userspace because they are a part
> of CPU model definition and they can't be disabled (as existing Intel
> features).
> 
> Fixes: 2a4209d6a9cb ("KVM: x86: Advertise fast REP string features inherent to the CPU")

Why is there a Fixes tag here?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v3 2/2] x86: KVM: Advertise AMD's speculation control features

[Borislav Petkov]

On Wed, Dec 04, 2024 at 04:43:45PM +0300, Maksim Davydov wrote:
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index 45f87a026bba..0a709d03ee5c 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -343,7 +343,10 @@
>  #define X86_FEATURE_AMD_IBPB		(13*32+12) /* Indirect Branch Prediction Barrier */
>  #define X86_FEATURE_AMD_IBRS		(13*32+14) /* Indirect Branch Restricted Speculation */
>  #define X86_FEATURE_AMD_STIBP		(13*32+15) /* Single Thread Indirect Branch Predictors */
> +#define X86_FEATURE_AMD_IBRS_ALWAYS_ON	(13*32+16) /* Indirect Branch Restricted Speculation always-on preferred */
>  #define X86_FEATURE_AMD_STIBP_ALWAYS_ON	(13*32+17) /* Single Thread Indirect Branch Predictors always-on preferred */
> +#define X86_FEATURE_AMD_IBRS_PREFERRED	(13*32+18) /* Indirect Branch Restricted Speculation is preferred over SW solution */
> +#define X86_FEATURE_AMD_IBRS_SAME_MODE	(13*32+19) /* Indirect Branch Restricted Speculation provides Same Mode protection */

This is an AMD-specific leaf - you don't need to put "AMD_" in front of every
bit.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v3 1/2] x86: KVM: Advertise FSRS and FSRC on AMD to userspace

[Sean Christopherson]

On Fri, Apr 11, 2025, Borislav Petkov wrote:
> On Wed, Dec 04, 2024 at 04:43:44PM +0300, Maksim Davydov wrote:
> > diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> > index 17b6590748c0..45f87a026bba 100644
> > --- a/arch/x86/include/asm/cpufeatures.h
> > +++ b/arch/x86/include/asm/cpufeatures.h
> > @@ -460,6 +460,8 @@
> >  #define X86_FEATURE_NULL_SEL_CLR_BASE	(20*32+ 6) /* Null Selector Clears Base */
> >  #define X86_FEATURE_AUTOIBRS		(20*32+ 8) /* Automatic IBRS */
> >  #define X86_FEATURE_NO_SMM_CTL_MSR	(20*32+ 9) /* SMM_CTL MSR is not present */
> > +#define X86_FEATURE_AMD_FSRS	        (20*32+10) /* AMD Fast short REP STOSB supported */
> > +#define X86_FEATURE_AMD_FSRC		(20*32+11) /* AMD Fast short REP CMPSB supported */
> 
> Since Intel has the same flags, you should do
> 
> 	if (cpu_has(c, X86_FEATURE_AMD_FSRS))
> 		set_cpu_cap(c, X86_FEATURE_FSRS);
> 
> and the other one too. Probably in init_amd() so that guest userspace doesn't
> need to differentiate between the two and you don't have to do...
> 
> >  #define X86_FEATURE_SBPB		(20*32+27) /* Selective Branch Prediction Barrier */
> >  #define X86_FEATURE_IBPB_BRTYPE		(20*32+28) /* MSR_PRED_CMD[IBPB] flushes all branch type predictions */
> > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> > index 097bdc022d0f..7bc095add8ee 100644
> > --- a/arch/x86/kvm/cpuid.c
> > +++ b/arch/x86/kvm/cpuid.c
> > @@ -799,8 +799,8 @@ void kvm_set_cpu_caps(void)
> >  
> >  	kvm_cpu_cap_mask(CPUID_8000_0021_EAX,
> >  		F(NO_NESTED_DATA_BP) | F(LFENCE_RDTSC) | 0 /* SmmPgCfgLock */ |
> > -		F(NULL_SEL_CLR_BASE) | F(AUTOIBRS) | 0 /* PrefetchCtlMsr */ |
> > -		F(WRMSR_XX_BASE_NS)
> > +		F(NULL_SEL_CLR_BASE) | F(AUTOIBRS) | F(AMD_FSRS) |
> > +		F(AMD_FSRC) | 0 /* PrefetchCtlMsr */ | F(WRMSR_XX_BASE_NS)
> 
> ... this.

KVM should still explicitly advertise support for AMD's flavor.  There are KVM
use cases where KVM's advertised CPUID support is used almost verbatim, in which
case not advertising AMD_FSRC would result it the features not be enumerated to
the guest.  Linux might cross-pollinate, but KVM can't rely on all software to do
so.

Re: [PATCH v3 1/2] x86: KVM: Advertise FSRS and FSRC on AMD to userspace

[Borislav Petkov]

On Fri, Apr 11, 2025 at 06:49:54AM -0700, Sean Christopherson wrote:
> KVM should still explicitly advertise support for AMD's flavor.  There are KVM
> use cases where KVM's advertised CPUID support is used almost verbatim, in which

So that means the vendor differentiation is done by the user and KVM shouldn't
even try to unify things...?

> case not advertising AMD_FSRC would result it the features not be enumerated to
> the guest.  Linux might cross-pollinate, but KVM can't rely on all software to do
> so.

Sure.

The AMD feature flags are called the same:

  CPUID_Fn80000021_EAX [Extended Feature 2 EAX] (Core::X86::Cpuid::FeatureExt2Eax)
  
  11 FSRC. Read-only. Reset: Fixed,1. Fast Short Repe Cmpsb supported.
  10 FSRS. Read-only. Reset: Fixed,1. Fast Short Rep Stosb supported.

just the CPUID leaf is different.

But I guess KVM isn't exporting CPUID *names* but CPUID *leafs* so the
internal naming doesn't matter.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v3 1/2] x86: KVM: Advertise FSRS and FSRC on AMD to userspace

[Sean Christopherson]

On Fri, Apr 11, 2025, Borislav Petkov wrote:
> On Fri, Apr 11, 2025 at 06:49:54AM -0700, Sean Christopherson wrote:
> > KVM should still explicitly advertise support for AMD's flavor.  There are KVM
> > use cases where KVM's advertised CPUID support is used almost verbatim, in which
> 
> So that means the vendor differentiation is done by the user

Yes.

> and KVM shouldn't even try to unify things...?

Yeah, more or less.  KVM doesn't ever unify CPUID features, in the sense of giving
userspace one way to query support.

For better or worse, KVM does stuff feature bits for various mitigations, e.g. so
that kernels looking for just one or the other will get the desired behavior.

	if (boot_cpu_has(X86_FEATURE_AMD_IBPB_RET) &&
	    boot_cpu_has(X86_FEATURE_AMD_IBPB) &&
	    boot_cpu_has(X86_FEATURE_AMD_IBRS))
		kvm_cpu_cap_set(X86_FEATURE_SPEC_CTRL);
	if (boot_cpu_has(X86_FEATURE_STIBP))
		kvm_cpu_cap_set(X86_FEATURE_INTEL_STIBP);
	if (boot_cpu_has(X86_FEATURE_AMD_SSBD))
		kvm_cpu_cap_set(X86_FEATURE_SPEC_CTRL_SSBD);

	...

	if (boot_cpu_has(X86_FEATURE_IBPB)) {
		kvm_cpu_cap_set(X86_FEATURE_AMD_IBPB);
		if (boot_cpu_has(X86_FEATURE_SPEC_CTRL) &&
		    !boot_cpu_has_bug(X86_BUG_EIBRS_PBRSB))
			kvm_cpu_cap_set(X86_FEATURE_AMD_IBPB_RET);
	}
	if (boot_cpu_has(X86_FEATURE_IBRS))
		kvm_cpu_cap_set(X86_FEATURE_AMD_IBRS);
	if (boot_cpu_has(X86_FEATURE_STIBP))
		kvm_cpu_cap_set(X86_FEATURE_AMD_STIBP);
	if (boot_cpu_has(X86_FEATURE_SPEC_CTRL_SSBD))
		kvm_cpu_cap_set(X86_FEATURE_AMD_SSBD);
	if (!boot_cpu_has_bug(X86_BUG_SPEC_STORE_BYPASS))
		kvm_cpu_cap_set(X86_FEATURE_AMD_SSB_NO);
	/*
	 * The preference is to use SPEC CTRL MSR instead of the
	 * VIRT_SPEC MSR.
	 */
	if (boot_cpu_has(X86_FEATURE_LS_CFG_SSBD) &&
	    !boot_cpu_has(X86_FEATURE_AMD_SSBD))
		kvm_cpu_cap_set(X86_FEATURE_VIRT_SSBD);

But we've generally agreed that KVM shouldn't do that going forward, because many
of the mitigations don't have strict 1:1 mappings between Intel and AMD, i.e.
deciding which bits to stuff gets dangerously close to defining policy, which KVM
definitely wants to stay away from.

> But I guess KVM isn't exporting CPUID *names* but CPUID *leafs* so the
> internal naming doesn't matter.

Yep, exactly.