On Thu, Aug 15, 2013 at 4:23 PM, Jan Kiszka <jan.kiszka@xxxxxx> wrote:
> On 2013-08-15 10:20, Arthur Chunqi Li wrote:
>> On Thu, Aug 15, 2013 at 4:13 PM, Jan Kiszka <jan.kiszka@xxxxxx> wrote:
>>> On 2013-08-15 10:09, Arthur Chunqi Li wrote:
>>>> On Thu, Aug 15, 2013 at 3:58 PM, Jan Kiszka <jan.kiszka@xxxxxx> wrote:
>>>>> On 2013-08-15 09:51, Arthur Chunqi Li wrote:
>>>>>> 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.
>>>>>
>>>>> Right, there is an i/o instruction missing below after the second memset
>>>>> - or I cannot follow what you are trying to test. The above inl would
>>>>> always trigger, independent of the wrap-around. Only if you clear both
>>>>> bitmaps, we get to the "interesting" scenario. So something is still
>>>>> wrong here, no?
>>>> Yes, we need to memset io_bit_map_a to 0 here. The above inl and the
>>>> test "if (stage == 9)" are cooperatively used to test I/O overrun:
>>>> test 4 bits width "in" to 0xFFFF.
>>>
>>> The point is that, according to our understanding of the SDM, we should
>>> even see a trap in this wrap-around scenario if both bitmaps are cleared.
>> Well, yep. I get the same understanding when I had first glance at
>> SDM, but currently IO will pass if every bits cleared. This is the
>> only pending problem that I asked Paolo and Gleb in a previous mail
>> thread, and they are both too busy as you told me and no response
>> until now :)
>
> That is strange when looking at kvm:
>
> static bool nested_vmx_exit_handled_io(struct kvm_vcpu *vcpu,
> struct vmcs12 *vmcs12)
> ...
> if (port < 0x8000)
> bitmap = vmcs12->io_bitmap_a;
> else if (port < 0x10000)
> bitmap = vmcs12->io_bitmap_b;
> else
> return 1;
>
> Are you testing with kvm.git next?
Sorry, there's something wrong with my previous test. It is now acts
as what we expect.
Arthur
>
> Jan
>
>
--
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
