>>> 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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
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. Thanks again for your help Ben
On 04/05/2013 12:13, 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. > > Thanks again for your help > > BenI will also look to getting this into XenServer testing ASAP ~Andrew> > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
>>> Ben Guthro <Ben.Guthro@citrix.com> 05/04/13 1:14 PM >>> > On May 3, 2013, at 10:58 AM, Jan Beulich <JBeulich@suse.com> wrote: >> 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). > >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.Meanwhile I realized that the patch will need a little further adjustment: Parts of the xsave modifications need to/can become conditional upon FPU state being saved/restored (which in particular may not be the case during the eager restore phase needed for AMD LWP, but which otherwise would also be a latent bug). This shouldn''t affect testing of what I sent on Friday, though. Furthermore I meanwhile also mentally restored what I had done on native Linux years ago, which would permit determining the needed save/restore layout (32 vs 64 bit) regardless of current guest execution mode. That would come with a price, though, since some parts of the save operation would need to be done twice (needing either two fxsave-s/xsave-s, or an additional fnstenv) - will need to do some measurements to see how bad this would turn out, and which of the two would be preferable. (This implicitly also tells me that native Linux ought to have the same issue when running 32-bit Windows in this verifier mode on 64-bit KVM. Kirk - did you ever run across FPU state inconsistency bug checks in the WHQL testing on KVM?) Jan
>>> On 5/5/2013 at 12:32 AM, in message <5185FCE8.59A : 120 : 30442>, Jan Beulichwrote:>>>> Ben Guthro <Ben.Guthro@citrix.com> 05/04/13 1:14 PM >>> >> On May 3, 2013, at 10:58 AM, Jan Beulich <JBeulich@suse.com> wrote: >>> 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). >> >>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. > > Meanwhile I realized that the patch will need a little further adjustment: > Parts of the xsave modifications need to/can become conditional upon > FPU state being saved/restored (which in particular may not be the case > during the eager restore phase needed for AMD LWP, but which otherwise > would also be a latent bug). This shouldn''t affect testing of what I sent > on Friday, though. > > Furthermore I meanwhile also mentally restored what I had done on native > Linux years ago, which would permit determining the needed save/restore > layout (32 vs 64 bit) regardless of current guest execution mode. That > would come with a price, though, since some parts of the save operation > would need to be done twice (needing either two fxsave-s/xsave-s, or an > additional fnstenv) - will need to do some measurements to see how bad > this would turn out, and which of the two would be preferable. (This > implicitly also tells me that native Linux ought to have the same issue when > running 32-bit Windows in this verifier mode on 64-bit KVM. Kirk - did you > ever run across FPU state inconsistency bug checks in the WHQL testing on > KVM?)No, no bug checks while running WHQL. Other than bug checks, would there be any other manifestation of the issue that I could look for?> > Jan
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