[PATCH] domain: adjust soft-reset arch dependency

Jan Beulich posted 1 patch 1 month, 2 weeks ago
Failed in applying to current master (apply log)
[PATCH] domain: adjust soft-reset arch dependency
Posted by Jan Beulich 1 month, 2 weeks ago
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@suse.com>

--- 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(
Re: [PATCH] domain: adjust soft-reset arch dependency
Posted by Oleksii Kurochko 1 month, 1 week ago
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@suse.com>

LGTM:

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

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(
Re: [PATCH] domain: adjust soft-reset arch dependency
Posted by Jan Beulich 1 month, 1 week ago
On 05.11.2025 12:33, Oleksii Kurochko wrote:
> 
> 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@suse.com>
> 
> LGTM:
> 
> Reviewed-by: Oleksii Kurochko<oleksii.kurochko@gmail.com>

Thanks, but one remark: Here as well as in your release-acks, can you please
get used to inserting a blank ahead of the opening angle bracket? Roger had
asked (me, not you) about its absence too, the other day.

Jan
Re: [PATCH] domain: adjust soft-reset arch dependency
Posted by Julien Grall 1 month, 1 week ago
Hi Jan,

On 30/10/2025 12:10, 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@suse.com>

Acked-by: Julien GralL <jgrall@amazon.com>

Cheers,

-- 
Julien Grall
Re: [PATCH] domain: adjust soft-reset arch dependency
Posted by Grygorii Strashko 1 month, 2 weeks ago
Hi Jan,

On 30.10.25 14:10, 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@suse.com>
> 

Thank you for your patch.

Reviewed-by: Grygorii Strashko <grygorii_strashko@epam.com>

But one question - soft_reset is applicable for domain, so wouldn't it
be better to note that in Kconfig option name:

   HAS_DOMAIN_SOFT_RESET

-- 
Best regards,
-grygorii
Re: [PATCH] domain: adjust soft-reset arch dependency
Posted by Jan Beulich 1 month, 1 week ago
On 31.10.2025 14:08, Grygorii Strashko wrote:
> Hi Jan,
> 
> On 30.10.25 14:10, 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@suse.com>
>>
> 
> Thank you for your patch.
> 
> Reviewed-by: Grygorii Strashko <grygorii_strashko@epam.com>

Thanks.

> But one question - soft_reset is applicable for domain, so wouldn't it
> be better to note that in Kconfig option name:
> 
>    HAS_DOMAIN_SOFT_RESET

I thought that "soft reset" is (going to remain) pretty unambiguous without the
"domain". If (in particular) other REST maintainers think differently, I'm open
to change the name. Generally my aim is to prefer reasonably short names for
variables and alike, as long as no ambiguity arises.

Jan