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

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

Jan