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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 29.10.2024 12:57, Alejandro Vallejo wrote:
> On Mon Oct 28, 2024 at 5:20 PM GMT, Andrew Cooper wrote:
>> 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?
> 
> I expect the contents to be something along these lines (in function form for
> clarity):
> 
>   struct xsave_struct *vcpu_map_xsave_area(struct vcpu *v)
>   {
>       if ( !v->domain->asi )
>           return v->arch.xsave_area;
> 
>       if ( likely(v == current) )
>           return percpu_fixmap(v, PCPU_FIX_XSAVE_AREA);
> 
>       /* Likely some new vmap-like abstraction after AMX */
>       return map_domain_page(v->arch.xsave_area_pg);
>   }

I'd like to ask that map_domain_page() be avoided here from the beginning, to
take AMX into account right away. I've been sitting on the AMX series for
years, and I'd consider it pretty unfair if it was me to take care of such an
aspect, when instead the series should (imo) long have landed.

Jan