Re: [PATCH 06/14] x86/hvm: Map/unmap xsave area in hvmemul_{get,put}_fpu()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.10.2024 16:49, Alejandro Vallejo wrote:
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -2371,7 +2371,8 @@ static int cf_check hvmemul_get_fpu(
>          alternative_vcall(hvm_funcs.fpu_dirty_intercept);
>      else if ( type == X86EMUL_FPU_fpu )
>      {
> -        const fpusse_t *fpu_ctxt = &curr->arch.xsave_area->fpu_sse;
> +        const struct xsave_struct *xsave_area = vcpu_map_xsave_area(curr);
> +        const fpusse_t *fpu_ctxt = &xsave_area->fpu_sse;
>  
>          /*
>           * Latch current register state so that we can back out changes
> @@ -2397,6 +2398,8 @@ static int cf_check hvmemul_get_fpu(
>              else
>                  ASSERT(fcw == fpu_ctxt->fcw);
>          }
> +
> +        vcpu_unmap_xsave_area(curr, xsave_area);
>      }

Same question as for the other patch: Mainly a cosmetic change, with no
actual map/unmap?

> @@ -2411,7 +2414,8 @@ static void cf_check hvmemul_put_fpu(
>  
>      if ( aux )
>      {
> -        fpusse_t *fpu_ctxt = &curr->arch.xsave_area->fpu_sse;
> +        struct xsave_struct *xsave_area = vcpu_map_xsave_area(curr);
> +        fpusse_t *fpu_ctxt = &xsave_area->fpu_sse;
>          bool dval = aux->dval;
>          int mode = hvm_guest_x86_mode(curr);
>  
> @@ -2465,6 +2469,8 @@ static void cf_check hvmemul_put_fpu(
>  
>          fpu_ctxt->fop = aux->op;
>  
> +        vcpu_unmap_xsave_area(curr, xsave_area);
> +
>          /* Re-use backout code below. */
>          backout = X86EMUL_FPU_fpu;
>      }

Same here. Because of the overhead concern, such places may be worthwhile to
gain brief comments.

Jan