Re: [PATCH v6 1/5] x86/boot: create a C bundle for 32 bit boot code and use it

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.10.2024 13:41, Roger Pau Monné wrote:
> On Thu, Oct 17, 2024 at 02:31:19PM +0100, Frediano Ziglio wrote:
>> @@ -25,14 +23,47 @@ $(obj32): XEN_CFLAGS := $(CFLAGS_x86_32) -fpic
>>  $(obj)/%.32.o: $(src)/%.c FORCE
>>      $(call if_changed_rule,cc_o_c)
>>  
>> +orphan-handling-$(call ld-option,--orphan-handling=error) := 
>> --orphan-handling=error
>>  LDFLAGS_DIRECT-$(call ld-option,--warn-rwx-segments) := 
>> --no-warn-rwx-segments
>>  LDFLAGS_DIRECT += $(LDFLAGS_DIRECT-y)
>>  LD32 := $(LD) $(subst x86_64,i386,$(LDFLAGS_DIRECT))
>>  
>> -%.bin: %.lnk
>> -    $(OBJCOPY) -j .text -O binary $< $@
>> +text_gap := 0x010200
>> +text_diff := 0x408020
>> +
>> +$(obj)/build32.base.lds: AFLAGS-y += -DGAP=$(text_gap) 
>> -DTEXT_DIFF=$(text_diff)
>> +$(obj)/build32.offset.lds: AFLAGS-y += -DGAP=$(text_gap) 
>> -DTEXT_DIFF=$(text_diff) -DFINAL
>> +$(obj)/build32.base.lds $(obj)/build32.offset.lds: $(src)/build32.lds.S 
>> FORCE
>> +    $(call if_changed_dep,cpp_lds_S)
>> +
>> +targets += build32.offset.lds build32.base.lds
>> +
>> +# link all 32bit objects together
>> +$(obj)/built-in-32.tmp.o: $(obj32)
>> +    $(LD32) -r -o $@ $^
>> +
>> +# link bundle with a given layout and extract a binary from it
>> +$(obj)/built-in-32.%.bin: $(obj)/build32.%.lds $(obj)/built-in-32.tmp.o
>> +    $(LD32) $(orphan-handling-y) -N -T $< -Map $(@:bin=map) -o $(@:bin=o) 
>> $(filter %.o,$^)
>> +    $(OBJCOPY) -j .text -O binary $(@:bin=o) $@
>> +    rm -f $(@:bin=o)
>> +
>> +quiet_cmd_combine = GEN     $@
>> +cmd_combine = \
>> +    $(PYTHON) $(srctree)/tools/combine_two_binaries.py \
>> +            --gap=$(text_gap) --text-diff=$(text_diff) \
>> +            --script $(obj)/build32.offset.lds \
>> +            --bin1 $(obj)/built-in-32.base.bin \
>> +            --bin2 $(obj)/built-in-32.offset.bin \
>> +            --map $(obj)/built-in-32.offset.map \
>> +            --exports cmdline_parse_early,reloc \
>> +            --output $@
> 
> See xen/Rules.mk, for consistency the indentation should be done with
> spaces when defining variables.  That would also allow to align the
> options.

And ideally also such that the options align with the first program
argument.

>> +
>> +targets += built-in-32.S
>>  
>> -%.lnk: %.32.o $(src)/build32.lds
>> -    $(LD32) -N -T $(filter %.lds,$^) -o $@ $<
>> +# generate final object file combining and checking above binaries
>> +$(obj)/built-in-32.S: $(obj)/built-in-32.base.bin 
>> $(obj)/built-in-32.offset.bin \
>> +            $(srctree)/tools/combine_two_binaries.py FORCE
> 
> Can you indent this using spaces also, so it's on the same column as
> the ':'?

The first $(obj) you mean, I think / hope?

Jan