diff mbox series

[RFC,v2,08/11] KVM: VMX: Advertise MITI_ENUM_RETPOLINE_S_SUPPORT

Message ID 20230414062545.270178-9-chao.gao@intel.com
State New
Headers
Series Intel IA32_SPEC_CTRL Virtualization |

Commit Message

Chao Gao April 14, 2023, 6:25 a.m. UTC 
  Allow guest to query if the underying VMM understands Retpoline and
report the statue of Retpoline in suprevisor mode i.e. CPL < 3 via
MSR_VIRTUAL_MITIGATION_ENUM/CTRL.

Disable RRSBA behavior by setting RRSBA_DIS_S for guest if guest is
using retpoline and the processor has the behavior.

Signed-off-by: Zhang Chen <chen.zhang@intel.com>
Signed-off-by: Chao Gao <chao.gao@intel.com>
Tested-by: Jiaan Lu <jiaan.lu@intel.com>
---
 arch/x86/kvm/vmx/vmx.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

Comments

Xiaoyao Li May 18, 2023, 10:25 a.m. UTC | #1 
On 4/14/2023 2:25 PM, Chao Gao wrote:
> Allow guest to query if the underying VMM understands Retpoline and
> report the statue of Retpoline in suprevisor mode i.e. CPL < 3 via
> MSR_VIRTUAL_MITIGATION_ENUM/CTRL.
> 
> Disable RRSBA behavior by setting RRSBA_DIS_S for guest if guest is
> using retpoline and the processor has the behavior.
> 
> Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Tested-by: Jiaan Lu <jiaan.lu@intel.com>
> ---
>   arch/x86/kvm/vmx/vmx.c | 24 +++++++++++++++++++++---
>   1 file changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 980498c4c30c..25afb4c3e55e 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -1944,8 +1944,8 @@ static inline bool is_vmx_feature_control_msr_valid(struct vcpu_vmx *vmx,
>   }
>   
>   #define VIRTUAL_ENUMERATION_VALID_BITS	VIRT_ENUM_MITIGATION_CTRL_SUPPORT
> -#define MITI_ENUM_VALID_BITS		0ULL
> -#define MITI_CTRL_VALID_BITS		0ULL
> +#define MITI_ENUM_VALID_BITS		MITI_ENUM_RETPOLINE_S_SUPPORT
> +#define MITI_CTRL_VALID_BITS		MITI_CTRL_RETPOLINE_S_USED
>   
>   static int vmx_get_msr_feature(struct kvm_msr_entry *msr)
>   {
> @@ -2173,7 +2173,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>   	struct vmx_uret_msr *msr;
>   	int ret = 0;
>   	u32 msr_index = msr_info->index;
> -	u64 data = msr_info->data, spec_ctrl_mask;
> +	u64 data = msr_info->data, arch_msr = 0, spec_ctrl_mask = 0;

Sugget to make arch_msr and spec_ctrl_mask as local variables of each 
case {} block

>   	u32 index;
>   
>   	switch (msr_index) {
> @@ -2488,6 +2488,24 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>   		if (data & ~MITI_CTRL_VALID_BITS)
>   			return 1;
>   
> +		if (boot_cpu_has(X86_FEATURE_ARCH_CAPABILITIES))
> +			rdmsrl(MSR_IA32_ARCH_CAPABILITIES, arch_msr);
> +
> +		if (data & MITI_CTRL_RETPOLINE_S_USED &&
> +		    kvm_cpu_cap_has(X86_FEATURE_RRSBA_CTRL) &&

why kvm_cpu_cap_has() is used here? it means whether KVM supports expose 
this feature to guest. But what we need here is whether host supports 
this feature. Though they might get the same result, we'd better use 
boot_cpu_has() or even read CPUID directly (since cpuid info can be 
changed by clearcpuid magic) to avoid confusion.

> +		    arch_msr & ARCH_CAP_RRSBA)
> +			spec_ctrl_mask |= SPEC_CTRL_RRSBA_DIS_S;
> +
> +		/*
> +		 * Intercept IA32_SPEC_CTRL to disallow guest from changing
> +		 * certain bits.
> +		 */
> +		if (spec_ctrl_mask && !cpu_has_spec_ctrl_virt())
> +			vmx_enable_intercept_for_msr(vcpu, MSR_IA32_SPEC_CTRL, MSR_TYPE_RW);
> +
> +		vmx_set_spec_ctrl_mask(vmx, spec_ctrl_mask);
> +		vmx_set_guest_spec_ctrl(vmx, vmx_get_guest_spec_ctrl(vmx));
> +
>   		vmx->msr_virtual_mitigation_ctrl = data;
>   		break;
>   	default:
Chao Gao May 19, 2023, 10:26 a.m. UTC | #2 
On Thu, May 18, 2023 at 06:25:30PM +0800, Xiaoyao Li wrote:
>> @@ -2173,7 +2173,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>>   	struct vmx_uret_msr *msr;
>>   	int ret = 0;
>>   	u32 msr_index = msr_info->index;
>> -	u64 data = msr_info->data, spec_ctrl_mask;
>> +	u64 data = msr_info->data, arch_msr = 0, spec_ctrl_mask = 0;
>
>Sugget to make arch_msr and spec_ctrl_mask as local variables of each case {}
>block

Sure. Will do

>
>>   	u32 index;
>>   	switch (msr_index) {
>> @@ -2488,6 +2488,24 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>>   		if (data & ~MITI_CTRL_VALID_BITS)
>>   			return 1;
>> +		if (boot_cpu_has(X86_FEATURE_ARCH_CAPABILITIES))
>> +			rdmsrl(MSR_IA32_ARCH_CAPABILITIES, arch_msr);
>> +
>> +		if (data & MITI_CTRL_RETPOLINE_S_USED &&
>> +		    kvm_cpu_cap_has(X86_FEATURE_RRSBA_CTRL) &&
>
>why kvm_cpu_cap_has() is used here? it means whether KVM supports expose this
>feature to guest. But what we need here is whether host supports this
>feature. Though they might get the same result, we'd better use
>boot_cpu_has() or even read CPUID directly (since cpuid info can be changed
>by clearcpuid magic) to avoid confusion.

OK. This makes sense. I will use boot_cpu_has(). clearcpuid sometimes is
helpful for debugging. I prefer to honor it.

Thanks.
Liu, Jingqi May 22, 2023, 9:43 a.m. UTC | #3 
On 4/14/2023 2:25 PM, Chao Gao wrote:
> Allow guest to query if the underying VMM understands Retpoline and
> report the statue of Retpoline in suprevisor mode i.e. CPL < 3 via
s/statue/status
> MSR_VIRTUAL_MITIGATION_ENUM/CTRL.
>
> Disable RRSBA behavior by setting RRSBA_DIS_S for guest if guest is
> using retpoline and the processor has the behavior.
>
> Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Tested-by: Jiaan Lu <jiaan.lu@intel.com>
> ---
Thanks,
Jingqi
diff mbox series

Patch

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 980498c4c30c..25afb4c3e55e 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -1944,8 +1944,8 @@  static inline bool is_vmx_feature_control_msr_valid(struct vcpu_vmx *vmx,
 }
 
 #define VIRTUAL_ENUMERATION_VALID_BITS	VIRT_ENUM_MITIGATION_CTRL_SUPPORT
-#define MITI_ENUM_VALID_BITS		0ULL
-#define MITI_CTRL_VALID_BITS		0ULL
+#define MITI_ENUM_VALID_BITS		MITI_ENUM_RETPOLINE_S_SUPPORT
+#define MITI_CTRL_VALID_BITS		MITI_CTRL_RETPOLINE_S_USED
 
 static int vmx_get_msr_feature(struct kvm_msr_entry *msr)
 {
@@ -2173,7 +2173,7 @@  static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 	struct vmx_uret_msr *msr;
 	int ret = 0;
 	u32 msr_index = msr_info->index;
-	u64 data = msr_info->data, spec_ctrl_mask;
+	u64 data = msr_info->data, arch_msr = 0, spec_ctrl_mask = 0;
 	u32 index;
 
 	switch (msr_index) {
@@ -2488,6 +2488,24 @@  static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		if (data & ~MITI_CTRL_VALID_BITS)
 			return 1;
 
+		if (boot_cpu_has(X86_FEATURE_ARCH_CAPABILITIES))
+			rdmsrl(MSR_IA32_ARCH_CAPABILITIES, arch_msr);
+
+		if (data & MITI_CTRL_RETPOLINE_S_USED &&
+		    kvm_cpu_cap_has(X86_FEATURE_RRSBA_CTRL) &&
+		    arch_msr & ARCH_CAP_RRSBA)
+			spec_ctrl_mask |= SPEC_CTRL_RRSBA_DIS_S;
+
+		/*
+		 * Intercept IA32_SPEC_CTRL to disallow guest from changing
+		 * certain bits.
+		 */
+		if (spec_ctrl_mask && !cpu_has_spec_ctrl_virt())
+			vmx_enable_intercept_for_msr(vcpu, MSR_IA32_SPEC_CTRL, MSR_TYPE_RW);
+
+		vmx_set_spec_ctrl_mask(vmx, spec_ctrl_mask);
+		vmx_set_guest_spec_ctrl(vmx, vmx_get_guest_spec_ctrl(vmx));
+
 		vmx->msr_virtual_mitigation_ctrl = data;
 		break;
 	default: