Haitao Shan
2010-Nov-09 06:22 UTC
[Xen-devel] [Patch] Allowing PV-OPS kernel to detect whether XSAVE is supported
Hi, Jeremy, This patch allows pv-ops kernel to detect whether XSAVE is supported (before masking it unconditionally through xen_cpuid). Can you please have review? Thanks! Signed-off-by: Shan Haitao <haitao.shan@intel.com> Shan Haitao _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2010-Nov-09 10:43 UTC
Re: [Xen-devel] [Patch] Allowing PV-OPS kernel to detect whether XSAVE is supported
On Tue, 2010-11-09 at 06:22 +0000, Haitao Shan wrote:> Hi, Jeremy, > > This patch allows pv-ops kernel to detect whether XSAVE is supported > (before masking it unconditionally through xen_cpuid). > Can you please have review? Thanks! > > Signed-off-by: Shan Haitao <haitao.shan@intel.com> > > Shan Haitao >> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c > index fd3803e..03bfaf7 100644 > --- a/arch/x86/xen/enlighten.c > +++ b/arch/x86/xen/enlighten.c > @@ -252,6 +252,13 @@ static __init void xen_init_cpuid_mask(void) > (1 << X86_FEATURE_MCA) | /* disable MCA */ > (1 << X86_FEATURE_APIC) | /* disable local APIC */ > (1 << X86_FEATURE_ACPI)); /* disable ACPI */ > + ax = 1; > + xen_cpuid(&ax, &bx, &cx, &dx); > + > + /* Xen will set CR4.OSXSAVE if supported and not disabled by force */For how long has the hypervisor had this behaviour? IIRC older hypervisors did not correctly expose/mask the *XSAVE CPUID flags and would causes PV guests to crash when they used *XSAVE features which weren''t actually available. In other words have you confirmed that this patch does not break the kernel running on older hypervisors such as Xen 4.0?> + if ( cx & (1 << (X86_FEATURE_XSAVE % 32)) && > + cx & (1 << (X86_FEATURE_OSXSAVE % 32)) ) > + return; > > cpuid_leaf1_ecx_mask &= ~(1 << (X86_FEATURE_XSAVE % 32)); /* disable XSAVE */ > }_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2010-Nov-09 10:51 UTC
Re: [Xen-devel] [Patch] Allowing PV-OPS kernel to detect whether XSAVE is supported
>>> On 09.11.10 at 11:43, Ian Campbell <Ian.Campbell@citrix.com> wrote: > On Tue, 2010-11-09 at 06:22 +0000, Haitao Shan wrote: >> Hi, Jeremy, >> >> This patch allows pv-ops kernel to detect whether XSAVE is supported >> (before masking it unconditionally through xen_cpuid). >> Can you please have review? Thanks! >> >> Signed-off-by: Shan Haitao <haitao.shan@intel.com> >> >> Shan Haitao >> > >> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c >> index fd3803e..03bfaf7 100644 >> --- a/arch/x86/xen/enlighten.c >> +++ b/arch/x86/xen/enlighten.c >> @@ -252,6 +252,13 @@ static __init void xen_init_cpuid_mask(void) >> (1 << X86_FEATURE_MCA) | /* disable MCA */ >> (1 << X86_FEATURE_APIC) | /* disable local APIC */ >> (1 << X86_FEATURE_ACPI)); /* disable ACPI */ >> + ax = 1; >> + xen_cpuid(&ax, &bx, &cx, &dx); >> + >> + /* Xen will set CR4.OSXSAVE if supported and not disabled by force > */ > > For how long has the hypervisor had this behaviour? IIRC older > hypervisors did not correctly expose/mask the *XSAVE CPUID flags and > would causes PV guests to crash when they used *XSAVE features which > weren''t actually available. > > In other words have you confirmed that this patch does not break the > kernel running on older hypervisors such as Xen 4.0?The problem was only with the XSAVE cpuid bit, not the OSXSAVE one (which gets turned on only when CR4.OSXSAVE gets set). Jan>> + if ( cx & (1 << (X86_FEATURE_XSAVE % 32)) && >> + cx & (1 << (X86_FEATURE_OSXSAVE % 32)) ) >> + return; >> >> cpuid_leaf1_ecx_mask &= ~(1 << (X86_FEATURE_XSAVE % 32)); /* disable > XSAVE */ >> } > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2010-Nov-09 10:54 UTC
Re: [Xen-devel] [Patch] Allowing PV-OPS kernel to detect whether XSAVE is supported
On Tue, 2010-11-09 at 10:51 +0000, Jan Beulich wrote:> >>> On 09.11.10 at 11:43, Ian Campbell <Ian.Campbell@citrix.com> wrote: > > On Tue, 2010-11-09 at 06:22 +0000, Haitao Shan wrote: > >> Hi, Jeremy, > >> > >> This patch allows pv-ops kernel to detect whether XSAVE is supported > >> (before masking it unconditionally through xen_cpuid). > >> Can you please have review? Thanks! > >> > >> Signed-off-by: Shan Haitao <haitao.shan@intel.com> > >> > >> Shan Haitao > >> > > > >> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c > >> index fd3803e..03bfaf7 100644 > >> --- a/arch/x86/xen/enlighten.c > >> +++ b/arch/x86/xen/enlighten.c > >> @@ -252,6 +252,13 @@ static __init void xen_init_cpuid_mask(void) > >> (1 << X86_FEATURE_MCA) | /* disable MCA */ > >> (1 << X86_FEATURE_APIC) | /* disable local APIC */ > >> (1 << X86_FEATURE_ACPI)); /* disable ACPI */ > >> + ax = 1; > >> + xen_cpuid(&ax, &bx, &cx, &dx); > >> + > >> + /* Xen will set CR4.OSXSAVE if supported and not disabled by force > > */ > > > > For how long has the hypervisor had this behaviour? IIRC older > > hypervisors did not correctly expose/mask the *XSAVE CPUID flags and > > would causes PV guests to crash when they used *XSAVE features which > > weren''t actually available. > > > > In other words have you confirmed that this patch does not break the > > kernel running on older hypervisors such as Xen 4.0? > > The problem was only with the XSAVE cpuid bit, not the OSXSAVE > one (which gets turned on only when CR4.OSXSAVE gets set).So if OSXSAVE is enabled we can also infer that XSAVE is safe to use, because XSAVE was fixed/implemented before OSXSAVE was? Seems reasonable. Ian.> > Jan > > >> + if ( cx & (1 << (X86_FEATURE_XSAVE % 32)) && > >> + cx & (1 << (X86_FEATURE_OSXSAVE % 32)) ) > >> + return; > >> > >> cpuid_leaf1_ecx_mask &= ~(1 << (X86_FEATURE_XSAVE % 32)); /* disable > > XSAVE */ > >> } > > > > > > _______________________________________________ > > Xen-devel mailing list > > Xen-devel@lists.xensource.com > > http://lists.xensource.com/xen-devel > > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2010-Nov-09 11:26 UTC
Re: [Xen-devel] [Patch] Allowing PV-OPS kernel to detect whether XSAVE is supported
>>> On 09.11.10 at 11:54, Ian Campbell <Ian.Campbell@eu.citrix.com> wrote: > So if OSXSAVE is enabled we can also infer that XSAVE is safe to use, > because XSAVE was fixed/implemented before OSXSAVE was? Seems > reasonable.Yes, that''s my understanding (prior to the recent changes, CR4.OSXSAVE got turned on only in HVM context if I followed things correctly). Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Haitao Shan
2010-Nov-09 15:12 UTC
Re: [Xen-devel] [Patch] Allowing PV-OPS kernel to detect whether XSAVE is supported
Yes. According to spec, while CPUID leaf 1 ECX.XSAVE reports whether processor supports XSAVE, CPUID leaf 1 ECX.OSXSAVE just reflects CR4.OSXSAVE, which is served as an indication from kernel (and hence VMM, in Xen''s case) to apps (including PV kernels, in xen''s case) that XSAVE is enabled and can be used safely. Shan Haitao 2010/11/9 Ian Campbell <Ian.Campbell@eu.citrix.com>:> On Tue, 2010-11-09 at 10:51 +0000, Jan Beulich wrote: >> >>> On 09.11.10 at 11:43, Ian Campbell <Ian.Campbell@citrix.com> wrote: >> > On Tue, 2010-11-09 at 06:22 +0000, Haitao Shan wrote: >> >> Hi, Jeremy, >> >> >> >> This patch allows pv-ops kernel to detect whether XSAVE is supported >> >> (before masking it unconditionally through xen_cpuid). >> >> Can you please have review? Thanks! >> >> >> >> Signed-off-by: Shan Haitao <haitao.shan@intel.com> >> >> >> >> Shan Haitao >> >> >> > >> >> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c >> >> index fd3803e..03bfaf7 100644 >> >> --- a/arch/x86/xen/enlighten.c >> >> +++ b/arch/x86/xen/enlighten.c >> >> @@ -252,6 +252,13 @@ static __init void xen_init_cpuid_mask(void) >> >> (1 << X86_FEATURE_MCA) | /* disable MCA */ >> >> (1 << X86_FEATURE_APIC) | /* disable local APIC */ >> >> (1 << X86_FEATURE_ACPI)); /* disable ACPI */ >> >> + ax = 1; >> >> + xen_cpuid(&ax, &bx, &cx, &dx); >> >> + >> >> + /* Xen will set CR4.OSXSAVE if supported and not disabled by force >> > */ >> > >> > For how long has the hypervisor had this behaviour? IIRC older >> > hypervisors did not correctly expose/mask the *XSAVE CPUID flags and >> > would causes PV guests to crash when they used *XSAVE features which >> > weren''t actually available. >> > >> > In other words have you confirmed that this patch does not break the >> > kernel running on older hypervisors such as Xen 4.0? >> >> The problem was only with the XSAVE cpuid bit, not the OSXSAVE >> one (which gets turned on only when CR4.OSXSAVE gets set). > > So if OSXSAVE is enabled we can also infer that XSAVE is safe to use, > because XSAVE was fixed/implemented before OSXSAVE was? Seems > reasonable. > > Ian. > >> >> Jan >> >> >> + if ( cx & (1 << (X86_FEATURE_XSAVE % 32)) && >> >> + cx & (1 << (X86_FEATURE_OSXSAVE % 32)) ) >> >> + return; >> >> >> >> cpuid_leaf1_ecx_mask &= ~(1 << (X86_FEATURE_XSAVE % 32)); /* disable >> > XSAVE */ >> >> } >> > >> > >> > _______________________________________________ >> > Xen-devel mailing list >> > Xen-devel@lists.xensource.com >> > http://lists.xensource.com/xen-devel >> >> >> > > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jeremy Fitzhardinge
2010-Nov-09 19:55 UTC
Re: [Xen-devel] [Patch] Allowing PV-OPS kernel to detect whether XSAVE is supported
On 11/08/2010 10:22 PM, Haitao Shan wrote:> Hi, Jeremy, > > This patch allows pv-ops kernel to detect whether XSAVE is supported > (before masking it unconditionally through xen_cpuid). > Can you please have review? Thanks! > > Signed-off-by: Shan Haitao <haitao.shan@intel.com> > > Shan Haitao >For future reference: Please post patches inline if possible. If you must use an attachment to prevent your mail system from corrupting the patch, please include a complete description of the patch (what is it trying to do, how does it do it, what is the outcome?) with signed-off-bys in the the patch itself.> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c index > fd3803e..03bfaf7 100644 --- a/arch/x86/xen/enlighten.c +++ > b/arch/x86/xen/enlighten.c @@ -252,6 +252,13 @@ static __init void > xen_init_cpuid_mask(void) (1 << X86_FEATURE_MCA) | /* disable MCA */ > (1 << X86_FEATURE_APIC) | /* disable local APIC */ (1 << > X86_FEATURE_ACPI)); /* disable ACPI */ + ax = 1; + xen_cpuid(&ax, &bx, > &cx, &dx); + + /* Xen will set CR4.OSXSAVE if supported and not > disabled by force */ + if ( cx & (1 << (X86_FEATURE_XSAVE % 32)) && + > cx & (1 << (X86_FEATURE_OSXSAVE % 32)) ) + return; > cpuid_leaf1_ecx_mask &= ~(1 << (X86_FEATURE_XSAVE % 32)); /* disable > XSAVE */I cleaned this up a bit (fixed formatting to Linux style, reversed the sense of the if() and masked OSXSAVE as well, even though it won''t make a difference). But I''m still a bit concerned about the back-compat issues around this. What happens if the guest OS does not support XSAVE (older versions of Linux)? Could usermode code see OSXSAVE set in the cpuid feature flags and attempt to use XSAVE, even if XSAVE isn''t set? In other words, can usermode ever normally see OSXSAVE set, but XSAVE clear? We can''t mask OSXSAVE to usermode because they can run the naked CPUID instruction, so I guess the only way to cause it to be cleared would be to clear CR4.OSXSAVE? But that seems like a very expensive thing to do on a vcpu context switch. How much testing of real usermode code have you done with this? How many combinations of XSAVE support in Xen, Linux and usermode have you tried? J _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Haitao Shan
2010-Nov-10 00:15 UTC
Re: [Xen-devel] [Patch] Allowing PV-OPS kernel to detect whether XSAVE is supported
2010/11/10 Jeremy Fitzhardinge <jeremy@goop.org>:> On 11/08/2010 10:22 PM, Haitao Shan wrote: >> Hi, Jeremy, >> >> This patch allows pv-ops kernel to detect whether XSAVE is supported >> (before masking it unconditionally through xen_cpuid). >> Can you please have review? Thanks! >> >> Signed-off-by: Shan Haitao <haitao.shan@intel.com> >> >> Shan Haitao >> > > For future reference: > > Please post patches inline if possible. > > If you must use an attachment to prevent your mail system from > corrupting the patch, please include a complete description of the patch > (what is it trying to do, how does it do it, what is the outcome?) with > signed-off-bys in the the patch itself.OK. I will follow.> >> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c index >> fd3803e..03bfaf7 100644 --- a/arch/x86/xen/enlighten.c +++ >> b/arch/x86/xen/enlighten.c @@ -252,6 +252,13 @@ static __init void >> xen_init_cpuid_mask(void) (1 << X86_FEATURE_MCA) | /* disable MCA */ >> (1 << X86_FEATURE_APIC) | /* disable local APIC */ (1 << >> X86_FEATURE_ACPI)); /* disable ACPI */ + ax = 1; + xen_cpuid(&ax, &bx, >> &cx, &dx); + + /* Xen will set CR4.OSXSAVE if supported and not >> disabled by force */ + if ( cx & (1 << (X86_FEATURE_XSAVE % 32)) && + >> cx & (1 << (X86_FEATURE_OSXSAVE % 32)) ) + return; >> cpuid_leaf1_ecx_mask &= ~(1 << (X86_FEATURE_XSAVE % 32)); /* disable >> XSAVE */ > > > I cleaned this up a bit (fixed formatting to Linux style, reversed the > sense of the if() and masked OSXSAVE as well, even though it won''t make > a difference). But I''m still a bit concerned about the back-compat > issues around this. > > What happens if the guest OS does not support XSAVE (older versions of > Linux)? Could usermode code see OSXSAVE set in the cpuid feature flags > and attempt to use XSAVE, even if XSAVE isn''t set? In other words, can > usermode ever normally see OSXSAVE set, but XSAVE clear?I think that is not possible. If usermode does not use native CPUID instruction, or CPUID can be virtualized any way, this won''t happen. Otherwise, usermode will either see both are set or unset. Since on a NON-XSAVE processor, you can not set CR4.OSXSAVE and won''t be reflected to CPUID. If user mode sees both are set, they can use it actually. So we initially set FP and SSE in XCR0 for guest. If user mode wants to use it, guest kernel still can manage the state using traditional FPU save/restore mechanism. If user mode wants to access more extended states, it has to acquire kernel''s support for turning on related bit in XCR0, which is finally controlled by Xen now. That''s the reason I chose to turn on OSXSAVE in Xen and don''t dynamically change it.> > We can''t mask OSXSAVE to usermode because they can run the naked CPUID > instruction, so I guess the only way to cause it to be cleared would be > to clear CR4.OSXSAVE? But that seems like a very expensive thing to do > on a vcpu context switch.Yes. This is the biggest concern when I wrote XSAVE patches. Otherwise, you will need to switch CR4 on entry to/exit from guest mode. Or as you said, switching CR4 when do a vcpu context switch, but whenever Xen itself want to use XSAVE instructions, it needs to turns on it and off it on the fly.> > How much testing of real usermode code have you done with this? How > many combinations of XSAVE support in Xen, Linux and usermode have you > tried?I have a few AVX test programs running both inside PV guest and HVM. However, I have to admit that my aim is to test Xen''s fpu context switching using Xsave and guest save/restore.> > J >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Haitao Shan
2010-Nov-10 00:18 UTC
Re: [Xen-devel] [Patch] Allowing PV-OPS kernel to detect whether XSAVE is supported
> Otherwise, usermode will either see both are set or unset. Since on aOh, that is not correct. I mean whenever user mode sees OSXSAVE, it sees both are set or unset. If user mode sees XSAVE only, that is legal of course. Shan Haitao 2010/11/10 Haitao Shan <maillists.shan@gmail.com>:> 2010/11/10 Jeremy Fitzhardinge <jeremy@goop.org>: >> On 11/08/2010 10:22 PM, Haitao Shan wrote: >>> Hi, Jeremy, >>> >>> This patch allows pv-ops kernel to detect whether XSAVE is supported >>> (before masking it unconditionally through xen_cpuid). >>> Can you please have review? Thanks! >>> >>> Signed-off-by: Shan Haitao <haitao.shan@intel.com> >>> >>> Shan Haitao >>> >> >> For future reference: >> >> Please post patches inline if possible. >> >> If you must use an attachment to prevent your mail system from >> corrupting the patch, please include a complete description of the patch >> (what is it trying to do, how does it do it, what is the outcome?) with >> signed-off-bys in the the patch itself. > OK. I will follow. > >> >>> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c index >>> fd3803e..03bfaf7 100644 --- a/arch/x86/xen/enlighten.c +++ >>> b/arch/x86/xen/enlighten.c @@ -252,6 +252,13 @@ static __init void >>> xen_init_cpuid_mask(void) (1 << X86_FEATURE_MCA) | /* disable MCA */ >>> (1 << X86_FEATURE_APIC) | /* disable local APIC */ (1 << >>> X86_FEATURE_ACPI)); /* disable ACPI */ + ax = 1; + xen_cpuid(&ax, &bx, >>> &cx, &dx); + + /* Xen will set CR4.OSXSAVE if supported and not >>> disabled by force */ + if ( cx & (1 << (X86_FEATURE_XSAVE % 32)) && + >>> cx & (1 << (X86_FEATURE_OSXSAVE % 32)) ) + return; >>> cpuid_leaf1_ecx_mask &= ~(1 << (X86_FEATURE_XSAVE % 32)); /* disable >>> XSAVE */ >> >> >> I cleaned this up a bit (fixed formatting to Linux style, reversed the >> sense of the if() and masked OSXSAVE as well, even though it won''t make >> a difference). But I''m still a bit concerned about the back-compat >> issues around this. >> >> What happens if the guest OS does not support XSAVE (older versions of >> Linux)? Could usermode code see OSXSAVE set in the cpuid feature flags >> and attempt to use XSAVE, even if XSAVE isn''t set? In other words, can >> usermode ever normally see OSXSAVE set, but XSAVE clear? > I think that is not possible. If usermode does not use native CPUID > instruction, or CPUID can be virtualized any way, this won''t happen. > Otherwise, usermode will either see both are set or unset. Since on a > NON-XSAVE processor, you can not set CR4.OSXSAVE and won''t be > reflected to CPUID. > > If user mode sees both are set, they can use it actually. So we > initially set FP and SSE in XCR0 for guest. If user mode wants to use > it, guest kernel still can manage the state using traditional FPU > save/restore mechanism. If user mode wants to access more extended > states, it has to acquire kernel''s support for turning on related bit > in XCR0, which is finally controlled by Xen now. > > That''s the reason I chose to turn on OSXSAVE in Xen and don''t > dynamically change it. > >> >> We can''t mask OSXSAVE to usermode because they can run the naked CPUID >> instruction, so I guess the only way to cause it to be cleared would be >> to clear CR4.OSXSAVE? But that seems like a very expensive thing to do >> on a vcpu context switch. > Yes. This is the biggest concern when I wrote XSAVE patches. > Otherwise, you will need to switch CR4 on entry to/exit from guest > mode. Or as you said, switching CR4 when do a vcpu context switch, but > whenever Xen itself want to use XSAVE instructions, it needs to turns > on it and off it on the fly. > >> >> How much testing of real usermode code have you done with this? How >> many combinations of XSAVE support in Xen, Linux and usermode have you >> tried? > I have a few AVX test programs running both inside PV guest and HVM. > However, I have to admit that my aim is to test Xen''s fpu context > switching using Xsave and guest save/restore. > >> >> J >> >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jeremy Fitzhardinge
2010-Nov-10 00:41 UTC
Re: [Xen-devel] [Patch] Allowing PV-OPS kernel to detect whether XSAVE is supported
On 11/09/2010 04:15 PM, Haitao Shan wrote:> > I think that is not possible. If usermode does not use native CPUID > instruction, or CPUID can be virtualized any way, this won''t happen. > Otherwise, usermode will either see both are set or unset. Since on a > NON-XSAVE processor, you can not set CR4.OSXSAVE and won''t be > reflected to CPUID.I see.> If user mode sees both are set, they can use it actually. So we > initially set FP and SSE in XCR0 for guest. If user mode wants to use > it, guest kernel still can manage the state using traditional FPU > save/restore mechanism. If user mode wants to access more extended > states, it has to acquire kernel''s support for turning on related bit > in XCR0, which is finally controlled by Xen now.Are you saying that usermode can use XSAVE, AVX and other instruction set extensions successfully if Xen supports them, even if the guest OS doesn''t? That sounds unlikely - what happens when, for example, an old Linux wants to context switch from one Linux task to another on a VCPU? How will the task''s context get saved/reloaded properly? Your kernel changes seem fine, and should allow a modern kernel to support XSAVE and all the CPU features that go along with it. But I''m really concerned that your Xen changes will cause previously working usermode programs to start erroneously using XSAVE when the guest kernel cannot support it.> I have a few AVX test programs running both inside PV guest and HVM. > However, I have to admit that my aim is to test Xen''s fpu context > switching using Xsave and guest save/restore. >Could you make them available for testing? IanJ: It looks like it would be useful to add some tests to the suite to make sure all this extended context is save/restored properly... Thanks, J _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Haitao Shan
2010-Nov-10 01:45 UTC
Re: [Xen-devel] [Patch] Allowing PV-OPS kernel to detect whether XSAVE is supported
>> If user mode sees both are set, they can use it actually. So we >> initially set FP and SSE in XCR0 for guest. If user mode wants to use >> it, guest kernel still can manage the state using traditional FPU >> save/restore mechanism. If user mode wants to access more extended >> states, it has to acquire kernel''s support for turning on related bit >> in XCR0, which is finally controlled by Xen now. > > Are you saying that usermode can use XSAVE, AVX and other instruction > set extensions successfully if Xen supports them, even if the guest OS > doesn''t? That sounds unlikely - what happens when, for example, an old > Linux wants to context switch from one Linux task to another on a VCPU? > How will the task''s context get saved/reloaded properly?Actually not. I mean user mode can use XSAVE instruction itself but not AVX or any future instruction extensions. Using the latter would require setting XCR0 properly, which is owned and managed by Xen itself. As I said, initially Xen sets XCR0 to be FPU/SSE only for guest (we do do XCR0 switch when vcpu context switch, this is a per-vcpu setting). Unless guest kernel requires to enable more extended states through XSETBV (this is ROOT ring 0 instruction only), it won''t be changed. And in this case, guest kernel can still use original FXSAVE mechanism to provide context switching support to its user space apps. It just means user mode can use XSAVE instruction if it wans to use this instruction to do save/restore itself. If user mode wants more, it has to check kernel''s SW interface support for turning on more extended states management (via XSETBV in kernel finally). But old kernels just do not have such kind of interface.> > Your kernel changes seem fine, and should allow a modern kernel to > support XSAVE and all the CPU features that go along with it. But I''m > really concerned that your Xen changes will cause previously working > usermode programs to start erroneously using XSAVE when the guest kernel > cannot support it.Well, I am concerned too. But I still think the current approach should be architecturally viable. But there may be bugs in my code ........> >> I have a few AVX test programs running both inside PV guest and HVM. >> However, I have to admit that my aim is to test Xen''s fpu context >> switching using Xsave and guest save/restore. >> > > Could you make them available for testing?I will return to you later for answer to this question. I needs to judge the correct Intel policy and channel when doing such a release.> > IanJ: It looks like it would be useful to add some tests to the suite to > make sure all this extended context is save/restored properly... > > Thanks, > J >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jeremy Fitzhardinge
2010-Nov-10 02:15 UTC
Re: [Xen-devel] [Patch] Allowing PV-OPS kernel to detect whether XSAVE is supported
On 11/09/2010 05:45 PM, Haitao Shan wrote:>>> If user mode sees both are set, they can use it actually. So we >>> initially set FP and SSE in XCR0 for guest. If user mode wants to use >>> it, guest kernel still can manage the state using traditional FPU >>> save/restore mechanism. If user mode wants to access more extended >>> states, it has to acquire kernel''s support for turning on related bit >>> in XCR0, which is finally controlled by Xen now. >> Are you saying that usermode can use XSAVE, AVX and other instruction >> set extensions successfully if Xen supports them, even if the guest OS >> doesn''t? That sounds unlikely - what happens when, for example, an old >> Linux wants to context switch from one Linux task to another on a VCPU? >> How will the task''s context get saved/reloaded properly? > Actually not. I mean user mode can use XSAVE instruction itself but > not AVX or any future instruction extensions. Using the latter would > require setting XCR0 properly, which is owned and managed by Xen > itself. As I said, initially Xen sets XCR0 to be FPU/SSE only for > guest (we do do XCR0 switch when vcpu context switch, this is a > per-vcpu setting). Unless guest kernel requires to enable more > extended states through XSETBV (this is ROOT ring 0 instruction only), > it won''t be changed.Ah, OK.>> Your kernel changes seem fine, and should allow a modern kernel to >> support XSAVE and all the CPU features that go along with it. But I''m >> really concerned that your Xen changes will cause previously working >> usermode programs to start erroneously using XSAVE when the guest kernel >> cannot support it. > Well, I am concerned too. But I still think the current approach > should be architecturally viable. But there may be bugs in my code > ........Well, that''s not as bad as baking in a flawed interface.>>> I have a few AVX test programs running both inside PV guest and HVM. >>> However, I have to admit that my aim is to test Xen''s fpu context >>> switching using Xsave and guest save/restore. >>> >> Could you make them available for testing? > I will return to you later for answer to this question. I needs to > judge the correct Intel policy and channel when doing such a release.Well, if you can''t release your actual test programs (which might not be appropriate in any case), then perhaps you could write something new to exercise this? Something that would enable as many instruction extensions as possible and continually makes sure the contexts are being properly saved and restored? Thanks, J _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2010-Nov-19 18:17 UTC
Re: [Xen-devel] [Patch] Allowing PV-OPS kernel to detect whether XSAVE is supported
Jeremy Fitzhardinge writes ("Re: [Xen-devel] [Patch] Allowing PV-OPS kernel
to detect whether XSAVE is supported"):> Well, if you can''t release your actual test programs (which might
not be
> appropriate in any case), then perhaps you could write something new to
> exercise this? Something that would enable as many instruction
> extensions as possible and continually makes sure the contexts are being
> properly saved and restored?
This would be a very good idea.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel