[v2] Disable domctl-op via CONFIG_MGMT_HYPERCALLS

[PATCH v2 07/26] xen/domctl: wrap domain_pause_by_systemcontroller() with MGMT_HYPERCALLS

Posted by Penny Zheng 4 months, 4 weeks ago

Function domain_pause_by_systemcontroller() is responsible for
XEN_DOMCTL_pausedomain domctl-op, and shall be wrapped around with
CONFIG_MGMT_HYPERCALLS.
Provide transient wrapping around XEN_DOMCTL_pausedomain-case, and it
will be removed on introducing CONFIG_MGMT_HYPERCALLS on the common/domctl.c
in the last.

Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
---
v1 -> v2:
- adapt to changes of "unify DOMCTL to MGMT_HYPERCALLS"
- provide transient wrapping around XEN_DOMCTL_pausedomain-case
---
 xen/common/domain.c | 2 ++
 xen/common/domctl.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index 775c339285..976172c7d3 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -1606,10 +1606,12 @@ static int _domain_pause_by_systemcontroller(struct domain *d, bool sync)
     return 0;
 }
 
+#ifdef CONFIG_MGMT_HYPERCALLS
 int domain_pause_by_systemcontroller(struct domain *d)
 {
     return _domain_pause_by_systemcontroller(d, true /* sync */);
 }
+#endif /* CONFIG_MGMT_HYPERCALLS */
 
 int domain_pause_by_systemcontroller_nosync(struct domain *d)
 {
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 71e712c1f3..0061d7972a 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -390,11 +390,13 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
         break;
     }
 
+#ifdef CONFIG_MGMT_HYPERCALLS
     case XEN_DOMCTL_pausedomain:
         ret = -EINVAL;
         if ( d != current->domain )
             ret = domain_pause_by_systemcontroller(d);
         break;
+#endif /* CONFIG_MGMT_HYPERCALLS */
 
     case XEN_DOMCTL_unpausedomain:
         ret = domain_unpause_by_systemcontroller(d);
-- 
2.34.1

Re: [PATCH v2 07/26] xen/domctl: wrap domain_pause_by_systemcontroller() with MGMT_HYPERCALLS

Posted by Jan Beulich 4 months, 4 weeks ago

On 10.09.2025 09:38, Penny Zheng wrote:
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -1606,10 +1606,12 @@ static int _domain_pause_by_systemcontroller(struct domain *d, bool sync)
>      return 0;
>  }
>  
> +#ifdef CONFIG_MGMT_HYPERCALLS
>  int domain_pause_by_systemcontroller(struct domain *d)
>  {
>      return _domain_pause_by_systemcontroller(d, true /* sync */);
>  }
> +#endif /* CONFIG_MGMT_HYPERCALLS */
>  
>  int domain_pause_by_systemcontroller_nosync(struct domain *d)
>  {

I would have ack-ed this if there was only this part, but ...

> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -390,11 +390,13 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
>          break;
>      }
>  
> +#ifdef CONFIG_MGMT_HYPERCALLS
>      case XEN_DOMCTL_pausedomain:
>          ret = -EINVAL;
>          if ( d != current->domain )
>              ret = domain_pause_by_systemcontroller(d);
>          break;
> +#endif /* CONFIG_MGMT_HYPERCALLS */
>  
>      case XEN_DOMCTL_unpausedomain:
>          ret = domain_unpause_by_systemcontroller(d);

... as expressed elsewhere I'm not happy about this one, as it'll need
undoing in a later patch of this same series.

Jan

RE: [PATCH v2 07/26] xen/domctl: wrap domain_pause_by_systemcontroller() with MGMT_HYPERCALLS

Posted by Penny, Zheng 4 months, 2 weeks ago

[Public]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Wednesday, September 10, 2025 11:09 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>; xen-
> devel@lists.xenproject.org
> Subject: Re: [PATCH v2 07/26] xen/domctl: wrap
> domain_pause_by_systemcontroller() with MGMT_HYPERCALLS
>
> On 10.09.2025 09:38, Penny Zheng wrote:
> > --- a/xen/common/domain.c
> > +++ b/xen/common/domain.c
> > @@ -1606,10 +1606,12 @@ static int
> _domain_pause_by_systemcontroller(struct domain *d, bool sync)
> >      return 0;
> >  }
> >
> > +#ifdef CONFIG_MGMT_HYPERCALLS
> >  int domain_pause_by_systemcontroller(struct domain *d)  {
> >      return _domain_pause_by_systemcontroller(d, true /* sync */);  }
> > +#endif /* CONFIG_MGMT_HYPERCALLS */
> >
> >  int domain_pause_by_systemcontroller_nosync(struct domain *d)
> >  {
>
> I would have ack-ed this if there was only this part, but ...
>
> > --- a/xen/common/domctl.c
> > +++ b/xen/common/domctl.c
> > @@ -390,11 +390,13 @@ long
> do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
> >          break;
> >      }
> >
> > +#ifdef CONFIG_MGMT_HYPERCALLS
> >      case XEN_DOMCTL_pausedomain:
> >          ret = -EINVAL;
> >          if ( d != current->domain )
> >              ret = domain_pause_by_systemcontroller(d);
> >          break;
> > +#endif /* CONFIG_MGMT_HYPERCALLS */
> >
> >      case XEN_DOMCTL_unpausedomain:
> >          ret = domain_unpause_by_systemcontroller(d);
>
> ... as expressed elsewhere I'm not happy about this one, as it'll need
> undoing in a later patch of this same series.
>

I shall admit that this kind of stub really helps me test MGMT_HYPERCALLS=n for this big serie commit by commit at the very beginning. Otherwise, it could be only disabled (and tested) in the end, and accumulate the mistakes...
But, as you said, all this transient thing needs to be reversed in the last, and I could accidently missing something and leave dead code...
As CONFIG_SYSCTL is already a prompt option, then maybe I need to raise a new commit to make it as def_bool again only for this patch serie transiently or just address it in " xen/sysctl: replace CONFIG_SYSCTL with CONFIG_MGMT_DOMCTL " ?

> Jan

Re: [PATCH v2 07/26] xen/domctl: wrap domain_pause_by_systemcontroller() with MGMT_HYPERCALLS

Posted by Jan Beulich 4 months, 2 weeks ago

On 24.09.2025 09:11, Penny, Zheng wrote:
> [Public]
> 
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: Wednesday, September 10, 2025 11:09 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>; xen-
>> devel@lists.xenproject.org
>> Subject: Re: [PATCH v2 07/26] xen/domctl: wrap
>> domain_pause_by_systemcontroller() with MGMT_HYPERCALLS
>>
>> On 10.09.2025 09:38, Penny Zheng wrote:
>>> --- a/xen/common/domain.c
>>> +++ b/xen/common/domain.c
>>> @@ -1606,10 +1606,12 @@ static int
>> _domain_pause_by_systemcontroller(struct domain *d, bool sync)
>>>      return 0;
>>>  }
>>>
>>> +#ifdef CONFIG_MGMT_HYPERCALLS
>>>  int domain_pause_by_systemcontroller(struct domain *d)  {
>>>      return _domain_pause_by_systemcontroller(d, true /* sync */);  }
>>> +#endif /* CONFIG_MGMT_HYPERCALLS */
>>>
>>>  int domain_pause_by_systemcontroller_nosync(struct domain *d)
>>>  {
>>
>> I would have ack-ed this if there was only this part, but ...
>>
>>> --- a/xen/common/domctl.c
>>> +++ b/xen/common/domctl.c
>>> @@ -390,11 +390,13 @@ long
>> do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
>>>          break;
>>>      }
>>>
>>> +#ifdef CONFIG_MGMT_HYPERCALLS
>>>      case XEN_DOMCTL_pausedomain:
>>>          ret = -EINVAL;
>>>          if ( d != current->domain )
>>>              ret = domain_pause_by_systemcontroller(d);
>>>          break;
>>> +#endif /* CONFIG_MGMT_HYPERCALLS */
>>>
>>>      case XEN_DOMCTL_unpausedomain:
>>>          ret = domain_unpause_by_systemcontroller(d);
>>
>> ... as expressed elsewhere I'm not happy about this one, as it'll need
>> undoing in a later patch of this same series.
>>
> 
> I shall admit that this kind of stub really helps me test MGMT_HYPERCALLS=n for this big serie commit by commit at the very beginning. Otherwise, it could be only disabled (and tested) in the end, and accumulate the mistakes...
> But, as you said, all this transient thing needs to be reversed in the last, and I could accidently missing something and leave dead code...
> As CONFIG_SYSCTL is already a prompt option, then maybe I need to raise a new commit to make it as def_bool again only for this patch serie transiently or just address it in " xen/sysctl: replace CONFIG_SYSCTL with CONFIG_MGMT_DOMCTL " ?

Removing the prompt again (whether in a separate patch or in the renaming one I
wouldn't care much) was what I suggested from the very beginning, but which also
is what faced Stefano's opposition.

Jan