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

["Zhang, Yang Z" <yang.z.zhang@xxxxxxxxx>]

Jan Beulich wrote on 2013-10-30:
>>>> 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.

Just looking into the code. The current logic is necessary since there are lots 
of codes depend on them, so cannot remove them simply.
I will rebase the patch with the fixing included.

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


Best regards,
Yang



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