On 12/7/20 12:17 PM, Sean Christopherson wrote:
On Mon, Dec 07, 2020, Krish Sadhukhan wrote:
According to sections "Canonicalization and Consistency Checks" and "Event
Injection" in APM vol 2
VMRUN exits with VMEXIT_INVALID error code if either:
- Reserved values of TYPE have been specified, or
- TYPE = 3 (exception) has been specified with a vector that does not
correspond to an exception (this includes vector 2, which is an NMI,
not an exception).
Signed-off-by: Krish Sadhukhan <krish.sadhukhan@xxxxxxxxxx>
---
arch/x86/include/asm/svm.h | 4 ++++
arch/x86/kvm/svm/nested.c | 14 ++++++++++++++
2 files changed, 18 insertions(+)
diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index 71d630bb5e08..d676f140cd19 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -341,9 +341,13 @@ struct vmcb {
#define SVM_EVTINJ_TYPE_MASK (7 << SVM_EVTINJ_TYPE_SHIFT)
#define SVM_EVTINJ_TYPE_INTR (0 << SVM_EVTINJ_TYPE_SHIFT)
+#define SVM_EVTINJ_TYPE_RESV1 (1 << SVM_EVTINJ_TYPE_SHIFT)
#define SVM_EVTINJ_TYPE_NMI (2 << SVM_EVTINJ_TYPE_SHIFT)
#define SVM_EVTINJ_TYPE_EXEPT (3 << SVM_EVTINJ_TYPE_SHIFT)
#define SVM_EVTINJ_TYPE_SOFT (4 << SVM_EVTINJ_TYPE_SHIFT)
+#define SVM_EVTINJ_TYPE_RESV5 (5 << SVM_EVTINJ_TYPE_SHIFT)
+#define SVM_EVTINJ_TYPE_RESV6 (6 << SVM_EVTINJ_TYPE_SHIFT)
+#define SVM_EVTINJ_TYPE_RESV7 (7 << SVM_EVTINJ_TYPE_SHIFT)
#define SVM_EVTINJ_VALID (1 << 31)
#define SVM_EVTINJ_VALID_ERR (1 << 11)
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 9e4c226dbf7d..fa51231c1f24 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -212,6 +212,9 @@ static bool svm_get_nested_state_pages(struct kvm_vcpu *vcpu)
static bool nested_vmcb_check_controls(struct vmcb_control_area *control)
{
+ u8 type, vector;
+ bool valid;
+
if ((vmcb_is_intercept(control, INTERCEPT_VMRUN)) == 0)
return false;
@@ -222,6 +225,17 @@ static bool nested_vmcb_check_controls(struct vmcb_control_area *control)
!npt_enabled)
return false;
+ valid = control->event_inj & SVM_EVTINJ_VALID;
+ type = control->event_inj & SVM_EVTINJ_TYPE_MASK;
The mask is shifted by 8, which means type is guaranteed to be 0 since the value
will be truncated when casting to a u8. I'm somewhat surprised neither
checkpatch nor checkpatch warns. The types are also shifted, so the easiest
thing is probably to store it as a u32, same as event_inj. I suspect your test
passes because type==0 is INTR and the test always injects #DE, which is likely
an illegal vector.
My bad. Will change it back to u32.
+ if (valid && (type == SVM_EVTINJ_TYPE_RESV1 ||
+ type >= SVM_EVTINJ_TYPE_RESV5))
+ return false;
+
+ vector = control->event_inj & SVM_EVTINJ_VEC_MASK;
+ if (valid && (type == SVM_EVTINJ_TYPE_EXEPT))
+ if (vector == NMI_VECTOR || vector > 31)
Preferred style is to combine these into a single statement.
The reason why I split them was to avoid using too many parentheses
which was what Paolo was objecting to. 😁
+ return false;
+
return true;
}
--
2.27.0
