From: Grygorii Strashko <grygorii_strashko@epam.com> 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@epam.com> --- 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 XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/Makefile +++ b/xen/arch/x86/Makefile @@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/include/asm/guest_access.h +++ b/xen/arch/x86/include/asm/guest_access.h @@ -XXX,XX +XXX,XX @@ #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)) : \ @@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -XXX,XX +XXX,XX @@ 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(); } @@ -XXX,XX +XXX,XX @@ 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); -- 2.34.1 From: Grygorii Strashko <grygorii_strashko@epam.com> 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)) This pattern works depending 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 static inline functions which account for above PV/HVM possible configurations with main intention to optimize code for (PV=n and HVM=y) case. For the case (PV=n and HVM=n) return "len" value indicating a failure (no guests should be possible in this case, which means no access to guest memory should ever happen). The measured (bloat-o-meter) improvement for (PV=n and HVM=y) case is about -11K. [teddy.astie@vates.tech: Suggested to use static inline functions vs macro combinations] Suggested-by: Teddy Astie <teddy.astie@vates.tech> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com> --- changes in v5: - rebase - drop moving usercopy.c as it is not needed since commit 7370966d1cb7 ("x86: move / split usercopy.c to / into arch-specific library") changes in v4: - move usercopy.c into arch/x86/pv/ - rework to always dynamically check for HVM vcpu(domain) by using is_hvm_vcpu() as requested by Jan Beulich changes in v3: - add raw_use_hvm_access() wrapper changes in v2: - use static inline functions instead of macro combinations xen/arch/x86/include/asm/guest_access.h | 78 ++++++++++++++++++------- 1 file changed, 58 insertions(+), 20 deletions(-) diff --git a/xen/arch/x86/include/asm/guest_access.h b/xen/arch/x86/include/asm/guest_access.h index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/include/asm/guest_access.h +++ b/xen/arch/x86/include/asm/guest_access.h @@ -XXX,XX +XXX,XX @@ #include <asm/hvm/guest_access.h> /* Raw access functions: no type checking. */ -#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)) -#define raw_copy_from_guest(dst, src, len) \ - (is_hvm_vcpu(current) ? \ - copy_from_user_hvm((dst), (src), (len)) : \ - copy_from_guest_pv(dst, src, len)) -#define raw_clear_guest(dst, len) \ - (is_hvm_vcpu(current) ? \ - clear_user_hvm((dst), (len)) : \ - clear_guest_pv(dst, len)) -#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)) -#define __raw_copy_from_guest(dst, src, len) \ - (is_hvm_vcpu(current) ? \ - copy_from_user_hvm((dst), (src), (len)) : \ - __copy_from_guest_pv(dst, src, len)) +static inline unsigned int raw_copy_to_guest(void *dst, const void *src, + unsigned int len) +{ + if ( is_hvm_vcpu(current) ) + return copy_to_user_hvm(dst, src, len); + + if ( !IS_ENABLED(CONFIG_PV) ) + return len; + + return copy_to_guest_pv(dst, src, len); +} + +static inline unsigned int raw_copy_from_guest(void *dst, const void *src, + unsigned int len) +{ + if ( is_hvm_vcpu(current) ) + return copy_from_user_hvm(dst, src, len); + + if ( !IS_ENABLED(CONFIG_PV) ) + return len; + + return copy_from_guest_pv(dst, src, len); +} + +static inline unsigned int raw_clear_guest(void *dst, unsigned int len) +{ + if ( is_hvm_vcpu(current) ) + return clear_user_hvm(dst, len); + + if ( !IS_ENABLED(CONFIG_PV) ) + return len; + + return clear_guest_pv(dst, len); +} + +static inline unsigned int __raw_copy_to_guest(void *dst, const void *src, + unsigned int len) +{ + if ( is_hvm_vcpu(current) ) + return copy_to_user_hvm(dst, src, len); + + if ( !IS_ENABLED(CONFIG_PV) ) + return len; + + return __copy_to_guest_pv(dst, src, len); +} + +static inline unsigned int __raw_copy_from_guest(void *dst, const void *src, + unsigned int len) +{ + if ( is_hvm_vcpu(current) ) + return copy_from_user_hvm(dst, src, len); + + if ( !IS_ENABLED(CONFIG_PV) ) + return len; + + return __copy_from_guest_pv(dst, src, len); +} /* * Pre-validate a guest handle. -- 2.34.1