Re: XSAVE/XRSTOR crash resurgence in 4.3

[Ben Guthro <ben.guthro@gmail.com>]

On Wed, Jul 17, 2013 at 9:07 AM, Ben Guthro <ben.guthro@gmail.com> wrote:
> On Wed, Jul 17, 2013 at 2:38 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 16.07.13 at 18:57, Ben Guthro <ben.guthro@gmail.com> wrote:
>>> On Tue, Jul 16, 2013 at 12:23 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>> On 15.07.13 at 15:49, Ben Guthro <ben.guthro@gmail.com> wrote:
>>>>> The following output was from a WinXP SP3 guest (32bit) on a Lenovo T430:
>>>>>
>>>>> (XEN) d1v0: fip=1b773d6e9a fdp=23773d1c48 w=8
>>>>> (XEN) d1v0: FIP=1b773d6e9a FDP=23773d1c48 w=8
>>>>> (XEN) d1v1: fip=1b79e78dee fdp=230012e3b4 w=8
>>>>> (XEN) d1v1: FIP=1b79e78dee FDP=230012e3b4 w=8
>>>>> (XEN) d1v1: fip=0000:79e78dee fdp=0000:0012e3b4
>>>>> (XEN) d1v1: fip=0000:79e78dee fdp=0000:0012e3b4
>>>>> (XEN) d1v1: fip=0000:79e78dee fdp=0000:0012e3b4
>>>>> (XEN) d1v1: fip=0000:79e78dee fdp=0000:0012e3b4
>>>>> (XEN) d1v1: fip=1b79e78dee fdp=230012d528 w=8
>>>>> (XEN) d1v1: FIP=1b79e78dee FDP=230012d528 w=8
>>>>> (XEN) d1v0: fip=4500000000 fdp=4b1000000000 w=8
>>>>> (XEN) d1v0: FIP=4500000000 FDP=4b1000000000 w=8
>>>>> (XEN) d1v1: fip=1b773d6e9a fdp=23773d1c48 w=8
>>>>> (XEN) d1v1: FIP=1b773d6e9a FDP=23773d1c48 w=8
>>>>
>>>> Well, it should have been quite obvious (to me) that this is
>>>> related to xsaveopt behavior (according to my own observations
>>>> of cases where the selector/offset fields don't get written). Could
>>>> you double check whether the system(s) you see the problem on
>>>> support xsaveopt, and if so, simply comment out the conditionals
>>>> that cause it to be used?
>>>
>>> They do support xsaveopt.
>>> I'll ifdef these out for tonight's test run.
>
> There was a problem with my commit for this test run, so I did not get
> the data I wanted.
>
> I will test with this patch tonight.

Testing runs over the past few nights were successful, with this patch.
While not totally conclusive - I would say that this solved the problem.

Thank you again.

Ben

>> And attached/below also a tentative fix.
>>
>> Jan
>>
>> --- a/xen/arch/x86/xstate.c
>> +++ b/xen/arch/x86/xstate.c
>> @@ -72,9 +72,24 @@ void xsave(struct vcpu *v, uint64_t mask
>>      if ( word_size <= 0 || !is_pv_32bit_vcpu(v) )
>>      {
>>          if ( cpu_has_xsaveopt )
>> +        {
>> +            /*
>> +             * xsaveopt may not write the FPU portion even when the respective
>> +             * mask bit is set. For the check further down to work we hence
>> +             * need to put the save image back into the state that it was in
>> +             * right after the previous xsaveopt.
>> +             */
>> +            if ( word_size > 0 &&
>> +                 (ptr->fpu_sse.x[FPU_WORD_SIZE_OFFSET] == 4 ||
>> +                  ptr->fpu_sse.x[FPU_WORD_SIZE_OFFSET] == 2) )
>> +            {
>> +                ptr->fpu_sse.fip.sel = 0;
>> +                ptr->fpu_sse.fdp.sel = 0;
>> +            }
>>              asm volatile ( ".byte 0x48,0x0f,0xae,0x37"
>>                             : "=m" (*ptr)
>>                             : "a" (lmask), "d" (hmask), "D" (ptr) );
>> +        }
>>          else
>>              asm volatile ( ".byte 0x48,0x0f,0xae,0x27"
>>                             : "=m" (*ptr)
>>
>>