Re: [PATCH v4 2/6] x86/boot: create a C bundle for 32 bit boot code and use it

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.10.2024 18:32, Frediano Ziglio wrote:
> On Mon, Oct 14, 2024 at 4:31 PM Anthony PERARD
> <anthony.perard@xxxxxxxxxx> wrote:
>>
>> On Mon, Oct 14, 2024 at 09:53:28AM +0100, Frediano Ziglio wrote:
>>> diff --git a/xen/arch/x86/boot/Makefile b/xen/arch/x86/boot/Makefile
>>> index 1199291d2b..23ad274c89 100644
>>> --- a/xen/arch/x86/boot/Makefile
>>> +++ b/xen/arch/x86/boot/Makefile
>>> @@ -1,4 +1,5 @@
>>>  obj-bin-y += head.o
>>> +obj-bin-y += built_in_32.o
>>
>> I don't quite like that this new object name is "built_in_32.o",
>> It's really closed to "built_in.*" which is used by Rules.mk to collect
>> all objects in a subdirectory. I don't really have a better suggestion at
>> the moment.
>>
> 
> It was cbundle.o before, but people preferred built_in_32.o.
> It's a collection of object files like built_in.o so it does not seem
> so bad to me.
> But seen other replies, some other people prefer "bundle".

Well, I for one don't really _prefer_ bundle. I merely see it as a possible
option to address Anthony's name ambiguity concern.

>>> +     $(LD32) $(orphan-handling-y) -N -T $< -Map 
>>> $(obj)/built_in_32.$(*F).map -o $(obj)/built_in_32.$(*F).o 
>>> $(obj)/built_in_32.tmp.o
>>
>> I think this wants to be: -T $(filter %.lds,$^) -Map $(patsubst 
>> %.bin,%.map,$@) -o $(patsubst %.bin,%.o,$@) $(filter %.o,$^)
>>
>> :-(, maybe that's lots of $(patsubst,), not sure which is better between
>> $(patsubst,) and using the stem $*.
>>
> 
> Not strong about stem or patsubst.
> The 2 filters seem good, they suggest lds for the script and objects
> for the input, which makes sense.
> 
>> Also, if something tries to use built_in_32.tmp.bin, we have a rule that
>> remove it's prerequisite.
>>
>> BTW, everything is kind of temporary in a build system, beside the few
>> files that we want to install on a machine, so having a target named
>> with "*tmp*" isn't great. But having a rule that create "*tmp*" file but
>> remove them before the end of its recipe is fine (or those *tmp* aren't
>> use outside of this recipe).
>>
> 
> Mumble... yes, I think the XX.tmp.o was a temporary internal rule file.
> So we still don't agree on one name, and now we want to find also
> another, tricky.
> More or less if it can help, one is a 32 bit object file that bundle
> together multiple 32 bits object files while the other is the
> conversion of the 32 bits bundle file to 64 bits.
> XXX_32.o and XXX_32as64.o ??

Whatever the eventual name (I don't care all that much), just one request:
Dashes instead of underscores please.

Jan