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 <jbeul...@suse.com>
>>>> Sent: Monday, August 4, 2025 3:43 PM
>>>> To: Penny, Zheng <penny.zh...@amd.com>
>>>> Cc: Huang, Ray <ray.hu...@amd.com>; Andrew Cooper
>>>> <andrew.coop...@citrix.com>; Roger Pau Monné <roger....@citrix.com>;
>>>> Anthony PERARD <anthony.per...@vates.tech>; Orzel, Michal
>>>> <michal.or...@amd.com>; Julien Grall <jul...@xen.org>; Stefano Stabellini
>>>> <sstabell...@kernel.org>; xen-devel@lists.xenproject.org
>>>> 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 <jbeul...@suse.com>
>>>>> Signed-off-by: Penny Zheng <penny.zh...@amd.com>
>>>>> ---
>>>>> 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
