On 7/26/2023 4:06 PM, Chao Gao wrote:
On Thu, Jul 20, 2023 at 11:03:45PM -0400, Yang Weijiang wrote:
Add VMX specific emulation for CET MSR read and write.
IBT feature is only available on Intel platforms now and the
virtualization interface to the control fields is vensor
specific, so split this part from the common code.
Signed-off-by: Yang Weijiang <weijiang.yang@xxxxxxxxx>
---
arch/x86/kvm/vmx/vmx.c | 40 ++++++++++++++++++++++++++++++++++++++++
arch/x86/kvm/x86.c | 7 -------
2 files changed, 40 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index c8d9870cfecb..b29817ec6f2e 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -2093,6 +2093,21 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
else
msr_info->data = vmx->pt_desc.guest.addr_a[index / 2];
break;
+ case MSR_IA32_U_CET:
+ case MSR_IA32_PL0_SSP ... MSR_IA32_PL3_SSP:
+ return kvm_get_msr_common(vcpu, msr_info);
kvm_get_msr_common() is called for the "default" case. so this can be dropped.
yes, I can drop these in vmx_get_msr(), just wanted to make it clearer,
Thanks!
+ case MSR_IA32_S_CET:
+ case MSR_KVM_GUEST_SSP:
+ case MSR_IA32_INT_SSP_TAB:
+ if (kvm_get_msr_common(vcpu, msr_info))
+ return 1;
+ if (msr_info->index == MSR_KVM_GUEST_SSP)
+ msr_info->data = vmcs_readl(GUEST_SSP);
+ else if (msr_info->index == MSR_IA32_S_CET)
+ msr_info->data = vmcs_readl(GUEST_S_CET);
+ else if (msr_info->index == MSR_IA32_INT_SSP_TAB)
+ msr_info->data = vmcs_readl(GUEST_INTR_SSP_TABLE);
+ break;
case MSR_IA32_DEBUGCTLMSR:
msr_info->data = vmcs_read64(GUEST_IA32_DEBUGCTL);
break;
@@ -2402,6 +2417,31 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
else
vmx->pt_desc.guest.addr_a[index / 2] = data;
break;
+#define VMX_CET_CONTROL_MASK (~GENMASK_ULL(9, 6))
bits9-6 are reserved for both intel and amd. Shouldn't this check be
done in the common code?
My thinking is, on AMD platform, bit 63:2 is anyway reserved since it
doesn't support IBT,
so the checks in common code for AMD is enough, when the execution flow
comes here,
it should be vmx, and need this additional check.
+#define CET_LEG_BITMAP_BASE(data) ((data) >> 12)
+#define CET_EXCLUSIVE_BITS (CET_SUPPRESS | CET_WAIT_ENDBR)
+ case MSR_IA32_PL0_SSP ... MSR_IA32_PL3_SSP:
+ return kvm_set_msr_common(vcpu, msr_info);
this hunk can be dropped as well.
In patch 16, these lines still need to be added back for PL{0,1,2}_SSP,
so would like keep it
here.
+ break;
+ case MSR_IA32_U_CET:
+ case MSR_IA32_S_CET:
+ case MSR_KVM_GUEST_SSP:
+ case MSR_IA32_INT_SSP_TAB:
+ if ((msr_index == MSR_IA32_U_CET ||
+ msr_index == MSR_IA32_S_CET) &&
+ ((data & ~VMX_CET_CONTROL_MASK) ||
+ !IS_ALIGNED(CET_LEG_BITMAP_BASE(data), 4) ||
+ (data & CET_EXCLUSIVE_BITS) == CET_EXCLUSIVE_BITS))
+ return 1;
how about
case MSR_IA32_U_CET:
case MSR_IA32_S_CET:
if ((data & ~VMX_CET_CONTROL_MASK) || ...
...
case MSR_KVM_GUEST_SSP:
case MSR_IA32_INT_SSP_TAB:
Do you mean to use "fallthrough"?
If not, for MSR_IA32_S_CET, we need vmcs_write() to handle it,
this is vmx specific code.
