Re: [Xen-devel] [xen-unstable test] 21288: regressions - FAIL

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 30.10.13 at 15:53, "Zhang, Yang Z" <yang.z.zhang@xxxxxxxxx> wrote:
> Jan Beulich wrote on 2013-10-30:
>>>>> On 30.10.13 at 01:30, xen.org <ian.jackson@xxxxxxxxxxxxx> wrote:
>>> flight 21288 xen-unstable real [real]
>>> http://www.chiark.greenend.org.uk/~xensrcts/logs/21288/ 
>>> 
>>> Regressions :-(
>>> 
>>> Tests which did not succeed and are blocking, including tests which
>>> could not be run:
>>>  test-amd64-amd64-xl-qemuu-winxpsp3  8 guest-saverestore   fail REGR.
>>>  vs. 21268 test-amd64-amd64-xl-qemut-winxpsp3  7 windows-install    
>>>  fail REGR. vs. 21268 test-amd64-amd64-xl-winxpsp3  7 windows-install  
>>>          fail REGR. vs. 21268 test-amd64-amd64-xl-win7-amd64  7
>>>  windows-install         fail REGR. vs. 21268
>>>  test-amd64-i386-xl-win7-amd64  7 windows-install          fail REGR.
>>>  vs. 21268 test-amd64-i386-xl-qemut-win7-amd64  7 windows-install   
>>>  fail REGR.
>> vs. 21268
>> 
>> All but the first one are due to the guests triple faulting while
>> booting. While - unless otherwise confirmed - I'd like to wait for
>> bisector results here, the prime suspect appears to be c9efe34c
>> ("VMX: Eliminate cr3 store/load vmexit when UG enabled") - any
>> thoughts? Did you perhaps not test this on a system without UG support?
> 
> Yes, the previous patch will cause the guest boot fail on non-UG platform. 
> The following patch is able to fix it.
> But going deep into the code, I am thinking whether the current logic that 
> set hw_cr[3] and guest_cr[3] from GUEST_CR3 is really necessary. Especially, 
> each VMexit does the same thing. 

Feel free to clean this up.

> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2411,7 +2411,8 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
>      if ( paging_mode_hap(v->domain) )
>      {
>          __vmread(GUEST_CR3, &v->arch.hvm_vcpu.hw_cr[3]);
> -        v->arch.hvm_vcpu.guest_cr[3] = v->arch.hvm_vcpu.hw_cr[3];
> +        if ( vmx_unrestricted_guest(v) || hvm_paging_enabled(v) )
> +            v->arch.hvm_vcpu.guest_cr[3] = v->arch.hvm_vcpu.hw_cr[3];
>      }
>  
>      __vmread(VM_EXIT_REASON, &exit_reason);

We'll need a proper patch in any event. As I'm going to be out of
the office for the rest of the week, I guess I'll revert the patch
for now, and you then better send another rev of it with the
above change included.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel