From: Grygorii Strashko <grygorii_strashko@epam.com>
Factor out COMPAT HVM code under ifdefs in preparation for making HVM
COMPAT code optional.
- hypercall-defs.c updated to always provide compat declaration for:
physdev_op, grant_table_op, grant_table_op. This reduces number of COMPAT
ifdefs in HVM code and lets compiler DCE do the job.
- Only 64-bit shinfo is supported with COMPAT=n, so struct
arch_domain->has_32bit_shinfo field is moved under COMPAT ifdef and
has_32bit_shinfo() is updated to account for COMPAT=n.
Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
---
changes in v2:
- update hypercall-defs.c to always provide compat declaration for:
physdev_op, grant_table_op, grant_table_op
- move struct arch_domain->has_32bit_shinfo is moved under COMPAT ifdef
- return hvm_hypercall()
- use ASSERT_UNREACHABLE() in hvm_do_multicall_call()
- constify has_32bit_shinfo() for COMPAT=n
xen/arch/x86/hvm/hvm.c | 16 ++++++++++++++++
xen/arch/x86/hvm/hypercall.c | 13 +++++++++++++
xen/arch/x86/include/asm/domain.h | 9 +++++++--
xen/include/hypercall-defs.c | 9 +++++++--
4 files changed, 43 insertions(+), 4 deletions(-)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 0fd3f95b6e0e..19524cb7a914 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -69,7 +69,9 @@
#include <public/version.h>
#include <public/vm_event.h>
+#ifdef CONFIG_COMPAT
#include <compat/hvm/hvm_op.h>
+#endif
bool __read_mostly hvm_enabled;
@@ -1255,6 +1257,7 @@ static int cf_check hvm_save_cpu_xsave_states(
return 0;
}
+#ifdef CONFIG_COMPAT
/*
* Structure layout conformity checks, documenting correctness of the cast in
* the invocation of validate_xstate() below.
@@ -1267,6 +1270,7 @@ CHECK_FIELD_(struct, xsave_hdr, xcomp_bv);
CHECK_FIELD_(struct, xsave_hdr, reserved);
#undef compat_xsave_hdr
#undef xen_xsave_hdr
+#endif /* CONFIG_COMPAT */
static int cf_check hvm_load_cpu_xsave_states(
struct domain *d, hvm_domain_context_t *h)
@@ -3991,8 +3995,14 @@ static void hvm_latch_shinfo_size(struct domain *d)
*/
if ( current->domain == d )
{
+#ifdef CONFIG_COMPAT
+ /*
+ * Only 64-bit shinfo is supported when COMPAT 32-bit hypercalls
+ * interface is disabled
+ */
d->arch.has_32bit_shinfo =
hvm_guest_x86_mode(current) != X86_MODE_64BIT;
+#endif
/*
* Make sure that the timebase in the shared info structure is correct.
@@ -4965,6 +4975,7 @@ static int do_altp2m_op(
#endif /* CONFIG_ALTP2M */
}
+#ifdef CONFIG_COMPAT
DEFINE_XEN_GUEST_HANDLE(compat_hvm_altp2m_op_t);
/*
@@ -4992,10 +5003,12 @@ DEFINE_XEN_GUEST_HANDLE(compat_hvm_altp2m_op_t);
CHECK_hvm_altp2m_op;
CHECK_hvm_altp2m_set_mem_access_multi;
+#endif /* CONFIG_COMPAT */
static int compat_altp2m_op(
XEN_GUEST_HANDLE_PARAM(void) arg)
{
+#ifdef CONFIG_COMPAT
int rc = 0;
struct compat_hvm_altp2m_op a;
union
@@ -5063,6 +5076,9 @@ static int compat_altp2m_op(
}
return rc;
+#else
+ return -EOPNOTSUPP;
+#endif /* CONFIG_COMPAT */
}
static int hvmop_get_mem_type(
diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c
index 52cae1d15312..1ee0193b69af 100644
--- a/xen/arch/x86/hvm/hypercall.c
+++ b/xen/arch/x86/hvm/hypercall.c
@@ -170,6 +170,7 @@ int hvm_hypercall(struct cpu_user_regs *regs)
HVM_DBG_LOG(DBG_LEVEL_HCALL, "hcall%lu(%x, %x, %x, %x, %x)", eax,
regs->ebx, regs->ecx, regs->edx, regs->esi, regs->edi);
+#ifdef CONFIG_COMPAT
curr->hcall_compat = true;
call_handlers_hvm32(eax, regs->eax, regs->ebx, regs->ecx, regs->edx,
regs->esi, regs->edi);
@@ -177,6 +178,9 @@ int hvm_hypercall(struct cpu_user_regs *regs)
if ( !curr->hcall_preempted && regs->eax != -ENOSYS )
clobber_regs(regs, eax, hvm, 32);
+#else
+ regs->eax = -ENOSYS;
+#endif
}
hvmemul_cache_restore(curr, token);
@@ -207,10 +211,19 @@ enum mc_disposition hvm_do_multicall_call(struct mc_state *state)
}
else
{
+#ifdef CONFIG_COMPAT
struct compat_multicall_entry *call = &state->compat_call;
call_handlers_hvm32(call->op, call->result, call->args[0], call->args[1],
call->args[2], call->args[3], call->args[4]);
+#else
+ /*
+ * code should never reach here in case !CONFIG_COMPAT as any
+ * 32-bit hypercall should bail out earlier from hvm_hypercall()
+ * with -EOPNOTSUPP
+ */
+ ASSERT_UNREACHABLE();
+#endif
}
return !hvm_get_cpl(curr) ? mc_continue : mc_preempt;
diff --git a/xen/arch/x86/include/asm/domain.h b/xen/arch/x86/include/asm/domain.h
index 5df8c7825333..0005f4450931 100644
--- a/xen/arch/x86/include/asm/domain.h
+++ b/xen/arch/x86/include/asm/domain.h
@@ -12,8 +12,11 @@
#include <public/vcpu.h>
#include <public/hvm/hvm_info_table.h>
-#define has_32bit_shinfo(d) ((d)->arch.has_32bit_shinfo)
-
+#ifdef CONFIG_COMPAT
+#define has_32bit_shinfo(d) ((d)->arch.has_32bit_shinfo)
+#else
+#define has_32bit_shinfo(d) ((void)(d), false)
+#endif
/*
* Set to true if either the global vector-type callback or per-vCPU
* LAPIC vectors are used. Assume all vCPUs will use
@@ -365,8 +368,10 @@ struct arch_domain
/* NB. protected by d->event_lock and by irq_desc[irq].lock */
struct radix_tree_root irq_pirq;
+#ifdef CONFIG_COMPAT
/* Is shared-info page in 32-bit format? */
bool has_32bit_shinfo;
+#endif
/* Is PHYSDEVOP_eoi to automatically unmask the event channel? */
bool auto_unmask;
diff --git a/xen/include/hypercall-defs.c b/xen/include/hypercall-defs.c
index cef08eeec1b8..08c01153ac56 100644
--- a/xen/include/hypercall-defs.c
+++ b/xen/include/hypercall-defs.c
@@ -80,6 +80,8 @@ rettype: compat int
#define PREFIX_compat
#endif
+#define PREFIX_compat_always compat
+
#ifdef CONFIG_ARM
#define PREFIX_dep dep
#define PREFIX_do_arm do_arm
@@ -104,10 +106,10 @@ defhandle: trap_info_compat_t
defhandle: physdev_op_compat_t
#endif
-prefix: do PREFIX_hvm PREFIX_compat PREFIX_do_arm
+prefix: do PREFIX_hvm PREFIX_compat_always PREFIX_do_arm
physdev_op(int cmd, void *arg)
-prefix: do PREFIX_hvm PREFIX_compat
+prefix: do PREFIX_hvm PREFIX_compat_always
#if defined(CONFIG_GRANT_TABLE) || defined(CONFIG_PV_SHIM)
grant_table_op(unsigned int cmd, void *uop, unsigned int count)
#endif
@@ -156,6 +158,9 @@ platform_op(compat_platform_op_t *u_xenpf_op)
#ifdef CONFIG_KEXEC
kexec_op(unsigned int op, void *uarg)
#endif
+#else /* CONFIG_COMPAT */
+prefix: PREFIX_compat_always
+memory_op(unsigned int cmd, void *arg)
#endif /* CONFIG_COMPAT */
#if defined(CONFIG_PV) || defined(CONFIG_ARM)
--
2.34.1Hi Jürgen,
Would it be possible for you to take a look at hypercall-defs.c changes?
On 19.11.25 21:30, Grygorii Strashko wrote:
> From: Grygorii Strashko <grygorii_strashko@epam.com>
>
> Factor out COMPAT HVM code under ifdefs in preparation for making HVM
> COMPAT code optional.
>
> - hypercall-defs.c updated to always provide compat declaration for:
> physdev_op, grant_table_op, grant_table_op. This reduces number of COMPAT
> ifdefs in HVM code and lets compiler DCE do the job.
>
> - Only 64-bit shinfo is supported with COMPAT=n, so struct
> arch_domain->has_32bit_shinfo field is moved under COMPAT ifdef and
> has_32bit_shinfo() is updated to account for COMPAT=n.
>
> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
> ---
> changes in v2:
> - update hypercall-defs.c to always provide compat declaration for:
> physdev_op, grant_table_op, grant_table_op
> - move struct arch_domain->has_32bit_shinfo is moved under COMPAT ifdef
> - return hvm_hypercall()
> - use ASSERT_UNREACHABLE() in hvm_do_multicall_call()
> - constify has_32bit_shinfo() for COMPAT=n
>
> xen/arch/x86/hvm/hvm.c | 16 ++++++++++++++++
> xen/arch/x86/hvm/hypercall.c | 13 +++++++++++++
> xen/arch/x86/include/asm/domain.h | 9 +++++++--
> xen/include/hypercall-defs.c | 9 +++++++--
> 4 files changed, 43 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 0fd3f95b6e0e..19524cb7a914 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -69,7 +69,9 @@
> #include <public/version.h>
> #include <public/vm_event.h>
>
> +#ifdef CONFIG_COMPAT
> #include <compat/hvm/hvm_op.h>
> +#endif
>
> bool __read_mostly hvm_enabled;
>
> @@ -1255,6 +1257,7 @@ static int cf_check hvm_save_cpu_xsave_states(
> return 0;
> }
>
> +#ifdef CONFIG_COMPAT
> /*
> * Structure layout conformity checks, documenting correctness of the cast in
> * the invocation of validate_xstate() below.
> @@ -1267,6 +1270,7 @@ CHECK_FIELD_(struct, xsave_hdr, xcomp_bv);
> CHECK_FIELD_(struct, xsave_hdr, reserved);
> #undef compat_xsave_hdr
> #undef xen_xsave_hdr
> +#endif /* CONFIG_COMPAT */
>
> static int cf_check hvm_load_cpu_xsave_states(
> struct domain *d, hvm_domain_context_t *h)
> @@ -3991,8 +3995,14 @@ static void hvm_latch_shinfo_size(struct domain *d)
> */
> if ( current->domain == d )
> {
> +#ifdef CONFIG_COMPAT
> + /*
> + * Only 64-bit shinfo is supported when COMPAT 32-bit hypercalls
> + * interface is disabled
> + */
> d->arch.has_32bit_shinfo =
> hvm_guest_x86_mode(current) != X86_MODE_64BIT;
> +#endif
>
> /*
> * Make sure that the timebase in the shared info structure is correct.
> @@ -4965,6 +4975,7 @@ static int do_altp2m_op(
> #endif /* CONFIG_ALTP2M */
> }
>
> +#ifdef CONFIG_COMPAT
> DEFINE_XEN_GUEST_HANDLE(compat_hvm_altp2m_op_t);
>
> /*
> @@ -4992,10 +5003,12 @@ DEFINE_XEN_GUEST_HANDLE(compat_hvm_altp2m_op_t);
>
> CHECK_hvm_altp2m_op;
> CHECK_hvm_altp2m_set_mem_access_multi;
> +#endif /* CONFIG_COMPAT */
>
> static int compat_altp2m_op(
> XEN_GUEST_HANDLE_PARAM(void) arg)
> {
> +#ifdef CONFIG_COMPAT
> int rc = 0;
> struct compat_hvm_altp2m_op a;
> union
> @@ -5063,6 +5076,9 @@ static int compat_altp2m_op(
> }
>
> return rc;
> +#else
> + return -EOPNOTSUPP;
> +#endif /* CONFIG_COMPAT */
> }
>
> static int hvmop_get_mem_type(
> diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c
> index 52cae1d15312..1ee0193b69af 100644
> --- a/xen/arch/x86/hvm/hypercall.c
> +++ b/xen/arch/x86/hvm/hypercall.c
> @@ -170,6 +170,7 @@ int hvm_hypercall(struct cpu_user_regs *regs)
> HVM_DBG_LOG(DBG_LEVEL_HCALL, "hcall%lu(%x, %x, %x, %x, %x)", eax,
> regs->ebx, regs->ecx, regs->edx, regs->esi, regs->edi);
>
> +#ifdef CONFIG_COMPAT
> curr->hcall_compat = true;
> call_handlers_hvm32(eax, regs->eax, regs->ebx, regs->ecx, regs->edx,
> regs->esi, regs->edi);
> @@ -177,6 +178,9 @@ int hvm_hypercall(struct cpu_user_regs *regs)
>
> if ( !curr->hcall_preempted && regs->eax != -ENOSYS )
> clobber_regs(regs, eax, hvm, 32);
> +#else
> + regs->eax = -ENOSYS;
> +#endif
> }
>
> hvmemul_cache_restore(curr, token);
> @@ -207,10 +211,19 @@ enum mc_disposition hvm_do_multicall_call(struct mc_state *state)
> }
> else
> {
> +#ifdef CONFIG_COMPAT
> struct compat_multicall_entry *call = &state->compat_call;
>
> call_handlers_hvm32(call->op, call->result, call->args[0], call->args[1],
> call->args[2], call->args[3], call->args[4]);
> +#else
> + /*
> + * code should never reach here in case !CONFIG_COMPAT as any
> + * 32-bit hypercall should bail out earlier from hvm_hypercall()
> + * with -EOPNOTSUPP
> + */
> + ASSERT_UNREACHABLE();
> +#endif
> }
>
> return !hvm_get_cpl(curr) ? mc_continue : mc_preempt;
> diff --git a/xen/arch/x86/include/asm/domain.h b/xen/arch/x86/include/asm/domain.h
> index 5df8c7825333..0005f4450931 100644
> --- a/xen/arch/x86/include/asm/domain.h
> +++ b/xen/arch/x86/include/asm/domain.h
> @@ -12,8 +12,11 @@
> #include <public/vcpu.h>
> #include <public/hvm/hvm_info_table.h>
>
> -#define has_32bit_shinfo(d) ((d)->arch.has_32bit_shinfo)
> -
> +#ifdef CONFIG_COMPAT
> +#define has_32bit_shinfo(d) ((d)->arch.has_32bit_shinfo)
> +#else
> +#define has_32bit_shinfo(d) ((void)(d), false)
> +#endif
> /*
> * Set to true if either the global vector-type callback or per-vCPU
> * LAPIC vectors are used. Assume all vCPUs will use
> @@ -365,8 +368,10 @@ struct arch_domain
> /* NB. protected by d->event_lock and by irq_desc[irq].lock */
> struct radix_tree_root irq_pirq;
>
> +#ifdef CONFIG_COMPAT
> /* Is shared-info page in 32-bit format? */
> bool has_32bit_shinfo;
> +#endif
>
> /* Is PHYSDEVOP_eoi to automatically unmask the event channel? */
> bool auto_unmask;
> diff --git a/xen/include/hypercall-defs.c b/xen/include/hypercall-defs.c
> index cef08eeec1b8..08c01153ac56 100644
> --- a/xen/include/hypercall-defs.c
> +++ b/xen/include/hypercall-defs.c
> @@ -80,6 +80,8 @@ rettype: compat int
> #define PREFIX_compat
> #endif
>
> +#define PREFIX_compat_always compat
> +
> #ifdef CONFIG_ARM
> #define PREFIX_dep dep
> #define PREFIX_do_arm do_arm
> @@ -104,10 +106,10 @@ defhandle: trap_info_compat_t
> defhandle: physdev_op_compat_t
> #endif
>
> -prefix: do PREFIX_hvm PREFIX_compat PREFIX_do_arm
> +prefix: do PREFIX_hvm PREFIX_compat_always PREFIX_do_arm
> physdev_op(int cmd, void *arg)
>
> -prefix: do PREFIX_hvm PREFIX_compat
> +prefix: do PREFIX_hvm PREFIX_compat_always
> #if defined(CONFIG_GRANT_TABLE) || defined(CONFIG_PV_SHIM)
> grant_table_op(unsigned int cmd, void *uop, unsigned int count)
> #endif
> @@ -156,6 +158,9 @@ platform_op(compat_platform_op_t *u_xenpf_op)
> #ifdef CONFIG_KEXEC
> kexec_op(unsigned int op, void *uarg)
> #endif
> +#else /* CONFIG_COMPAT */
> +prefix: PREFIX_compat_always
> +memory_op(unsigned int cmd, void *arg)
> #endif /* CONFIG_COMPAT */
>
> #if defined(CONFIG_PV) || defined(CONFIG_ARM)
--
Best regards,
-grygorii
On 2025-11-19 14:30, Grygorii Strashko wrote: > From: Grygorii Strashko <grygorii_strashko@epam.com> > > Factor out COMPAT HVM code under ifdefs in preparation for making HVM > COMPAT code optional. > > - hypercall-defs.c updated to always provide compat declaration for: > physdev_op, grant_table_op, grant_table_op. This reduces number of COMPAT > ifdefs in HVM code and lets compiler DCE do the job. > > - Only 64-bit shinfo is supported with COMPAT=n, so struct > arch_domain->has_32bit_shinfo field is moved under COMPAT ifdef and > has_32bit_shinfo() is updated to account for COMPAT=n. > > Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com> > diff --git a/xen/arch/x86/include/asm/domain.h b/xen/arch/x86/include/asm/domain.h > index 5df8c7825333..0005f4450931 100644 > --- a/xen/arch/x86/include/asm/domain.h > +++ b/xen/arch/x86/include/asm/domain.h > @@ -12,8 +12,11 @@ > #include <public/vcpu.h> > #include <public/hvm/hvm_info_table.h> > > -#define has_32bit_shinfo(d) ((d)->arch.has_32bit_shinfo) > - > +#ifdef CONFIG_COMPAT > +#define has_32bit_shinfo(d) ((d)->arch.has_32bit_shinfo) > +#else > +#define has_32bit_shinfo(d) ((void)(d), false) (void)(d) is to avoid an any potential unreferenced 'd' messages? Just using false builds for me, but your way is a little more robust. > +#endif > /* > * Set to true if either the global vector-type callback or per-vCPU > * LAPIC vectors are used. Assume all vCPUs will use Reviewed-by: Jason Andryuk <jason.andryuk@amd.com> Regards, Jason
On 02.12.25 21:26, Jason Andryuk wrote: > On 2025-11-19 14:30, Grygorii Strashko wrote: >> From: Grygorii Strashko <grygorii_strashko@epam.com> >> >> Factor out COMPAT HVM code under ifdefs in preparation for making HVM >> COMPAT code optional. >> >> - hypercall-defs.c updated to always provide compat declaration for: >> physdev_op, grant_table_op, grant_table_op. This reduces number of COMPAT >> ifdefs in HVM code and lets compiler DCE do the job. >> >> - Only 64-bit shinfo is supported with COMPAT=n, so struct >> arch_domain->has_32bit_shinfo field is moved under COMPAT ifdef and >> has_32bit_shinfo() is updated to account for COMPAT=n. >> >> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com> > >> diff --git a/xen/arch/x86/include/asm/domain.h b/xen/arch/x86/include/asm/domain.h >> index 5df8c7825333..0005f4450931 100644 >> --- a/xen/arch/x86/include/asm/domain.h >> +++ b/xen/arch/x86/include/asm/domain.h >> @@ -12,8 +12,11 @@ >> #include <public/vcpu.h> >> #include <public/hvm/hvm_info_table.h> >> -#define has_32bit_shinfo(d) ((d)->arch.has_32bit_shinfo) >> - >> +#ifdef CONFIG_COMPAT >> +#define has_32bit_shinfo(d) ((d)->arch.has_32bit_shinfo) >> +#else >> +#define has_32bit_shinfo(d) ((void)(d), false) > > (void)(d) is to avoid an any potential unreferenced 'd' messages? It's a generic request to have macro arguments evaluated consistently. > > Just using false builds for me, but your way is a little more robust. > >> +#endif >> /* >> * Set to true if either the global vector-type callback or per-vCPU >> * LAPIC vectors are used. Assume all vCPUs will use > Reviewed-by: Jason Andryuk <jason.andryuk@amd.com> > > Regards, > Jason -- Best regards, -grygorii
© 2016 - 2025 Red Hat, Inc.