Re: [PATCH v2 1/5] vmx: Refactor vpid_sync_vcpu_gva()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 11.09.2026 11:34, Teddy Astie wrote:
> Le 10/09/2026 à 17:45, Jan Beulich a écrit :
>> On 31.07.2026 16:46, Teddy Astie wrote:
>>> --- a/xen/arch/x86/include/asm/hvm/vmx/vmx.h
>>> +++ b/xen/arch/x86/include/asm/hvm/vmx/vmx.h
>>> @@ -460,19 +460,16 @@ static inline void vpid_sync_vcpu_gva(struct vcpu *v, 
>>> unsigned long gva)
>>>        * If individual address invalidation is not supported, we escalate to
>>>        * use single context invalidation.
>>>        */
>>> -    if ( likely(cpu_has_vmx_vpid_invvpid_individual_addr) )
>>> -        goto execute_invvpid;
>>> -
>>> -    type = INVVPID_SINGLE_CONTEXT;
>>> +    if ( unlikely(!cpu_has_vmx_vpid_invvpid_individual_addr) )
>>> +        type = INVVPID_SINGLE_CONTEXT;
>>>   
>>>       /*
>>>        * If single context invalidation is not supported, we escalate to
>>>        * use all context invalidation.
>>>        */
>>> -    if ( !cpu_has_vmx_vpid_invvpid_single_context )
>>> +    if ( unlikely(!cpu_has_vmx_vpid_invvpid_single_context) )
>>>           type = INVVPID_ALL_CONTEXT;
>>>   
>>> -execute_invvpid:
>>>       __invvpid(type, v->arch.hvm.n1asid.asid, (u64)gva);
>>>   }
>>
>> But this isn't functionally identical to the original, at least in the
>> cpu_has_vmx_vpid_invvpid_individual_addr &&
>> !cpu_has_vmx_vpid_invvpid_single_context case. We'd now end up using
>> INVVPID_ALL_CONTEXT there, when it wants to continue to be
>> INVVPID_INDIVIDUAL_ADDR.
>>
> 
> Yes, though a good question is to know if there exist a CPU that has 
> INVVPID_INDIVIDUAL_ADDR without INVVPID_SINGLE_CONTEXT; otherwise, we 
> can just say that this is not supposed to happen.
> 
> One alternative I have in mind would be to have cascading fallbacks to 
> "larger scope" alternatives.
> 
> i.e
> 
> static inline void vpid_sync_all(void)
> {
>      __invvpid(INVVPID_ALL_CONTEXT, 0, 0);
> }
> 
> static inline void vpid_sync_vcpu_context(const struct vcpu *v)
> {
>      /*
>       * If single context invalidation is not supported, we escalate to
>       * use all context invalidation.
>       */
>      if ( unlikely(!cpu_has_vmx_vpid_invvpid_single_context) )
>          return vpid_sync_all();
> 
>      __invvpid(INVVPID_SINGLE_CONTEXT, v->arch.hvm.n1asid.asid, 0);
> }
> 
> static inline void vpid_sync_vcpu_gva(struct vcpu *v, unsigned long gva)
> {
>      /*
>       * If individual address invalidation is not supported, we escalate to
>       * use single context invalidation.
>       */
>      if ( unlikely(!cpu_has_vmx_vpid_invvpid_individual_addr) )
>          return vpid_sync_vcpu_context(v);
> 
>      __invvpid(INVVPID_INDIVIDUAL_ADDR, v->arch.hvm.n1asid.asid, (u64)gva);
> }
> 
> That requires introducing vpid_sync_vcpu_context() ahead, but would 
> avoid the corner case without making things too complex.

I think that would be fine. Ideally in the process the unnecessary cast and
the const-vs-not inconsistency would also be addressed.

Jan