[kvm-unit-tests PATCH] x86: vmx: add regression test for posted interrupts

[kvm-unit-tests PATCH] x86: vmx: add regression test for posted interrupts

[Oliver Upton]

If a guest blocks interrupts for the entirety of running in root mode
(RFLAGS.IF=0), a pending interrupt corresponding to the posted-interrupt
vector set in the VMCS should result in an interrupt posting to the vIRR
at VM-entry. However, on KVM this is not the case. The pending interrupt
is not recognized as the posted-interrupt vector and instead results in
an external interrupt VM-exit.

Add a regression test to exercise this issue.

Signed-off-by: Oliver Upton <oupton@google.com>
---
 lib/x86/asm/bitops.h |  8 +++++
 x86/vmx_tests.c      | 76 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 84 insertions(+)

diff --git a/lib/x86/asm/bitops.h b/lib/x86/asm/bitops.h
index 13a25ec9853d..ce5743538f65 100644
--- a/lib/x86/asm/bitops.h
+++ b/lib/x86/asm/bitops.h
@@ -13,4 +13,12 @@
 
 #define HAVE_BUILTIN_FLS 1
 
+static inline void test_and_set_bit(long nr, unsigned long *addr)
+{
+	asm volatile("lock; bts %1, %0"
+		     : "+m" (*addr)
+		     : "Ir" (nr)
+		     : "memory");
+}
+
 #endif
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index d2084ae9e8ce..9ba9a5d452a2 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -10430,6 +10430,81 @@ static void atomic_switch_overflow_msrs_test(void)
 		test_skip("Test is only supported on KVM");
 }
 
+#define PI_VECTOR 0xe0
+#define PI_TEST_VECTOR 0x21
+
+static void enable_posted_interrupts(void)
+{
+	void *pi_desc = alloc_page();
+
+	vmcs_set_bits(PIN_CONTROLS, PIN_POST_INTR);
+	vmcs_set_bits(EXI_CONTROLS, EXI_INTA);
+	vmcs_write(PINV, PI_VECTOR);
+	vmcs_write(POSTED_INTR_DESC_ADDR, (u64)pi_desc);
+}
+
+static unsigned long *get_pi_desc(void)
+{
+	return (unsigned long *)vmcs_read(POSTED_INTR_DESC_ADDR);
+}
+
+static void post_interrupt(u8 vector, u32 dest)
+{
+	unsigned long *pi_desc = get_pi_desc();
+
+	test_and_set_bit(vector, pi_desc);
+	test_and_set_bit(256, pi_desc);
+	apic_icr_write(PI_VECTOR, dest);
+}
+
+static struct vmx_posted_interrupt_test_args {
+	bool isr_fired;
+} vmx_posted_interrupt_test_args;
+
+static void vmx_posted_interrupt_test_isr(isr_regs_t *regs)
+{
+	volatile struct vmx_posted_interrupt_test_args *args
+			= &vmx_posted_interrupt_test_args;
+
+	args->isr_fired = true;
+	eoi();
+}
+
+static void vmx_posted_interrupt_test_guest(void)
+{
+	handle_irq(PI_TEST_VECTOR, vmx_posted_interrupt_test_isr);
+	irq_enable();
+	vmcall();
+	asm volatile("nop");
+	vmcall();
+}
+
+static void vmx_posted_interrupt_test(void)
+{
+	volatile struct vmx_posted_interrupt_test_args *args
+			= &vmx_posted_interrupt_test_args;
+
+	if (!cpu_has_apicv()) {
+		report_skip(__func__);
+		return;
+	}
+
+	enable_vid();
+	enable_posted_interrupts();
+	test_set_guest(vmx_posted_interrupt_test_guest);
+
+	enter_guest();
+	skip_exit_vmcall();
+
+	irq_disable();
+	post_interrupt(PI_TEST_VECTOR, apic_id());
+	enter_guest();
+
+	skip_exit_vmcall();
+	TEST_ASSERT(args->isr_fired);
+	enter_guest();
+}
+
 #define TEST(name) { #name, .v2 = name }
 
 /* name/init/guest_main/exit_handler/syscall_handler/guest_regs */
@@ -10533,5 +10608,6 @@ struct vmx_test vmx_tests[] = {
 	TEST(rdtsc_vmexit_diff_test),
 	TEST(vmx_mtf_test),
 	TEST(vmx_mtf_pdpte_test),
+	TEST(vmx_posted_interrupt_test),
 	{ NULL, NULL, NULL, NULL, NULL, {0} },
 };
-- 
2.28.0.806.g8561365e88-goog

Re: [kvm-unit-tests PATCH] x86: vmx: add regression test for posted interrupts

[Jim Mattson]

On Tue, Oct 6, 2020 at 2:26 PM Oliver Upton <oupton@google.com> wrote:
>
> If a guest blocks interrupts for the entirety of running in root mode
> (RFLAGS.IF=0), a pending interrupt corresponding to the posted-interrupt
> vector set in the VMCS should result in an interrupt posting to the vIRR
> at VM-entry. However, on KVM this is not the case. The pending interrupt
> is not recognized as the posted-interrupt vector and instead results in
> an external interrupt VM-exit.
>
> Add a regression test to exercise this issue.
>
> Signed-off-by: Oliver Upton <oupton@google.com>
> ---
>  lib/x86/asm/bitops.h |  8 +++++
>  x86/vmx_tests.c      | 76 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 84 insertions(+)
>
> diff --git a/lib/x86/asm/bitops.h b/lib/x86/asm/bitops.h
> index 13a25ec9853d..ce5743538f65 100644
> --- a/lib/x86/asm/bitops.h
> +++ b/lib/x86/asm/bitops.h
> @@ -13,4 +13,12 @@
>
>  #define HAVE_BUILTIN_FLS 1
>
> +static inline void test_and_set_bit(long nr, unsigned long *addr)
> +{
> +       asm volatile("lock; bts %1, %0"
> +                    : "+m" (*addr)
> +                    : "Ir" (nr)
> +                    : "memory");
> +}
> +
>  #endif
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index d2084ae9e8ce..9ba9a5d452a2 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -10430,6 +10430,81 @@ static void atomic_switch_overflow_msrs_test(void)
>                 test_skip("Test is only supported on KVM");
>  }
>
> +#define PI_VECTOR 0xe0
> +#define PI_TEST_VECTOR 0x21
> +
> +static void enable_posted_interrupts(void)
> +{
> +       void *pi_desc = alloc_page();
> +
> +       vmcs_set_bits(PIN_CONTROLS, PIN_POST_INTR);
> +       vmcs_set_bits(EXI_CONTROLS, EXI_INTA);
> +       vmcs_write(PINV, PI_VECTOR);
> +       vmcs_write(POSTED_INTR_DESC_ADDR, (u64)pi_desc);
> +}
> +
> +static unsigned long *get_pi_desc(void)
> +{
> +       return (unsigned long *)vmcs_read(POSTED_INTR_DESC_ADDR);
> +}
> +
> +static void post_interrupt(u8 vector, u32 dest)
> +{
> +       unsigned long *pi_desc = get_pi_desc();
> +
> +       test_and_set_bit(vector, pi_desc);
> +       test_and_set_bit(256, pi_desc);
> +       apic_icr_write(PI_VECTOR, dest);
> +}
> +
> +static struct vmx_posted_interrupt_test_args {
> +       bool isr_fired;
> +} vmx_posted_interrupt_test_args;
> +
> +static void vmx_posted_interrupt_test_isr(isr_regs_t *regs)
> +{
> +       volatile struct vmx_posted_interrupt_test_args *args
> +                       = &vmx_posted_interrupt_test_args;
> +
> +       args->isr_fired = true;
> +       eoi();
> +}
> +
> +static void vmx_posted_interrupt_test_guest(void)
> +{
> +       handle_irq(PI_TEST_VECTOR, vmx_posted_interrupt_test_isr);
> +       irq_enable();
> +       vmcall();
> +       asm volatile("nop");
> +       vmcall();
> +}
> +
> +static void vmx_posted_interrupt_test(void)
> +{
> +       volatile struct vmx_posted_interrupt_test_args *args
> +                       = &vmx_posted_interrupt_test_args;
> +
> +       if (!cpu_has_apicv()) {
> +               report_skip(__func__);
> +               return;
> +       }
> +
> +       enable_vid();
> +       enable_posted_interrupts();
> +       test_set_guest(vmx_posted_interrupt_test_guest);
> +
> +       enter_guest();
> +       skip_exit_vmcall();
> +
> +       irq_disable();
> +       post_interrupt(PI_TEST_VECTOR, apic_id());
> +       enter_guest();
> +
> +       skip_exit_vmcall();
> +       TEST_ASSERT(args->isr_fired);
> +       enter_guest();
> +}
> +
>  #define TEST(name) { #name, .v2 = name }
>
>  /* name/init/guest_main/exit_handler/syscall_handler/guest_regs */
> @@ -10533,5 +10608,6 @@ struct vmx_test vmx_tests[] = {
>         TEST(rdtsc_vmexit_diff_test),
>         TEST(vmx_mtf_test),
>         TEST(vmx_mtf_pdpte_test),
> +       TEST(vmx_posted_interrupt_test),
>         { NULL, NULL, NULL, NULL, NULL, {0} },
>  };
> --
> 2.28.0.806.g8561365e88-goog
>

Ping.

Re: [kvm-unit-tests PATCH] x86: vmx: add regression test for posted interrupts

[Paolo Bonzini]

On 06/10/20 23:25, Oliver Upton wrote:
> If a guest blocks interrupts for the entirety of running in root mode
> (RFLAGS.IF=0), a pending interrupt corresponding to the posted-interrupt
> vector set in the VMCS should result in an interrupt posting to the vIRR
> at VM-entry. However, on KVM this is not the case. The pending interrupt
> is not recognized as the posted-interrupt vector and instead results in
> an external interrupt VM-exit.
> 
> Add a regression test to exercise this issue.
> 
> Signed-off-by: Oliver Upton <oupton@google.com>

I am a bit confused.  Is this testing the KVM or the bare metal 
behavior?  Or was this fixed in KVM already?

Paolo

> ---
>   lib/x86/asm/bitops.h |  8 +++++
>   x86/vmx_tests.c      | 76 ++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 84 insertions(+)
> 
> diff --git a/lib/x86/asm/bitops.h b/lib/x86/asm/bitops.h
> index 13a25ec9853d..ce5743538f65 100644
> --- a/lib/x86/asm/bitops.h
> +++ b/lib/x86/asm/bitops.h
> @@ -13,4 +13,12 @@
>   
>   #define HAVE_BUILTIN_FLS 1
>   
> +static inline void test_and_set_bit(long nr, unsigned long *addr)
> +{
> +	asm volatile("lock; bts %1, %0"
> +		     : "+m" (*addr)
> +		     : "Ir" (nr)
> +		     : "memory");
> +}
> +
>   #endif
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index d2084ae9e8ce..9ba9a5d452a2 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -10430,6 +10430,81 @@ static void atomic_switch_overflow_msrs_test(void)
>   		test_skip("Test is only supported on KVM");
>   }
>   
> +#define PI_VECTOR 0xe0
> +#define PI_TEST_VECTOR 0x21
> +
> +static void enable_posted_interrupts(void)
> +{
> +	void *pi_desc = alloc_page();
> +
> +	vmcs_set_bits(PIN_CONTROLS, PIN_POST_INTR);
> +	vmcs_set_bits(EXI_CONTROLS, EXI_INTA);
> +	vmcs_write(PINV, PI_VECTOR);
> +	vmcs_write(POSTED_INTR_DESC_ADDR, (u64)pi_desc);
> +}
> +
> +static unsigned long *get_pi_desc(void)
> +{
> +	return (unsigned long *)vmcs_read(POSTED_INTR_DESC_ADDR);
> +}
> +
> +static void post_interrupt(u8 vector, u32 dest)
> +{
> +	unsigned long *pi_desc = get_pi_desc();
> +
> +	test_and_set_bit(vector, pi_desc);
> +	test_and_set_bit(256, pi_desc);
> +	apic_icr_write(PI_VECTOR, dest);
> +}
> +
> +static struct vmx_posted_interrupt_test_args {
> +	bool isr_fired;
> +} vmx_posted_interrupt_test_args;
> +
> +static void vmx_posted_interrupt_test_isr(isr_regs_t *regs)
> +{
> +	volatile struct vmx_posted_interrupt_test_args *args
> +			= &vmx_posted_interrupt_test_args;
> +
> +	args->isr_fired = true;
> +	eoi();
> +}
> +
> +static void vmx_posted_interrupt_test_guest(void)
> +{
> +	handle_irq(PI_TEST_VECTOR, vmx_posted_interrupt_test_isr);
> +	irq_enable();
> +	vmcall();
> +	asm volatile("nop");
> +	vmcall();
> +}
> +
> +static void vmx_posted_interrupt_test(void)
> +{
> +	volatile struct vmx_posted_interrupt_test_args *args
> +			= &vmx_posted_interrupt_test_args;
> +
> +	if (!cpu_has_apicv()) {
> +		report_skip(__func__);
> +		return;
> +	}
> +
> +	enable_vid();
> +	enable_posted_interrupts();
> +	test_set_guest(vmx_posted_interrupt_test_guest);
> +
> +	enter_guest();
> +	skip_exit_vmcall();
> +
> +	irq_disable();
> +	post_interrupt(PI_TEST_VECTOR, apic_id());
> +	enter_guest();
> +
> +	skip_exit_vmcall();
> +	TEST_ASSERT(args->isr_fired);
> +	enter_guest();
> +}
> +
>   #define TEST(name) { #name, .v2 = name }
>   
>   /* name/init/guest_main/exit_handler/syscall_handler/guest_regs */
> @@ -10533,5 +10608,6 @@ struct vmx_test vmx_tests[] = {
>   	TEST(rdtsc_vmexit_diff_test),
>   	TEST(vmx_mtf_test),
>   	TEST(vmx_mtf_pdpte_test),
> +	TEST(vmx_posted_interrupt_test),
>   	{ NULL, NULL, NULL, NULL, NULL, {0} },
>   };
> 

Re: [kvm-unit-tests PATCH] x86: vmx: add regression test for posted interrupts

[Oliver Upton]

On Fri, Dec 11, 2020 at 5:20 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 06/10/20 23:25, Oliver Upton wrote:
> > If a guest blocks interrupts for the entirety of running in root mode
> > (RFLAGS.IF=0), a pending interrupt corresponding to the posted-interrupt
> > vector set in the VMCS should result in an interrupt posting to the vIRR
> > at VM-entry. However, on KVM this is not the case. The pending interrupt
> > is not recognized as the posted-interrupt vector and instead results in
> > an external interrupt VM-exit.
> >
> > Add a regression test to exercise this issue.
> >
> > Signed-off-by: Oliver Upton <oupton@google.com>
>
> I am a bit confused.  Is this testing the KVM or the bare metal
> behavior?  Or was this fixed in KVM already?

This is a directed test case for
25bb2cf97139 ("KVM: nVMX: Morph notification vector IRQ on nested
VM-Enter to pending PI")

My local version of this patch has changed a bit. I'll send a v2 shortly.

--
Thanks,
Oliver

>
> Paolo
>
> > ---
> >   lib/x86/asm/bitops.h |  8 +++++
> >   x86/vmx_tests.c      | 76 ++++++++++++++++++++++++++++++++++++++++++++
> >   2 files changed, 84 insertions(+)
> >
> > diff --git a/lib/x86/asm/bitops.h b/lib/x86/asm/bitops.h
> > index 13a25ec9853d..ce5743538f65 100644
> > --- a/lib/x86/asm/bitops.h
> > +++ b/lib/x86/asm/bitops.h
> > @@ -13,4 +13,12 @@
> >
> >   #define HAVE_BUILTIN_FLS 1
> >
> > +static inline void test_and_set_bit(long nr, unsigned long *addr)
> > +{
> > +     asm volatile("lock; bts %1, %0"
> > +                  : "+m" (*addr)
> > +                  : "Ir" (nr)
> > +                  : "memory");
> > +}
> > +
> >   #endif
> > diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> > index d2084ae9e8ce..9ba9a5d452a2 100644
> > --- a/x86/vmx_tests.c
> > +++ b/x86/vmx_tests.c
> > @@ -10430,6 +10430,81 @@ static void atomic_switch_overflow_msrs_test(void)
> >               test_skip("Test is only supported on KVM");
> >   }
> >
> > +#define PI_VECTOR 0xe0
> > +#define PI_TEST_VECTOR 0x21
> > +
> > +static void enable_posted_interrupts(void)
> > +{
> > +     void *pi_desc = alloc_page();
> > +
> > +     vmcs_set_bits(PIN_CONTROLS, PIN_POST_INTR);
> > +     vmcs_set_bits(EXI_CONTROLS, EXI_INTA);
> > +     vmcs_write(PINV, PI_VECTOR);
> > +     vmcs_write(POSTED_INTR_DESC_ADDR, (u64)pi_desc);
> > +}
> > +
> > +static unsigned long *get_pi_desc(void)
> > +{
> > +     return (unsigned long *)vmcs_read(POSTED_INTR_DESC_ADDR);
> > +}
> > +
> > +static void post_interrupt(u8 vector, u32 dest)
> > +{
> > +     unsigned long *pi_desc = get_pi_desc();
> > +
> > +     test_and_set_bit(vector, pi_desc);
> > +     test_and_set_bit(256, pi_desc);
> > +     apic_icr_write(PI_VECTOR, dest);
> > +}
> > +
> > +static struct vmx_posted_interrupt_test_args {
> > +     bool isr_fired;
> > +} vmx_posted_interrupt_test_args;
> > +
> > +static void vmx_posted_interrupt_test_isr(isr_regs_t *regs)
> > +{
> > +     volatile struct vmx_posted_interrupt_test_args *args
> > +                     = &vmx_posted_interrupt_test_args;
> > +
> > +     args->isr_fired = true;
> > +     eoi();
> > +}
> > +
> > +static void vmx_posted_interrupt_test_guest(void)
> > +{
> > +     handle_irq(PI_TEST_VECTOR, vmx_posted_interrupt_test_isr);
> > +     irq_enable();
> > +     vmcall();
> > +     asm volatile("nop");
> > +     vmcall();
> > +}
> > +
> > +static void vmx_posted_interrupt_test(void)
> > +{
> > +     volatile struct vmx_posted_interrupt_test_args *args
> > +                     = &vmx_posted_interrupt_test_args;
> > +
> > +     if (!cpu_has_apicv()) {
> > +             report_skip(__func__);
> > +             return;
> > +     }
> > +
> > +     enable_vid();
> > +     enable_posted_interrupts();
> > +     test_set_guest(vmx_posted_interrupt_test_guest);
> > +
> > +     enter_guest();
> > +     skip_exit_vmcall();
> > +
> > +     irq_disable();
> > +     post_interrupt(PI_TEST_VECTOR, apic_id());
> > +     enter_guest();
> > +
> > +     skip_exit_vmcall();
> > +     TEST_ASSERT(args->isr_fired);
> > +     enter_guest();
> > +}
> > +
> >   #define TEST(name) { #name, .v2 = name }
> >
> >   /* name/init/guest_main/exit_handler/syscall_handler/guest_regs */
> > @@ -10533,5 +10608,6 @@ struct vmx_test vmx_tests[] = {
> >       TEST(rdtsc_vmexit_diff_test),
> >       TEST(vmx_mtf_test),
> >       TEST(vmx_mtf_pdpte_test),
> > +     TEST(vmx_posted_interrupt_test),
> >       { NULL, NULL, NULL, NULL, NULL, {0} },
> >   };
> >
>

Re: [kvm-unit-tests PATCH] x86: vmx: add regression test for posted interrupts

[Paolo Bonzini]

On 12/12/20 00:41, Oliver Upton wrote:
> On Fri, Dec 11, 2020 at 5:20 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> On 06/10/20 23:25, Oliver Upton wrote:
>>> If a guest blocks interrupts for the entirety of running in root mode
>>> (RFLAGS.IF=0), a pending interrupt corresponding to the posted-interrupt
>>> vector set in the VMCS should result in an interrupt posting to the vIRR
>>> at VM-entry. However, on KVM this is not the case. The pending interrupt
>>> is not recognized as the posted-interrupt vector and instead results in
>>> an external interrupt VM-exit.
>>>
>>> Add a regression test to exercise this issue.
>>>
>>> Signed-off-by: Oliver Upton <oupton@google.com>
>>
>> I am a bit confused.  Is this testing the KVM or the bare metal
>> behavior?  Or was this fixed in KVM already?
> 
> This is a directed test case for
> 25bb2cf97139 ("KVM: nVMX: Morph notification vector IRQ on nested
> VM-Enter to pending PI")

Ok, thanks.  However, the patch currently fails like this:

	Test suite: vmx_posted_interrupt_test
	VM-Fail on vmlaunch: error number is 7. See Intel 30.4.

I haven't debugged it, so for now I suggest that you just move it to a 
separate suite.

Paolo

Re: [kvm-unit-tests PATCH] x86: vmx: add regression test for posted interrupts

[Paolo Bonzini]

On 12/12/20 01:24, Oliver Upton wrote:
>     Ok, thanks.  However, the patch currently fails like this:
> 
>              Test suite: vmx_posted_interrupt_test
>              VM-Fail on vmlaunch: error number is 7. See Intel 30.4.
> 
>     I haven't debugged it, so for now I suggest that you just move it to a
>     separate suite.
> 
> 
> 
> What commit are you on? Not reproducing for me.

kvm-unit-tests master + this patch

Linux kvm/queue

It works if I just invoke the test, but not with "./run_tests.sh vmx" 
or, equivalently

./x86/run x86/vmx.flat -smp 1 -cpu host,+vmx -append 
"-exit_monitor_from_l2_test -ept_access* -vmx_smp* -vmx_vmcs_shadow_test 
-atomic_switch_overflow_msrs_test -vmx_init_signal_test 
-vmx_apic_passthrough_tpr_threshold_test

Paolo