[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 05/14] x86/xstate: Map/unmap xsave area in xstate_set_init() and handle_setbv()
On Tue Oct 29, 2024 at 1:31 PM GMT, Jan Beulich wrote: > On 29.10.2024 14:00, Alejandro Vallejo wrote: > > On Tue Oct 29, 2024 at 8:26 AM GMT, Jan Beulich wrote: > >> On 28.10.2024 16:49, Alejandro Vallejo wrote: > >>> --- a/xen/arch/x86/xstate.c > >>> +++ b/xen/arch/x86/xstate.c > >>> @@ -993,7 +993,12 @@ int handle_xsetbv(u32 index, u64 new_bv) > >>> > >>> clts(); > >>> if ( curr->fpu_dirtied ) > >>> - asm ( "stmxcsr %0" : "=m" > >>> (curr->arch.xsave_area->fpu_sse.mxcsr) ); > >>> + { > >>> + struct xsave_struct *xsave_area = vcpu_map_xsave_area(curr); > >>> + > >>> + asm ( "stmxcsr %0" : "=m" (xsave_area->fpu_sse.mxcsr) ); > >>> + vcpu_unmap_xsave_area(curr, xsave_area); > >>> + } > >> > >> Since it's curr that we're dealing with, is this largely a cosmetic > >> change? I.e. > >> there's no going to be any actual map/unmap operation in that case? > >> Otherwise > >> I'd be inclined to say that an actual map/unmap is pretty high overhead > >> for a > >> mere store of a 32-bit value. > > > > Somewhat. > > > > See the follow-up reply to patch2 with something resembling what I expect > > the > > wrappers to have. In short, yes, I expect "current" to not require > > mapping/unmapping; but I still would rather see those sites using the same > > wrappers for auditability. After we settle on a particular interface, we can > > let the implementation details creep out if that happens to be clearer, but > > it's IMO easier to work this way for the time being until those details > > crystalise. > > Sure. As expressed in a later reply on the same topic, what I'm after are > brief > comments indicating that despite the function names involved, no actual > mapping > operations will be carried out in these cases, thus addressing concerns > towards > the overhead involved. > > Jan Right, I can add those to the sites using exclusively "current". That's no problem. Cheers, Alejandro
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |