[Public]
> -----Original Message-----
> From: Jan Beulich <jbeul...@suse.com>
> Sent: Friday, July 25, 2025 1:38 PM
> To: Stefano Stabellini <sstabell...@kernel.org>
> Cc: Penny, Zheng <penny.zh...@amd.com>; Huang, Ray
> <ray.hu...@amd.com>; Julien Grall <jul...@xen.org>; Bertrand Marquis
> <bertrand.marq...@arm.com>; Orzel, Michal <michal.or...@amd.com>;
> Volodymyr Babchuk <volodymyr_babc...@epam.com>; Andrew Cooper
> <andrew.coop...@citrix.com>; Anthony PERARD <anthony.per...@vates.tech>;
> Roger Pau Monné <roger....@citrix.com>; Daniel P. Smith
> <dpsm...@apertussolutions.com>; xen-devel@lists.xenproject.org
> Subject: Re: [PATCH v1] xen: move getdomaininfo() to domain.c
>
> On 25.07.2025 03:21, Stefano Stabellini wrote:
> > On Thu, 24 Jul 2025, Jan Beulich wrote:
> >> On 23.07.2025 22:30, Stefano Stabellini wrote:
> >>> On Wed, 23 Jul 2025, Jan Beulich wrote:
> >>>> On 23.07.2025 02:46, Stefano Stabellini wrote:
> >>>>> On Tue, 22 Jul 2025, Jan Beulich wrote:
> >>>>>> On 22.07.2025 07:04, Penny Zheng wrote:
> >>>>>>> Function getdomaininfo() is not only invoked by domctl-op, but
> >>>>>>> also sysctl-op, so it shall better live in domain.c, rather than
> >>>>>>> domctl.c. Which is also applied for arch_get_domain_info().
> >>>>>>> Style corrections shall be applied at the same time while moving
> >>>>>>> these functions, such as converting u64 to uint64_t.
> >>>>>>>
> >>>>>>> The movement could also fix CI error of a randconfig picking
> >>>>>>> both SYSCTL=y and PV_SHIM_EXCLUSIVE=y results in sysctl.c being
> >>>>>>> built, but domctl.c not being built, which leaves getdomaininfo()
> undefined, causing linking to fail.
> >>>>>>>
> >>>>>>> Fixes: 34317c508294 ("xen/sysctl: wrap around sysctl hypercall")
> >>>>>>> Reported-by: Jan Beulich <jbeul...@suse.com>
> >>>>>>> Signed-off-by: Penny Zheng <penny.zh...@amd.com>
> >>>>>>
> >>>>>> I'm not convinced of this approach. In the longer run this would
> >>>>>> mean wrapping everything you move in "#if defined(CONFIG_SYSCTL)
> >>>>>> || defined(CONFIG_DOMCTL)", which I consider undesirable. Without
> >>>>>> DOMCTL, the usefulness of XEN_SYSCTL_getdomaininfolist is at
> >>>>>> least questionable. Therefore adding more isolated "#ifdef
> >>>>>> CONFIG_DOMCTL" just there may be an option. Similarly, as
> >>>>>> mentioned on the other thread, having SYSCTL depend on DOMCTL is an
> approach which imo wants at least considering. And there surely are further
> options.
> >>>>>>
> >>>>>> As indicated elsewhere, my preference goes towards reverting the
> >>>>>> final one or two patches of that series. They can be re-applied
> >>>>>> once the dependencies were properly sorted, which may (as per
> >>>>>> above) involve properly introducing a DOMCTL Kconfig setting first.
> >>>>>
> >>>>> I don't think this is a good idea.
> >>>>
> >>>> And implicitly you say that what I put under question in the first
> >>>> paragraph is a good way forward?
> >>>
> >>> I think it is OK.
> >>>
> >>> I also think "having SYSCTL depend on DOMCTL" is certainly worth
> >>> thinking about. In terms of privilege and potential for interference
> >>> with other domains sysctl and domctl don't seem different so it is
> >>> unlikely one would want to disable one but not the other.
> >>>
> >>> Another idea is to have a single kconfig for both SYSCTL and DOMCTL:
> >>> we don't necessarily need to offer individual kconfig for every feature.
> >>> From a safety point of view, we want to disable them both.
> >>
> >> Then again (and going against the thought of making SYSCTL depend on
> >> DOMCTL) there may be a desire to query / alter certain properties of
> >> the system as a whole, without also having that need for individual
> >> domains. But yes, covering both with a single control also is an option to
> consider.
> >
> > If making SYSCTL depend on DOMCTL and/or a single kconfig for both
> > SYSCTL and DOMCTL are both way forward, then we can take this patch as
> > is?
>
> In both of the named cases this patch simply wouldn't be needed. Once the
> conversion work was done, that is. And to be frank, I'm not happy to see the
> function move out and then back in.
>
I think the new patch "move domctl.o out of PV_SHIM_EXCLUSIVE " could also fix
getdomaininfo() linking error
> Jan
