[PATCH] vmx: Introduce vcpu single context VPID invalidation

Teddy Astie posted 1 patch 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/83d0e7dfc076e9453fb6537e5948c03d90e947da.1748594036.git.teddy.astie@vates.tech
xen/arch/x86/include/asm/hvm/vmx/vmx.h | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
[PATCH] vmx: Introduce vcpu single context VPID invalidation
Posted by Teddy Astie 5 months ago
Introduce vpid_sync_vcpu_context to do a single-context invalidation
on the vpid attached to the vcpu as a alternative to per-gva and all-contexts
invlidations.

Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
---
Extracted from SEV series.
This will be used for instance in fixed-ASID patches (in SEV series).
---
 xen/arch/x86/include/asm/hvm/vmx/vmx.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/xen/arch/x86/include/asm/hvm/vmx/vmx.h b/xen/arch/x86/include/asm/hvm/vmx/vmx.h
index d85b52b9d5..1269c318dc 100644
--- a/xen/arch/x86/include/asm/hvm/vmx/vmx.h
+++ b/xen/arch/x86/include/asm/hvm/vmx/vmx.h
@@ -451,6 +451,23 @@ static inline void ept_sync_all(void)
 
 void ept_sync_domain(struct p2m_domain *p2m);
 
+static inline void vpid_sync_vcpu_context(struct vcpu *v)
+{
+    int type = INVVPID_SINGLE_CONTEXT;
+
+    /*
+     * If single context invalidation is not supported, we escalate to
+     * use all context invalidation.
+     */
+    if ( likely(cpu_has_vmx_vpid_invvpid_single_context) )
+        goto execute_invvpid;
+
+    type = INVVPID_ALL_CONTEXT;
+
+execute_invvpid:
+    __invvpid(type, v->arch.hvm.n1asid.asid, 0);
+}
+
 static inline void vpid_sync_vcpu_gva(struct vcpu *v, unsigned long gva)
 {
     int type = INVVPID_INDIVIDUAL_ADDR;
-- 
2.49.0



Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech
Re: [PATCH] vmx: Introduce vcpu single context VPID invalidation
Posted by Jan Beulich 4 months, 3 weeks ago
On 30.05.2025 10:48, Teddy Astie wrote:
> Introduce vpid_sync_vcpu_context to do a single-context invalidation
> on the vpid attached to the vcpu as a alternative to per-gva and all-contexts
> invlidations.
> 
> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
> ---
> Extracted from SEV series.
> This will be used for instance in fixed-ASID patches (in SEV series).

I think it should be in that series, which may still be some long way out.
Until then we'd carry dead/unreachable code (disliked by Misra), and we'd
risk that this bit-rots because of being unused.

> --- a/xen/arch/x86/include/asm/hvm/vmx/vmx.h
> +++ b/xen/arch/x86/include/asm/hvm/vmx/vmx.h
> @@ -451,6 +451,23 @@ static inline void ept_sync_all(void)
>  
>  void ept_sync_domain(struct p2m_domain *p2m);
>  
> +static inline void vpid_sync_vcpu_context(struct vcpu *v)

pointer-to-const?

> +{
> +    int type = INVVPID_SINGLE_CONTEXT;

Please don't use plain int when all values possibly held in a variable are
non-negative.

> +    /*
> +     * If single context invalidation is not supported, we escalate to
> +     * use all context invalidation.
> +     */
> +    if ( likely(cpu_has_vmx_vpid_invvpid_single_context) )
> +        goto execute_invvpid;
> +
> +    type = INVVPID_ALL_CONTEXT;
> +
> +execute_invvpid:

There no reason at all to use "goto" here (and with that replaced there's
then also no style issue with the label placement).

Jan

> +    __invvpid(type, v->arch.hvm.n1asid.asid, 0);
> +}
> +
>  static inline void vpid_sync_vcpu_gva(struct vcpu *v, unsigned long gva)
>  {
>      int type = INVVPID_INDIVIDUAL_ADDR;
Re: [PATCH] vmx: Introduce vcpu single context VPID invalidation
Posted by Teddy Astie 4 months, 3 weeks ago
Le 05/06/2025 à 16:51, Jan Beulich a écrit :
> On 30.05.2025 10:48, Teddy Astie wrote:
>> Introduce vpid_sync_vcpu_context to do a single-context invalidation
>> on the vpid attached to the vcpu as a alternative to per-gva and all-contexts
>> invlidations.
>>
>> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
>> ---
>> Extracted from SEV series.
>> This will be used for instance in fixed-ASID patches (in SEV series).
> 
> I think it should be in that series, which may still be some long way out.
> Until then we'd carry dead/unreachable code (disliked by Misra), and we'd
> risk that this bit-rots because of being unused.
> 

Yes, that make sense, it should exist along with patches that make use 
of it.

>> --- a/xen/arch/x86/include/asm/
hvm/vmx/vmx.h
>> +++ b/xen/arch/x86/include/asm/hvm/vmx/vmx.h
>> @@ -451,6 +451,23 @@ static inline void ept_sync_all(void)
>>   
>>   void ept_sync_domain(struct p2m_domain *p2m);
>>   
>> +static inline void vpid_sync_vcpu_context(struct vcpu *v)
> 
> pointer-to-const?
> 
>> +{
>> +    int type = INVVPID_SINGLE_CONTEXT;
> 
> Please don't use plain int when all values possibly held in a variable are
> non-negative.
> 

I mostly took the code from other vcpu_sync_* functions. I will take a 
look as a separate patch on reworking the existing vpid_sync_vcpu_gva 
with these proposed changes, as it has the same problem.

>> +    /*
>> +     * If single context invalidation is not supported, we escalate to
>> +     * use all context invalidation.
>> +     */
>> +    if ( likely(cpu_has_vmx_vpid_invvpid_single_context) )
>> +        goto execute_invvpid;
>> +
>> +    type = INVVPID_ALL_CONTEXT;
>> +
>> +execute_invvpid:
> 
> There no reason at all to use "goto" here (and with that replaced there's
> then also no style issue with the label placement).
> 

Should a similar treatment be made for vpid_sync_vcpu_gva ?

> Jan
> 
>> +    __invvpid(type, v->arch.hvm.n1asid.asid, 0);
>> +}
>> +
>>   static inline void vpid_sync_vcpu_gva(struct vcpu *v, unsigned long gva)
>>   {
>>       int type = INVVPID_INDIVIDUAL_ADDR;
> 
> 

Teddy



Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech
Re: [PATCH] vmx: Introduce vcpu single context VPID invalidation
Posted by Jan Beulich 4 months, 3 weeks ago
On 05.06.2025 17:17, Teddy Astie wrote:
> Le 05/06/2025 à 16:51, Jan Beulich a écrit :
>> On 30.05.2025 10:48, Teddy Astie wrote:
>>> +    /*
>>> +     * If single context invalidation is not supported, we escalate to
>>> +     * use all context invalidation.
>>> +     */
>>> +    if ( likely(cpu_has_vmx_vpid_invvpid_single_context) )
>>> +        goto execute_invvpid;
>>> +
>>> +    type = INVVPID_ALL_CONTEXT;
>>> +
>>> +execute_invvpid:
>>
>> There no reason at all to use "goto" here (and with that replaced there's
>> then also no style issue with the label placement).
> 
> Should a similar treatment be made for vpid_sync_vcpu_gva ?

I wouldn't require anyone to do a re-work, but the latest when it is touched
the next time it likely should be polished some. For context, while iirc we
didn't accept that rule, Misra in principle also demands that "goto" not be
used.

Jan