RE: [Xen-devel] [PATCH]Fix the bug of guest os installation failure and win2k boot failure

["Xu, Dongxiao" <dongxiao.xu@xxxxxxxxx>]

Hi, Keir, 

    Thanks for your response.

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

    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.

    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. 

    There may be some points that I haven't considered. Thanks for comments!

Best Regards,
-- Dongxiao
    



-----Original Message-----
From: Keir Fraser [mailto:keir.fraser@xxxxxxxxxxxxx] 
Sent: 2008年3月17日 16:32
To: Xu, Dongxiao; xen-devel@xxxxxxxxxxxxxxxxxxx
Subject: Re: [Xen-devel] [PATCH]Fix the bug of guest os installation failure 
and win2k boot failure

Hi Dongxiao,

Thanks for tracking this one down this far! I'm not fully convinced by your
analysis however, for the following reasons:

 1. In real mode we must surely have CPL==SS.DPL==0, otherwise the write of
CR0.PE would have been disallowed. Hence all I/O port operations must surely
succeed in our emulated real mode.

 2. I don't understand your changes to the shadow emulation path. Firstly,
the shadow emulator does not provide callback functions for I/O-port
operations, so they can never be emulated. Secondly, even if we do fall into
one of the generate_exception_if(!mode_iopl(), EXC_GP) statements, the
shadow emulator does not provide an inject_hw_exception() callback and hence
no exception will be generated and instead the emulation will (correctly)
fail.

This makes sense, since the nasty guest-installation problems have only
appeared (as far as I know) since the old I/O vmexit path was changed to use
x86_emulate(). Before that the 4-instruction shadow emulator and also the
real-mode emulator were working pretty much fine!

So... with regard to what is wrong with the I/O vmexit path. I agree that
the CPL-IOPL check is redundant, but it *should* work! Are we simply falling
down because we are not also checking the TSS bitmap?

Removing the IOPL checks from x86_emulate() may be the right thing to do,
but I would like to really understand the underlying root-cause problem
first.

 -- Keir

On 17/3/08 08:08, "Xu, Dongxiao" <dongxiao.xu@xxxxxxxxx> wrote:

> Hi, Keir,
> 
>       This patch is to fix the problem of Linux guest installation failure and
> Windows 2000 boot failure.
> 
> ????? In the early code, we use vmx_vmexit_handler() -> vmx_io_instruction()
> function to emulate I/O instructions. But now, we use vmx_vmexit_handler() ->
> handle_mmio -> hvm_emulate_one() -> x86_emulate() to emulate I/O instructions.
> Also nowadays, the realmode emulation code walks through the path:
> vmx_realmode() -> realmode_emulate_one() -> hvm_emulate_one() ->
> x86_emulate().
> 
> ????? The I/O handle code in x86_emulate() checks the cpl and iopl value, and
> if cpl > iopl, it will generate a GP fault. This causes Linux guest
> installation failure and Windows 2000 boot failure. I think this check code
> may be not reasonable for two aspects:
> 
> ????? 1. If x86_emulate() is called from vmexit or from realmode emulation, I
> think this line of code is not needed, because:
> 
> ??????????? a). In I/O emulation, the cpu has already checked the cpl, iopl,
> and also the I/O bitmap before vmx_vmexit_handler() is called,
> ??????????? b). For realmode, we shouldn't check the cpl and iopl, because any
> I/O operation is permitted in realmode.
> 
> ????? 2. If x86_emulate() is called from multi.c, which emulates up to four
> instructions when dealing with PAE guest page tables. In this condition, the
> check is needed, but it is not correct, it should follow the code as follows,
> which is stated in the Intel SDM:
> 
> 　If (cpl <= iopl)
> 　??? Do I/O operation;
> 　Else {
> 　??? If (I/O permission bit for the port == 0)
> 　??????? Do I/O operation;
> 　??? Else 
> 　??????? Generate GP fault;
> 　} 
> 
> ????? Now this patch remove the cpl and iopl check in I/O handler code in
> x86_emulate() function. And it checks the four instructions which would be
> emulated by multi.c, if any of them is IN/INS/OUT/OUTS, or REP
> IN/INS/OUT/OUTS, we will break that four-instruction emulation, and let the
> I/O instruction walk through the path of vmx_vmexit_handler() -> handle_mmio
> -> hvm_emulate_one() -> x86_emulate().
> 
>       Another way to solve this issue could be that, we put the entire io
> permission check in x86_emulate(), and use a flag to indicate whether we
> should do the check. If x86_emulate() is called by vmexit or realmode
> emulation, we skip this check; if it is called by multi.c, then we do the io
> permission check. But it may be a bit complex for hypervisor to read guest
> process’s TSS and find and check its io bitmap.
> 
> BTW: Why the existence code doesn't check the LOCK prefix (which should cause
> #UD injected to guest)
>       
>       Welcome for your comment, thanks!
> 
> Signed-off-by: Xu Dongxiao <dongxiao.xu@xxxxxxxxx>
> 
> Best Regards,
> --Dongxiao
> 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel