Jan Kiszka <jan.kiszka@xxxxxx> writes:
> From: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
>
> Consistently make sure we are not affected by any compiler reordering
> when evaluating the current stage.
Should we prevent accidental calls to the variable directly by moving
get/set to vmx.c or a separate file in lib/x86 altogether ?
> Signed-off-by: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
> ---
> x86/vmx_tests.c | 80 ++++++++++++++++++++++++++++-----------------------------
> 1 file changed, 40 insertions(+), 40 deletions(-)
>
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index d0ce365..bf7aa2c 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -415,13 +415,13 @@ static void cr_shadowing_main()
> // Test read through
> set_stage(0);
> guest_cr0 = read_cr0();
> - if (stage == 1)
> + if (get_stage() == 1)
> report("Read through CR0", 0);
> else
> vmcall();
> set_stage(1);
> guest_cr4 = read_cr4();
> - if (stage == 2)
> + if (get_stage() == 2)
> report("Read through CR4", 0);
> else
> vmcall();
> @@ -430,13 +430,13 @@ static void cr_shadowing_main()
> guest_cr4 = guest_cr4 ^ (X86_CR4_TSD | X86_CR4_DE);
> set_stage(2);
> write_cr0(guest_cr0);
> - if (stage == 3)
> + if (get_stage() == 3)
> report("Write throuth CR0", 0);
> else
> vmcall();
> set_stage(3);
> write_cr4(guest_cr4);
> - if (stage == 4)
> + if (get_stage() == 4)
> report("Write through CR4", 0);
> else
> vmcall();
> @@ -444,7 +444,7 @@ static void cr_shadowing_main()
> set_stage(4);
> vmcall();
> cr0 = read_cr0();
> - if (stage != 5) {
> + if (get_stage() != 5) {
> if (cr0 == guest_cr0)
> report("Read shadowing CR0", 1);
> else
> @@ -452,7 +452,7 @@ static void cr_shadowing_main()
> }
> set_stage(5);
> cr4 = read_cr4();
> - if (stage != 6) {
> + if (get_stage() != 6) {
> if (cr4 == guest_cr4)
> report("Read shadowing CR4", 1);
> else
> @@ -461,13 +461,13 @@ static void cr_shadowing_main()
> // Test write shadow (same value with shadow)
> set_stage(6);
> write_cr0(guest_cr0);
> - if (stage == 7)
> + if (get_stage() == 7)
> report("Write shadowing CR0 (same value with shadow)", 0);
> else
> vmcall();
> set_stage(7);
> write_cr4(guest_cr4);
> - if (stage == 8)
> + if (get_stage() == 8)
> report("Write shadowing CR4 (same value with shadow)", 0);
> else
> vmcall();
> @@ -478,7 +478,7 @@ static void cr_shadowing_main()
> "mov %%rsi, %%cr0\n\t"
> ::"m"(tmp)
> :"rsi", "memory", "cc");
> - if (stage != 9)
> + if (get_stage() != 9)
> report("Write shadowing different X86_CR0_TS", 0);
> else
> report("Write shadowing different X86_CR0_TS", 1);
> @@ -488,7 +488,7 @@ static void cr_shadowing_main()
> "mov %%rsi, %%cr0\n\t"
> ::"m"(tmp)
> :"rsi", "memory", "cc");
> - if (stage != 10)
> + if (get_stage() != 10)
> report("Write shadowing different X86_CR0_MP", 0);
> else
> report("Write shadowing different X86_CR0_MP", 1);
> @@ -498,7 +498,7 @@ static void cr_shadowing_main()
> "mov %%rsi, %%cr4\n\t"
> ::"m"(tmp)
> :"rsi", "memory", "cc");
> - if (stage != 11)
> + if (get_stage() != 11)
> report("Write shadowing different X86_CR4_TSD", 0);
> else
> report("Write shadowing different X86_CR4_TSD", 1);
> @@ -508,7 +508,7 @@ static void cr_shadowing_main()
> "mov %%rsi, %%cr4\n\t"
> ::"m"(tmp)
> :"rsi", "memory", "cc");
> - if (stage != 12)
> + if (get_stage() != 12)
> report("Write shadowing different X86_CR4_DE", 0);
> else
> report("Write shadowing different X86_CR4_DE", 1);
> @@ -584,31 +584,31 @@ static int cr_shadowing_exit_handler()
> switch (get_stage()) {
> case 4:
> report("Read shadowing CR0", 0);
> - set_stage(stage + 1);
> + set_stage(get_stage() + 1);
> break;
> case 5:
> report("Read shadowing CR4", 0);
> - set_stage(stage + 1);
> + set_stage(get_stage() + 1);
> break;
> case 6:
> report("Write shadowing CR0 (same value)", 0);
> - set_stage(stage + 1);
> + set_stage(get_stage() + 1);
> break;
> case 7:
> report("Write shadowing CR4 (same value)", 0);
> - set_stage(stage + 1);
> + set_stage(get_stage() + 1);
> break;
> case 8:
> case 9:
> // 0x600 encodes "mov %esi, %cr0"
> if (exit_qual == 0x600)
> - set_stage(stage + 1);
> + set_stage(get_stage() + 1);
> break;
> case 10:
> case 11:
> // 0x604 encodes "mov %esi, %cr4"
> if (exit_qual == 0x604)
> - set_stage(stage + 1);
> + set_stage(get_stage() + 1);
> break;
> default:
> // Should not reach here
> @@ -648,7 +648,7 @@ static void iobmp_main()
> set_stage(0);
> inb(0x5000);
> outb(0x0, 0x5000);
> - if (stage != 0)
> + if (get_stage() != 0)
> report("I/O bitmap - I/O pass", 0);
> else
> report("I/O bitmap - I/O pass", 1);
> @@ -656,39 +656,39 @@ static void iobmp_main()
> ((u8 *)io_bitmap_a)[0] = 0xFF;
> set_stage(2);
> inb(0x0);
> - if (stage != 3)
> + if (get_stage() != 3)
> report("I/O bitmap - trap in", 0);
> else
> report("I/O bitmap - trap in", 1);
> set_stage(3);
> outw(0x0, 0x0);
> - if (stage != 4)
> + if (get_stage() != 4)
> report("I/O bitmap - trap out", 0);
> else
> report("I/O bitmap - trap out", 1);
> set_stage(4);
> inl(0x0);
> - if (stage != 5)
> + if (get_stage() != 5)
> report("I/O bitmap - I/O width, long", 0);
> // test low/high IO port
> set_stage(5);
> ((u8 *)io_bitmap_a)[0x5000 / 8] = (1 << (0x5000 % 8));
> inb(0x5000);
> - if (stage == 6)
> + if (get_stage() == 6)
> report("I/O bitmap - I/O port, low part", 1);
> else
> report("I/O bitmap - I/O port, low part", 0);
> set_stage(6);
> ((u8 *)io_bitmap_b)[0x1000 / 8] = (1 << (0x1000 % 8));
> inb(0x9000);
> - if (stage == 7)
> + if (get_stage() == 7)
> report("I/O bitmap - I/O port, high part", 1);
> else
> report("I/O bitmap - I/O port, high part", 0);
> // test partial pass
> set_stage(7);
> inl(0x4FFF);
> - if (stage == 8)
> + if (get_stage() == 8)
> report("I/O bitmap - partial pass", 1);
> else
> report("I/O bitmap - partial pass", 0);
> @@ -697,18 +697,18 @@ static void iobmp_main()
> memset(io_bitmap_a, 0x0, PAGE_SIZE);
> memset(io_bitmap_b, 0x0, PAGE_SIZE);
> inl(0xFFFF);
> - if (stage == 9)
> + if (get_stage() == 9)
> report("I/O bitmap - overrun", 1);
> else
> report("I/O bitmap - overrun", 0);
> set_stage(9);
> vmcall();
> outb(0x0, 0x0);
> - report("I/O bitmap - ignore unconditional exiting", stage == 9);
> + report("I/O bitmap - ignore unconditional exiting", get_stage() == 9);
> set_stage(10);
> vmcall();
> outb(0x0, 0x0);
> - report("I/O bitmap - unconditional exiting", stage == 11);
> + report("I/O bitmap - unconditional exiting", get_stage() == 11);
> }
>
> static int iobmp_exit_handler()
> @@ -726,7 +726,7 @@ static int iobmp_exit_handler()
> switch (get_stage()) {
> case 0:
> case 1:
> - set_stage(stage + 1);
> + set_stage(get_stage() + 1);
> break;
> case 2:
> if ((exit_qual & VMX_IO_SIZE_MASK) != _VMX_IO_BYTE)
> @@ -737,7 +737,7 @@ static int iobmp_exit_handler()
> report("I/O bitmap - I/O direction, in", 0);
> else
> report("I/O bitmap - I/O direction, in", 1);
> - set_stage(stage + 1);
> + set_stage(get_stage() + 1);
> break;
> case 3:
> if ((exit_qual & VMX_IO_SIZE_MASK) != _VMX_IO_WORD)
> @@ -748,36 +748,36 @@ static int iobmp_exit_handler()
> report("I/O bitmap - I/O direction, out", 1);
> else
> report("I/O bitmap - I/O direction, out", 0);
> - set_stage(stage + 1);
> + set_stage(get_stage() + 1);
> break;
> case 4:
> if ((exit_qual & VMX_IO_SIZE_MASK) != _VMX_IO_LONG)
> report("I/O bitmap - I/O width, long", 0);
> else
> report("I/O bitmap - I/O width, long", 1);
> - set_stage(stage + 1);
> + set_stage(get_stage() + 1);
> break;
> case 5:
> if (((exit_qual & VMX_IO_PORT_MASK) >> VMX_IO_PORT_SHIFT) == 0x5000)
> - set_stage(stage + 1);
> + set_stage(get_stage() + 1);
> break;
> case 6:
> if (((exit_qual & VMX_IO_PORT_MASK) >> VMX_IO_PORT_SHIFT) == 0x9000)
> - set_stage(stage + 1);
> + set_stage(get_stage() + 1);
> break;
> case 7:
> if (((exit_qual & VMX_IO_PORT_MASK) >> VMX_IO_PORT_SHIFT) == 0x4FFF)
> - set_stage(stage + 1);
> + set_stage(get_stage() + 1);
> break;
> case 8:
> if (((exit_qual & VMX_IO_PORT_MASK) >> VMX_IO_PORT_SHIFT) == 0xFFFF)
> - set_stage(stage + 1);
> + set_stage(get_stage() + 1);
> break;
> case 9:
> case 10:
> ctrl_cpu0 = vmcs_read(CPU_EXEC_CTRL0);
> vmcs_write(CPU_EXEC_CTRL0, ctrl_cpu0 & ~CPU_IO);
> - set_stage(stage + 1);
> + set_stage(get_stage() + 1);
> break;
> default:
> // Should not reach here
> @@ -948,13 +948,13 @@ static void insn_intercept_main()
> case INSN_CPU0:
> case INSN_CPU1:
> case INSN_ALWAYS_TRAP:
> - if (stage != cur_insn + 1)
> + if (get_stage() != cur_insn + 1)
> report(insn_table[cur_insn].name, 0);
> else
> report(insn_table[cur_insn].name, 1);
> break;
> case INSN_NEVER_TRAP:
> - if (stage == cur_insn + 1)
> + if (get_stage() == cur_insn + 1)
> report(insn_table[cur_insn].name, 0);
> else
> report(insn_table[cur_insn].name, 1);
> @@ -985,7 +985,7 @@ static int insn_intercept_exit_handler()
> if (insn_table[cur_insn].test_field & FIELD_INSN_INFO)
> pass = pass && insn_table[cur_insn].insn_info == insn_info;
> if (pass)
> - set_stage(stage + 1);
> + set_stage(get_stage() + 1);
> vmcs_write(GUEST_RIP, guest_rip + insn_len);
> return VMX_TEST_RESUME;
> }
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
