Re: [Xen-devel] XSAVE flavors
On Thu, Feb 04, 2016 at 01:51:34AM -0700, Jan Beulich wrote: > >> >> And I'm afraid there's yet one more issue: If my reading of the > >> >> SDM is right, then the offsets at which components get saved > >> >> by XSAVEC / XSAVES aren't fixed, but depend on RFBM (as that's > >> >> what gets stored into xcomp_bv[62:0]). xstate_comp_offsets[], > >> >> otoh, gets computed based on all available features, irrespective > >> >> of vcpu_xsave_mask() returning four different values depending > >> >> on current guest state. I can't see how get_xsave_addr() can > >> >> work correctly without honoring xcomp_bv. Nor can I convince > >> >> myself that state can't get corrupted / lost, e.g. when a save > >> >> with v->fpu_dirtied set is followed by one with v->fpu_dirtied > >> >> clear. > >> >> > >> >> Am I misunderstanding what the SDM writes? > >> >> > >> > Yes. you are right. This is a issue. I will find a way to solve > >> > this. > >> > >> Thanks. > > > > For xstate_comp_offsets is only used in get_xsave_addr when performing > > migration. > > I intend to recaculte xstate_comp_offsets based on the > > vcpu->arch.xsavec_area.save_hdr.xcomp_bv > > before get_xsave_addr is called. > > I don't think that'll suffice, as it won't deal with the lazy XSAVE[SC] > possibly overwriting data written by the non-lazy one. See the > effectively three different values returned by vcpu_xsave_mask() > (the fourth one is impossible since the function won't ever get > called with both v->fpu_dirtied and v->arch.nonlazy_xstate_used > clear). Oh, Yes, Thanks. The way I think to solve the problem is: if v->fpu_dirtied is clear and v->arch.nonlazy_xstate_used is set, vcpu_xsave_mask will return XSTATE_ALL (only if xsave[sc] is support). But this will do some overhead save. > ___ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel > ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] XSAVE flavors
>>> On 04.02.16 at 07:49, wrote: > On Tue, Feb 02, 2016 at 02:42:38AM -0700, Jan Beulich wrote: >> >> > With this another question then is whether, when both XSAVEC >> >> > and XSAVEOPT are available, it is indeed always better to use >> >> > XSAVEC (as the code is doing after your enabling). >> > Yes. >> > But current no machine only support xsavec not support xsaves. >> > I enable xsavec for "xsavec is a feature". >> >> But this shouldn't preclude the code being in reasonable shape >> also for the case where a CPU has XSAVEC but no XSAVES. The >> more that right now we don't really need XSAVES (since we don't >> yet allow any bit to get set in XSS). >> > Actually, when I enable xsaves/xsavec, I have put xsavec into > consideration. If xsavec is used we also need to guarntee that xcomp_bv > never has any bits clear which are set in xstate_bv and the compaction > bit is set. > > Those guarntee and xsavec specific code in my patch is always behind "if( > cpu_has_xsavec )" > or " if ( cpu_has_xsaves || cpu_has_xsavec )". > Please remind me if there is some other things I am not aware. I'm not pointing out any correctness issue here, all I'm asking is whether the current model is really the best one performance wise. >> >> And I'm afraid there's yet one more issue: If my reading of the >> >> SDM is right, then the offsets at which components get saved >> >> by XSAVEC / XSAVES aren't fixed, but depend on RFBM (as that's >> >> what gets stored into xcomp_bv[62:0]). xstate_comp_offsets[], >> >> otoh, gets computed based on all available features, irrespective >> >> of vcpu_xsave_mask() returning four different values depending >> >> on current guest state. I can't see how get_xsave_addr() can >> >> work correctly without honoring xcomp_bv. Nor can I convince >> >> myself that state can't get corrupted / lost, e.g. when a save >> >> with v->fpu_dirtied set is followed by one with v->fpu_dirtied >> >> clear. >> >> >> >> Am I misunderstanding what the SDM writes? >> >> >> > Yes. you are right. This is a issue. I will find a way to solve >> > this. >> >> Thanks. > > For xstate_comp_offsets is only used in get_xsave_addr when performing > migration. > I intend to recaculte xstate_comp_offsets based on the > vcpu->arch.xsavec_area.save_hdr.xcomp_bv > before get_xsave_addr is called. I don't think that'll suffice, as it won't deal with the lazy XSAVE[SC] possibly overwriting data written by the non-lazy one. See the effectively three different values returned by vcpu_xsave_mask() (the fourth one is impossible since the function won't ever get called with both v->fpu_dirtied and v->arch.nonlazy_xstate_used clear). > The patch will be sent out after Chinese New Year holiday. That's fine of course. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] XSAVE flavors
On Tue, Feb 02, 2016 at 02:42:38AM -0700, Jan Beulich wrote: > >> > With this another question then is whether, when both XSAVEC > >> > and XSAVEOPT are available, it is indeed always better to use > >> > XSAVEC (as the code is doing after your enabling). > > Yes. > > But current no machine only support xsavec not support xsaves. > > I enable xsavec for "xsavec is a feature". > > But this shouldn't preclude the code being in reasonable shape > also for the case where a CPU has XSAVEC but no XSAVES. The > more that right now we don't really need XSAVES (since we don't > yet allow any bit to get set in XSS). > Actually, when I enable xsaves/xsavec, I have put xsavec into consideration. If xsavec is used we also need to guarntee that xcomp_bv never has any bits clear which are set in xstate_bv and the compaction bit is set. Those guarntee and xsavec specific code in my patch is always behind "if( cpu_has_xsavec )" or " if ( cpu_has_xsaves || cpu_has_xsavec )". Please remind me if there is some other things I am not aware. > >> And I'm afraid there's yet one more issue: If my reading of the > >> SDM is right, then the offsets at which components get saved > >> by XSAVEC / XSAVES aren't fixed, but depend on RFBM (as that's > >> what gets stored into xcomp_bv[62:0]). xstate_comp_offsets[], > >> otoh, gets computed based on all available features, irrespective > >> of vcpu_xsave_mask() returning four different values depending > >> on current guest state. I can't see how get_xsave_addr() can > >> work correctly without honoring xcomp_bv. Nor can I convince > >> myself that state can't get corrupted / lost, e.g. when a save > >> with v->fpu_dirtied set is followed by one with v->fpu_dirtied > >> clear. > >> > >> Am I misunderstanding what the SDM writes? > >> > > Yes. you are right. This is a issue. I will find a way to solve > > this. > > Thanks. For xstate_comp_offsets is only used in get_xsave_addr when performing migration. I intend to recaculte xstate_comp_offsets based on the vcpu->arch.xsavec_area.save_hdr.xcomp_bv before get_xsave_addr is called. The patch will be sent out after Chinese New Year holiday. Thanks > > Jan > > > ___ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel > ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] XSAVE flavors
>>> On 02.02.16 at 07:31, wrote: > On Tue, Jan 26, 2016 at 08:12:20AM -0700, Jan Beulich wrote: >> >>> On 26.01.16 at 15:33, wrote: >> > originally I only meant to inquire about the state of the promised >> > alternatives improvement to the XSAVE code. However, while >> > looking over the code in question again I stumbled across a >> > separate issue: XSAVES, just like XSAVEOPT, may use the >> > "modified" optimization. However, the fcs and fds handling code >> > that has been present around the use of XSAVEOPT did not also >> > get applied to the XSAVES path. I suppose this was just an >> > oversight? > Really sorry for late response. The alternatives on xsave code is ok a > couples > weeks ago, the patch solve xsaves use modified optimization problem. > I will send it now. Thanks, But this response of yours covers only one half of what I've pointed out. >> > With this another question then is whether, when both XSAVEC >> > and XSAVEOPT are available, it is indeed always better to use >> > XSAVEC (as the code is doing after your enabling). > Yes. > But current no machine only support xsavec not support xsaves. > I enable xsavec for "xsavec is a feature". But this shouldn't preclude the code being in reasonable shape also for the case where a CPU has XSAVEC but no XSAVES. The more that right now we don't really need XSAVES (since we don't yet allow any bit to get set in XSS). >> And I'm afraid there's yet one more issue: If my reading of the >> SDM is right, then the offsets at which components get saved >> by XSAVEC / XSAVES aren't fixed, but depend on RFBM (as that's >> what gets stored into xcomp_bv[62:0]). xstate_comp_offsets[], >> otoh, gets computed based on all available features, irrespective >> of vcpu_xsave_mask() returning four different values depending >> on current guest state. I can't see how get_xsave_addr() can >> work correctly without honoring xcomp_bv. Nor can I convince >> myself that state can't get corrupted / lost, e.g. when a save >> with v->fpu_dirtied set is followed by one with v->fpu_dirtied >> clear. >> >> Am I misunderstanding what the SDM writes? >> > Yes. you are right. This is a issue. I will find a way to solve > this. Thanks. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] XSAVE flavors
On Tue, Jan 26, 2016 at 08:12:20AM -0700, Jan Beulich wrote: > >>> On 26.01.16 at 15:33, wrote: > > originally I only meant to inquire about the state of the promised > > alternatives improvement to the XSAVE code. However, while > > looking over the code in question again I stumbled across a > > separate issue: XSAVES, just like XSAVEOPT, may use the > > "modified" optimization. However, the fcs and fds handling code > > that has been present around the use of XSAVEOPT did not also > > get applied to the XSAVES path. I suppose this was just an > > oversight? Really sorry for late response. The alternatives on xsave code is ok a couples weeks ago, the patch solve xsaves use modified optimization problem. I will send it now. > > > > With this another question then is whether, when both XSAVEC > > and XSAVEOPT are available, it is indeed always better to use > > XSAVEC (as the code is doing after your enabling). Yes. But current no machine only support xsavec not support xsaves. I enable xsavec for "xsavec is a feature". > > And I'm afraid there's yet one more issue: If my reading of the > SDM is right, then the offsets at which components get saved > by XSAVEC / XSAVES aren't fixed, but depend on RFBM (as that's > what gets stored into xcomp_bv[62:0]). xstate_comp_offsets[], > otoh, gets computed based on all available features, irrespective > of vcpu_xsave_mask() returning four different values depending > on current guest state. I can't see how get_xsave_addr() can > work correctly without honoring xcomp_bv. Nor can I convince > myself that state can't get corrupted / lost, e.g. when a save > with v->fpu_dirtied set is followed by one with v->fpu_dirtied > clear. > > Am I misunderstanding what the SDM writes? > Yes. you are right. This is a issue. I will find a way to solve this. > Jan > ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] XSAVE flavors
>>> On 26.01.16 at 15:33, wrote: > originally I only meant to inquire about the state of the promised > alternatives improvement to the XSAVE code. However, while > looking over the code in question again I stumbled across a > separate issue: XSAVES, just like XSAVEOPT, may use the > "modified" optimization. However, the fcs and fds handling code > that has been present around the use of XSAVEOPT did not also > get applied to the XSAVES path. I suppose this was just an > oversight? > > With this another question then is whether, when both XSAVEC > and XSAVEOPT are available, it is indeed always better to use > XSAVEC (as the code is doing after your enabling). And I'm afraid there's yet one more issue: If my reading of the SDM is right, then the offsets at which components get saved by XSAVEC / XSAVES aren't fixed, but depend on RFBM (as that's what gets stored into xcomp_bv[62:0]). xstate_comp_offsets[], otoh, gets computed based on all available features, irrespective of vcpu_xsave_mask() returning four different values depending on current guest state. I can't see how get_xsave_addr() can work correctly without honoring xcomp_bv. Nor can I convince myself that state can't get corrupted / lost, e.g. when a save with v->fpu_dirtied set is followed by one with v->fpu_dirtied clear. Am I misunderstanding what the SDM writes? Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] XSAVE flavors
Shuai, originally I only meant to inquire about the state of the promised alternatives improvement to the XSAVE code. However, while looking over the code in question again I stumbled across a separate issue: XSAVES, just like XSAVEOPT, may use the "modified" optimization. However, the fcs and fds handling code that has been present around the use of XSAVEOPT did not also get applied to the XSAVES path. I suppose this was just an oversight? With this another question then is whether, when both XSAVEC and XSAVEOPT are available, it is indeed always better to use XSAVEC (as the code is doing after your enabling). Thanks, Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
