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 09:33:32AM +0100, Frediano Ziglio wrote:
> On Tue, Oct 15, 2024 at 2:51 PM Anthony PERARD <anthony.perard@xxxxxxxxxx> 
> wrote:
> > On Mon, Oct 14, 2024 at 05:32:26PM +0100, 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:

> > BTW, do we need the rules that generate the input of this rules
> > (built_in_32.tmp.o that is), or can this one takes all 32bit objects as
> > input?
> >
>
> Better not to do it In some conditions it can generate slightly
> different results (like different object alignments) making the
> algorithm fail.

Ok. Thanks for the explanation.

> > > > > +# generate final object file combining and checking above binaries
> > > > > +$(obj)/built_in_32.S: $(obj)/built_in_32.other.bin 
> > > > > $(obj)/built_in_32.final.bin
> > > >
> > > > So, "other" isn't part of "final", I don't really know what those two
> > > > things contains so naming wise I can't suggest anything useful.
> >
> > Instead of "other", is "control" (like in science experiment where you
> > have a control group), or "offseted" (which seems to be how this second
> > binary is constructed) would be better names for this *.bin? It seems
> > the script take both input and play the game of the 7 differences, to
> > find clues about the location of some symbols, right?.
> >
>
> I don't know the game and I think people not familiar with it won't
> find new names more readable but less.

Sorry, the "game" as nothing to do with the name I've proposed. I was
just asking if the script take both *.bin and was looking for
differences.

(The game of 7 differences is simple: there's two similar pictures and
you just look for the 7 differences between them, that's it.)

> Not saying that current names are good, they just need to be located
> at different addresses with some "magic" in the middle.

Well to me "other" evoke a binary that contains functions that are not
in "final", but instead they both contain the sames functions with
slight variation of placement in the file (with added offset, gap), as I
understand. But if you don't like my proposal, so be it.

> > 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.

Cheers,

--

Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech