|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [XEN][PATCH] xen/x86: guest_access: optimize raw_x_guest() for PV and HVM combinations
Le 31/10/2025 à 22:25, Grygorii Strashko a écrit :
> From: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
>
> Xen uses below pattern for raw_x_guest() functions:
>
> define raw_copy_to_guest(dst, src, len) \
> (is_hvm_vcpu(current) ? \
> copy_to_user_hvm((dst), (src), (len)) : \
> copy_to_guest_pv(dst, src, len))
>
> How this pattern is working depends on CONFIG_PV/CONFIG_HVM as:
> - PV=y and HVM=y
> Proper guest access function is selected depending on domain type.
> - PV=y and HVM=n
> Only PV domains are possible. is_hvm_domain/vcpu() will constify to "false"
> and compiler will optimize code and skip HVM specific part.
> - PV=n and HVM=y
> Only HVM domains are possible. is_hvm_domain/vcpu() will not be constified.
> No PV specific code will be optimized by compiler.
> - PV=n and HVM=n
> No guests should possible. The code will still follow PV path.
>
> Rework raw_x_guest() code to use required functions explicitly for each
> combination of CONFIG_PV/CONFIG_HVM with main intention to optimize code for
> (PV=n and HVM=y) case.
>
> For the case (PV=n and HVM=n) empty stubs are created which return (1)
> indicating failure. Hence, no guests should possible in this case -
> which means no access to guest memory should ever happen.
> The two calls of __raw_copy_to_guest() in
> common/domain.c->update_runstate_area()
> are fixed for this case by explicitly cast the return value to void
> (MISRA C Rule 17.7).
>
> Finally build arch/x86/usercopy.c only for PV=y.
>
> The measured (bloat-o-meter) improvement for (PV=n and HVM=y) case is:
> add/remove: 0/10 grow/shrink: 2/90 up/down: 163/-30932 (-30769)
> Total: Before=1937113, After=1906344, chg -1.59%
>
> Signed-off-by: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
> ---
> xen/arch/x86/Makefile | 2 +-
> xen/arch/x86/include/asm/guest_access.h | 38 +++++++++++++++++++++++++
> xen/common/domain.c | 10 ++++---
> 3 files changed, 45 insertions(+), 5 deletions(-)
>
> diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
> index 407571c510e1..27f131ffeb61 100644
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -71,7 +71,7 @@ obj-y += time.o
> obj-y += traps-setup.o
> obj-y += traps.o
> obj-$(CONFIG_INTEL) += tsx.o
> -obj-y += usercopy.o
> +obj-$(CONFIG_PV) += usercopy.o
> obj-y += x86_emulate.o
> obj-$(CONFIG_TBOOT) += tboot.o
> obj-y += hpet.o
> diff --git a/xen/arch/x86/include/asm/guest_access.h
> b/xen/arch/x86/include/asm/guest_access.h
> index 69716c8b41bb..36aeb89524ab 100644
> --- a/xen/arch/x86/include/asm/guest_access.h
> +++ b/xen/arch/x86/include/asm/guest_access.h
> @@ -13,6 +13,7 @@
> #include <asm/hvm/guest_access.h>
>
> /* Raw access functions: no type checking. */
> +#if defined(CONFIG_PV) && defined(CONFIG_HVM)
> #define raw_copy_to_guest(dst, src, len) \
> (is_hvm_vcpu(current) ? \
> copy_to_user_hvm((dst), (src), (len)) : \
> @@ -34,6 +35,43 @@
> copy_from_user_hvm((dst), (src), (len)) : \
> __copy_from_guest_pv(dst, src, len))
>
> +#elif defined(CONFIG_HVM)
> +#define raw_copy_to_guest(dst, src, len) \
> + copy_to_user_hvm((dst), (src), (len))
> +#define raw_copy_from_guest(dst, src, len) \
> + copy_from_user_hvm((dst), (src), (len))
> +#define raw_clear_guest(dst, len) \
> + clear_user_hvm((dst), (len))
> +#define __raw_copy_to_guest(dst, src, len) \
> + copy_to_user_hvm((dst), (src), (len))
> +#define __raw_copy_from_guest(dst, src, len) \
> + copy_from_user_hvm((dst), (src), (len))
> +
> +#elif defined(CONFIG_PV)
> +#define raw_copy_to_guest(dst, src, len) \
> + copy_to_guest_pv(dst, src, len)
> +#define raw_copy_from_guest(dst, src, len) \
> + copy_from_guest_pv(dst, src, len)
> +#define raw_clear_guest(dst, len) \
> + clear_guest_pv(dst, len)
> +#define __raw_copy_to_guest(dst, src, len) \
> + __copy_to_guest_pv(dst, src, len)
> +#define __raw_copy_from_guest(dst, src, len) \
> + __copy_from_guest_pv(dst, src, len)
> +
> +#else
> +#define raw_copy_to_guest(dst, src, len) \
> + ((void)(dst), (void)(src), (void)(len), 1)
> +#define raw_copy_from_guest(dst, src, len) \
> + ((void)(dst), (void)(src), (void)(len), 1)
> +#define raw_clear_guest(dst, len) \
> + ((void)(dst), (void)(len), 1)
> +#define __raw_copy_to_guest(dst, src, len) \
> + ((void)(dst), (void)(src), (void)(len), 1)
> +#define __raw_copy_from_guest(dst, src, len) \
> + ((void)(dst), (void)(src), (void)(len), 1)
> +#endif
> +
> /*
> * Pre-validate a guest handle.
> * Allows use of faster __copy_* functions.
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 4f91316ad93e..c603edcc7d46 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -1985,8 +1985,9 @@ bool update_runstate_area(struct vcpu *v)
> #endif
> guest_handle--;
> runstate.state_entry_time |= XEN_RUNSTATE_UPDATE;
> - __raw_copy_to_guest(guest_handle,
> - (void *)(&runstate.state_entry_time + 1) - 1, 1);
> + (void)__raw_copy_to_guest(guest_handle,
> + (void *)(&runstate.state_entry_time + 1) -
> 1,
> + 1);
> smp_wmb();
> }
>
> @@ -2008,8 +2009,9 @@ bool update_runstate_area(struct vcpu *v)
> {
> runstate.state_entry_time &= ~XEN_RUNSTATE_UPDATE;
> smp_wmb();
> - __raw_copy_to_guest(guest_handle,
> - (void *)(&runstate.state_entry_time + 1) - 1, 1);
> + (void)__raw_copy_to_guest(guest_handle,
> + (void *)(&runstate.state_entry_time + 1) -
> 1,
> + 1);
> }
>
> update_guest_memory_policy(v, &policy);
Alternatively, we can make all the raw_* functions `static inline` and
have something like this which should have the same effect with much
less redundancy.
static inline
unsigned int raw_copy_to_user_hvm(void *to, const void *from,
unsigned int len)
{
if ( IS_ENABLED(CONFIG_HVM) &&
(!IS_ENABLED(CONFIG_PV) || is_hvm_vcpu(current) )
copy_to_user_hvm(to, from, len);
else if ( IS_ENABLED(CONFIG_PV) )
copy_to_guest_pv(to, from, len);
}
Teddy
--
Teddy Astie | Vates XCP-ng Developer
XCP-ng & Xen Orchestra - Vates solutions
web: https://vates.tech
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |