Re: [PATCH kvm-unit-tests 3/4] x86: remove test_for_exception

[Paolo Bonzini <pbonzini@redhat.com>]

On 15/12/2015 19:32, David Matlack wrote:
> What do you think about this simplification?
> 
> bool test_for_exception(unsigned int ex, void (*trigger_func)(void *data),
>                         void *data)
>         jmp_buf jmpbuf;
>         int ret;
> 
>         handle_exception(ex, exception_handler);
>         ret = set_exception_jmpbuf(&jmpbuf);
>         if (ret == 0)
>                 trigger_func(data);
>         handle_exception(ex, NULL);
>         return ret;
> }
> 
> Then trigger_func can be very simple, e.g.
> 
> static void do_write_apicbase(u64 data)
> {
>         wrmsr(MSR_IA32_APICBASE, data);
> }

I agree that's the best of both worlds.

-------- 8< ------------
>From de1c13978b785e2aca82060ad3ccecdc04e75802 Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Tue, 12 Jan 2016 13:40:19 +0100
Subject: [PATCH kvm-unit-tests] x86: simplify test_for_exception trigger
 functions

test_for_exception makes the unit tests simpler, but it requires the
trigger function to be in on the joke: before, by calling
set_exception_return; now, by calling set_exception_jmpbuf and
handle_exception(NULL).  The new setjmp-based implementation
lets us move all of this into test_for_exception itself, so
that the trigger_func can be very simple.

Suggested-by: David Matlack <dmatlack@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 lib/x86/desc.c | 10 +++++++---
 x86/apic.c     |  5 +----
 x86/vmx.c      | 12 +++---------
 3 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index acf29e3..17aedcf 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -333,11 +333,15 @@ static void exception_handler(struct ex_regs *regs)
 bool test_for_exception(unsigned int ex, void (*trigger_func)(void *data),
 			void *data)
 {
+	jmp_buf jmpbuf;
+	int ret;
+
 	handle_exception(ex, exception_handler);
-	exception = false;
-	trigger_func(data);
+	ret = set_exception_jmpbuf(&jmpbuf);
+	if (ret == 0)
+		trigger_func(data);
 	handle_exception(ex, NULL);
-	return exception;
+	return ret;
 }
 
 void __set_exception_jmpbuf(jmp_buf *addr)
diff --git a/x86/apic.c b/x86/apic.c
index 2e04c82..61a0a76 100644
--- a/x86/apic.c
+++ b/x86/apic.c
@@ -63,10 +63,7 @@ static void test_tsc_deadline_timer(void)
 
 static void do_write_apicbase(void *data)
 {
-    jmp_buf jmpbuf;
-    if (set_exception_jmpbuf(jmpbuf) == 0) {
-        wrmsr(MSR_IA32_APICBASE, *(u64 *)data);
-    }
+    wrmsr(MSR_IA32_APICBASE, *(u64 *)data);
 }
 
 void test_enable_x2apic(void)
diff --git a/x86/vmx.c b/x86/vmx.c
index 12ec396..3fa1a73 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -617,19 +617,13 @@ static void init_vmx(void)
 
 static void do_vmxon_off(void *data)
 {
-	jmp_buf jmpbuf;
-	if (set_exception_jmpbuf(jmpbuf) == 0) {
-		vmx_on();
-		vmx_off();
-	}
+	vmx_on();
+	vmx_off();
 }
 
 static void do_write_feature_control(void *data)
 {
-	jmp_buf jmpbuf;
-	if (set_exception_jmpbuf(jmpbuf) == 0) {
-		wrmsr(MSR_IA32_FEATURE_CONTROL, 0);
-	}
+	wrmsr(MSR_IA32_FEATURE_CONTROL, 0);
 }
 
 static int test_vmx_feature_control(void)
-- 
2.5.0