Message ID | 914ca67ac7207c5c13cc2737b654ac2899f58270.1402842281.git.jan.kiszka@web.de (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Jan Kiszka <jan.kiszka@web.de> writes: > From: Jan Kiszka <jan.kiszka@siemens.com> > > 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@siemens.com> > --- > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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; }