[PATCH v2 26/26] xen/domctl: wrap common/domctl.c with CONFIG_MGMT_HYPERCALLS

Penny Zheng posted 26 patches 3 days, 22 hours ago
[PATCH v2 26/26] xen/domctl: wrap common/domctl.c with CONFIG_MGMT_HYPERCALLS
Posted by Penny Zheng 3 days, 22 hours ago
Wrap domctl hypercall def and domctl.o with CONFIG_MGMT_HYPERCALLS,
and remove all #ifdef CONFIG_MGMT_HYPERCALLS wrappings in common/domctl.c
With MGMT_HYPERCALLS=n, we need to provide stub for
domctl_lock_{acquire,release}(), as it may be invoked by hvm_set_param().

Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
---
v1 -> v2:
- remove stub in common/domctl.c
- combine the original commit of "xen/domctl: provide stub for
 domctl_lock_{acquire,release}"
- adapt to changes of "unify DOMCTL to MGMT_HYPERCALLS"
---
 xen/common/Kconfig           |  2 +-
 xen/common/Makefile          |  2 +-
 xen/common/domctl.c          | 24 ------------------------
 xen/include/hypercall-defs.c |  4 +++-
 xen/include/xen/domain.h     |  9 +++++++++
 5 files changed, 14 insertions(+), 27 deletions(-)

diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 1aedd00b12..da207a7183 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -654,7 +654,7 @@ config MGMT_HYPERCALLS
 	help
 	  This option shall only be disabled on some dom0less systems, or
 	  PV shim on x86, to reduce Xen footprint via managing unnessary
-	  hypercalls, like sysctl, etc.
+	  hypercalls, like sysctl, domctl, etc.
 
 config PM_OP
 	bool "Enable Performance Management Operation"
diff --git a/xen/common/Makefile b/xen/common/Makefile
index fdf826f218..45c0bda000 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -11,7 +11,7 @@ obj-$(filter-out $(CONFIG_X86),$(CONFIG_ACPI)) += device.o
 obj-$(CONFIG_DEVICE_TREE_PARSE) += device-tree/
 obj-$(CONFIG_IOREQ_SERVER) += dm.o
 obj-y += domain.o
-obj-y += domctl.o
+obj-$(CONFIG_MGMT_HYPERCALLS) += domctl.o
 obj-y += domid.o
 obj-y += event_2l.o
 obj-y += event_channel.o
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 5657b95089..71e712c1f3 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -44,14 +44,12 @@ static int nodemask_to_xenctl_bitmap(struct xenctl_bitmap *xenctl_nodemap,
                                    MAX_NUMNODES);
 }
 
-#ifdef CONFIG_MGMT_HYPERCALLS
 static int xenctl_bitmap_to_nodemask(nodemask_t *nodemask,
                                      const struct xenctl_bitmap *xenctl_nodemap)
 {
     return xenctl_bitmap_to_bitmap(nodemask_bits(nodemask), xenctl_nodemap,
                                    MAX_NUMNODES);
 }
-#endif /* CONFIG_MGMT_HYPERCALLS */
 
 void getdomaininfo(struct domain *d, struct xen_domctl_getdomaininfo *info)
 {
@@ -114,9 +112,7 @@ void getdomaininfo(struct domain *d, struct xen_domctl_getdomaininfo *info)
 
     memcpy(info->handle, d->handle, sizeof(xen_domain_handle_t));
 
-#ifdef CONFIG_MGMT_HYPERCALLS
     arch_get_domain_info(d, info);
-#endif
 }
 
 bool domctl_lock_acquire(void)
@@ -394,26 +390,22 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
         break;
     }
 
-#ifdef CONFIG_MGMT_HYPERCALLS
     case XEN_DOMCTL_pausedomain:
         ret = -EINVAL;
         if ( d != current->domain )
             ret = domain_pause_by_systemcontroller(d);
         break;
-#endif /* CONFIG_MGMT_HYPERCALLS */
 
     case XEN_DOMCTL_unpausedomain:
         ret = domain_unpause_by_systemcontroller(d);
         break;
 
-#ifdef CONFIG_MGMT_HYPERCALLS
     case XEN_DOMCTL_resumedomain:
         if ( d == current->domain ) /* no domain_pause() */
             ret = -EINVAL;
         else
             domain_resume(d);
         break;
-#endif /* CONFIG_MGMT_HYPERCALLS */
 
     case XEN_DOMCTL_createdomain:
     {
@@ -473,7 +465,6 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
         break;
     }
 
-#ifdef CONFIG_MGMT_HYPERCALLS
     case XEN_DOMCTL_soft_reset:
     case XEN_DOMCTL_soft_reset_cont:
         if ( d == current->domain ) /* no domain_pause() */
@@ -510,14 +501,12 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
             ret = domain_set_node_affinity(d, &new_affinity);
         break;
     }
-#endif /* CONFIG_MGMT_HYPERCALLS */
 
     case XEN_DOMCTL_getnodeaffinity:
         ret = nodemask_to_xenctl_bitmap(&op->u.nodeaffinity.nodemap,
                                         &d->node_affinity);
         break;
 
-#ifdef CONFIG_MGMT_HYPERCALLS
     case XEN_DOMCTL_setvcpuaffinity:
     case XEN_DOMCTL_getvcpuaffinity:
         ret = vcpu_affinity_domctl(d, op->cmd, &op->u.vcpuaffinity);
@@ -527,7 +516,6 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
         ret = sched_adjust(d, &op->u.scheduler_op);
         copyback = 1;
         break;
-#endif /* CONFIG_MGMT_HYPERCALLS */
 
     case XEN_DOMCTL_getdomaininfo:
         ret = xsm_getdomaininfo(XSM_XS_PRIV, d);
@@ -540,7 +528,6 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
         copyback = 1;
         break;
 
-#ifdef CONFIG_MGMT_HYPERCALLS
     case XEN_DOMCTL_getvcpucontext:
     {
         vcpu_guest_context_u c = { .nat = NULL };
@@ -589,7 +576,6 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
         xfree(c.nat);
         break;
     }
-#endif /* CONFIG_MGMT_HYPERCALLS */
 
     case XEN_DOMCTL_getvcpuinfo:
     {
@@ -750,11 +736,9 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
         break;
     }
 
-#ifdef CONFIG_MGMT_HYPERCALLS
     case XEN_DOMCTL_settimeoffset:
         domain_set_time_offset(d, op->u.settimeoffset.time_offset_seconds);
         break;
-#endif /* CONFIG_MGMT_HYPERCALLS */
 
     case XEN_DOMCTL_set_target:
     {
@@ -810,11 +794,9 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
         break;
 #endif
 
-#ifdef CONFIG_MGMT_HYPERCALLS
     case XEN_DOMCTL_set_virq_handler:
         ret = set_global_virq_handler(d, op->u.set_virq_handler.virq);
         break;
-#endif /* CONFIG_MGMT_HYPERCALLS */
 
     case XEN_DOMCTL_setvnumainfo:
     {
@@ -842,7 +824,6 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
             copyback = 1;
         break;
 
-#ifdef CONFIG_MGMT_HYPERCALLS
     case XEN_DOMCTL_assign_device:
     case XEN_DOMCTL_test_assign_device:
     case XEN_DOMCTL_deassign_device:
@@ -863,7 +844,6 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
             ret = hypercall_create_continuation(
                 __HYPERVISOR_domctl, "h", u_domctl);
         break;
-#endif /* CONFIG_MGMT_HYPERCALLS */
 
     case XEN_DOMCTL_set_llc_colors:
         if ( op->u.set_llc_colors.pad )
@@ -884,11 +864,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
         break;
 
     default:
-#ifdef CONFIG_MGMT_HYPERCALLS
         ret = arch_do_domctl(op, d, u_domctl);
-#else
-        ret = -EOPNOTSUPP;
-#endif /* CONFIG_MGMT_HYPERCALLS */
         break;
     }
 
diff --git a/xen/include/hypercall-defs.c b/xen/include/hypercall-defs.c
index 02d7b93e80..cbd547f724 100644
--- a/xen/include/hypercall-defs.c
+++ b/xen/include/hypercall-defs.c
@@ -200,7 +200,9 @@ sysctl(xen_sysctl_t *u_sysctl)
 #if defined(CONFIG_X86) && defined(CONFIG_PAGING) && defined(CONFIG_MGMT_HYPERCALLS)
 paging_domctl_cont(xen_domctl_t *u_domctl)
 #endif
+#ifdef CONFIG_MGMT_HYPERCALLS
 domctl(xen_domctl_t *u_domctl)
+#endif
 #ifndef CONFIG_PV_SHIM_EXCLUSIVE
 platform_op(xen_platform_op_t *u_xenpf_op)
 #endif
@@ -279,8 +281,8 @@ hvm_op                             do       do       do       do       do
 #endif
 #ifdef CONFIG_MGMT_HYPERCALLS
 sysctl                             do       do       do       do       do
-#endif
 domctl                             do       do       do       do       do
+#endif
 #ifdef CONFIG_KEXEC
 kexec_op                           compat   do       -        -        -
 #endif
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index 11d2505420..19dd85150a 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -151,8 +151,17 @@ void arch_dump_domain_info(struct domain *d);
 
 int arch_vcpu_reset(struct vcpu *v);
 
+#ifdef CONFIG_MGMT_HYPERCALLS
 bool domctl_lock_acquire(void);
 void domctl_lock_release(void);
+#else
+static inline bool domctl_lock_acquire(void)
+{
+    return false;
+}
+
+static inline void domctl_lock_release(void) {}
+#endif /* CONFIG_MGMT_HYPERCALLS */
 
 /*
  * Continue the current hypercall via func(data) on specified cpu.
-- 
2.34.1
Re: [PATCH v2 26/26] xen/domctl: wrap common/domctl.c with CONFIG_MGMT_HYPERCALLS
Posted by Jan Beulich 2 days, 17 hours ago
On 10.09.2025 09:38, Penny Zheng wrote:
> --- a/xen/include/xen/domain.h
> +++ b/xen/include/xen/domain.h
> @@ -151,8 +151,17 @@ void arch_dump_domain_info(struct domain *d);
>  
>  int arch_vcpu_reset(struct vcpu *v);
>  
> +#ifdef CONFIG_MGMT_HYPERCALLS
>  bool domctl_lock_acquire(void);
>  void domctl_lock_release(void);
> +#else
> +static inline bool domctl_lock_acquire(void)
> +{
> +    return false;

I.e. a someone invoking hvm_set_param() with HVM_PARAM_IDENT_PT will loop
indefinitely on getting back -ERESTART? Imo you simply cannot get things
right here with a stub: Either you have the above issue, or you put some
future new user of the function at risk.

Setting HVM_PARAM_IDENT_PT being a toolstack-only operation, I think that
needs making conditional upon CONFIG_MGMT_HYPERCALLS right in this series,
such that the last caller of these lock/unlock functions disappears.

Jan
Re: [PATCH v2 26/26] xen/domctl: wrap common/domctl.c with CONFIG_MGMT_HYPERCALLS
Posted by Jan Beulich 3 days, 16 hours ago
On 10.09.2025 09:38, Penny Zheng wrote:
> Wrap domctl hypercall def and domctl.o with CONFIG_MGMT_HYPERCALLS,
> and remove all #ifdef CONFIG_MGMT_HYPERCALLS wrappings in common/domctl.c
> With MGMT_HYPERCALLS=n, we need to provide stub for
> domctl_lock_{acquire,release}(), as it may be invoked by hvm_set_param().
> 
> Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
> ---
> v1 -> v2:
> - remove stub in common/domctl.c
> - combine the original commit of "xen/domctl: provide stub for
>  domctl_lock_{acquire,release}"
> - adapt to changes of "unify DOMCTL to MGMT_HYPERCALLS"
> ---
>  xen/common/Kconfig           |  2 +-
>  xen/common/Makefile          |  2 +-
>  xen/common/domctl.c          | 24 ------------------------
>  xen/include/hypercall-defs.c |  4 +++-
>  xen/include/xen/domain.h     |  9 +++++++++
>  5 files changed, 14 insertions(+), 27 deletions(-)

Please see all the removals of #ifdef-s below for why I was arguing towards
the Kconfig control wanting to (re)gain its prompt last. These #ifdef-s will
have been added by earlier patches in the series (which I didn't look at
yet), and that kind of churn could have been avoided.

Jan