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

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Wed, Oct 16, 2024 at 04:05:00PM +0100, Frediano Ziglio wrote:
> On Wed, Oct 16, 2024 at 12:25 PM Anthony PERARD > <anthony.perard@xxxxxxxxxx> 
> wrote:
> > On Wed, Oct 16, 2024 at 09:33:32AM +0100, Frediano Ziglio wrote:
> > > On Tue, Oct 15, 2024 at 2:51 PM Anthony PERARD 
> > > <anthony.perard@xxxxxxxxxx> wrote:
> > > > I can think of one example where $(if_changed,) is going to really help,
> > > > by looking at this command line:
> > > >     One does update the .c file to add a function that they like to
> > > >     export, run `make`, realize they forgot to update the makefile so
> > > >     update it, run `make`, it's still doesn't work...
> > > >     Maybe run `make clean; make`, or something else...
> > > >
> > > > So, could you use $(if_changed,) ?
> > > > Probably:
> > > > quiet_cmd_combine = GEN     $@
> > > > cmd_combine = $(PYTHON) ...
> > > > $(obj)/built_in_32.S: $(obj)/built_in_32.other.bin 
> > > > $(obj)/built_in_32.final.bin FORCE
> > > >     $(call if_changes,combine)
> > > > targets += built_in_32.S
> > > >
> > > > GEN, for generate, or it could be PY instead, because python script can
> > > > be slow to compile which could explain why the build system output is
> > > > making a pause on this target (on slow machines that is). Or it could be
> > > > COMBINE, or something else, but it's not really necessary to explain,
> > > > the target name is often enough to figure out what's happening, when
> > > > needed.
> > > >
> > >
> > > It just looks more complicated to me.
> >
> > I'm sorry if writing makefile is complicated. GNU make doesn't help with
> > writing build system that work well, especially when doing incremental
> > builds. So we need to use more complicated construction, especially for
> > a complex project like Xen.
> >
>
> It was more a balance consideration. Considering the cases that seem
> to solve (and your case did not much apply) I don't feel that worth.
> Also, dependency to Makefile would solve without additional macros and
> indirection. Do you mind posting a full working change?

Sure, here it is (I notice I've misspell the macro name in what I've written
earlier):

quiet_cmd_combine = GEN     $@
cmd_combine = \
    $(PYTHON) $(srctree)/tools/combine_two_binaries.py \
        --script $(obj)/build32.final.lds \
        --bin1 $(obj)/built_in_32.other.bin \
        --bin2 $(obj)/built_in_32.final.bin \
        --map $(obj)/built_in_32.final.map \
        --exports cmdline_parse_early,reloc \
        --output $@

targets += built_in_32.S

$(obj)/built_in_32.S: $(obj)/built_in_32.other.bin $(obj)/built_in_32.final.bin 
\
            $(srctree)/tools/combine_two_binaries.py FORCE
        $(call if_changed,combine)

Cheers,

--

Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech