[PATCH v3 26/28] xen/domctl: wrap arch-specific domctl-op with CONFIG_MGMT_HYPERCALLS

Penny Zheng posted 28 patches 2 weeks, 3 days ago
Only 27 patches received!
[PATCH v3 26/28] xen/domctl: wrap arch-specific domctl-op with CONFIG_MGMT_HYPERCALLS
Posted by Penny Zheng 2 weeks, 3 days 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 too. Otherwise they will all become unreachable when
MGMT_HYPERCALLS=n, and hence violating Misra Rule 2.1.
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
- 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
  - hvm_vmtrace_output_position
    - hvm_funcs.vmtrace_output_position
- paging_domctl_cont
- make policy.o/msr.o depend on CONFIG_MGMT_HYPERCALLS
For ARM:
- subarch_do_domctl
We put the guardian in Makefile for the arch-specific domctl.o compilation.

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
---
v2 -> v3:
- add missing guardian in arm/Makefile
- cover RTIT_CTL{,STATUS}_MASK #define-s
- add missing guardian for .vmtrace_output_position
- guard the whole static inline function
- address "violating Misra rule 2.1" in commit message
- remove transient wrapping around default-case and arch_get_domain_info()
---
 xen/Kconfig.debug                  | 2 +-
 xen/arch/arm/Makefile              | 2 +-
 xen/arch/arm/arm32/Makefile        | 2 +-
 xen/arch/arm/arm64/Makefile        | 2 +-
 xen/arch/x86/Makefile              | 2 +-
 xen/arch/x86/hvm/hvm.c             | 2 ++
 xen/arch/x86/hvm/pmtimer.c         | 2 ++
 xen/arch/x86/hvm/vmx/vmx.c         | 4 ++++
 xen/arch/x86/include/asm/hvm/hvm.h | 4 ++++
 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/include/hypercall-defs.c       | 4 ++--
 xen/lib/x86/Makefile               | 4 ++--
 14 files changed, 25 insertions(+), 11 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/Makefile b/xen/arch/arm/Makefile
index 2aff1a1630..2f4593f5eb 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -20,7 +20,7 @@ obj-$(CONFIG_IOREQ_SERVER) += dm.o
 obj-$(CONFIG_DOM0LESS_BOOT) += dom0less-build.init.o
 obj-y += domain.o
 obj-y += domain_build.init.o
-obj-y += domctl.o
+obj-$(CONFIG_MGMT_HYPERCALLS) += domctl.o
 obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
 obj-y += efi/
 obj-y += gic.o
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/x86/Makefile b/xen/arch/x86/Makefile
index 615cd101b8..7253ca32af 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/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index e3dacc909b..e77b0c03ed 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -5259,6 +5259,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;
@@ -5301,6 +5302,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/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 4394990131..5c2e8a8d92 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2568,6 +2568,7 @@ static bool cf_check vmx_get_pending_event(
     return true;
 }
 
+#ifdef CONFIG_MGMT_HYPERCALLS
 /*
  * We only let vmtrace agents see and modify a subset of bits in MSR_RTIT_CTL.
  * These all pertain to data-emitted into the trace buffer(s).  Must not
@@ -2700,6 +2701,7 @@ static int cf_check vmtrace_output_position(struct vcpu *v, uint64_t *pos)
     *pos = v->arch.msrs->rtit.output_offset;
     return v->arch.hvm.vmx.ipt_active;
 }
+#endif /* CONFIG_MGMT_HYPERCALLS */
 
 #if defined(CONFIG_MGMT_HYPERCALLS) || defined(CONFIG_MEM_SHARING)
 static int cf_check vmtrace_reset(struct vcpu *v)
@@ -2886,10 +2888,12 @@ 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,
     .vmtrace_output_position = vmtrace_output_position,
     .vmtrace_set_option = vmtrace_set_option,
     .vmtrace_get_option = vmtrace_get_option,
+#endif
 #if defined(CONFIG_MGMT_HYPERCALLS) || defined(CONFIG_MEM_SHARING)
     .vmtrace_reset = vmtrace_reset,
 #endif
diff --git a/xen/arch/x86/include/asm/hvm/hvm.h b/xen/arch/x86/include/asm/hvm/hvm.h
index 9d6cb42d48..a88c69e3ff 100644
--- a/xen/arch/x86/include/asm/hvm/hvm.h
+++ b/xen/arch/x86/include/asm/hvm/hvm.h
@@ -236,10 +236,12 @@ struct hvm_function_table {
 #endif
 
     /* vmtrace */
+#ifdef CONFIG_MGMT_HYPERCALLS
     int (*vmtrace_control)(struct vcpu *v, bool enable, bool reset);
     int (*vmtrace_output_position)(struct vcpu *v, uint64_t *pos);
     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
 #if defined(CONFIG_MGMT_HYPERCALLS) || defined(CONFIG_MEM_SHARING)
     int (*vmtrace_reset)(struct vcpu *v);
 #endif
@@ -742,6 +744,7 @@ static inline bool altp2m_vcpu_emulate_ve(struct vcpu *v)
 bool altp2m_vcpu_emulate_ve(struct vcpu *v);
 #endif /* CONFIG_ALTP2M */
 
+#ifdef CONFIG_MGMT_HYPERCALLS
 static inline int hvm_vmtrace_control(struct vcpu *v, bool enable, bool reset)
 {
     if ( hvm_funcs.vmtrace_control )
@@ -776,6 +779,7 @@ static inline int hvm_vmtrace_get_option(
 
     return -EOPNOTSUPP;
 }
+#endif /* CONFIG_MGMT_HYPERCALLS */
 
 #if defined(CONFIG_MGMT_HYPERCALLS) || defined(CONFIG_MEM_SHARING)
 static inline int hvm_vmtrace_reset(struct vcpu *v)
diff --git a/xen/arch/x86/include/asm/p2m.h b/xen/arch/x86/include/asm/p2m.h
index 9016e88411..3574e2eecd 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 af7b7f2538..6113a271dc 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -2322,6 +2322,7 @@ out:
     return rc;
 }
 
+#ifdef CONFIG_MGMT_HYPERCALLS
 int mem_sharing_domctl(struct domain *d, struct xen_domctl_mem_sharing_op *mec)
 {
     int rc;
@@ -2339,6 +2340,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/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..89fb4bba27 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) += msr.o
+obj-$(CONFIG_MGMT_HYPERCALLS) += policy.o
-- 
2.34.1
Re: [PATCH v3 26/28] xen/domctl: wrap arch-specific domctl-op with CONFIG_MGMT_HYPERCALLS
Posted by Jan Beulich 5 hours ago
On 13.10.2025 12:15, 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 too. Otherwise they will all become unreachable when
> MGMT_HYPERCALLS=n, and hence violating Misra Rule 2.1.
> 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
> - 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
>   - hvm_vmtrace_output_position
>     - hvm_funcs.vmtrace_output_position
> - paging_domctl_cont
> - make policy.o/msr.o depend on CONFIG_MGMT_HYPERCALLS
> For ARM:
> - subarch_do_domctl
> We put the guardian in Makefile for the arch-specific domctl.o compilation.
> 
> 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
> ---
> v2 -> v3:
> - add missing guardian in arm/Makefile
> - cover RTIT_CTL{,STATUS}_MASK #define-s
> - add missing guardian for .vmtrace_output_position
> - guard the whole static inline function
> - address "violating Misra rule 2.1" in commit message
> - remove transient wrapping around default-case and arch_get_domain_info()
> ---
>  xen/Kconfig.debug                  | 2 +-
>  xen/arch/arm/Makefile              | 2 +-
>  xen/arch/arm/arm32/Makefile        | 2 +-
>  xen/arch/arm/arm64/Makefile        | 2 +-
>  xen/arch/x86/Makefile              | 2 +-
>  xen/arch/x86/hvm/hvm.c             | 2 ++
>  xen/arch/x86/hvm/pmtimer.c         | 2 ++
>  xen/arch/x86/hvm/vmx/vmx.c         | 4 ++++
>  xen/arch/x86/include/asm/hvm/hvm.h | 4 ++++
>  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/include/hypercall-defs.c       | 4 ++--
>  xen/lib/x86/Makefile               | 4 ++--
>  14 files changed, 25 insertions(+), 11 deletions(-)

This again might better be split, perhaps (at least) into an Arm and an x86 patch.
Possibly the do_vmtrace_op() set of changes could also be a separate patch.

> --- 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

Likely better

	depends on MGMT_HYPERCALLS
	depends on X86

such that when (if ever) another arch is to be added, a simpler modification will
do.

> --- a/xen/arch/x86/mm/mem_sharing.c
> +++ b/xen/arch/x86/mm/mem_sharing.c
> @@ -2322,6 +2322,7 @@ out:
>      return rc;
>  }
>  
> +#ifdef CONFIG_MGMT_HYPERCALLS
>  int mem_sharing_domctl(struct domain *d, struct xen_domctl_mem_sharing_op *mec)
>  {
>      int rc;
> @@ -2339,6 +2340,7 @@ int mem_sharing_domctl(struct domain *d, struct xen_domctl_mem_sharing_op *mec)
>  
>      return rc;
>  }
> +#endif /* CONFIG_MGMT_HYPERCALLS */

See again my MEM_SHARING related remark.

> --- 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) += msr.o
> +obj-$(CONFIG_MGMT_HYPERCALLS) += policy.o

What about e.g. x86_cpuid_copy_{to,from}_buffer(), living in cpuid.o?

Jan