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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.08.2025 01:14, Stefano Stabellini wrote:
> 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?

It first needs re-basing, I expect. There were also other anomalies, like a
requirement to commit together with another patch.

Jan