[v3,08/21] KVM:x86: Init kvm_caps.supported_xss with supported feature bits

Message ID 20230511040857.6094-9-weijiang.yang@intel.com
State New
Headers
Series Enable CET Virtualization |

Commit Message

Yang, Weijiang May 11, 2023, 4:08 a.m. UTC
  Initialize kvm_caps.supported_xss with host XSS msr value AND XSS mask.
KVM_SUPPORTED_XSS holds all potential supported feature bits, the result
represents all KVM supported feature bits which is used for swapping guest
and host FPU contents.

Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 arch/x86/kvm/vmx/vmx.c | 1 -
 arch/x86/kvm/x86.c     | 6 +++++-
 2 files changed, 5 insertions(+), 2 deletions(-)
  

Comments

Chao Gao June 6, 2023, 8:38 a.m. UTC | #1
On Thu, May 11, 2023 at 12:08:44AM -0400, Yang Weijiang wrote:
>Initialize kvm_caps.supported_xss with host XSS msr value AND XSS mask.
>KVM_SUPPORTED_XSS holds all potential supported feature bits,

>the result
>represents all KVM supported feature bits which is used for swapping guest
>and host FPU contents.

do you mean kvm_caps.supported_xss by "the result"? I don't see how
fpu_swap_kvm_fpstate() uses kvm_caps.supported_xss.

>
>Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
>---
> arch/x86/kvm/vmx/vmx.c | 1 -
> arch/x86/kvm/x86.c     | 6 +++++-
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
>diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>index 44fb619803b8..c872a5aafa50 100644
>--- a/arch/x86/kvm/vmx/vmx.c
>+++ b/arch/x86/kvm/vmx/vmx.c
>@@ -7806,7 +7806,6 @@ static __init void vmx_set_cpu_caps(void)
> 		kvm_cpu_cap_set(X86_FEATURE_UMIP);
> 
> 	/* CPUID 0xD.1 */
>-	kvm_caps.supported_xss = 0;

AMD has the same statement. Do you need to remove that one?

> 	if (!cpu_has_vmx_xsaves())
> 		kvm_cpu_cap_clear(X86_FEATURE_XSAVES);
> 
>diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>index ab3360a10933..d2975ca96ac5 100644
>--- a/arch/x86/kvm/x86.c
>+++ b/arch/x86/kvm/x86.c
>@@ -223,6 +223,8 @@ static struct kvm_user_return_msrs __percpu *user_return_msrs;
> 				| XFEATURE_MASK_BNDCSR | XFEATURE_MASK_AVX512 \
> 				| XFEATURE_MASK_PKRU | XFEATURE_MASK_XTILE)
> 
>+#define KVM_SUPPORTED_XSS     0
>+
> u64 __read_mostly host_efer;
> EXPORT_SYMBOL_GPL(host_efer);
> 
>@@ -9472,8 +9474,10 @@ static int __kvm_x86_vendor_init(struct kvm_x86_init_ops *ops)
> 
> 	rdmsrl_safe(MSR_EFER, &host_efer);
> 
>-	if (boot_cpu_has(X86_FEATURE_XSAVES))
>+	if (boot_cpu_has(X86_FEATURE_XSAVES)) {
> 		rdmsrl(MSR_IA32_XSS, host_xss);
>+		kvm_caps.supported_xss = host_xss & KVM_SUPPORTED_XSS;
>+	}
> 
> 	kvm_init_pmu_capability(ops->pmu_ops);
> 
>-- 
>2.27.0
>
  
Yang, Weijiang June 8, 2023, 5:42 a.m. UTC | #2
On 6/6/2023 4:38 PM, Chao Gao wrote:
> On Thu, May 11, 2023 at 12:08:44AM -0400, Yang Weijiang wrote:
>> Initialize kvm_caps.supported_xss with host XSS msr value AND XSS mask.
>> KVM_SUPPORTED_XSS holds all potential supported feature bits,
>> the result
>> represents all KVM supported feature bits which is used for swapping guest
>> and host FPU contents.
> do you mean kvm_caps.supported_xss by "the result"? I don't see how
> fpu_swap_kvm_fpstate() uses kvm_caps.supported_xss.

The wording is not accurate, what I meant is : the resulting bits are 
supported by

fpu_swap_kvm_fpstate(). I will change the commit logs in next version, thanks!

>
>> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
>> ---
>> arch/x86/kvm/vmx/vmx.c | 1 -
>> arch/x86/kvm/x86.c     | 6 +++++-
>> 2 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>> index 44fb619803b8..c872a5aafa50 100644
>> --- a/arch/x86/kvm/vmx/vmx.c
>> +++ b/arch/x86/kvm/vmx/vmx.c
>> @@ -7806,7 +7806,6 @@ static __init void vmx_set_cpu_caps(void)
>> 		kvm_cpu_cap_set(X86_FEATURE_UMIP);
>>
>> 	/* CPUID 0xD.1 */
>> -	kvm_caps.supported_xss = 0;
> AMD has the same statement. Do you need to remove that one?

Since it appears in svm.c, so I assume Allen(AMD) will change it

in his following up patch series.

[...]
  

Patch

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 44fb619803b8..c872a5aafa50 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7806,7 +7806,6 @@  static __init void vmx_set_cpu_caps(void)
 		kvm_cpu_cap_set(X86_FEATURE_UMIP);
 
 	/* CPUID 0xD.1 */
-	kvm_caps.supported_xss = 0;
 	if (!cpu_has_vmx_xsaves())
 		kvm_cpu_cap_clear(X86_FEATURE_XSAVES);
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ab3360a10933..d2975ca96ac5 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -223,6 +223,8 @@  static struct kvm_user_return_msrs __percpu *user_return_msrs;
 				| XFEATURE_MASK_BNDCSR | XFEATURE_MASK_AVX512 \
 				| XFEATURE_MASK_PKRU | XFEATURE_MASK_XTILE)
 
+#define KVM_SUPPORTED_XSS     0
+
 u64 __read_mostly host_efer;
 EXPORT_SYMBOL_GPL(host_efer);
 
@@ -9472,8 +9474,10 @@  static int __kvm_x86_vendor_init(struct kvm_x86_init_ops *ops)
 
 	rdmsrl_safe(MSR_EFER, &host_efer);
 
-	if (boot_cpu_has(X86_FEATURE_XSAVES))
+	if (boot_cpu_has(X86_FEATURE_XSAVES)) {
 		rdmsrl(MSR_IA32_XSS, host_xss);
+		kvm_caps.supported_xss = host_xss & KVM_SUPPORTED_XSS;
+	}
 
 	kvm_init_pmu_capability(ops->pmu_ops);