[Public]
Hi
> -----Original Message-----
> From: Jan Beulich <jbeul...@suse.com>
> Sent: Tuesday, June 10, 2025 9:05 PM
> To: Penny, Zheng <penny.zh...@amd.com>
> Cc: Huang, Ray <ray.hu...@amd.com>; Stabellini, Stefano
> <stefano.stabell...@amd.com>; Andrew Cooper <andrew.coop...@citrix.com>;
> Anthony PERARD <anthony.per...@vates.tech>; Orzel, Michal
> <michal.or...@amd.com>; Julien Grall <jul...@xen.org>; Roger Pau Monné
> <roger....@citrix.com>; Stefano Stabellini <sstabell...@kernel.org>; Sergiy
> Kibrik
> <sergiy_kib...@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.stabell...@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
