diff mbox series

[kvm-unit-tests,v2] memory: Skip tests for instructions that are absent

Message ID 20230403164411.388475-1-pbonzini@redhat.com (mailing list archive)
State New, archived
Headers show
Series [kvm-unit-tests,v2] memory: Skip tests for instructions that are absent | expand

Commit Message

Paolo Bonzini April 3, 2023, 4:44 p.m. UTC 
Checking that instructions are absent is broken when running with CPU
models other than the bare metal processor's, because neither VMX nor SVM have
intercept controls for the instructions.

This can even happen with "-cpu max" when running under nested
virtualization, which is the current situation in the Fedora KVM job
on Cirrus-CI:

FAIL: clflushopt (ABSENT)
FAIL: clwb (ABSENT)

In other words it looks like the features have been marked as disabled
in the L0 host, while the hardware supports them.

Reported-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 x86/memory.c | 83 +++++++++++++++++++++++++++++++---------------------
 1 file changed, 49 insertions(+), 34 deletions(-)

Comments

Sean Christopherson April 3, 2023, 4:53 p.m. UTC | #1 
On Mon, Apr 03, 2023, Paolo Bonzini wrote:
> Checking that instructions are absent is broken when running with CPU
> models other than the bare metal processor's, because neither VMX nor SVM have
> intercept controls for the instructions.
> 
> This can even happen with "-cpu max" when running under nested
> virtualization, which is the current situation in the Fedora KVM job
> on Cirrus-CI:
> 
> FAIL: clflushopt (ABSENT)
> FAIL: clwb (ABSENT)
> 
> In other words it looks like the features have been marked as disabled
> in the L0 host, while the hardware supports them.
> 
> Reported-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  x86/memory.c | 83 +++++++++++++++++++++++++++++++---------------------
>  1 file changed, 49 insertions(+), 34 deletions(-)
> 
> diff --git a/x86/memory.c b/x86/memory.c
> index 351e7c0..58ef835 100644
> --- a/x86/memory.c
> +++ b/x86/memory.c
> @@ -25,53 +25,68 @@ static void handle_ud(struct ex_regs *regs)
>  
>  int main(int ac, char **av)
>  {
> -	int expected;
> -
>  	handle_exception(UD_VECTOR, handle_ud);
>  
>  	/* 3-byte instructions: */
>  	isize = 3;

We can clean this up even further by utilizing TRY_ASM().  Though I think this is
a good excuse to add things like asm_safe(), asm_safe_report(), and asm_safe_report_ex(),
to cut down on the boilerplate even more.  I'll send patches on top.
diff mbox series

Patch

diff --git a/x86/memory.c b/x86/memory.c
index 351e7c0..58ef835 100644
--- a/x86/memory.c
+++ b/x86/memory.c
@@ -25,53 +25,68 @@  static void handle_ud(struct ex_regs *regs)
 
 int main(int ac, char **av)
 {
-	int expected;
-
 	handle_exception(UD_VECTOR, handle_ud);
 
 	/* 3-byte instructions: */
 	isize = 3;
 
-	expected = !this_cpu_has(X86_FEATURE_CLFLUSH); /* CLFLUSH */
-	ud = 0;
-	asm volatile("clflush (%0)" : : "b" (&target));
-	report(ud == expected, "clflush (%s)", expected ? "ABSENT" : "present");
+	if (this_cpu_has(X86_FEATURE_CLFLUSH)) { /* CLFLUSH */
+		ud = 0;
+		asm volatile("clflush (%0)" : : "b" (&target));
+		report(!ud, "clflush");
+	} else {
+		report_skip("clflush");
+	}
 
-	expected = !this_cpu_has(X86_FEATURE_XMM); /* SSE */
-	ud = 0;
-	asm volatile("sfence");
-	report(ud == expected, "sfence (%s)", expected ? "ABSENT" : "present");
+	if (this_cpu_has(X86_FEATURE_XMM)) { /* SSE */
+		ud = 0;
+		asm volatile("sfence");
+		report(!ud, "sfence");
+	} else {
+		report_skip("sfence");
+	}
 
-	expected = !this_cpu_has(X86_FEATURE_XMM2); /* SSE2 */
-	ud = 0;
-	asm volatile("lfence");
-	report(ud == expected, "lfence (%s)", expected ? "ABSENT" : "present");
-
-	ud = 0;
-	asm volatile("mfence");
-	report(ud == expected, "mfence (%s)", expected ? "ABSENT" : "present");
+	if (this_cpu_has(X86_FEATURE_XMM2)) { /* SSE2 */
+		ud = 0;
+		asm volatile("lfence");
+		report(!ud, "lfence");
+		ud = 0;
+		asm volatile("mfence");
+		report(!ud, "mfence");
+	} else {
+		report_skip("lfence");
+		report_skip("mfence");
+	}
 
 	/* 4-byte instructions: */
 	isize = 4;
 
-	expected = !this_cpu_has(X86_FEATURE_CLFLUSHOPT); /* CLFLUSHOPT */
-	ud = 0;
-	/* clflushopt (%rbx): */
-	asm volatile(".byte 0x66, 0x0f, 0xae, 0x3b" : : "b" (&target));
-	report(ud == expected, "clflushopt (%s)",
-	       expected ? "ABSENT" : "present");
+	if (this_cpu_has(X86_FEATURE_CLFLUSHOPT)) { /* CLFLUSHOPT */
+		ud = 0;
+		/* clflushopt (%rbx): */
+		asm volatile(".byte 0x66, 0x0f, 0xae, 0x3b" : : "b" (&target));
+		report(!ud, "clflushopt");
+	} else {
+		report_skip("clflushopt");
+	}
 
-	expected = !this_cpu_has(X86_FEATURE_CLWB); /* CLWB */
-	ud = 0;
-	/* clwb (%rbx): */
-	asm volatile(".byte 0x66, 0x0f, 0xae, 0x33" : : "b" (&target));
-	report(ud == expected, "clwb (%s)", expected ? "ABSENT" : "present");
+	if (this_cpu_has(X86_FEATURE_CLWB)) { /* CLWB */
+		ud = 0;
+		/* clwb (%rbx): */
+		asm volatile(".byte 0x66, 0x0f, 0xae, 0x33" : : "b" (&target));
+		report(!ud, "clwb");
+	} else {
+		report_skip("clwb");
+	}
 
-	expected = !this_cpu_has(X86_FEATURE_PCOMMIT); /* PCOMMIT */
-	ud = 0;
-	/* pcommit: */
-	asm volatile(".byte 0x66, 0x0f, 0xae, 0xf8");
-	report(ud == expected, "pcommit (%s)", expected ? "ABSENT" : "present");
+	if (this_cpu_has(X86_FEATURE_PCOMMIT)) { /* PCOMMIT */
+		ud = 0;
+		/* pcommit: */
+		asm volatile(".byte 0x66, 0x0f, 0xae, 0xf8");
+		report(!ud, "pcommit");
+	} else {
+		report_skip("pcommit");
+	}
 
 	return report_summary();
 }