Re: [PATCH 1/2] x86: move arch_generic_hweightl() to arch-specific library

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.11.2025 15:17, Andrew Cooper wrote:
> On 26/11/2025 2:09 pm, Jan Beulich wrote:
>> On 26.11.2025 15:05, Andrew Cooper wrote:
>>> On 26/11/2025 1:58 pm, Jan Beulich wrote:
>>>> On 26.11.2025 14:51, Andrew Cooper wrote:
>>>>> On 26/11/2025 1:24 pm, Jan Beulich wrote:
>>>>>> diff --git a/xen/arch/x86/arch.mk b/xen/arch/x86/arch.mk
>>>>>> index 16368a498bb7..a0ee050c931b 100644
>>>>>> --- a/xen/arch/x86/arch.mk
>>>>>> +++ b/xen/arch/x86/arch.mk
>>>>>> @@ -3,6 +3,8 @@
>>>>>>  
>>>>>>  export XEN_IMG_OFFSET := 0x200000
>>>>>>  
>>>>>> +ALL_LIBS-y += arch/x86/lib/lib.a
>>>>>> +
>>>>> Oh, I'd realised it was this easy, I'd have done so straight away when
>>>>> adding x86's custom arch_generic_hweightl().
>>>>>
>>>>> I assumed it was going to be more complicated getting the order of the
>>>>> arch specific lib correct with the generic lib.
>>>>>
>>>>> More concretely.  Given an x86 lib, we should move things like
>>>>> arch/x86/memcpy.S to it.
>>>>>
>>>>> Therefore, when we have common/lib.a and arch/lib.a, do we guarantee to
>>>>> have arch/lib.a with higher precedence so for matching functions the
>>>>> arch specific one guarantees to be taken?
>>>> Not with the change above, it would need to become
>>>>
>>>> ALL_LIBS-y := arch/x86/lib/lib.a $(ALL_LIBS-y)
>>>>
>>>> to achieve that, requiring that ALL_LIBS-y won't change into a 
>>>> lazy-expansion
>>>> variable. If that's okay (please confirm), I can adjust the patch.
>>>>
>>>> Things would be yet easier if every arch had a lib/lib.a, as then in
>>>> xen/Makefile we could simply have
>>>>
>>>> ALL_LIBS-y                := arch/$(SRCARCH)/lib/lib.a
>>>> ALL_LIBS-y                += lib/lib.a
>>>>
>>>> Alternatively we could move the setting of ALL_LIBS-y in xen/Makefile to
>>>> after the arch/$(SRCARCH)/arch.mk inclusion. I'd be a little wary of that,
>>>> though, as it would then be different from ALL_OBJS-y.
>>> I think this would be better handled by common code.
>>>
>>> Arches are going to want a lib.a eventually.  ARM even has
>>> arch/arm/arm{32,64}/lib/ but like x86 they're just simple obj-y += at
>>> the moment.
>>>
>>> However, arches shouldn't be forced to make an empty one simply to build.
>>>
>>> Does this work:
>>>
>>> ALL_LIBS-y                := $(wildcard arch/$(SRCARCH)/lib/lib.a)
>>> ALL_LIBS-y                += lib/lib.a
>>>
>>> ?  If so, I think it's the nicest option.
>> I had thought of doing it this way initially, but on a fresh build
>> arch/$(SRCARCH)/lib/lib.a wouldn't be there when the Makefile is read in.
>> Whether switching ALL_LIBS-y to be a lazy-expansion variable would work
>> I didn't try; I'd prefer not to change the kind of variable that it is.
> 
> Hmm.  What about:
> 
>     $(filter arch%,$(ALL_LIBS-y)) $(filter-out arch%,$(ALL_LIBS-y))
> 
> in the link, at which point it doesn't matter about the exact order in
> ALL_LIBS-y?

Hmm, we could apparently do something like that, but looking at how ALL_LIBS
(note: not ALL_LIBS-y) is used, I wonder if I didn't make us depend on
unspecified behavior in f301f9a9e84f ("lib: collect library files in an
archive"): I don't think it is well-defined whether undefined symbols are
resolved by pulling in archive members when doing a relocatable link. (Of
course this is only a tangential aspect here.)

Another question is whether it is really always going to be the case that
arch-specific libraries want to take precedence over common ones.

To summarize, I think by this point I'd still prefer the

ALL_LIBS-y := arch/x86/lib/lib.a $(ALL_LIBS-y)

approach mentioned earlier.

Jan