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

[Jan Beulich <jbeulich@suse.com>]

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