[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