Re: [PATCH 02/14] x86/xstate: Create map/unmap primitives for xsave areas

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 28/10/2024 3:49 pm, Alejandro Vallejo wrote:
> diff --git a/xen/arch/x86/include/asm/xstate.h 
> b/xen/arch/x86/include/asm/xstate.h
> index 07017cc4edfd..36260459667c 100644
> --- a/xen/arch/x86/include/asm/xstate.h
> +++ b/xen/arch/x86/include/asm/xstate.h
> @@ -143,4 +143,24 @@ static inline bool xstate_all(const struct vcpu *v)
>             (v->arch.xcr0_accum & XSTATE_LAZY & ~XSTATE_FP_SSE);
>  }
>  
> +/*
> + * Fetch a pointer to the XSAVE area of a vCPU
> + *
> + * If ASI is enabled for the domain, this mapping is pCPU-local.
> + *
> + * @param v Owner of the XSAVE area
> + */
> +#define vcpu_map_xsave_area(v) ((v)->arch.xsave_area)
> +
> +/*
> + * Drops the XSAVE area of a vCPU and nullifies its pointer on exit.
> + *
> + * If ASI is enabled and v is not the currently scheduled vCPU then the
> + * per-pCPU mapping is removed from the address space.
> + *
> + * @param v           vCPU logically owning xsave_area
> + * @param xsave_area  XSAVE blob of v
> + */
> +#define vcpu_unmap_xsave_area(v, x) ({ (x) = NULL; })
> +

Is there a preview of how these will end up looking with the real ASI
bits in place?

Having a macro-that-reads-like-a-function mutating x by name, rather
than by pointer, is somewhat rude.  This is why we capitalise
XFREE()/etc which have a similar pattern; to make it clear it's a macro
and potentially doing weird things with scopes.

~Andrew