[PATCH v4 04/20] xen: introduce CONFIG_SYSCTL

Penny Zheng posted 20 patches 5 months ago
There is a newer version of this series
[PATCH v4 04/20] xen: introduce CONFIG_SYSCTL
Posted by Penny Zheng 5 months ago
From: Stefano Stabellini <stefano.stabellini@amd.com>

We introduce a new Kconfig CONFIG_SYSCTL, which shall only be disabled
on some dom0less systems or PV shim on x86, to reduce Xen footprint.

Making SYSCTL without prompt is transient and it will be fixed in the final
patch. Also, we will also state unsetting SYSCTL in pvshim_defconfig to
explicitly make it unavailable for PV shim in the final patch.

Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
v2 -> v3:
- remove "intend to" in commit message
---
v3 -> v4:
- introduce the option without prompt (simply "defbool y") to eliminate the
need for transient "#ifdef CONFIG_SYSCTL"
- calling out the transient scenario in commit message
---
 xen/common/Kconfig | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 3d66d09397..28e6ac2142 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -579,4 +579,15 @@ config BUDDY_ALLOCATOR_SIZE
 	  Amount of memory reserved for the buddy allocator to serve Xen heap,
 	  working alongside the colored one.
 
+menu "Supported hypercall interfaces"
+	visible if EXPERT
+
+config SYSCTL
+	bool "Enable sysctl hypercall"
+	def_bool y
+	help
+	  This option shall only be disabled on some dom0less systems,
+	  to reduce Xen footprint.
+endmenu
+
 endmenu
-- 
2.34.1
Re: [PATCH v4 04/20] xen: introduce CONFIG_SYSCTL
Posted by Jan Beulich 4 months, 3 weeks ago
On 28.05.2025 11:16, Penny Zheng wrote:
> From: Stefano Stabellini <stefano.stabellini@amd.com>
> 
> We introduce a new Kconfig CONFIG_SYSCTL, which shall only be disabled
> on some dom0less systems or PV shim on x86, to reduce Xen footprint.
> 
> Making SYSCTL without prompt is transient and it will be fixed in the final

Nit: s/fixed/adjusted/ ? It's not a bug, after all.

> patch. Also, we will also state unsetting SYSCTL in pvshim_defconfig to
> explicitly make it unavailable for PV shim in the final patch.

Even without the double "also" this reads odd. But it's also unclear what
it has to do here, nor whether what is being said is actually correct.

> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -579,4 +579,15 @@ config BUDDY_ALLOCATOR_SIZE
>  	  Amount of memory reserved for the buddy allocator to serve Xen heap,
>  	  working alongside the colored one.
>  
> +menu "Supported hypercall interfaces"
> +	visible if EXPERT
> +
> +config SYSCTL
> +	bool "Enable sysctl hypercall"
> +	def_bool y

Why def_bool when you already have bool on the earlier line?

> +	help
> +	  This option shall only be disabled on some dom0less systems,
> +	  to reduce Xen footprint.

This isn't overly useful (but possibly misleading) as long as the prompt
isn#t going to be visible, yet.

> +endmenu

Blank line please ahead of this one.

Jan
RE: [PATCH v4 04/20] xen: introduce CONFIG_SYSCTL
Posted by Penny, Zheng 4 months, 3 weeks ago
[Public]

Hi

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Tuesday, June 10, 2025 9:05 PM
> To: Penny, Zheng <penny.zheng@amd.com>
> Cc: Huang, Ray <Ray.Huang@amd.com>; Stabellini, Stefano
> <stefano.stabellini@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>; Sergiy Kibrik
> <Sergiy_Kibrik@epam.com>; xen-devel@lists.xenproject.org
> Subject: Re: [PATCH v4 04/20] xen: introduce CONFIG_SYSCTL
>
> On 28.05.2025 11:16, Penny Zheng wrote:
> > From: Stefano Stabellini <stefano.stabellini@amd.com>
> >
> > We introduce a new Kconfig CONFIG_SYSCTL, which shall only be disabled
> > on some dom0less systems or PV shim on x86, to reduce Xen footprint.
> >
> > Making SYSCTL without prompt is transient and it will be fixed in the
> > final
>
> Nit: s/fixed/adjusted/ ? It's not a bug, after all.
>

Understood.

> > patch. Also, we will also state unsetting SYSCTL in pvshim_defconfig
> > to explicitly make it unavailable for PV shim in the final patch.
>
> Even without the double "also" this reads odd. But it's also unclear what it has to do
> here, nor whether what is being said is actually correct.
>

Hmmm, How about  "
The consequences of introducing "CONFIG_SYSCTL=y" in .config file generated from pvshim_defconfig
is transient and will be also fixed in the final."

> > --- a/xen/common/Kconfig
> > +++ b/xen/common/Kconfig
> > @@ -579,4 +579,15 @@ config BUDDY_ALLOCATOR_SIZE
> >       Amount of memory reserved for the buddy allocator to serve Xen heap,
> >       working alongside the colored one.
> >
> > +menu "Supported hypercall interfaces"
> > +   visible if EXPERT
> > +
> > +config SYSCTL
> > +   bool "Enable sysctl hypercall"
> > +   def_bool y
>
> Why def_bool when you already have bool on the earlier line?
>

Ack, then here maybe a simple
"
config SYSCTL
        def_bool y
"
 is enough.

> > +   help
> > +     This option shall only be disabled on some dom0less systems,
> > +     to reduce Xen footprint.
>
> This isn't overly useful (but possibly misleading) as long as the prompt isn#t going
> to be visible, yet.
>

Understood, I'll remove the description here and add it in the final when prompt is visible

> > +endmenu
>
> Blank line please ahead of this one.
>

Ack

> Jan
Re: [PATCH v4 04/20] xen: introduce CONFIG_SYSCTL
Posted by Jan Beulich 4 months, 3 weeks ago
On 12.06.2025 06:35, Penny, Zheng wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: Tuesday, June 10, 2025 9:05 PM
>>
>> On 28.05.2025 11:16, Penny Zheng wrote:
>>> From: Stefano Stabellini <stefano.stabellini@amd.com>
>>>
>>> We introduce a new Kconfig CONFIG_SYSCTL, which shall only be disabled
>>> on some dom0less systems or PV shim on x86, to reduce Xen footprint.
>>>
>>> Making SYSCTL without prompt is transient and it will be fixed in the
>>> final
>>
>> Nit: s/fixed/adjusted/ ? It's not a bug, after all.
> 
> Understood.

At the risk of being overly blunt - did you really? You use ...

>>> patch. Also, we will also state unsetting SYSCTL in pvshim_defconfig
>>> to explicitly make it unavailable for PV shim in the final patch.
>>
>> Even without the double "also" this reads odd. But it's also unclear what it has to do
>> here, nor whether what is being said is actually correct.
> 
> Hmmm, How about  "
> The consequences of introducing "CONFIG_SYSCTL=y" in .config file generated from pvshim_defconfig
> is transient and will be also fixed in the final."

... "fixed" again right away, in the same misleading way. Apart from
this - yes, this wording is quite a bit better.

>>> --- a/xen/common/Kconfig
>>> +++ b/xen/common/Kconfig
>>> @@ -579,4 +579,15 @@ config BUDDY_ALLOCATOR_SIZE
>>>       Amount of memory reserved for the buddy allocator to serve Xen heap,
>>>       working alongside the colored one.
>>>
>>> +menu "Supported hypercall interfaces"
>>> +   visible if EXPERT
>>> +
>>> +config SYSCTL
>>> +   bool "Enable sysctl hypercall"
>>> +   def_bool y
>>
>> Why def_bool when you already have bool on the earlier line?
>>
> 
> Ack, then here maybe a simple
> "
> config SYSCTL
>         def_bool y
> "
>  is enough.

Indeed; see my later reply on this same topic.

Jan
Re: [PATCH v4 04/20] xen: introduce CONFIG_SYSCTL
Posted by Jan Beulich 4 months, 3 weeks ago
On 10.06.2025 15:05, Jan Beulich wrote:
> On 28.05.2025 11:16, Penny Zheng wrote:
>> --- a/xen/common/Kconfig
>> +++ b/xen/common/Kconfig
>> @@ -579,4 +579,15 @@ config BUDDY_ALLOCATOR_SIZE
>>  	  Amount of memory reserved for the buddy allocator to serve Xen heap,
>>  	  working alongside the colored one.
>>  
>> +menu "Supported hypercall interfaces"
>> +	visible if EXPERT
>> +
>> +config SYSCTL
>> +	bool "Enable sysctl hypercall"
>> +	def_bool y
> 
> Why def_bool when you already have bool on the earlier line?

It took me until the last patch to properly figure what's wrong here.
Whether "def_bool" or "default", neither makes the prompt invisible.
So you want to keep that line but remove the earlier one, for that to
be added in the final patch.

Jan