[v2] x86: Address Space Isolation FPU preparations

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

Posted by Alejandro Vallejo 1 year, 3 months ago

No functional change.

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
---
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);
 }
 
 /*
@@ -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);
 }
 
 void xsave(struct vcpu *v, uint64_t mask)
-- 
2.47.0

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

Posted by Jan Beulich 1 year, 1 month ago

On 05.11.2024 15:33, Alejandro Vallejo wrote:
> No functional change.
> 
> Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
> ---
> 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

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

Posted by Alejandro Vallejo 1 year, 1 month ago

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@cloud.com>
> > ---
> > 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