Re: XSAVE IRC thread

[Ben Guthro <Benjamin.Guthro@citrix.com>]

On 05/04/2013 07:13 AM, Ben Guthro wrote:
>
> On May 3, 2013, at 10:58 AM, Jan Beulich <JBeulich@suse.com>
>   wrote:
>
>>>>> On 30.04.13 at 18:24, Mark Roddy <mark.roddy@citrix.com> wrote:
>>> The two values are pointers to the before and after regions:
>>>
>>> 1: kd> dd 841c4880
>>> 841c4880  0000027f 00000000 6e83506e 0000001b
>>> 841c4890  01d113d8 00000023 00001f80 0000ffff
>>> 841c48a0  00000000 00000000 00000000 00000000
>>> 841c48b0  00000000 00000000 00000000 00000000
>>> 841c48c0  00000000 00000000 00000000 00000000
>>> 841c48d0  00000000 00000000 00000000 00000000
>>> 841c48e0  00000000 00000000 00000000 00000000
>>> 841c48f0  00000000 00000000 00000000 00000000
>>>
>>> 1: kd> dd 841c4600
>>> 841c4600  0000027f 00000000 6e83506e 00000000
>>> 841c4610  01d113d8 00000000 00001f80 0000ffff
>>> 841c4620  00000000 00000000 00000000 00000000
>>> 841c4630  00000000 00000000 00000000 00000000
>>> 841c4640  00000000 00000000 00000000 00000000
>>> 841c4650  00000000 00000000 00000000 00000000
>>> 841c4660  00000000 00000000 00000000 00000000
>>> 841c4670  00000000 00000000 00000000 00000000
>>>
>>> I don't know if that helps, but obviously there are differences, two of
>>> them, at offsets 0xc and 0x12.
>>>
>>> " Interrupt Service Routine 88CAC91C has changed extended thread context.
>>> Context saved before executing ISR: 841C4880. Context saved after executing
>>> ISR: 841C4600."
>>>
>>> 841C4880 is before and 841C4600 is after.
>>
>> Attached a patch that I think should address this problem. It's
>> against the tip of the staging tree, and doesn't apply without
>> adjustment to 4.2 (and making it work for 4.1 would be quite a
>> bit more work) - please let me know whether that's sufficient for
>> you testing this, or whether you need me to do any backporting.
>>
>> I didn't verify this with any Windows, but since the same issue
>> can - if one is looking for it - be observed on PV Linux, I did verify
>> the patch to help there.
>>
>> I'd like to note though that while this is expected to help with
>> 32-bit guests, and with a 64-bit guest kernels doing such checking
>> after using the respective save (and possibly restore) instructions
>> with a 64-bit operand size, the hypervisor has no way of knowing
>> whether the context actually belongs to a 32-bit process while the
>> guest is in kernel (64-bit) mode. That means that from a 32-bit
>> app's perspective, inconsistencies could still be observed under
>> certain conditions (but the case where the hypervisor side save
>> happens after a VM exit from user mode should also work with
>> that patch). I don't see any way to hide that, other than running
>> on CPUs that don't save/restore the selector values at all
>> anymore (Intel at least has a feature bit for this).
>>
>> Jan
>>
>> <x86-FPU-preserve-selectors.patch>
>
> Jan,
>
> Thank you very much for looking into this so quickly.
>
> Our QA infrastructure is currently set up for testing against our XenClient product built on Xen 4.2.2.
> Since this is an intermittent failure, in order to reduce the number of variables in testing this solution,
> I'll look into backporting this on Mon to 4.2, and report back after a night, or two of testing.

Preliminary testing results look good - no xsave crashes with this patch.

Modulo your concerns from your other email, I'd say this solution looks 
good.

Ben