diff mbox

[4/5] VMX: Validate capability MSRs

Message ID 6a1106d8fefb4c808cdacc2a0632a5a55bd825b6.1402842281.git.jan.kiszka@web.de (mailing list archive)
State New, archived
Headers show

Commit Message

Jan Kiszka June 15, 2014, 2:24 p.m. UTC
From: Jan Kiszka <jan.kiszka@siemens.com>

Check for required-0 or required-1 bits as well as known field value
restrictions. Also check the consistency between VMX_*_CTLS and
VMX_TRUE_*_CTLS and between CR0/4_FIXED0 and CR0/4_FIXED1.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 x86/vmx.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 x86/vmx.h |  5 +++--
 2 files changed, 75 insertions(+), 3 deletions(-)

Comments

Paolo Bonzini June 16, 2014, 11 a.m. UTC | #1
Il 15/06/2014 16:24, Jan Kiszka ha scritto:
> +	for (n = 0; n < ARRAY_SIZE(vmx_ctl_msr); n++) {
> +		val = rdmsr(vmx_ctl_msr[n].index);
> +		default1 = vmx_ctl_msr[n].default1;
> +		ok = (val & default1) == default1 &&
> +			((((u32)val) ^ (val >> 32)) & ~(val >> 32)) == 0;

Ouch, this can sure be make this more readable.

Please unify these:

union vmx_ctrl_pin {
         u64 val;
         struct {
                 u32 set, clr;
         };
};

union vmx_ctrl_cpu {
         u64 val;
         struct {
                 u32 set, clr;
         };
};

union vmx_ctrl_exit {
         u64 val;
         struct {
                 u32 set, clr;
         };
};

union vmx_ctrl_ent {
         u64 val;
         struct {
                 u32 set, clr;
         };
};


into a single "union vmx_ctl_msr", and use this union for val and 
true_val as well.

Paolo

> +		if (ok && basic.ctrl) {
> +			true_val = rdmsr(vmx_ctl_msr[n].true_index);
> +			ok = (val >> 32) == (true_val >> 32) &&
> +				((u32)(val ^ true_val) & ~default1) == 0;
> +		}
> +		report(vmx_ctl_msr[n].name, ok);


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Kiszka June 16, 2014, 11:26 a.m. UTC | #2
On 2014-06-16 13:00, Paolo Bonzini wrote:
> Il 15/06/2014 16:24, Jan Kiszka ha scritto:
>> +    for (n = 0; n < ARRAY_SIZE(vmx_ctl_msr); n++) {
>> +        val = rdmsr(vmx_ctl_msr[n].index);
>> +        default1 = vmx_ctl_msr[n].default1;
>> +        ok = (val & default1) == default1 &&
>> +            ((((u32)val) ^ (val >> 32)) & ~(val >> 32)) == 0;
> 
> Ouch, this can sure be make this more readable.
> 
> Please unify these:
> 
> union vmx_ctrl_pin {
>         u64 val;
>         struct {
>                 u32 set, clr;
>         };
> };
> 
> union vmx_ctrl_cpu {
>         u64 val;
>         struct {
>                 u32 set, clr;
>         };
> };
> 
> union vmx_ctrl_exit {
>         u64 val;
>         struct {
>                 u32 set, clr;
>         };
> };
> 
> union vmx_ctrl_ent {
>         u64 val;
>         struct {
>                 u32 set, clr;
>         };
> };
> 
> 
> into a single "union vmx_ctl_msr", and use this union for val and
> true_val as well.

OK, will do.

Jan
diff mbox

Patch

diff --git a/x86/vmx.c b/x86/vmx.c
index 1182eef..84c514b 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -635,6 +635,76 @@  static void test_vmptrst(void)
 	report("test vmptrst", (!ret) && (vmcs1 == vmcs2));
 }
 
+struct vmx_ctl_msr {
+	const char *name;
+	u32 index, true_index;
+	u32 default1;
+} vmx_ctl_msr[] = {
+	{ "MSR_IA32_VMX_PINBASED_CTLS", MSR_IA32_VMX_PINBASED_CTLS,
+	  MSR_IA32_VMX_TRUE_PIN, 0x16 },
+	{ "MSR_IA32_VMX_PROCBASED_CTLS", MSR_IA32_VMX_PROCBASED_CTLS,
+	  MSR_IA32_VMX_TRUE_PROC, 0x401e172 },
+	{ "MSR_IA32_VMX_PROCBASED_CTLS2", MSR_IA32_VMX_PROCBASED_CTLS2,
+	  MSR_IA32_VMX_PROCBASED_CTLS2, 0 },
+	{ "MSR_IA32_VMX_EXIT_CTLS", MSR_IA32_VMX_EXIT_CTLS,
+	  MSR_IA32_VMX_TRUE_EXIT, 0x36dff },
+	{ "MSR_IA32_VMX_ENTRY_CTLS", MSR_IA32_VMX_ENTRY_CTLS,
+	  MSR_IA32_VMX_TRUE_ENTRY, 0x11ff },
+};
+
+static void test_vmx_caps(void)
+{
+	u64 val, true_val, default1, fixed0, fixed1;
+	unsigned int n;
+	bool ok;
+
+	printf("\nTest suite: VMX capability reporting\n");
+
+	report("MSR_IA32_VMX_BASIC",
+	       (basic.revision & (1ul << 31)) == 0 &&
+	       basic.size > 0 && basic.size <= 4096 &&
+	       (basic.type == 0 || basic.type == 6) &&
+	       basic.reserved1 == 0 && basic.reserved2 == 0);
+
+	val = rdmsr(MSR_IA32_VMX_MISC);
+	report("MSR_IA32_VMX_MISC",
+	       (!(ctrl_cpu_rev[1].clr & CPU_URG) || val & (1ul << 5)) &&
+	       ((val >> 16) & 0x1ff) <= 256 &&
+	       (val & 0xc0007e00) == 0);
+
+	for (n = 0; n < ARRAY_SIZE(vmx_ctl_msr); n++) {
+		val = rdmsr(vmx_ctl_msr[n].index);
+		default1 = vmx_ctl_msr[n].default1;
+		ok = (val & default1) == default1 &&
+			((((u32)val) ^ (val >> 32)) & ~(val >> 32)) == 0;
+		if (ok && basic.ctrl) {
+			true_val = rdmsr(vmx_ctl_msr[n].true_index);
+			ok = (val >> 32) == (true_val >> 32) &&
+				((u32)(val ^ true_val) & ~default1) == 0;
+		}
+		report(vmx_ctl_msr[n].name, ok);
+	}
+
+	fixed0 = rdmsr(MSR_IA32_VMX_CR0_FIXED0);
+	fixed1 = rdmsr(MSR_IA32_VMX_CR0_FIXED1);
+	report("MSR_IA32_VMX_IA32_VMX_CR0_FIXED0/1",
+	       ((fixed0 ^ fixed1) & ~fixed1) == 0);
+
+	fixed0 = rdmsr(MSR_IA32_VMX_CR4_FIXED0);
+	fixed1 = rdmsr(MSR_IA32_VMX_CR4_FIXED1);
+	report("MSR_IA32_VMX_IA32_VMX_CR4_FIXED0/1",
+	       ((fixed0 ^ fixed1) & ~fixed1) == 0);
+
+	val = rdmsr(MSR_IA32_VMX_VMCS_ENUM);
+	report("MSR_IA32_VMX_VMCS_ENUM",
+	       (val & 0x3e) >= 0x2a &&
+	       (val & 0xfffffffffffffc01Ull) == 0);
+
+	val = rdmsr(MSR_IA32_VMX_EPT_VPID_CAP);
+	report("MSR_IA32_VMX_EPT_VPID_CAP",
+	       (val & 0xfffff07ef9eebebeUll) == 0);
+}
+
 /* This function can only be called in guest */
 static void __attribute__((__used__)) hypercall(u32 hypercall_no)
 {
@@ -777,7 +847,7 @@  static int test_run(struct vmx_test *test)
 	regs = test->guest_regs;
 	vmcs_write(GUEST_RFLAGS, regs.rflags | 0x2);
 	launched = 0;
-	printf("\nTest suite : %s\n", test->name);
+	printf("\nTest suite: %s\n", test->name);
 	vmx_run();
 	if (vmx_off()) {
 		printf("%s : vmxoff failed.\n", __func__);
@@ -816,6 +886,7 @@  int main(void)
 		goto exit;
 	}
 	test_vmxoff();
+	test_vmx_caps();
 
 	while (vmx_tests[++i].name != NULL)
 		if (test_run(&vmx_tests[i]))
diff --git a/x86/vmx.h b/x86/vmx.h
index 69a5385..38ec3c5 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -46,12 +46,13 @@  union vmx_basic {
 	struct {
 		u32 revision;
 		u32	size:13,
-			: 3,
+			reserved1: 3,
 			width:1,
 			dual:1,
 			type:4,
 			insouts:1,
-			ctrl:1;
+			ctrl:1,
+			reserved2:8;
 	};
 };