Re: [PATCH] xen/x86: move domctl.o out of PV_SHIM_EXCLUSIVE

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 25 Aug 2025, Jan Beulich wrote:
> On 19.08.2025 01:51, Stefano Stabellini wrote:
> > On Mon, 18 Aug 2025, Jan Beulich wrote:
> >> On 18.08.2025 15:28, Oleksii Kurochko wrote:
> >>> On 8/18/25 10:31 AM, Jan Beulich wrote:
> >>>> On 15.08.2025 12:27, Penny Zheng wrote:
> >>>>> In order to fix CI error of a randconfig picking both 
> >>>>> PV_SHIM_EXCLUSIVE=y and
> >>>>> HVM=y results in hvm.c being built, but domctl.c not being built, which 
> >>>>> leaves
> >>>>> a few functions, like domctl_lock_acquire/release() undefined, causing 
> >>>>> linking
> >>>>> to fail.
> >>>>> To fix that, we intend to move domctl.o out of the PV_SHIM_EXCLUSIVE 
> >>>>> Makefile
> >>>>> /hypercall-defs section, with this adjustment, we also need to release
> >>>>> redundant vnuma_destroy() stub definition from PV_SHIM_EXCLUSIVE 
> >>>>> guardian,
> >>>>> to not break compilation
> >>>>> Above change will leave dead code in the shim binary temporarily and 
> >>>>> will be
> >>>>> fixed with the introduction of domctl-op wrapping.
> >>>> Well, "temporarily" is now getting interesting. While v1 of "Introduce
> >>>> CONFIG_DOMCTL" was submitted in time to still be eligible for taking into
> >>>> 4.21, that - as indicated elsewhere - is moving us further in an unwanted
> >>>> direction.
> >>>
> >>> Do you mean that specifically this patch or the whole patch series is 
> >>> moving us
> >>> in unwanted direction? (1)
> >>
> >> That series. We said we don't want individual CONFIG_SYSCTL, 
> >> CONFIG_DOMCTL, etc.
> >> Instead a single umbrella option wants introducing. Which means there 
> >> series
> >> doesn't need re-doing from scratch, but it may end up being a significant 
> >> re-
> >> work, especially considering that CONFIG_SYSCTL is already in the codebase 
> >> and
> >> hence now also needs replacing.
> > 
> > I would not characterize this series as "moving us in an unwanted
> > direction". Yes, it introduces a separate CONFIG_DOMCTL, which we
> > agreed we do not want. However, simplifying it to reuse a single
> > CONFIG is a minor improvement that can be addressed in v2. The main
> > challenge in this series is adding the #ifdef in the appropriate
> > places, and using a single CONFIG for domctl and sysctl would
> > actually help.
> 
> Well, when are we going to see a v2 then which does this? Of the three
> options I mentioned in the earlier reply, Oleksii favored the revert
> path, leaving open the get-everything-in one. For the latter, however,
> we need to see relatively constant progress now, or else time will run
> out. Whether to commit the patch here really depends on what route we
> settle on for 4.21.

While I share Jan's view that now is a good time to make progress on
"Introduce CONFIG_DOMCTL", and I also believe that this is the best
course of action, I would like to share a few thoughts on the other
alternatives.

My understanding is that the PV_SHIM_EXCLUSIVE option is no longer
widely used, and a small size increase would not compromise its
functionality and should be tolerable.

In general, we are slow at getting larger series reviewed and committed,
and choosing to revert rather than accept fixes tends to make us even
slower, which is undesirable.