[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 08/12] x86/emulator: Refactor FXSAVE_AREA to use wrappers



On Wed Mar 5, 2025 at 3:29 PM GMT, Jan Beulich wrote:
> On 10.01.2025 14:28, Alejandro Vallejo wrote:
> > --- a/xen/arch/x86/x86_emulate/blk.c
> > +++ b/xen/arch/x86/x86_emulate/blk.c
> > @@ -11,9 +11,12 @@
> >      !defined(X86EMUL_NO_SIMD)
> >  # ifdef __XEN__
> >  #  include <asm/xstate.h>
> > -#  define FXSAVE_AREA ((void *)&current->arch.xsave_area->fpu_sse)
> > +/* Has a fastpath for `current`, so there's no actual map */
> > +#  define FXSAVE_AREA ((void *)VCPU_MAP_XSAVE_AREA(current))
> > +#  define UNMAP_FXSAVE_AREA(x) VCPU_UNMAP_XSAVE_AREA(current, x)
> >  # else
> >  #  define FXSAVE_AREA get_fpu_save_area()
> > +#  define UNMAP_FXSAVE_AREA(x) ((void)(x))
> >  # endif
> >  #endif
>
> While preparing to commit this I felt a little uneasy. The mapping aspect
> is ...

Thanks for coming back to it :)

>
> > @@ -292,6 +295,9 @@ int x86_emul_blk(
> >          }
> >          else
> >              asm volatile ( "fxrstor %0" :: "m" (*fxsr) );
> > +
> > +        UNMAP_FXSAVE_AREA(fxsr);
> > +
> >          break;
> >      }
> >  
> > @@ -320,6 +326,9 @@ int x86_emul_blk(
> >  
> >          if ( fxsr != ptr ) /* i.e. s->op_bytes < sizeof(*fxsr) */
> >              memcpy(ptr, fxsr, s->op_bytes);
> > +
> > +        UNMAP_FXSAVE_AREA(fxsr);
> > +
> >          break;
> >      }
> >  
>
> ... is entirely invisible at the use sites. This imo calls for making
> mistakes, and hence the existing macro better would be adjusted to become
> MAP_FXSAVE_AREA().
>
> Jan

I prefer it that way too; I was simply trying to minimize the diff. Would you
like me to resend it with that adjustment?

Cheers,
Alejandro



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.