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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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.


> >>   Hence I'm not sure this can even be counted as an in-time
> >> submission. Plus it looks to be pretty extensive re-work in some areas.
> > 
> > It doesn't clear based on change log why this patch is sent outside 
> > "Introduce
> > CONFIG_DOMCTL" (2) as it looks the same as in (2) and it was reverted once 
> > with
> > the reason "for breaking the x86 build". (I haven't checked what was 
> > changed so
> > it won't lead to build issue again.)
> 
> Before we can even consider further work in the intended direction, the 
> present
> randconfig build issue wants sorting. Which supposedly this patch alone does.

I think we should take this patch right away.

I also think we should consider "Introduce CONFIG_DOMCTL" for 4.21.