Skip to contentSkip to conversation

svm: Verify the effect of V_INTR_MASKING on INTR interrupts

PATCH Superseded Cathy Avery 2 participants 2 messages Dec 3, 2019 KVM

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 svm: Verify the effect of V_INTR_MASKING on INTR interrupts

The test confirms the influence of the V_INTR_MASKING bit on RFLAGS.IF. The expectation is while running a guest with V_INTR_MASKING cleared to zero:

- EFLAGS.IF controls both virtual and physical interrupts.

While running a guest with V_INTR_MASKING set to 1:

- The host EFLAGS.IF at the time of the VMRUN is saved and controls physical interrupts while the guest is running.

- The guest value of EFLAGS.IF controls virtual interrupts only.

As discussed previously, this patch also modifies the vmrun loop ( test_run ) to allow running with HIF=0

---
 x86/svm.c | 106 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 104 insertions(+), 2 deletions(-)
x86/svm.c +104 -2
index 0360d8d..fb5796f 100644
@@ -44,6 +44,8 @@ u64 runs;
 u8 *io_bitmap;
 u8 io_bitmap_area[16384];
 
+u64 set_host_if = 1;
+
 #define MSR_BITMAP_SIZE 8192
 
 u8 *msr_bitmap;
@@ -266,21 +268,24 @@ static void test_run(struct test *test, struct vmcb *vmcb)
         tsc_start = rdtsc();
         asm volatile (
             "clgi \n\t"
+            "cmp $0, set_host_if\n\t"
+            "jz 1f\n\t"
+            "sti \n\t"
+            "1: \n\t"
             "vmload \n\t"
             "mov regs+0x80, %%r15\n\t"  // rflags
             "mov %%r15, 0x170(%0)\n\t"
             "mov regs, %%r15\n\t"       // rax
             "mov %%r15, 0x1f8(%0)\n\t"
             LOAD_GPR_C
-            "sti \n\t"		// only used if V_INTR_MASKING=1
             "vmrun \n\t"
-            "cli \n\t"
             SAVE_GPR_C
             "mov 0x170(%0), %%r15\n\t"  // rflags
             "mov %%r15, regs+0x80\n\t"
             "mov 0x1f8(%0), %%r15\n\t"  // rax
             "mov %%r15, regs\n\t"
             "vmsave \n\t"
+            "cli \n\t"
             "stgi"
             : : "a"(vmcb_phys)
             : "rbx", "rcx", "rdx", "rsi",
@@ -307,6 +312,7 @@ static bool default_supported(void)
 static void default_prepare(struct test *test)
 {
     vmcb_ident(test->vmcb);
+    cli();
 }
 
 static bool default_finished(struct test *test)
@@ -1386,6 +1392,99 @@ static bool pending_event_check(struct test *test)
     return get_test_stage(test) == 2;
 }
 
+static void pending_event_prepare_vmask(struct test *test)
+{
+    default_prepare(test);
+
+    pending_event_ipi_fired = false;
+
+    set_host_if = 0;
+
+    handle_irq(0xf1, pending_event_ipi_isr);
+
+    apic_icr_write(APIC_DEST_SELF | APIC_DEST_PHYSICAL |
+              APIC_DM_FIXED | 0xf1, 0);
+
+    set_test_stage(test, 0);
+}
+
+static void pending_event_test_vmask(struct test *test)
+{
+    if (pending_event_ipi_fired == true) {
+        set_test_stage(test, -1);
+        report("Interrupt preceeded guest", false);
+        vmmcall();
+    }
+
+    irq_enable();
+    asm volatile ("nop");
+    irq_disable();
+
+    if (pending_event_ipi_fired != true) {
+        set_test_stage(test, -1);
+        report("Interrupt not triggered by guest", false);
+    }
+
+    vmmcall();
+
+    irq_enable();
+    asm volatile ("nop");
+    irq_disable();
+}
+
+static bool pending_event_finished_vmask(struct test *test)
+{
+    if ( test->vmcb->control.exit_code != SVM_EXIT_VMMCALL) {
+        report("VM_EXIT return to host is not EXIT_VMMCALL exit reason 0x%x", false,
+                test->vmcb->control.exit_code);
+        return true;
+    }
+
+    switch (get_test_stage(test)) {
+    case 0:
+        test->vmcb->save.rip += 3;
+
+        pending_event_ipi_fired = false;
+
+        test->vmcb->control.int_ctl |= V_INTR_MASKING_MASK;
+
+        apic_icr_write(APIC_DEST_SELF | APIC_DEST_PHYSICAL |
+              APIC_DM_FIXED | 0xf1, 0);
+
+        break;
+
+    case 1:
+        if (pending_event_ipi_fired == true) {
+            report("Interrupt triggered by guest", false);
+            return true;
+        }
+
+        irq_enable();
+        asm volatile ("nop");
+        irq_disable();
+
+        if (pending_event_ipi_fired != true) {
+            report("Interrupt not triggered by host", false);
+            return true;
+        }
+
+        break;
+
+    default:
+        return true;
+    }
+
+    inc_test_stage(test);
+
+    return get_test_stage(test) == 2;
+}
+
+static bool pending_event_check_vmask(struct test *test)
+{
+    set_host_if = 1;
+    return get_test_stage(test) == 2;
+}
+
 static struct test tests[] = {
     { "null", default_supported, default_prepare, null_test,
       default_finished, null_check },
@@ -1438,6 +1537,9 @@ static struct test tests[] = {
       lat_svm_insn_finished, lat_svm_insn_check },
     { "pending_event", default_supported, pending_event_prepare,
       pending_event_test, pending_event_finished, pending_event_check },
+    { "pending_event_vmask", default_supported, pending_event_prepare_vmask,
+      pending_event_test_vmask, pending_event_finished_vmask,
+      pending_event_check_vmask },
 };
 
 int main(int ac, char **av)
Signature · 1 line
2.20.1
maintainer
#1 lore raw
Quoted 8 lines · Cathy Avery

The test confirms the influence of the V_INTR_MASKING bit on RFLAGS.IF. The expectation is while running a guest with V_INTR_MASKING cleared to zero:

- EFLAGS.IF controls both virtual and physical interrupts.

While running a guest with V_INTR_MASKING set to 1:

Quoted 7 lines

- The host EFLAGS.IF at the time of the VMRUN is saved and controls physical interrupts while the guest is running.

- The guest value of EFLAGS.IF controls virtual interrupts only.

As discussed previously, this patch also modifies the vmrun loop ( test_run ) to allow running with HIF=0

Looks good, just a couple nits below.

Quoted 10 lines
---
 x86/svm.c | 106 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 104 insertions(+), 2 deletions(-)
x86/svm.c +0 -0
index 0360d8d..fb5796f 100644

x86/svm.c u64 runs; · line 47

Quoted 4 lines
x86/svm.c +1 -0
@@ -44,6 +44,8 @@ u64 runs;
 u8 *io_bitmap;
 u8 io_bitmap_area[16384];
 
+u64 set_host_if = 1;

"u8 set_host_if;" here

x86/svm.c static void test_run(struct test *test, struct vmcb *vmcb)

Quoted 5 lines
x86/svm.c +1 -0
+
 #define MSR_BITMAP_SIZE 8192
 
 u8 *msr_bitmap;
@@ -266,21 +268,24 @@ static void test_run(struct test *test, struct vmcb *vmcb)

... "set_host_if = 1;" before invoking the ->prepare function...

x86/svm.c static void test_run(struct test *test, struct vmcb *vmcb) · line 271

Quoted 4 lines
x86/svm.c +1 -0
         tsc_start = rdtsc();
         asm volatile (
             "clgi \n\t"
+            "cmp $0, set_host_if\n\t"

... and cmpb here.

Quoted 23 lines
x86/svm.c +4 -2
+            "jz 1f\n\t"
+            "sti \n\t"
+            "1: \n\t"
             "vmload \n\t"
             "mov regs+0x80, %%r15\n\t"  // rflags
             "mov %%r15, 0x170(%0)\n\t"
             "mov regs, %%r15\n\t"       // rax
             "mov %%r15, 0x1f8(%0)\n\t"
             LOAD_GPR_C
-            "sti \n\t"		// only used if V_INTR_MASKING=1
             "vmrun \n\t"
-            "cli \n\t"
             SAVE_GPR_C
             "mov 0x170(%0), %%r15\n\t"  // rflags
             "mov %%r15, regs+0x80\n\t"
             "mov 0x1f8(%0), %%r15\n\t"  // rax
             "mov %%r15, regs\n\t"
             "vmsave \n\t"
+            "cli \n\t"
             "stgi"
             : : "a"(vmcb_phys)
             : "rbx", "rcx", "rdx", "rsi",

x86/svm.c static bool default_supported(void) · line 315

Quoted 4 lines
x86/svm.c +1 -0
@@ -307,6 +312,7 @@ static bool default_supported(void)
 static void default_prepare(struct test *test)
 {
     vmcb_ident(test->vmcb);
+    cli();

I think this is not needed, perhaps it's a relic of rebasing your previous patch?

Thanks,

Paolo

Quoted 114 lines
x86/svm.c +96 -0
 }
 
 static bool default_finished(struct test *test)
@@ -1386,6 +1392,99 @@ static bool pending_event_check(struct test *test)
     return get_test_stage(test) == 2;
 }
 
+static void pending_event_prepare_vmask(struct test *test)
+{
+    default_prepare(test);
+
+    pending_event_ipi_fired = false;
+
+    set_host_if = 0;
+
+    handle_irq(0xf1, pending_event_ipi_isr);
+
+    apic_icr_write(APIC_DEST_SELF | APIC_DEST_PHYSICAL |
+              APIC_DM_FIXED | 0xf1, 0);
+
+    set_test_stage(test, 0);
+}
+
+static void pending_event_test_vmask(struct test *test)
+{
+    if (pending_event_ipi_fired == true) {
+        set_test_stage(test, -1);
+        report("Interrupt preceeded guest", false);
+        vmmcall();
+    }
+
+    irq_enable();
+    asm volatile ("nop");
+    irq_disable();
+
+    if (pending_event_ipi_fired != true) {
+        set_test_stage(test, -1);
+        report("Interrupt not triggered by guest", false);
+    }
+
+    vmmcall();
+
+    irq_enable();
+    asm volatile ("nop");
+    irq_disable();
+}
+
+static bool pending_event_finished_vmask(struct test *test)
+{
+    if ( test->vmcb->control.exit_code != SVM_EXIT_VMMCALL) {
+        report("VM_EXIT return to host is not EXIT_VMMCALL exit reason 0x%x", false,
+                test->vmcb->control.exit_code);
+        return true;
+    }
+
+    switch (get_test_stage(test)) {
+    case 0:
+        test->vmcb->save.rip += 3;
+
+        pending_event_ipi_fired = false;
+
+        test->vmcb->control.int_ctl |= V_INTR_MASKING_MASK;
+
+        apic_icr_write(APIC_DEST_SELF | APIC_DEST_PHYSICAL |
+              APIC_DM_FIXED | 0xf1, 0);
+
+        break;
+
+    case 1:
+        if (pending_event_ipi_fired == true) {
+            report("Interrupt triggered by guest", false);
+            return true;
+        }
+
+        irq_enable();
+        asm volatile ("nop");
+        irq_disable();
+
+        if (pending_event_ipi_fired != true) {
+            report("Interrupt not triggered by host", false);
+            return true;
+        }
+
+        break;
+
+    default:
+        return true;
+    }
+
+    inc_test_stage(test);
+
+    return get_test_stage(test) == 2;
+}
+
+static bool pending_event_check_vmask(struct test *test)
+{
+    set_host_if = 1;
+    return get_test_stage(test) == 2;
+}
+
 static struct test tests[] = {
     { "null", default_supported, default_prepare, null_test,
       default_finished, null_check },
@@ -1438,6 +1537,9 @@ static struct test tests[] = {
       lat_svm_insn_finished, lat_svm_insn_check },
     { "pending_event", default_supported, pending_event_prepare,
       pending_event_test, pending_event_finished, pending_event_check },
+    { "pending_event_vmask", default_supported, pending_event_prepare_vmask,
+      pending_event_test_vmask, pending_event_finished_vmask,
+      pending_event_check_vmask },
 };
 
 int main(int ac, char **av)