Keir Fraser wrote:
>> For vmexit code path: When I debug this issue, I found that for
>> some I/O vmexits which happened when /sbin/loader is executed, the
>> cpl is 3, the iopl is 0, so when
>> "generate_exception_if(!mode_iopl(), EXC_GP)" checks cpl and iopl
>> relationship, it push out a GP fault and makes the guest
>> installation fail. Actually before the code enters
>> vmx_vmexit_handler(), the processor has already checked the I/O
>> permission. So here I think that line of check code is not needed.
>
> Ah, and so we assume that the I/O access is actually permitted by the
> TSS I/O bitmap?
When a VMexit with reason IO_INSTRUCTION occurs, VMM can tell that: in
guest OS,
the execution has enough privilege to access the port ( (an actually
virtual port, from VMM's
perspective), because if ( vCPL > vIOPL || vTR.io_bitmap[port]==1), CPU
will raise #GP to guest directly without VM-exit.
> Seems reasonable, and so we could add that check or
> just remove the CPL-IOPL check. I agree again.
>
>> Also we haven't found any bug caused by the 4-instruction
>> emulation till now. Adding the change in the shadow code path is
>> because: There may be I/O instructions among the 4 instructions in
>> theory. In this case, I think a full check of cpl, iopl, and the I/O
>> bitmap is needed. So we may either add the I/O permission check in
>> software, or break the 4-instruction emulate and let processor do
>> the I/O permission check, then emulate it by
>> vmx_vmexit_handler()->handle_mmio() code path. Here the patch uses
>> the second way.
>
> I think you misunderstand. The shadow emulator *never* emulates I/O
> port accesses or exception deliveries. Those callback functions are
> simply not implemented and are left as NULL.
Those callback functions -- what are they? -- do you mean the following?
static struct x86_emulate_ops hvm_emulate_ops = {
....
.read_io = hvmemul_read_io,
.write_io = hvmemul_write_io,
...
};
>Hence we will fail the
> IOPL check, but we will also fail to deliver EXC_GP, and so we will
> simply return X86EMUL_UNHANDLEABLE, which is the right thing to do.
> So, no bug here and your changes to the shadow emulator are not
> required.
>
> I will try out a version of your patch which is basically just your
> x86_emulate.c changes, and see how that works out for me.
>
> Keir
Thanks
-- Dexuan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|