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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 5 Aug 2025, Jan Beulich wrote:
> On 05.08.2025 05:38, Penny, Zheng wrote:
> > [Public]
> > 
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@xxxxxxxx>
> >> Sent: Monday, August 4, 2025 3:43 PM
> >> To: Penny, Zheng <penny.zheng@xxxxxxx>
> >> Cc: Huang, Ray <Ray.Huang@xxxxxxx>; Andrew Cooper
> >> <andrew.cooper3@xxxxxxxxxx>; Roger Pau Monné <roger.pau@xxxxxxxxxx>;
> >> Anthony PERARD <anthony.perard@xxxxxxxxxx>; Orzel, Michal
> >> <Michal.Orzel@xxxxxxx>; Julien Grall <julien@xxxxxxx>; Stefano Stabellini
> >> <sstabellini@xxxxxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
> >> Subject: Re: [PATCH v1 01/25] xen/x86: move domctl.o out of
> >> PV_SHIM_EXCLUSIVE
> >>
> >> On 03.08.2025 11:47, 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 and paging_domctl
> >>> hypercall-defs 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 CONFIG_DOMCTL.
> >>>
> >>> Fixes: 568f806cba4c ("xen/x86: remove "depends on
> >>> !PV_SHIM_EXCLUSIVE"")
> >>> Reported-by: Jan Beulich <jbeulich@xxxxxxxx>
> >>> Signed-off-by: Penny Zheng <Penny.Zheng@xxxxxxx>
> >>> ---
> >>> v1 -> v2:
> >>> - remove paging_domctl hypercall-defs
> >>
> >> And you've run this through a full round of testing this time, in 
> >> isolation?
> > 
> > This commit shall be committed together with "xen/x86: complement 
> > PG_log_dirty wrapping", (I've added in change log, idk why it didn't get 
> > delivered in the mail list in the last).
> 
> And "committed together" still means the two at least build okay independently
> (i.e. allowing the build-each-commit job to succeed)?
> 
> As to the missing indication thereof in the submission: Patch 01 has a revlog,
> so if anything was missing there you must have added it some other way. But
> the cover letter is lacking that information as well. (As indicated earlier,
> to increase the chance of such a remark actually being noticed, it's best put
> in both places.)
> 
> > As PG_log_dirty is disabled on PV mode, paging_domctl() will still have 
> > "undefined reference" on PV mode, which gets fixed in "xen/x86: complement 
> > PG_log_dirty wrapping".  I thought it better sits there.
> > If it doesn't comply with the commit policy, I'll move according fix here.
> 
> Let me post a pair of patches dealing with part of the problem, in an imo
> (longer term) more desirable way.

With this patch https://marc.info/?l=xen-devel&m=175438069103017
committed, can we go ahead with this patch, to resolve the outstanding
build problem?

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>