[PATCH v2 24/26] xen/domctl: wrap arch-specific domctl-op with CONFIG_MGMT_HYPERCALLS

Penny Zheng posted 26 patches 5 months ago
There is a newer version of this series
[PATCH v2 24/26] xen/domctl: wrap arch-specific domctl-op with CONFIG_MGMT_HYPERCALLS
Posted by Penny Zheng 5 months ago
Function arch_do_domctl() is responsible for arch-specific domctl-op,
and shall be wrapped with CONFIG_MGMT_HYPERCALLS
Tracking its calling chain and the following functions shall be wrapped with
CONFIG_MGMT_HYPERCALLS:
For x86:
- hvm_save_one
- hvm_acpi_power_button
- hvm_acpi_sleep_button
- hvm_debug_op
- mem_sharing_domctl
- make P2M_AUDIT depend on CONFIG_MGMT_HYPERCALLS
- make PG_log_dirty depend on CONFIG_MGMT_HYPERCALLS
- make policy.o depend on CONFIG_MGMT_HYPERCALLS
- do_vmtrace_op
  - hvm_vmtrace_control
    - hvm_funcs.vmtrace_control
  - hvm_vmtrace_get_option
    - hvm_funcs.vmtrace_get_option
  - hvm_vmtrace_set_option
    - hvm_funcs.vmtrace_set_option
- paging_domctl_cont
For ARM:
- subarch_do_domctl

Also, remove all #ifdef CONFIG_MGMT_HYPERCALLS-s in arch-specific domctl.c, as
we put the guardian in Makefile for the whole file.
Wrap default-case and arch_get_domain_info() transiently with
CONFIG_MGMT_HYPERCALLS, and it will be removed when introducing
CONFIG_MGMT_HYPERCALLS on the common/domctl.c in the last.

Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
---
v1 -> v2:
- split out xsm parts
- adapt to changes of "unify DOMCTL to MGMT_HYPERCALLS"
- wrap default-case and arch_get_domain_info() transiently
---
 xen/Kconfig.debug                  |  2 +-
 xen/arch/arm/arm32/Makefile        |  2 +-
 xen/arch/arm/arm64/Makefile        |  2 +-
 xen/arch/arm/domctl.c              |  2 --
 xen/arch/x86/Makefile              |  2 +-
 xen/arch/x86/domctl.c              |  2 --
 xen/arch/x86/hvm/hvm.c             |  2 ++
 xen/arch/x86/hvm/pmtimer.c         |  2 ++
 xen/arch/x86/hvm/save.c            |  2 ++
 xen/arch/x86/hvm/vmx/vmx.c         |  6 ++++++
 xen/arch/x86/include/asm/hvm/hvm.h | 10 ++++++++++
 xen/arch/x86/include/asm/p2m.h     |  2 +-
 xen/arch/x86/include/asm/paging.h  |  2 +-
 xen/arch/x86/mm/mem_sharing.c      |  2 ++
 xen/common/domctl.c                |  6 ++++++
 xen/include/hypercall-defs.c       |  4 ++--
 xen/lib/x86/Makefile               |  2 +-
 17 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index a69615cd63..0dd44d2b10 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -15,7 +15,7 @@ if DEBUG || EXPERT
 
 config GDBSX
 	bool "Guest debugging with gdbsx"
-	depends on X86
+	depends on X86 && MGMT_HYPERCALLS
 	default y
 	help
 	  If you want to enable support for debugging guests from dom0 via
diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
index 531168f58a..f8cbf14211 100644
--- a/xen/arch/arm/arm32/Makefile
+++ b/xen/arch/arm/arm32/Makefile
@@ -4,7 +4,7 @@ obj-$(CONFIG_MPU) += mpu/
 
 obj-y += cache.o
 obj-$(CONFIG_EARLY_PRINTK) += debug.o
-obj-y += domctl.o
+obj-$(CONFIG_MGMT_HYPERCALLS) += domctl.o
 obj-y += domain.o
 obj-y += entry.o
 obj-y += head.o
diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
index 6491c5350b..6b77a15abe 100644
--- a/xen/arch/arm/arm64/Makefile
+++ b/xen/arch/arm/arm64/Makefile
@@ -6,7 +6,7 @@ obj-y += cache.o
 obj-y += cpufeature.o
 obj-$(CONFIG_HARDEN_BRANCH_PREDICTOR) += bpi.o
 obj-$(CONFIG_EARLY_PRINTK) += debug.o
-obj-y += domctl.o
+obj-$(CONFIG_MGMT_HYPERCALLS) += domctl.o
 obj-y += domain.o
 obj-y += entry.o
 obj-y += head.o
diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
index d3263e4d03..ad914c915f 100644
--- a/xen/arch/arm/domctl.c
+++ b/xen/arch/arm/domctl.c
@@ -184,7 +184,6 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain *d,
     }
 }
 
-#ifdef CONFIG_MGMT_HYPERCALLS
 void arch_get_info_guest(struct vcpu *v, vcpu_guest_context_u c)
 {
     struct vcpu_guest_context *ctxt = c.nat;
@@ -200,7 +199,6 @@ void arch_get_info_guest(struct vcpu *v, vcpu_guest_context_u c)
     if ( !test_bit(_VPF_down, &v->pause_flags) )
         ctxt->flags |= VGCF_online;
 }
-#endif /* CONFIG_MGMT_HYPERCALLS */
 
 /*
  * Local variables:
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index a7bfe4c0b1..8427dc52fd 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -28,7 +28,7 @@ obj-y += delay.o
 obj-y += desc.o
 obj-bin-y += dmi_scan.init.o
 obj-y += domain.o
-obj-y += domctl.o
+obj-$(CONFIG_MGMT_HYPERCALLS) += domctl.o
 obj-bin-y += dom0_build.init.o
 obj-y += domain_page.o
 obj-y += e820.o
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index ea5f5b20cf..6153e3c07e 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1370,7 +1370,6 @@ long arch_do_domctl(
     return ret;
 }
 
-#ifdef CONFIG_MGMT_HYPERCALLS
 #ifdef CONFIG_COMPAT
 #define xen_vcpu_guest_context vcpu_guest_context
 #define fpu_ctxt fpu_ctxt.x
@@ -1563,7 +1562,6 @@ void arch_get_info_guest(struct vcpu *v, vcpu_guest_context_u c)
     c(vm_assist = d->vm_assist);
 #undef c
 }
-#endif /* CONFIG_MGMT_HYPERCALLS */
 
 /*
  * Local variables:
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index b044dc2ecb..08bb1ba857 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -5223,6 +5223,7 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
     return rc;
 }
 
+#ifdef CONFIG_MGMT_HYPERCALLS
 int hvm_debug_op(struct vcpu *v, int32_t op)
 {
     int rc = 0;
@@ -5265,6 +5266,7 @@ int hvm_debug_op(struct vcpu *v, int32_t op)
 
     return rc;
 }
+#endif /* CONFIG_MGMT_HYPERCALLS */
 
 #ifdef CONFIG_VM_EVENT
 void hvm_toggle_singlestep(struct vcpu *v)
diff --git a/xen/arch/x86/hvm/pmtimer.c b/xen/arch/x86/hvm/pmtimer.c
index 87a7a01c9f..f080f7561d 100644
--- a/xen/arch/x86/hvm/pmtimer.c
+++ b/xen/arch/x86/hvm/pmtimer.c
@@ -56,6 +56,7 @@ static void pmt_update_sci(PMTState *s)
         hvm_isa_irq_deassert(s->vcpu->domain, SCI_IRQ);
 }
 
+#ifdef CONFIG_MGMT_HYPERCALLS
 void hvm_acpi_power_button(struct domain *d)
 {
     PMTState *s = &d->arch.hvm.pl_time->vpmt;
@@ -81,6 +82,7 @@ void hvm_acpi_sleep_button(struct domain *d)
     pmt_update_sci(s);
     spin_unlock(&s->lock);
 }
+#endif /* CONFIG_MGMT_HYPERCALLS */
 
 /* Set the correct value in the timer, accounting for time elapsed
  * since the last time we did that. */
diff --git a/xen/arch/x86/hvm/save.c b/xen/arch/x86/hvm/save.c
index 8ab6405706..0d966911a2 100644
--- a/xen/arch/x86/hvm/save.c
+++ b/xen/arch/x86/hvm/save.c
@@ -121,6 +121,7 @@ size_t hvm_save_size(struct domain *d)
     return sz;
 }
 
+#ifdef CONFIG_MGMT_HYPERCALLS
 /*
  * Extract a single instance of a save record, by marshalling all records of
  * that type and copying out the one we need.
@@ -195,6 +196,7 @@ int hvm_save_one(struct domain *d, unsigned int typecode, unsigned int instance,
     xfree(ctxt.data);
     return rv;
 }
+#endif /* CONFIG_MGMT_HYPERCALLS */
 
 int hvm_save(struct domain *d, hvm_domain_context_t *h)
 {
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 4cf5da70ad..056f46673e 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2585,6 +2585,7 @@ static bool cf_check vmx_get_pending_event(
     (RTIT_STATUS_FILTER_EN | RTIT_STATUS_CONTEXT_EN | RTIT_STATUS_TRIGGER_EN | \
      RTIT_STATUS_ERROR | RTIT_STATUS_STOPPED)
 
+#ifdef CONFIG_MGMT_HYPERCALLS
 static int cf_check vmtrace_get_option(
     struct vcpu *v, uint64_t key, uint64_t *output)
 {
@@ -2693,6 +2694,7 @@ static int cf_check vmtrace_control(struct vcpu *v, bool enable, bool reset)
 
     return 0;
 }
+#endif /* CONFIG_MGMT_HYPERCALLS */
 
 static int cf_check vmtrace_output_position(struct vcpu *v, uint64_t *pos)
 {
@@ -2883,10 +2885,14 @@ static struct hvm_function_table __initdata_cf_clobber vmx_function_table = {
     .altp2m_vcpu_emulate_ve = vmx_vcpu_emulate_ve,
     .altp2m_vcpu_emulate_vmfunc = vmx_vcpu_emulate_vmfunc,
 #endif
+#ifdef CONFIG_MGMT_HYPERCALLS
     .vmtrace_control = vmtrace_control,
+#endif
     .vmtrace_output_position = vmtrace_output_position,
+#ifdef CONFIG_MGMT_HYPERCALLS
     .vmtrace_set_option = vmtrace_set_option,
     .vmtrace_get_option = vmtrace_get_option,
+#endif
     .vmtrace_reset = vmtrace_reset,
 
     .get_reg = vmx_get_reg,
diff --git a/xen/arch/x86/include/asm/hvm/hvm.h b/xen/arch/x86/include/asm/hvm/hvm.h
index b2c75b733e..6e5ec65a8e 100644
--- a/xen/arch/x86/include/asm/hvm/hvm.h
+++ b/xen/arch/x86/include/asm/hvm/hvm.h
@@ -239,10 +239,14 @@ struct hvm_function_table {
 #endif
 
     /* vmtrace */
+#ifdef CONFIG_MGMT_HYPERCALLS
     int (*vmtrace_control)(struct vcpu *v, bool enable, bool reset);
+#endif
     int (*vmtrace_output_position)(struct vcpu *v, uint64_t *pos);
+#ifdef CONFIG_MGMT_HYPERCALLS
     int (*vmtrace_set_option)(struct vcpu *v, uint64_t key, uint64_t value);
     int (*vmtrace_get_option)(struct vcpu *v, uint64_t key, uint64_t *value);
+#endif
     int (*vmtrace_reset)(struct vcpu *v);
 
     uint64_t (*get_reg)(struct vcpu *v, unsigned int reg);
@@ -747,8 +751,10 @@ bool altp2m_vcpu_emulate_ve(struct vcpu *v);
 
 static inline int hvm_vmtrace_control(struct vcpu *v, bool enable, bool reset)
 {
+#ifdef CONFIG_MGMT_HYPERCALLS
     if ( hvm_funcs.vmtrace_control )
         return alternative_call(hvm_funcs.vmtrace_control, v, enable, reset);
+#endif
 
     return -EOPNOTSUPP;
 }
@@ -765,8 +771,10 @@ static inline int hvm_vmtrace_output_position(struct vcpu *v, uint64_t *pos)
 static inline int hvm_vmtrace_set_option(
     struct vcpu *v, uint64_t key, uint64_t value)
 {
+#ifdef CONFIG_MGMT_HYPERCALLS
     if ( hvm_funcs.vmtrace_set_option )
         return alternative_call(hvm_funcs.vmtrace_set_option, v, key, value);
+#endif
 
     return -EOPNOTSUPP;
 }
@@ -774,8 +782,10 @@ static inline int hvm_vmtrace_set_option(
 static inline int hvm_vmtrace_get_option(
     struct vcpu *v, uint64_t key, uint64_t *value)
 {
+#ifdef CONFIG_MGMT_HYPERCALLS
     if ( hvm_funcs.vmtrace_get_option )
         return alternative_call(hvm_funcs.vmtrace_get_option, v, key, value);
+#endif
 
     return -EOPNOTSUPP;
 }
diff --git a/xen/arch/x86/include/asm/p2m.h b/xen/arch/x86/include/asm/p2m.h
index 1856cc396c..f29605df54 100644
--- a/xen/arch/x86/include/asm/p2m.h
+++ b/xen/arch/x86/include/asm/p2m.h
@@ -20,7 +20,7 @@
 #include <asm/page.h>    /* for pagetable_t */
 
 /* Debugging and auditing of the P2M code? */
-#if !defined(NDEBUG) && defined(CONFIG_HVM)
+#if !defined(NDEBUG) && defined(CONFIG_HVM) && defined(CONFIG_MGMT_HYPERCALLS)
 #define P2M_AUDIT     1
 #else
 #define P2M_AUDIT     0
diff --git a/xen/arch/x86/include/asm/paging.h b/xen/arch/x86/include/asm/paging.h
index 1b0694bb36..db3e5b8f31 100644
--- a/xen/arch/x86/include/asm/paging.h
+++ b/xen/arch/x86/include/asm/paging.h
@@ -55,7 +55,7 @@
 #define PG_translate   0
 #define PG_external    0
 #endif
-#ifdef CONFIG_PAGING
+#if defined(CONFIG_PAGING) && defined(CONFIG_MGMT_HYPERCALLS)
 /* Enable log dirty mode */
 #define PG_log_dirty   (XEN_DOMCTL_SHADOW_ENABLE_LOG_DIRTY << PG_mode_shift)
 #else
diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index d7cbf2047b..3210cf5553 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -2319,6 +2319,7 @@ out:
     return rc;
 }
 
+#ifdef CONFIG_MGMT_HYPERCALLS
 int mem_sharing_domctl(struct domain *d, struct xen_domctl_mem_sharing_op *mec)
 {
     int rc;
@@ -2336,6 +2337,7 @@ int mem_sharing_domctl(struct domain *d, struct xen_domctl_mem_sharing_op *mec)
 
     return rc;
 }
+#endif /* CONFIG_MGMT_HYPERCALLS */
 
 void arch_dump_shared_mem_info(void)
 {
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index c87c28cea2..5657b95089 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -114,7 +114,9 @@ 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)
@@ -882,7 +884,11 @@ 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 cd2c801af6..02d7b93e80 100644
--- a/xen/include/hypercall-defs.c
+++ b/xen/include/hypercall-defs.c
@@ -197,7 +197,7 @@ dm_op(domid_t domid, unsigned int nr_bufs, xen_dm_op_buf_t *bufs)
 #ifdef CONFIG_MGMT_HYPERCALLS
 sysctl(xen_sysctl_t *u_sysctl)
 #endif
-#if defined(CONFIG_X86) && defined(CONFIG_PAGING)
+#if defined(CONFIG_X86) && defined(CONFIG_PAGING) && defined(CONFIG_MGMT_HYPERCALLS)
 paging_domctl_cont(xen_domctl_t *u_domctl)
 #endif
 domctl(xen_domctl_t *u_domctl)
@@ -296,7 +296,7 @@ dm_op                              compat   do       compat   do       do
 hypfs_op                           do       do       do       do       do
 #endif
 mca                                do       do       -        -        -
-#if defined(CONFIG_X86) && defined(CONFIG_PAGING)
+#if defined(CONFIG_X86) && defined(CONFIG_PAGING) && defined(CONFIG_MGMT_HYPERCALLS)
 paging_domctl_cont                 do       do       do       do       -
 #endif
 
diff --git a/xen/lib/x86/Makefile b/xen/lib/x86/Makefile
index 780ea05db1..ee5bec225e 100644
--- a/xen/lib/x86/Makefile
+++ b/xen/lib/x86/Makefile
@@ -1,3 +1,3 @@
 obj-y += cpuid.o
 obj-y += msr.o
-obj-y += policy.o
+obj-$(CONFIG_MGMT_HYPERCALLS) += policy.o
-- 
2.34.1
Re: [PATCH v2 24/26] xen/domctl: wrap arch-specific domctl-op with CONFIG_MGMT_HYPERCALLS
Posted by Jan Beulich 5 months ago
On 10.09.2025 09:38, Penny Zheng wrote:
> Function arch_do_domctl() is responsible for arch-specific domctl-op,
> and shall be wrapped with CONFIG_MGMT_HYPERCALLS
> Tracking its calling chain and the following functions shall be wrapped with
> CONFIG_MGMT_HYPERCALLS:
> For x86:
> - hvm_save_one
> - hvm_acpi_power_button
> - hvm_acpi_sleep_button
> - hvm_debug_op
> - mem_sharing_domctl
> - make P2M_AUDIT depend on CONFIG_MGMT_HYPERCALLS
> - make PG_log_dirty depend on CONFIG_MGMT_HYPERCALLS
> - make policy.o depend on CONFIG_MGMT_HYPERCALLS
> - do_vmtrace_op
>   - hvm_vmtrace_control
>     - hvm_funcs.vmtrace_control
>   - hvm_vmtrace_get_option
>     - hvm_funcs.vmtrace_get_option
>   - hvm_vmtrace_set_option
>     - hvm_funcs.vmtrace_set_option
> - paging_domctl_cont
> For ARM:
> - subarch_do_domctl
> 
> Also, remove all #ifdef CONFIG_MGMT_HYPERCALLS-s in arch-specific domctl.c, as
> we put the guardian in Makefile for the whole file.
> Wrap default-case and arch_get_domain_info() transiently with
> CONFIG_MGMT_HYPERCALLS, and it will be removed when introducing
> CONFIG_MGMT_HYPERCALLS on the common/domctl.c in the last.
> 
> Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
> ---
> v1 -> v2:
> - split out xsm parts
> - adapt to changes of "unify DOMCTL to MGMT_HYPERCALLS"
> - wrap default-case and arch_get_domain_info() transiently
> ---
>  xen/Kconfig.debug                  |  2 +-
>  xen/arch/arm/arm32/Makefile        |  2 +-
>  xen/arch/arm/arm64/Makefile        |  2 +-
>  xen/arch/arm/domctl.c              |  2 --

Isn't there a change missing to arm/Makefile? Or else, how can ...

> --- a/xen/arch/arm/domctl.c
> +++ b/xen/arch/arm/domctl.c
> @@ -184,7 +184,6 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain *d,
>      }
>  }
>  
> -#ifdef CONFIG_MGMT_HYPERCALLS
>  void arch_get_info_guest(struct vcpu *v, vcpu_guest_context_u c)
>  {
>      struct vcpu_guest_context *ctxt = c.nat;
> @@ -200,7 +199,6 @@ void arch_get_info_guest(struct vcpu *v, vcpu_guest_context_u c)
>      if ( !test_bit(_VPF_down, &v->pause_flags) )
>          ctxt->flags |= VGCF_online;
>  }
> -#endif /* CONFIG_MGMT_HYPERCALLS */

... this be correct?

> --- a/xen/arch/x86/hvm/save.c
> +++ b/xen/arch/x86/hvm/save.c
> @@ -121,6 +121,7 @@ size_t hvm_save_size(struct domain *d)
>      return sz;
>  }

Both this and ...

> +#ifdef CONFIG_MGMT_HYPERCALLS
>  /*
>   * Extract a single instance of a save record, by marshalling all records of
>   * that type and copying out the one we need.
> @@ -195,6 +196,7 @@ int hvm_save_one(struct domain *d, unsigned int typecode, unsigned int instance,
>      xfree(ctxt.data);
>      return rv;
>  }
> +#endif /* CONFIG_MGMT_HYPERCALLS */
>  
>  int hvm_save(struct domain *d, hvm_domain_context_t *h)
>  {

... this and hvm_load() (and some others) will end up unreachable when
MGMT_HYPERCALLS=n and MEM_SHARING=n.

> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2585,6 +2585,7 @@ static bool cf_check vmx_get_pending_event(
>      (RTIT_STATUS_FILTER_EN | RTIT_STATUS_CONTEXT_EN | RTIT_STATUS_TRIGGER_EN | \
>       RTIT_STATUS_ERROR | RTIT_STATUS_STOPPED)
>  
> +#ifdef CONFIG_MGMT_HYPERCALLS
>  static int cf_check vmtrace_get_option(
>      struct vcpu *v, uint64_t key, uint64_t *output)
>  {

This #ifdef wants to move up a few lines, to also cover the two #define-s.

> @@ -2693,6 +2694,7 @@ static int cf_check vmtrace_control(struct vcpu *v, bool enable, bool reset)
>  
>      return 0;
>  }
> +#endif /* CONFIG_MGMT_HYPERCALLS */
>  
>  static int cf_check vmtrace_output_position(struct vcpu *v, uint64_t *pos)
>  {
> @@ -2883,10 +2885,14 @@ static struct hvm_function_table __initdata_cf_clobber vmx_function_table = {
>      .altp2m_vcpu_emulate_ve = vmx_vcpu_emulate_ve,
>      .altp2m_vcpu_emulate_vmfunc = vmx_vcpu_emulate_vmfunc,
>  #endif
> +#ifdef CONFIG_MGMT_HYPERCALLS
>      .vmtrace_control = vmtrace_control,
> +#endif
>      .vmtrace_output_position = vmtrace_output_position,

Why would this remain? Patch 05 makes VM_EVENT dependent upon MGMT_HYPERCALLS,
and outside of domctl.c the only other caller is in vm_event.c.

> @@ -747,8 +751,10 @@ bool altp2m_vcpu_emulate_ve(struct vcpu *v);
>  
>  static inline int hvm_vmtrace_control(struct vcpu *v, bool enable, bool reset)
>  {
> +#ifdef CONFIG_MGMT_HYPERCALLS
>      if ( hvm_funcs.vmtrace_control )
>          return alternative_call(hvm_funcs.vmtrace_control, v, enable, reset);
> +#endif
>  
>      return -EOPNOTSUPP;
>  }
> @@ -765,8 +771,10 @@ static inline int hvm_vmtrace_output_position(struct vcpu *v, uint64_t *pos)
>  static inline int hvm_vmtrace_set_option(
>      struct vcpu *v, uint64_t key, uint64_t value)
>  {
> +#ifdef CONFIG_MGMT_HYPERCALLS
>      if ( hvm_funcs.vmtrace_set_option )
>          return alternative_call(hvm_funcs.vmtrace_set_option, v, key, value);
> +#endif
>  
>      return -EOPNOTSUPP;
>  }
> @@ -774,8 +782,10 @@ static inline int hvm_vmtrace_set_option(
>  static inline int hvm_vmtrace_get_option(
>      struct vcpu *v, uint64_t key, uint64_t *value)
>  {
> +#ifdef CONFIG_MGMT_HYPERCALLS
>      if ( hvm_funcs.vmtrace_get_option )
>          return alternative_call(hvm_funcs.vmtrace_get_option, v, key, value);
> +#endif
>  
>      return -EOPNOTSUPP;
>  }

Why #ifdef inside the functions? The sole users each are in domctl.c.

> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -114,7 +114,9 @@ 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
>  }

This shouldn't be necessary; instead imo patch 18 should be extended to cover
getdomainfo() altogether.

> --- a/xen/lib/x86/Makefile
> +++ b/xen/lib/x86/Makefile
> @@ -1,3 +1,3 @@
>  obj-y += cpuid.o
>  obj-y += msr.o
> -obj-y += policy.o
> +obj-$(CONFIG_MGMT_HYPERCALLS) += policy.o

Fair parts of cpuid.c also become unreachable. And all of msr.c afaics.

Jan
RE: [PATCH v2 24/26] xen/domctl: wrap arch-specific domctl-op with CONFIG_MGMT_HYPERCALLS
Posted by Penny, Zheng 4 months ago
[Public]

Hi,

Sorry for the late reply. Just come back from national holiday.

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Thursday, September 11, 2025 9:03 PM
> To: Penny, Zheng <penny.zheng@amd.com>
> Cc: Huang, Ray <Ray.Huang@amd.com>; Andrew Cooper
> <andrew.cooper3@citrix.com>; Anthony PERARD <anthony.perard@vates.tech>;
> Orzel, Michal <Michal.Orzel@amd.com>; Julien Grall <julien@xen.org>; Roger
> Pau Monné <roger.pau@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>;
> Bertrand Marquis <bertrand.marquis@arm.com>; Volodymyr Babchuk
> <Volodymyr_Babchuk@epam.com>; Tamas K Lengyel <tamas@tklengyel.com>;
> Daniel P. Smith <dpsmith@apertussolutions.com>; xen-
> devel@lists.xenproject.org
> Subject: Re: [PATCH v2 24/26] xen/domctl: wrap arch-specific domctl-op with
> CONFIG_MGMT_HYPERCALLS
>
> > --- a/xen/lib/x86/Makefile
> > +++ b/xen/lib/x86/Makefile
> > @@ -1,3 +1,3 @@
> >  obj-y += cpuid.o
> >  obj-y += msr.o
> > -obj-y += policy.o
> > +obj-$(CONFIG_MGMT_HYPERCALLS) += policy.o
>
> Fair parts of cpuid.c also become unreachable. And all of msr.c afaics.
>

I just found that the functions defined here, as helpers/libraries, are used in tools/libs/guest/xg_cpuid_x86.c too. Emmm, to make compiler happy, I still need to provide stubs for them when MGMT_HYPERCALLS=n. Or any better suggestion?

> Jan
Re: [PATCH v2 24/26] xen/domctl: wrap arch-specific domctl-op with CONFIG_MGMT_HYPERCALLS
Posted by Jan Beulich 3 months, 4 weeks ago
On 11.10.2025 08:44, Penny, Zheng wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: Thursday, September 11, 2025 9:03 PM
>>
>>> --- a/xen/lib/x86/Makefile
>>> +++ b/xen/lib/x86/Makefile
>>> @@ -1,3 +1,3 @@
>>>  obj-y += cpuid.o
>>>  obj-y += msr.o
>>> -obj-y += policy.o
>>> +obj-$(CONFIG_MGMT_HYPERCALLS) += policy.o
>>
>> Fair parts of cpuid.c also become unreachable. And all of msr.c afaics.
>>
> 
> I just found that the functions defined here, as helpers/libraries, are used in tools/libs/guest/xg_cpuid_x86.c too. Emmm, to make compiler happy, I still need to provide stubs for them when MGMT_HYPERCALLS=n. Or any better suggestion?

How does the Makefile change here affect tools/libs/guest/? What would you
need stubs for there?

Jan
RE: [PATCH v2 24/26] xen/domctl: wrap arch-specific domctl-op with CONFIG_MGMT_HYPERCALLS
Posted by Penny, Zheng 3 months, 4 weeks ago
[Public]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Monday, October 13, 2025 2:41 PM
> To: Penny, Zheng <penny.zheng@amd.com>
> Cc: Huang, Ray <Ray.Huang@amd.com>; Andrew Cooper
> <andrew.cooper3@citrix.com>; Anthony PERARD <anthony.perard@vates.tech>;
> Orzel, Michal <Michal.Orzel@amd.com>; Julien Grall <julien@xen.org>; Roger Pau
> Monné <roger.pau@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>;
> Bertrand Marquis <bertrand.marquis@arm.com>; Volodymyr Babchuk
> <Volodymyr_Babchuk@epam.com>; Tamas K Lengyel <tamas@tklengyel.com>;
> Daniel P. Smith <dpsmith@apertussolutions.com>; xen-devel@lists.xenproject.org
> Subject: Re: [PATCH v2 24/26] xen/domctl: wrap arch-specific domctl-op with
> CONFIG_MGMT_HYPERCALLS
>
> On 11.10.2025 08:44, Penny, Zheng wrote:
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@suse.com>
> >> Sent: Thursday, September 11, 2025 9:03 PM
> >>
> >>> --- a/xen/lib/x86/Makefile
> >>> +++ b/xen/lib/x86/Makefile
> >>> @@ -1,3 +1,3 @@
> >>>  obj-y += cpuid.o
> >>>  obj-y += msr.o
> >>> -obj-y += policy.o
> >>> +obj-$(CONFIG_MGMT_HYPERCALLS) += policy.o
> >>
> >> Fair parts of cpuid.c also become unreachable. And all of msr.c afaics.
> >>
> >
> > I just found that the functions defined here, as helpers/libraries, are used in
> tools/libs/guest/xg_cpuid_x86.c too. Emmm, to make compiler happy, I still need to
> provide stubs for them when MGMT_HYPERCALLS=n. Or any better suggestion?
>
> How does the Makefile change here affect tools/libs/guest/? What would you need
> stubs for there?
>

Like Function xc_cpu_policy_is_compatible() in tools/libs/guest/xg_cpuid_x86.c is also using x86_cpu_policies_are_compatible() to do the comparison between host and guest. If making xen/lib/x86/plocy.o guarded by MGMT_HYPERCALLS, we will have "undefined reference " error. Imo, it is not suitable to guard files tools/libs/guest/xg_cpuid_x86.o with MGMT_HYPERCALLS.
So it we still want to have the Makefile change here, we need to provide stubs. Or we remove the Makefile change here, as the role of these functions is more of the library,

> Jan
Re: [PATCH v2 24/26] xen/domctl: wrap arch-specific domctl-op with CONFIG_MGMT_HYPERCALLS
Posted by Jan Beulich 3 months, 4 weeks ago
On 13.10.2025 09:18, Penny, Zheng wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: Monday, October 13, 2025 2:41 PM
>>
>> On 11.10.2025 08:44, Penny, Zheng wrote:
>>>> -----Original Message-----
>>>> From: Jan Beulich <jbeulich@suse.com>
>>>> Sent: Thursday, September 11, 2025 9:03 PM
>>>>
>>>>> --- a/xen/lib/x86/Makefile
>>>>> +++ b/xen/lib/x86/Makefile
>>>>> @@ -1,3 +1,3 @@
>>>>>  obj-y += cpuid.o
>>>>>  obj-y += msr.o
>>>>> -obj-y += policy.o
>>>>> +obj-$(CONFIG_MGMT_HYPERCALLS) += policy.o
>>>>
>>>> Fair parts of cpuid.c also become unreachable. And all of msr.c afaics.
>>>>
>>>
>>> I just found that the functions defined here, as helpers/libraries, are used in
>> tools/libs/guest/xg_cpuid_x86.c too. Emmm, to make compiler happy, I still need to
>> provide stubs for them when MGMT_HYPERCALLS=n. Or any better suggestion?
>>
>> How does the Makefile change here affect tools/libs/guest/? What would you need
>> stubs for there?
>>
> 
> Like Function xc_cpu_policy_is_compatible() in tools/libs/guest/xg_cpuid_x86.c is also using x86_cpu_policies_are_compatible() to do the comparison between host and guest. If making xen/lib/x86/plocy.o guarded by MGMT_HYPERCALLS, we will have "undefined reference " error. Imo, it is not suitable to guard files tools/libs/guest/xg_cpuid_x86.o with MGMT_HYPERCALLS.

Correct, but I still don't see what you're getting at. This Makefile is used in
the hypervisor build only. In tools/libs/guest/Makefile.common we have

ifeq ($(CONFIG_X86),y) # Add libx86 to the build
vpath %.c ../../../xen/lib/x86

OBJS-y                 += cpuid.o msr.o policy.o
endif

Jan
RE: [PATCH v2 24/26] xen/domctl: wrap arch-specific domctl-op with CONFIG_MGMT_HYPERCALLS
Posted by Penny, Zheng 3 months, 4 weeks ago
[Public]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Monday, October 13, 2025 4:30 PM
> To: Penny, Zheng <penny.zheng@amd.com>
> Cc: Huang, Ray <Ray.Huang@amd.com>; Andrew Cooper
> <andrew.cooper3@citrix.com>; Anthony PERARD <anthony.perard@vates.tech>;
> Orzel, Michal <Michal.Orzel@amd.com>; Julien Grall <julien@xen.org>; Roger Pau
> Monné <roger.pau@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>;
> Bertrand Marquis <bertrand.marquis@arm.com>; Volodymyr Babchuk
> <Volodymyr_Babchuk@epam.com>; Tamas K Lengyel <tamas@tklengyel.com>;
> Daniel P. Smith <dpsmith@apertussolutions.com>; xen-devel@lists.xenproject.org
> Subject: Re: [PATCH v2 24/26] xen/domctl: wrap arch-specific domctl-op with
> CONFIG_MGMT_HYPERCALLS
>
> On 13.10.2025 09:18, Penny, Zheng wrote:
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@suse.com>
> >> Sent: Monday, October 13, 2025 2:41 PM
> >>
> >> On 11.10.2025 08:44, Penny, Zheng wrote:
> >>>> -----Original Message-----
> >>>> From: Jan Beulich <jbeulich@suse.com>
> >>>> Sent: Thursday, September 11, 2025 9:03 PM
> >>>>
> >>>>> --- a/xen/lib/x86/Makefile
> >>>>> +++ b/xen/lib/x86/Makefile
> >>>>> @@ -1,3 +1,3 @@
> >>>>>  obj-y += cpuid.o
> >>>>>  obj-y += msr.o
> >>>>> -obj-y += policy.o
> >>>>> +obj-$(CONFIG_MGMT_HYPERCALLS) += policy.o
> >>>>
> >>>> Fair parts of cpuid.c also become unreachable. And all of msr.c afaics.
> >>>>
> >>>
> >>> I just found that the functions defined here, as helpers/libraries,
> >>> are used in
> >> tools/libs/guest/xg_cpuid_x86.c too. Emmm, to make compiler happy, I
> >> still need to provide stubs for them when MGMT_HYPERCALLS=n. Or any
> better suggestion?
> >>
> >> How does the Makefile change here affect tools/libs/guest/? What
> >> would you need stubs for there?
> >>
> >
> > Like Function xc_cpu_policy_is_compatible() in tools/libs/guest/xg_cpuid_x86.c
> is also using x86_cpu_policies_are_compatible() to do the comparison between
> host and guest. If making xen/lib/x86/plocy.o guarded by MGMT_HYPERCALLS,
> we will have "undefined reference " error. Imo, it is not suitable to guard files
> tools/libs/guest/xg_cpuid_x86.o with MGMT_HYPERCALLS.
>
> Correct, but I still don't see what you're getting at. This Makefile is used in the
> hypervisor build only. In tools/libs/guest/Makefile.common we have
>
> ifeq ($(CONFIG_X86),y) # Add libx86 to the build vpath %.c ../../../xen/lib/x86
>
> OBJS-y                 += cpuid.o msr.o policy.o
> endif
>

Oh, true...
They are separately compiled with different Makefile.
It is fair parts of cpuid.c(x86_cpuid_copy_to{,from}_buffer) which got "undefined reference". Lets omit this part of change.

> Jan