Re: [PATCH]Fix the bug of guest os installation failure and win2k boot failure

[Keir Fraser <keir.fraser@eu.citrix.com>]

On 17/3/08 10:06, "Xu, Dongxiao" <dongxiao.xu@intel.com> wrote:

>     Realmode emulation code path for I/O is fine. I just think that realmode
> I/O doesn't need that line of check code.

Okay, understood and I agree.

>     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? 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. 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.

 Thanks!
 Keir