On 06/20/2018 05:30 PM, Marc Orr wrote:
This patch adds a test for the prereq checks done as a part of a nested
VM launch related to event injection.
Signed-off-by: Marc Orr <marcorr@xxxxxxxxxx>
---
Changelog since v1:
- Add positive test case for interrupt type != INTR_TYPE_RESERVED
- Renamed nr to vector
- Add positive test case for interrupt type == INTR_TYPE_NMI_INTR
- Explicitly encode 0 vecotr (i.e., DE_VECTOR)
- Updated deliver errorcode checks to test postive cases
- Moved test for INTR_TYPE_OTHER_EVENT after test for INTR_TYPE_RESERVED
lib/x86/processor.h | 8 ++
x86/types.h | 1 +
x86/vmx.h | 27 ++++++
x86/vmx_tests.c | 231 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 267 insertions(+)
diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index 247386207bb0..886170cfa163 100644
--- a/lib/x86/processor.h
+++ b/lib/x86/processor.h
@@ -15,6 +15,14 @@
# define S "4"
#endif
+#define DF_VECTOR 8
+#define TS_VECTOR 10
+#define NP_VECTOR 11
+#define SS_VECTOR 12
+#define GP_VECTOR 13
+#define PF_VECTOR 14
+#define AC_VECTOR 17
+
#define X86_CR0_PE 0x00000001
#define X86_CR0_MP 0x00000002
#define X86_CR0_TS 0x00000008
diff --git a/x86/types.h b/x86/types.h
index fd22743990f1..047556e854d6 100644
--- a/x86/types.h
+++ b/x86/types.h
@@ -3,6 +3,7 @@
#define DE_VECTOR 0
#define DB_VECTOR 1
+#define NMI_VECTOR 2
#define BP_VECTOR 3
#define OF_VECTOR 4
#define BR_VECTOR 5
diff --git a/x86/vmx.h b/x86/vmx.h
index bdcaac0edc01..116bbd515beb 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -420,6 +420,15 @@ enum Intr_type {
#define INTR_INFO_INTR_TYPE_SHIFT 8
+#define INTR_TYPE_EXT_INTR (0 << 8) /* external interrupt */
+#define INTR_TYPE_RESERVED (1 << 8) /* reserved */
+#define INTR_TYPE_NMI_INTR (2 << 8) /* NMI */
+#define INTR_TYPE_HARD_EXCEPTION (3 << 8) /* processor exception */
+#define INTR_TYPE_SOFT_INTR (4 << 8) /* software interrupt */
+#define INTR_TYPE_PRIV_SW_EXCEPTION (5 << 8) /* priv. software exception */
+#define INTR_TYPE_SOFT_EXCEPTION (6 << 8) /* software exception */
+#define INTR_TYPE_OTHER_EVENT (7 << 8) /* other event */
+
/*
* VM-instruction error numbers
*/
@@ -712,6 +721,24 @@ static inline bool invvpid(unsigned long type, u64 vpid, u64 gla)
return ret;
}
+static inline int enable_unrestricted_guest(void)
+{
+ if (!(ctrl_cpu_rev[0].clr & CPU_SECONDARY))
+ return -1;
+
+ if (!(ctrl_cpu_rev[1].clr & CPU_URG))
+ return -1;
+
+ vmcs_write(CPU_EXEC_CTRL0, vmcs_read(CPU_EXEC_CTRL0) | CPU_SECONDARY);
+ vmcs_write(CPU_EXEC_CTRL1, vmcs_read(CPU_EXEC_CTRL1) | CPU_URG);
+ return 0;
+}
+
+static inline void disable_unrestricted_guest(void)
+{
+ vmcs_write(CPU_EXEC_CTRL1, vmcs_read(CPU_EXEC_CTRL1) & ~CPU_URG);
+}
+
const char *exit_reason_description(u64 reason);
void print_vmexit_info();
void print_vmentry_failure_info(struct vmentry_failure *failure);
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 0c1a6952f9ea..98f95df1316b 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -3561,6 +3561,236 @@ static void try_tpr_threshold_and_vtpr(unsigned threshold, unsigned vtpr)
report_prefix_pop();
}
+static void test_invalid_event_injection(void)
+{
+ u32 ent_intr_info_save = vmcs_read(ENT_INTR_INFO);
+ u32 ent_intr_error_save = vmcs_read(ENT_INTR_ERROR);
+ u32 ent_inst_len_save = vmcs_read(ENT_INST_LEN);
+ u32 primary_save = vmcs_read(CPU_EXEC_CTRL0);
+ u32 secondary_save = vmcs_read(CPU_EXEC_CTRL1);
+ u64 guest_cr0_save = vmcs_read(GUEST_CR0);
+ u32 ent_intr_info_base = INTR_INFO_VALID_MASK;
+ u32 ent_intr_info, ent_intr_err, ent_intr_len;
+ u32 cnt;
+
+ /* Setup */
+ report_prefix_push("invalid event injection");
+ vmcs_write(ENT_INTR_ERROR, 0x00000000);
+ vmcs_write(ENT_INST_LEN, 0x00000001);
+
+ /* The field’s interruption type is not set to a reserved value. */
+ ent_intr_info = ent_intr_info_base | INTR_TYPE_RESERVED | DE_VECTOR;
This test is all about checking that the interruption type is not a
reserved value. Is DE_VECTOR relevant here ?
+ report_prefix_pushf("%s, VM-entry intr info=0x%x",
+ "RESERVED interruption type invalid",
+ ent_intr_info);
+ vmcs_write(ENT_INTR_INFO, ent_intr_info);
+ test_vmx_controls(false, false);
+ report_prefix_pop();
+
Don't we want to just remove INTR_TYPE_RESERVED and run the test again
for positive test case ?
+ ent_intr_info = ent_intr_info_base | INTR_TYPE_HARD_EXCEPTION |
+ DE_VECTOR;
+ report_prefix_pushf("%s, VM-entry intr info=0x%x",
+ "RESERVED interruption type invalid",
+ ent_intr_info);
+ vmcs_write(ENT_INTR_INFO, ent_intr_info);
+ test_vmx_controls(true, false);
+ report_prefix_pop();
+
+ /* If the interruption type is other event, the vector is 0. */
+ ent_intr_info = ent_intr_info_base | INTR_TYPE_OTHER_EVENT | DB_VECTOR;
+ report_prefix_pushf("%s, VM-entry intr info=0x%x",
+ "(OTHER EVENT && vector != 0) invalid",
+ ent_intr_info);
+ vmcs_write(ENT_INTR_INFO, ent_intr_info);
+ test_vmx_controls(false, false);
+ report_prefix_pop();
It's more readable if you place this test right after the one for
INTR_TYPE_RESERVED, because the SDM describes them together.
+
+ /* If the interruption type is NMI, the vector is 2 (negative case). */
+ ent_intr_info = ent_intr_info_base | INTR_TYPE_NMI_INTR | DE_VECTOR;
+ report_prefix_pushf("%s, VM-entry intr info=0x%x",
+ "(NMI && vector != 2) invalid", ent_intr_info);
+ vmcs_write(ENT_INTR_INFO, ent_intr_info);
+ test_vmx_controls(false, false);
+ report_prefix_pop();
+
+ /* If the interruption type is NMI, the vector is 2 (positive case). */
+ ent_intr_info = ent_intr_info_base | INTR_TYPE_NMI_INTR | NMI_VECTOR;
+ report_prefix_pushf("%s, VM-entry intr info=0x%x",
+ "(NMI && vector == 2) valid", ent_intr_info);
+ vmcs_write(ENT_INTR_INFO, ent_intr_info);
+ test_vmx_controls(true, false);
+ report_prefix_pop();
+
+ /*
+ * If the interruption type
+ * is HW exception, the vector is at most 31.
+ */
+ ent_intr_info = ent_intr_info_base | INTR_TYPE_HARD_EXCEPTION | 0x20;
+ report_prefix_pushf("%s, VM-entry intr info=0x%x",
+ "(HW exception && vector > 31) invalid",
+ ent_intr_info);
+ vmcs_write(ENT_INTR_INFO, ent_intr_info);
+ test_vmx_controls(false, false);
+ report_prefix_pop();
+
+ /*
+ * deliver-error-code is 1 iff either
+ * (a) the "unrestricted guest" VM-execution control is 0
+ * (b) CR0.PE is set.
+ */
+ ent_intr_info = ent_intr_info_base | INTR_INFO_DELIVER_CODE_MASK |
+ INTR_TYPE_HARD_EXCEPTION | GP_VECTOR;
+ report_prefix_pushf("%s, VM-entry intr info=0x%x",
+ "error code <-> (!URG || prot_mode)",
+ ent_intr_info);
+ enable_unrestricted_guest();
+ vmcs_write(GUEST_CR0, guest_cr0_save & ~X86_CR0_PE & ~X86_CR0_PG);
+ vmcs_write(ENT_INTR_INFO, ent_intr_info);
+ test_vmx_controls(false, false);
+ report_prefix_pop();
May be you can add another test with the same conditions except
unrestricted guest being disabled ?
+
+ ent_intr_info = ent_intr_info_base | INTR_TYPE_HARD_EXCEPTION |
+ GP_VECTOR;
You are missing INTR_INFO_DELIVER_CODE_MASK. Or are you purposely
turning that bit off ? If you are purposely turning the bit off, the
test should pass. The SDM says that if INTR_INFO_DELIVER_CODE_MASK is
set, certain conditions need to be fulfilled. But if it's not set,
there's nothing to test for failure.
+ report_prefix_pushf("%s, VM-entry intr info=0x%x",
+ "error code <-> (!URG || prot_mode)",
+ ent_intr_info);
+ vmcs_write(GUEST_CR0, guest_cr0_save | X86_CR0_PE);
+ vmcs_write(ENT_INTR_INFO, ent_intr_info);
+ test_vmx_controls(false, false);
+ report_prefix_pop();
+
+ /* deliver-error-code is 1 iff the interruption type is HW exception */
+ report_prefix_push("error code <-> HW exception");
+ for (cnt = 0; cnt < 8; cnt++) {
+ u32 exception_type_mask = cnt << 8;
+ u32 deliver_error_code_mask =
+ exception_type_mask != INTR_TYPE_HARD_EXCEPTION ?
+ INTR_INFO_DELIVER_CODE_MASK : 0;
+
+ ent_intr_info = ent_intr_info_base | deliver_error_code_mask |
+ exception_type_mask | GP_VECTOR;
+ report_prefix_pushf("VM-entry intr info=0x%x", ent_intr_info);
+ vmcs_write(ENT_INTR_INFO, ent_intr_info);
+ test_vmx_controls(false, false);
+ report_prefix_pop();
+ }
+ report_prefix_pop();
+
+ /*
+ * deliver-error-code is 1 iff the the vector
+ * indicates an exception that would normally deliver an error code
+ */
+ report_prefix_push("error code <-> vector delivers error code");
+ for (cnt = 0; cnt < 32; cnt++) {
+ bool has_error_code = false;
+ u32 deliver_error_code_mask;
+
+ switch (cnt) {
+ case DF_VECTOR:
+ case TS_VECTOR:
+ case NP_VECTOR:
+ case SS_VECTOR:
+ case GP_VECTOR:
+ case PF_VECTOR:
+ case AC_VECTOR:
+ has_error_code = true;
+ }
+
+ /* Negative case */
+ deliver_error_code_mask = has_error_code ?
+ 0 :
+ INTR_INFO_DELIVER_CODE_MASK;
+ ent_intr_info = ent_intr_info_base | deliver_error_code_mask |
+ INTR_TYPE_HARD_EXCEPTION | cnt;
+ report_prefix_pushf("VM-entry intr info=0x%x", ent_intr_info);
+ vmcs_write(ENT_INTR_INFO, ent_intr_info);
+ test_vmx_controls(false, false);
+ report_prefix_pop();
+
+ /* Positive case */
+ deliver_error_code_mask = has_error_code ?
+ INTR_INFO_DELIVER_CODE_MASK :
+ 0;
+ ent_intr_info = ent_intr_info_base | deliver_error_code_mask |
+ INTR_TYPE_HARD_EXCEPTION | cnt;
+ report_prefix_pushf("VM-entry intr info=0x%x", ent_intr_info);
+ vmcs_write(ENT_INTR_INFO, ent_intr_info);
+ test_vmx_controls(true, false);
+ report_prefix_pop();
+ }
+ report_prefix_pop();
Do we need the extra call to report_prefix_pop() at the end of each loop ?
+
+ /* Reserved bits in the field (30:12) are 0. */
+ report_prefix_push("reserved bits clear");
+ for (cnt = 12; cnt <= 30; cnt++) {
+ ent_intr_info = ent_intr_info_base |
+ INTR_INFO_DELIVER_CODE_MASK |
+ INTR_TYPE_HARD_EXCEPTION | GP_VECTOR |
+ (1U << cnt);
Do we need to set INTR_INFO_DELIVER_CODE_MASK, INTR_TYPE_HARD_EXCEPTION
and GP_VECTOR here ? All we are testing here are bits [30:12].
+ report_prefix_pushf("VM-entry intr info=0x%x", ent_intr_info);
+ vmcs_write(ENT_INTR_INFO, ent_intr_info);
+ test_vmx_controls(false, false);
+ report_prefix_pop();
+ }
+ report_prefix_pop();
+
+ /*
+ * If deliver-error-code is 1
+ * bits 31:15 of the VM-entry exception error-code field are 0.
+ */
+ ent_intr_info = ent_intr_info_base | INTR_INFO_DELIVER_CODE_MASK |
+ INTR_TYPE_HARD_EXCEPTION | GP_VECTOR;
Again, do we need INTR_TYPE_HARD_EXCEPTION and GP_VECTOR here ?
+ report_prefix_pushf("%s, VM-entry intr info=0x%x",
+ "VM-entry exception error code[31:15] clear",
+ ent_intr_info);
+ vmcs_write(ENT_INTR_INFO, ent_intr_info);
+ for (cnt = 15; cnt <= 31; cnt++) {
+ ent_intr_err = 1U << cnt;
+ report_prefix_pushf("VM-entry intr error=0x%x", ent_intr_err);
+ vmcs_write(ENT_INTR_ERROR, ent_intr_err);
+ test_vmx_controls(false, false);
+ report_prefix_pop();
+ }
+ vmcs_write(ENT_INTR_ERROR, 0x00000000);
+ report_prefix_pop();
+
+ /*
+ * If the interruption type is software interrupt, software exception,
+ * or privileged software exception, the VM-entry instruction-length
+ * field is in the range 0–15.
+ */
+
+ ent_intr_info = ent_intr_info_base | INTR_TYPE_SOFT_EXCEPTION;
+ report_prefix_pushf("%s, VM-entry intr info=0x%x",
+ "VM-entry instruction-length check", ent_intr_info);
+ vmcs_write(ENT_INTR_INFO, ent_intr_info);
+
+ /* Instruction length set to -1 (0xFFFFFFFF) should fail */
+ ent_intr_len = -1;
+ report_prefix_pushf("VM-entry intr length = 0x%x", ent_intr_len);
+ vmcs_write(ENT_INST_LEN, ent_intr_len);
+ test_vmx_controls(false, false);
+ report_prefix_pop();
+
+ /* Instruction length set to 16 should fail */
+ ent_intr_len = 0x00000010;
+ report_prefix_pushf("VM-entry intr length = 0x%x", ent_intr_len);
+ vmcs_write(ENT_INST_LEN, 0x00000010);
+ test_vmx_controls(false, false);
+ report_prefix_pop();
+
+ report_prefix_pop();
You should also test INTR_TYPE_PRIV_SW_EXCEPTION and INTR_TYPE_SOFT_INTR
in addition to INTR_TYPE_SOFT_EXCEPTION.
+
+ /* Cleanup */
+ vmcs_write(ENT_INTR_INFO, ent_intr_info_save);
+ vmcs_write(ENT_INTR_ERROR, ent_intr_error_save);
+ vmcs_write(ENT_INST_LEN, ent_inst_len_save);
+ vmcs_write(CPU_EXEC_CTRL0, primary_save);
+ vmcs_write(CPU_EXEC_CTRL1, secondary_save);
+ vmcs_write(GUEST_CR0, guest_cr0_save);
+ report_prefix_pop();
+}
+
/*
* Test interesting vTPR values for a given TPR threshold.
*/
@@ -3807,6 +4037,7 @@ static void vmx_controls_test(void)
test_apic_virt_addr();
test_tpr_threshold();
test_nmi_ctrls();
+ test_invalid_event_injection();
}
static bool valid_vmcs_for_vmentry(void)
