Re: [PATCH] domain: adjust soft-reset arch dependency

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 10/30/25 1:10 PM, Jan Beulich wrote:

> Arm's arch_domain_soft_reset() returning -ENOSYS is quite unhelpful: This
> way a domain will be crashed if a tool stack mistakenly invokes
> XEN_DOMCTL_soft_reset on it. Instead the tool stack should be told of its
> mistake.
>
> Introduce HAS_SOFT_RESET, implied only by x86. "imply" rather than
> "select" such that HAS_SOFT_RESET can later gain a dependency on
> MGMT_HYPERCALLS. That way HAS_SOFT_RESET will go off when
> MGMT_HYPERCALLS is off.
>
> Check the new setting early in domctl handling, and compile out the thus
> dead (when HAS_SOFT_RESET=n) domain_soft_reset() as well as its dedicated
> helpers.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>     
LGTM:

Reviewed-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>

Thanks.

~ Oleksii
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -875,11 +875,6 @@ void arch_domain_unpause(struct domain *
>  {
>  }
>  
> -int arch_domain_soft_reset(struct domain *d)
> -{
> -    return -ENOSYS;
> -}
> -
>  void arch_domain_creation_finished(struct domain *d)
>  {
>      p2m_domain_creation_finished(d);
> --- a/xen/arch/ppc/stubs.c
> +++ b/xen/arch/ppc/stubs.c
> @@ -214,11 +214,6 @@ void arch_domain_unpause(struct domain *
>      BUG_ON("unimplemented");
>  }
>  
> -int arch_domain_soft_reset(struct domain *d)
> -{
> -    BUG_ON("unimplemented");
> -}
> -
>  void arch_domain_creation_finished(struct domain *d)
>  {
>      BUG_ON("unimplemented");
> --- a/xen/arch/riscv/stubs.c
> +++ b/xen/arch/riscv/stubs.c
> @@ -188,11 +188,6 @@ void arch_domain_unpause(struct domain *
>      BUG_ON("unimplemented");
>  }
>  
> -int arch_domain_soft_reset(struct domain *d)
> -{
> -    BUG_ON("unimplemented");
> -}
> -
>  void arch_domain_creation_finished(struct domain *d)
>  {
>      BUG_ON("unimplemented");
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -29,6 +29,7 @@ config X86
>  	select HAS_PCI_MSI
>  	select HAS_PIRQ
>  	select HAS_SCHED_GRANULARITY
> +	imply HAS_SOFT_RESET
>  	select HAS_UBSAN
>  	select HAS_VMAP
>  	select HAS_VPCI if HVM
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1030,6 +1030,7 @@ void arch_domain_unpause(struct domain *
>          viridian_time_domain_thaw(d);
>  }
>  
> +#ifdef CONFIG_HAS_SOFT_RESET
>  int arch_domain_soft_reset(struct domain *d)
>  {
>      struct page_info *page = virt_to_page(d->shared_info), *new_page;
> @@ -1131,6 +1132,7 @@ int arch_domain_soft_reset(struct domain
>  
>      return ret;
>  }
> +#endif /* CONFIG_HAS_SOFT_RESET */
>  
>  void arch_domain_creation_finished(struct domain *d)
>  {
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -155,6 +155,9 @@ config HAS_PMAP
>  config HAS_SCHED_GRANULARITY
>  	bool
>  
> +config HAS_SOFT_RESET
> +	bool
> +
>  config HAS_STACK_PROTECTOR
>  	bool
>  
> --- a/xen/common/argo.c
> +++ b/xen/common/argo.c
> @@ -2351,6 +2351,7 @@ argo_destroy(struct domain *d)
>      write_unlock(&L1_global_argo_rwlock);
>  }
>  
> +#ifdef CONFIG_HAS_SOFT_RESET
>  void
>  argo_soft_reset(struct domain *d)
>  {
> @@ -2374,3 +2375,4 @@ argo_soft_reset(struct domain *d)
>  
>      write_unlock(&L1_global_argo_rwlock);
>  }
> +#endif /* CONFIG_HAS_SOFT_RESET */
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -1687,6 +1687,7 @@ void domain_unpause_except_self(struct d
>          domain_unpause(d);
>  }
>  
> +#ifdef CONFIG_HAS_SOFT_RESET
>  int domain_soft_reset(struct domain *d, bool resuming)
>  {
>      struct vcpu *v;
> @@ -1724,6 +1725,7 @@ int domain_soft_reset(struct domain *d,
>  
>      return rc;
>  }
> +#endif /* CONFIG_HAS_SOFT_RESET */
>  
>  int vcpu_reset(struct vcpu *v)
>  {
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -466,6 +466,12 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe
>  
>      case XEN_DOMCTL_soft_reset:
>      case XEN_DOMCTL_soft_reset_cont:
> +        if ( !IS_ENABLED(CONFIG_HAS_SOFT_RESET) )
> +        {
> +            ret = -EOPNOTSUPP;
> +            break;
> +        }
> +
>          if ( d == current->domain ) /* no domain_pause() */
>          {
>              ret = -EINVAL;
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -3962,6 +3962,7 @@ int gnttab_release_mappings(struct domai
>      return 0;
>  }
>  
> +#ifdef CONFIG_HAS_SOFT_RESET
>  void grant_table_warn_active_grants(struct domain *d)
>  {
>      struct grant_table *gt = d->grant_table;
> @@ -4006,6 +4007,7 @@ void grant_table_warn_active_grants(stru
>  
>  #undef WARN_GRANT_MAX
>  }
> +#endif /* CONFIG_HAS_SOFT_RESET */
>  
>  void
>  grant_table_destroy(
>