diff mbox series

[v4,3/6] KVM: selftests: Introduce UCALL_UNHANDLED for unhandled vector reporting

Message ID 20210611011020.3420067-4-ricarkol@google.com (mailing list archive)
State New, archived
Headers show
Series KVM: selftests: arm64 exception handling and debug test | expand

Commit Message

Ricardo Koller June 11, 2021, 1:10 a.m. UTC
x86, the only arch implementing exception handling, reports unhandled
vectors using port IO at a specific port number. This replicates what
ucall already does.

Introduce a new ucall type, UCALL_UNHANDLED, for guests to report
unhandled exceptions. Then replace the x86 unhandled vector exception
reporting to use it instead of port IO.  This new ucall type will be
used in the next commits by arm64 to report unhandled vectors as well.

Tested: Forcing a page fault in the ./x86_64/xapic_ipi_test
	halter_guest_code() shows this:

	$ ./x86_64/xapic_ipi_test
	...
	  Unexpected vectored event in guest (vector:0xe)

Signed-off-by: Ricardo Koller <ricarkol@google.com>
---
 .../testing/selftests/kvm/include/kvm_util.h  |  1 +
 .../selftests/kvm/include/x86_64/processor.h  |  2 --
 .../selftests/kvm/lib/x86_64/processor.c      | 19 ++++++++-----------
 3 files changed, 9 insertions(+), 13 deletions(-)

Comments

Sean Christopherson July 29, 2021, 6:15 p.m. UTC | #1
On Thu, Jun 10, 2021, Ricardo Koller wrote:
> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> index fcd8e3855111..beb76d6deaa9 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> @@ -349,6 +349,7 @@ enum {
>  	UCALL_SYNC,
>  	UCALL_ABORT,
>  	UCALL_DONE,
> +	UCALL_UNHANDLED,
>  };

...

> @@ -1254,16 +1254,13 @@ void vm_install_exception_handler(struct kvm_vm *vm, int vector,
>  
>  void assert_on_unhandled_exception(struct kvm_vm *vm, uint32_t vcpuid)
>  {
> -	if (vcpu_state(vm, vcpuid)->exit_reason == KVM_EXIT_IO
> -		&& vcpu_state(vm, vcpuid)->io.port == UNEXPECTED_VECTOR_PORT
> -		&& vcpu_state(vm, vcpuid)->io.size == 4) {
> -		/* Grab pointer to io data */
> -		uint32_t *data = (void *)vcpu_state(vm, vcpuid)
> -			+ vcpu_state(vm, vcpuid)->io.data_offset;
> -
> -		TEST_ASSERT(false,
> -			    "Unexpected vectored event in guest (vector:0x%x)",
> -			    *data);
> +	struct ucall uc;
> +
> +	if (get_ucall(vm, vcpuid, &uc) == UCALL_UNHANDLED) {

UCALL_UNHANDLED is a bit of an odd name.  Without the surrounding context, I would
have no idea that it's referring to an unhandled event, e.g. my gut reaction would
be that it means the ucall itself was unhandled.  Maybe UCALL_UNHANDLED_EVENT?
It's rather long, but I don't think that will be problematic for any of the code.


> +		uint64_t vector = uc.args[0];
> +
> +		TEST_FAIL("Unexpected vectored event in guest (vector:0x%lx)",
> +			  vector);
>  	}
>  }
>  
> -- 
> 2.32.0.272.g935e593368-goog
>
Ricardo Koller July 30, 2021, 1:10 a.m. UTC | #2
On Thu, Jul 29, 2021 at 06:15:27PM +0000, Sean Christopherson wrote:
> On Thu, Jun 10, 2021, Ricardo Koller wrote:
> > diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> > index fcd8e3855111..beb76d6deaa9 100644
> > --- a/tools/testing/selftests/kvm/include/kvm_util.h
> > +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> > @@ -349,6 +349,7 @@ enum {
> >  	UCALL_SYNC,
> >  	UCALL_ABORT,
> >  	UCALL_DONE,
> > +	UCALL_UNHANDLED,
> >  };
> 
> ...
> 
> > @@ -1254,16 +1254,13 @@ void vm_install_exception_handler(struct kvm_vm *vm, int vector,
> >  
> >  void assert_on_unhandled_exception(struct kvm_vm *vm, uint32_t vcpuid)
> >  {
> > -	if (vcpu_state(vm, vcpuid)->exit_reason == KVM_EXIT_IO
> > -		&& vcpu_state(vm, vcpuid)->io.port == UNEXPECTED_VECTOR_PORT
> > -		&& vcpu_state(vm, vcpuid)->io.size == 4) {
> > -		/* Grab pointer to io data */
> > -		uint32_t *data = (void *)vcpu_state(vm, vcpuid)
> > -			+ vcpu_state(vm, vcpuid)->io.data_offset;
> > -
> > -		TEST_ASSERT(false,
> > -			    "Unexpected vectored event in guest (vector:0x%x)",
> > -			    *data);
> > +	struct ucall uc;
> > +
> > +	if (get_ucall(vm, vcpuid, &uc) == UCALL_UNHANDLED) {
> 
> UCALL_UNHANDLED is a bit of an odd name.  Without the surrounding context, I would
> have no idea that it's referring to an unhandled event, e.g. my gut reaction would
> be that it means the ucall itself was unhandled. Maybe UCALL_UNHANDLED_EVENT?

I see. I can send a new patch (this was commited as 75275d7fbe) with a
new name. The only name I can think of that's more descriptive would be
UCALL_UNHANDLED_EXCEPTION, but that's even longer.

> It's rather long, but I don't think that will be problematic for any of the code.
> 
> 
> > +		uint64_t vector = uc.args[0];
> > +
> > +		TEST_FAIL("Unexpected vectored event in guest (vector:0x%lx)",
> > +			  vector);
> >  	}
> >  }
> >  
> > -- 
> > 2.32.0.272.g935e593368-goog
> >
Sean Christopherson July 30, 2021, 4:24 p.m. UTC | #3
On Thu, Jul 29, 2021, Ricardo Koller wrote:
> On Thu, Jul 29, 2021 at 06:15:27PM +0000, Sean Christopherson wrote:
> > On Thu, Jun 10, 2021, Ricardo Koller wrote:
> > > +	struct ucall uc;
> > > +
> > > +	if (get_ucall(vm, vcpuid, &uc) == UCALL_UNHANDLED) {
> > 
> > UCALL_UNHANDLED is a bit of an odd name.  Without the surrounding context, I would
> > have no idea that it's referring to an unhandled event, e.g. my gut reaction would
> > be that it means the ucall itself was unhandled. Maybe UCALL_UNHANDLED_EVENT?
> 
> I see. I can send a new patch (this was commited as 75275d7fbe) with a
> new name.

Eh, no need to post another patch.  If it can be fixed up in tree, great, if not,
no big deal.

> The only name I can think of that's more descriptive would be
> UCALL_UNHANDLED_EXCEPTION, but that's even longer.

Unfortunately, EXCEPTION is incorrect as x86 will route unexpected IRQs through
this as well.
diff mbox series

Patch

diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index fcd8e3855111..beb76d6deaa9 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -349,6 +349,7 @@  enum {
 	UCALL_SYNC,
 	UCALL_ABORT,
 	UCALL_DONE,
+	UCALL_UNHANDLED,
 };
 
 #define UCALL_MAX_ARGS 6
diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
index e9f584991332..92a62c6999bc 100644
--- a/tools/testing/selftests/kvm/include/x86_64/processor.h
+++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
@@ -53,8 +53,6 @@ 
 #define CPUID_PKU		(1ul << 3)
 #define CPUID_LA57		(1ul << 16)
 
-#define UNEXPECTED_VECTOR_PORT 0xfff0u
-
 /* General Registers in 64-Bit Mode */
 struct gpr64_regs {
 	u64 rax;
diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c
index 257c5c33d04e..a217515a9bc2 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
@@ -1201,7 +1201,7 @@  static void set_idt_entry(struct kvm_vm *vm, int vector, unsigned long addr,
 
 void kvm_exit_unexpected_vector(uint32_t value)
 {
-	outl(UNEXPECTED_VECTOR_PORT, value);
+	ucall(UCALL_UNHANDLED, 1, value);
 }
 
 void route_exception(struct ex_regs *regs)
@@ -1254,16 +1254,13 @@  void vm_install_exception_handler(struct kvm_vm *vm, int vector,
 
 void assert_on_unhandled_exception(struct kvm_vm *vm, uint32_t vcpuid)
 {
-	if (vcpu_state(vm, vcpuid)->exit_reason == KVM_EXIT_IO
-		&& vcpu_state(vm, vcpuid)->io.port == UNEXPECTED_VECTOR_PORT
-		&& vcpu_state(vm, vcpuid)->io.size == 4) {
-		/* Grab pointer to io data */
-		uint32_t *data = (void *)vcpu_state(vm, vcpuid)
-			+ vcpu_state(vm, vcpuid)->io.data_offset;
-
-		TEST_ASSERT(false,
-			    "Unexpected vectored event in guest (vector:0x%x)",
-			    *data);
+	struct ucall uc;
+
+	if (get_ucall(vm, vcpuid, &uc) == UCALL_UNHANDLED) {
+		uint64_t vector = uc.args[0];
+
+		TEST_FAIL("Unexpected vectored event in guest (vector:0x%lx)",
+			  vector);
 	}
 }