Re: [XEN][PATCH] xen/x86: guest_access: optimize raw_x_guest() for PV and HVM combinations

["Teddy Astie" <teddy.astie@xxxxxxxxxx>]

Le 06/11/2025 à 18:00, Jason Andryuk a écrit :
> On 2025-11-06 11:33, Grygorii Strashko wrote:
>> Hi Teddy, Jan,
>>
>> On 06.11.25 17:57, Teddy Astie wrote:
>>> Le 31/10/2025 à 22:25, Grygorii Strashko a écrit :
>> Can try.
>
> Yes, I was thinking something like Teddy suggested:
>
> #define raw_copy_to_guest(dst, src, len)        \
>          (is_hvm_vcpu(current) ? copy_to_user_hvm(dst, src, len) :
>           is_pv_vcpu(current) ? copy_to_guest_pv(dst, src, len) :
>           fail_copy(dst, src, len))
>
> But that made the think the is_{hvm,pv}_{vcpu,domain}() could be
> optimized for when only 1 of HVM or PV is enabled.
>
> Regards,
> Jason
>
> xen: Optimize is_hvm/pv_domain() for single domain type
>
> is_hvm_domain() and is_pv_domain() hardcode the false conditions for
> HVM=n and PV=n, but they still leave the XEN_DOMCTL_CDF_hvm flag
> checking.  When only one of PV or HVM is set, the result can be hard
> coded since the other is impossible.  Notably, this removes the
> evaluate_nospec() lfences.
>
> Signed-off-by: Jason Andryuk <jason.andryuk@xxxxxxx>
> ---
> Untested.
>
> HVM=y PV=n bloat-o-meter:
>
> add/remove: 3/6 grow/shrink: 19/212 up/down: 3060/-60349 (-57289)
>
> Full bloat-o-meter below.
> ---
>   xen/include/xen/sched.h | 18 ++++++++++++++----
>   1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index f680fb4fa1..12f10d9cc8 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -1176,8 +1176,13 @@ static always_inline bool
> is_hypercall_target(const struct domain *d)
>
>   static always_inline bool is_pv_domain(const struct domain *d)
>   {
> -    return IS_ENABLED(CONFIG_PV) &&
> -        evaluate_nospec(!(d->options & XEN_DOMCTL_CDF_hvm));
> +    if ( !IS_ENABLED(CONFIG_PV) )
> +        return false;
> +
> +    if ( IS_ENABLED(CONFIG_HVM) )
> +        return evaluate_nospec(!(d->options & XEN_DOMCTL_CDF_hvm));
> +
> +    return true;
>   }
>
>   static always_inline bool is_pv_vcpu(const struct vcpu *v)
> @@ -1218,8 +1223,13 @@ static always_inline bool is_pv_64bit_vcpu(const
> struct vcpu *v)
>
>   static always_inline bool is_hvm_domain(const struct domain *d)
>   {
> -    return IS_ENABLED(CONFIG_HVM) &&
> -        evaluate_nospec(d->options & XEN_DOMCTL_CDF_hvm);
> +    if ( !IS_ENABLED(CONFIG_HVM) )
> +        return false;
> +
> +    if ( IS_ENABLED(CONFIG_PV) )
> +        return evaluate_nospec(d->options & XEN_DOMCTL_CDF_hvm);
> +
> +    return true;
>   }
>
>   static always_inline bool is_hvm_vcpu(const struct vcpu *v)

While I like the idea, it may slightly impact some logic as special
domains (dom_xen and dom_io) are now considered HVM domains (when !PV &&
HVM) instead of "neither PV nor HVM".
We want at least to make sure we're not silently breaking something
elsewhere.

Teddy


--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech