On 13.11.25 14:30, Jan Beulich wrote:
On 11.11.2025 18:54, Grygorii Strashko wrote:
@@ -3991,7 +3995,7 @@ static void hvm_latch_shinfo_size(struct domain *d)
*/
if ( current->domain == d )
{
- d->arch.has_32bit_shinfo =
+ d->arch.has_32bit_shinfo = IS_ENABLED(CONFIG_COMPAT) &&
hvm_guest_x86_mode(current) != X86_MODE_64BIT;
I think this could need commenting on if we really want to put it in this shape.
ok.
But why would we retain the has_32bit_shinfo field in the first place when
COMPAT=n?
Here I'm not sure. May be other can comment.
@@ -4965,6 +4969,7 @@ static int do_altp2m_op(
#endif /* CONFIG_ALTP2M */
}
+#ifdef CONFIG_COMPAT
DEFINE_XEN_GUEST_HANDLE(compat_hvm_altp2m_op_t);
/*
@@ -5064,6 +5069,12 @@ static int compat_altp2m_op(
return rc;
}
+#else
+static int compat_altp2m_op(XEN_GUEST_HANDLE_PARAM(void) arg)
+{
+ return -EOPNOTSUPP;
+}
+#endif /* CONFIG_COMPAT */
I'm not in favor of repeating the function "header". Imo such #ifdef-s better
go inside respective functions' bodies.
ok.
--- a/xen/arch/x86/hvm/hypercall.c
+++ b/xen/arch/x86/hvm/hypercall.c
@@ -29,10 +29,12 @@ long hvm_memory_op(unsigned long cmd,
XEN_GUEST_HANDLE_PARAM(void) arg)
return -ENOSYS;
}
- if ( !vcpu_is_hcall_compat(current) )
- rc = do_memory_op(cmd, arg);
- else
+#ifdef CONFIG_COMPAT
+ if ( vcpu_is_hcall_compat(current) )
rc = compat_memory_op(cmd, arg);
+ else
+#endif
+ rc = do_memory_op(cmd, arg);
Why would this be needed when vcpu_is_hcall_compat() already honors
CONFIG_COMPAT?
(Same question then applies elsewhere, of course.)
This I do not like by myself, but I was not able to find other options :(
hypercall-defs.h is autogenerated and it's the only one place where functions
declarations like do_x_op() are appeared or disappeared.
So build is failing without ifdefs as compiler can't find compat_memory_op()
declaration.
@@ -171,6 +177,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);
@@ -178,6 +185,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 = -EOPNOTSUPP;
+#endif
I'm generally against most attempts to use ENOSYS, but here it should be used:
The top-level hypercalls are (effectively) unimplemented in this mode.
ok.
@@ -208,10 +218,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
+ */
+ unreachable();
I.e. you rather mean ASSERT_UNREACHABLE()?
ok.
--
Best regards,
-grygorii
