Re: [PATCH v2 08/13] x86/xstate: Map/unmap xsave area in {compress,expand}_xsave_states()

["Alejandro Vallejo" <alejandro.vallejo@xxxxxxxxx>]

On Mon Dec 9, 2024 at 4:20 PM GMT, Jan Beulich wrote:
> On 05.11.2024 15:33, Alejandro Vallejo wrote:
> > No functional change.
> > 
> > Signed-off-by: Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>
> > ---
> > v2:
> >   * No change
> > ---
> >  xen/arch/x86/xstate.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
> > index 401bdad2eb0d..6db7ec2ea6a9 100644
> > --- a/xen/arch/x86/xstate.c
> > +++ b/xen/arch/x86/xstate.c
> > @@ -177,7 +177,7 @@ static void setup_xstate_comp(uint16_t *comp_offsets,
> >   */
> >  void expand_xsave_states(const struct vcpu *v, void *dest, unsigned int 
> > size)
> >  {
> > -    const struct xsave_struct *xstate = v->arch.xsave_area;
> > +    const struct xsave_struct *xstate = VCPU_MAP_XSAVE_AREA(v);
> >      const void *src;
> >      uint16_t comp_offsets[sizeof(xfeature_mask)*8];
> >      u64 xstate_bv = xstate->xsave_hdr.xstate_bv;
> > @@ -228,6 +228,8 @@ void expand_xsave_states(const struct vcpu *v, void 
> > *dest, unsigned int size)
> >  
> >          valid &= ~feature;
> >      }
> > +
> > +    VCPU_UNMAP_XSAVE_AREA(v, xstate);
> >  }
>
> In the middle of these two hunks there's an early return.
>
> > @@ -242,7 +244,7 @@ void expand_xsave_states(const struct vcpu *v, void 
> > *dest, unsigned int size)
> >   */
> >  void compress_xsave_states(struct vcpu *v, const void *src, unsigned int 
> > size)
> >  {
> > -    struct xsave_struct *xstate = v->arch.xsave_area;
> > +    struct xsave_struct *xstate = VCPU_MAP_XSAVE_AREA(v);
> >      void *dest;
> >      uint16_t comp_offsets[sizeof(xfeature_mask)*8];
> >      u64 xstate_bv, valid;
> > @@ -294,6 +296,8 @@ void compress_xsave_states(struct vcpu *v, const void 
> > *src, unsigned int size)
> >  
> >          valid &= ~feature;
> >      }
> > +
> > +    VCPU_UNMAP_XSAVE_AREA(v, xstate);
> >  }
>
> Same here.
>
> Jan

Doh! Yes, good catch. I'll "goto out" on both rather than the early exit to
ensure the unmap is invoked in the "already (de)compressed" cases.

Cheers,
Alejandro