On Thu, Aug 15, 2013 at 3:40 PM, Jan Kiszka <jan.kiszka@xxxxxx> wrote:
> On 2013-08-13 17:56, Arthur Chunqi Li wrote:
>> Add test cases for I/O bitmaps, including corner cases.
>
> Would be good to briefly list the corner cases here.
>
>>
>> Signed-off-by: Arthur Chunqi Li <yzt356@xxxxxxxxx>
>> ---
>> x86/vmx.h | 6 +-
>> x86/vmx_tests.c | 167 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 170 insertions(+), 3 deletions(-)
>>
>> diff --git a/x86/vmx.h b/x86/vmx.h
>> index 18961f1..dba8b20 100644
>> --- a/x86/vmx.h
>> +++ b/x86/vmx.h
>> @@ -417,15 +417,15 @@ enum Ctrl1 {
>> "popf\n\t"
>>
>> #define VMX_IO_SIZE_MASK 0x7
>> -#define _VMX_IO_BYTE 1
>> -#define _VMX_IO_WORD 2
>> +#define _VMX_IO_BYTE 0
>> +#define _VMX_IO_WORD 1
>> #define _VMX_IO_LONG 3
>> #define VMX_IO_DIRECTION_MASK (1ul << 3)
>> #define VMX_IO_IN (1ul << 3)
>> #define VMX_IO_OUT 0
>> #define VMX_IO_STRING (1ul << 4)
>> #define VMX_IO_REP (1ul << 5)
>> -#define VMX_IO_OPRAND_DX (1ul << 6)
>> +#define VMX_IO_OPRAND_IMM (1ul << 6)
>> #define VMX_IO_PORT_MASK 0xFFFF0000
>> #define VMX_IO_PORT_SHIFT 16
>>
>> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
>> index 44be3f4..ad28c4c 100644
>> --- a/x86/vmx_tests.c
>> +++ b/x86/vmx_tests.c
>> @@ -2,10 +2,13 @@
>> #include "msr.h"
>> #include "processor.h"
>> #include "vm.h"
>> +#include "io.h"
>>
>> u64 ia32_pat;
>> u64 ia32_efer;
>> u32 stage;
>> +void *io_bitmap_a, *io_bitmap_b;
>> +u16 ioport;
>>
>> static inline void vmcall()
>> {
>> @@ -473,6 +476,168 @@ static int cr_shadowing_exit_handler()
>> return VMX_TEST_VMEXIT;
>> }
>>
>> +static void iobmp_init()
>> +{
>> + u32 ctrl_cpu0;
>> +
>> + io_bitmap_a = alloc_page();
>> + io_bitmap_a = alloc_page();
>> + memset(io_bitmap_a, 0x0, PAGE_SIZE);
>> + memset(io_bitmap_b, 0x0, PAGE_SIZE);
>> + ctrl_cpu0 = vmcs_read(CPU_EXEC_CTRL0);
>> + ctrl_cpu0 |= CPU_IO_BITMAP;
>> + ctrl_cpu0 &= (~CPU_IO);
>> + vmcs_write(CPU_EXEC_CTRL0, ctrl_cpu0);
>> + vmcs_write(IO_BITMAP_A, (u64)io_bitmap_a);
>> + vmcs_write(IO_BITMAP_B, (u64)io_bitmap_b);
>> +}
>> +
>> +static void iobmp_main()
>> +{
>> +/*
>> + data = (u8 *)io_bitmap_b;
>> + ioport = 0xffff;
>> + data[(ioport - 0x8000) /8] |= (1 << (ioport % 8));
>> + inb(ioport);
>> + outb(0, ioport);
>> +*/
>
> Forgotten debug code?
>
>> + // stage 0, test IO pass
>> + set_stage(0);
>> + inb(0x5000);
>> + outb(0x0, 0x5000);
>> + if (stage != 0)
>> + report("I/O bitmap - I/O pass", 0);
>> + else
>> + report("I/O bitmap - I/O pass", 1);
>> + // test IO width, in/out
>> + ((u8 *)io_bitmap_a)[0] = 0xFF;
>> + set_stage(2);
>> + inb(0x0);
>> + if (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)
>> + report("I/O bitmap - trap out", 0);
>> + else
>> + report("I/O bitmap - trap out", 1);
>> + set_stage(4);
>> + inl(0x0);
>
> Forgot to check the progress?
>
>> + // test low/high IO port
>> + set_stage(5);
>> + ((u8 *)io_bitmap_a)[0x5000 / 8] = (1 << (0x5000 % 8));
>> + inb(0x5000);
>> + if (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)
>> + 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)
>> + report("I/O bitmap - partial pass", 1);
>> + else
>> + report("I/O bitmap - partial pass", 0);
>> + // test overrun
>> + set_stage(8);
>> + memset(io_bitmap_b, 0xFF, PAGE_SIZE);
>> + inl(0xFFFF);
>
> Let's check the expected stage also here.
The check is below "if (stage == 9)", the following "memset" is just
used to prevent I/O mask to printf.
>
>> + memset(io_bitmap_b, 0x0, PAGE_SIZE);
>
> Note that you still have io_bitmap_a[0] != 0 here. You probably want to
> clear it in order to have a clean setup.
>
>> + if (stage == 9)
>> + report("I/O bitmap - overrun", 1);
>> + else
>> + report("I/O bitmap - overrun", 0);
>> +
>> + return;
>> +}
>> +
>> +static int iobmp_exit_handler()
>> +{
>> + u64 guest_rip;
>> + ulong reason, exit_qual;
>> + u32 insn_len;
>> + //u32 ctrl_cpu0;
>> +
>> + guest_rip = vmcs_read(GUEST_RIP);
>> + reason = vmcs_read(EXI_REASON) & 0xff;
>> + exit_qual = vmcs_read(EXI_QUALIFICATION);
>> + insn_len = vmcs_read(EXI_INST_LEN);
>> + switch (reason) {
>> + case VMX_IO:
>> + switch (stage) {
>> + case 2:
>> + if ((exit_qual & VMX_IO_SIZE_MASK) != _VMX_IO_BYTE)
>> + report("I/O bitmap - I/O width, byte", 0);
>> + else
>> + report("I/O bitmap - I/O width, byte", 1);
>> + if (!(exit_qual & VMX_IO_IN))
>> + report("I/O bitmap - I/O direction, in", 0);
>> + else
>> + report("I/O bitmap - I/O direction, in", 1);
>> + set_stage(stage + 1);
>> + break;
>> + case 3:
>> + if ((exit_qual & VMX_IO_SIZE_MASK) != _VMX_IO_WORD)
>> + report("I/O bitmap - I/O width, word", 0);
>> + else
>> + report("I/O bitmap - I/O width, word", 1);
>> + if (!(exit_qual & VMX_IO_IN))
>> + report("I/O bitmap - I/O direction, out", 1);
>> + else
>> + report("I/O bitmap - I/O direction, out", 0);
>> + set_stage(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);
>> + break;
>> + case 5:
>> + if (((exit_qual & VMX_IO_PORT_MASK) >> VMX_IO_PORT_SHIFT) == 0x5000)
>> + set_stage(stage + 1);
>
> else
> ...?
We don't need "else" here, if we don't increase "stage" here, it means
test fail, which is the same as if it doesn't cause vmexit.
>
> Here and below.
>
>> + break;
>> + case 6:
>> + if (((exit_qual & VMX_IO_PORT_MASK) >> VMX_IO_PORT_SHIFT) == 0x9000)
>> + set_stage(stage + 1);
>> + break;
>> + case 7:
>> + if (((exit_qual & VMX_IO_PORT_MASK) >> VMX_IO_PORT_SHIFT) == 0x4FFF)
>> + set_stage(stage + 1);
>> + //ctrl_cpu0 = vmcs_read(CPU_EXEC_CTRL0);
>> + //ctrl_cpu0 &= (~CPU_IO_BITMAP);
>> + //vmcs_write(CPU_EXEC_CTRL0, ctrl_cpu0);
>> + break;
>> + case 8:
>> + if (((exit_qual & VMX_IO_PORT_MASK) >> VMX_IO_PORT_SHIFT) == 0xFFFF)
>> + set_stage(stage + 1);
>> + break;
>> + case 0:
>> + case 1:
>> + set_stage(stage + 1);
>> + default:
>
> Unexpected stage?
>
>> + break;
>> + }
>> + vmcs_write(GUEST_RIP, guest_rip + insn_len);
>> + return VMX_TEST_RESUME;
>> + default:
>> + printf("guest_rip = 0x%llx\n", guest_rip);
>> + printf("\tERROR : Undefined exit reason, reason = %d.\n", reason);
>> + break;
>> + }
>> + return VMX_TEST_VMEXIT;
>> +}
>> +
>> /* name/init/guest_main/exit_handler/syscall_handler/guest_regs
>> basic_* just implement some basic functions */
>> struct vmx_test vmx_tests[] = {
>> @@ -486,5 +651,7 @@ struct vmx_test vmx_tests[] = {
>> test_ctrl_efer_exit_handler, basic_syscall_handler, {0} },
>> { "CR shadowing", basic_init, cr_shadowing_main,
>> cr_shadowing_exit_handler, basic_syscall_handler, {0} },
>> + { "I/O bitmap", iobmp_init, iobmp_main, iobmp_exit_handler,
>> + basic_syscall_handler, {0} },
>> { NULL, NULL, NULL, NULL, NULL, {0} },
>> };
>>
>
> Jan
>
--
Arthur Chunqi Li
Department of Computer Science
School of EECS
Peking University
Beijing, China
--
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
