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?
