Re: [kvm-unit-tests PATCH v2 5/8] s390x: use C pre-processor for linker script generation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Nina Schoetterl-Glausch <nsg@xxxxxxxxxxxxx> writes:

> On Wed, 2023-02-01 at 11:00 +0100, Marc Hartmayer wrote:
>> Nina Schoetterl-Glausch <nsg@xxxxxxxxxxxxx> writes:
>> 
>> > On Thu, 2023-01-19 at 12:40 +0100, Marc Hartmayer wrote:
>> > > Use the C pre-processor for the linker script generation. For example,
>> > > this enables us the use of constants in the "linker scripts" `*.lds.S`.
>> > > 
>> > > Signed-off-by: Marc Hartmayer <mhartmay@xxxxxxxxxxxxx>
>> > > ---
>> > >  .gitignore                                  | 1 +
>> > >  s390x/Makefile                              | 6 ++++--
>> > >  s390x/{flat.lds => flat.lds.S}              | 0
>> > >  s390x/snippets/asm/{flat.lds => flat.lds.S} | 0
>> > >  s390x/snippets/c/{flat.lds => flat.lds.S}   | 0
>> > >  5 files changed, 5 insertions(+), 2 deletions(-)
>> > >  rename s390x/{flat.lds => flat.lds.S} (100%)
>> > >  rename s390x/snippets/asm/{flat.lds => flat.lds.S} (100%)
>> > >  rename s390x/snippets/c/{flat.lds => flat.lds.S} (100%)
>> > > 
>> > > diff --git a/.gitignore b/.gitignore
>> > > index 601822d67325..29f352c5ceb6 100644
>> > > --- a/.gitignore
>> > > +++ b/.gitignore
>> > > @@ -31,3 +31,4 @@ cscope.*
>> > >  /s390x/comm.key
>> > >  /s390x/snippets/*/*.hdr
>> > >  /s390x/snippets/*/*.*obj
>> > > +/s390x/**/*.lds
>> > > diff --git a/s390x/Makefile b/s390x/Makefile
>> > > index 8719f0c837cf..44ccca8102d6 100644
>> > > --- a/s390x/Makefile
>> > > +++ b/s390x/Makefile
>> > > @@ -76,7 +76,7 @@ CFLAGS += -fno-delete-null-pointer-checks
>> > >  LDFLAGS += -nostdlib -Wl,--build-id=none
>> > >  
>> > >  # We want to keep intermediate files
>> > > -.PRECIOUS: %.o
>> > > +.PRECIOUS: %.o %.lds
>> > >  
>> > >  asm-offsets = lib/$(ARCH)/asm-offsets.h
>> > >  include $(SRCDIR)/scripts/asm-offsets.mak
>> > > @@ -159,6 +159,8 @@ $(SNIPPET_DIR)/c/%.gbin: $(SNIPPET_DIR)/c/%.o $(snippet_lib) $(FLATLIBS) $(SNIPP
>> > >  %.hdr.obj: %.hdr
>> > >  	$(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $< $@
>> > >  
>> > > +%.lds: %.lds.S
>> > > +	$(CPP) $(autodepend-flags) $(CPPFLAGS) -P -C -o $@ $<
>> > 
>> > Where is CPP defined?
>> > Do you need the $(autodepend-flags)? It generates a rule with target flat.lds.o.
>>                                                                        ^^^^^^^^^^
>> 
>> Where does it generate a new Makefile rule? It generates the dependency
>> file used for the dependency tracking with the name: .flat.lds.S.d (or
>> similar)
>
> Yes and that file contains the rule.
> cat s390x/.flat.d
> flat.lds.o: /foo/bar/s390x/flat.lds.S \
>  /foo/bar/lib/asm/asm-offsets.h \
>  /foo/bar/lib/generated/asm-offsets.h

Yep, right… thanks for pointing it out! So I could either don’t use
$(autodepend-flags) or adapt it:

I could change autodepend-flags to:

--- i/Makefile
+++ w/Makefile
@@ -94,7 +94,7 @@ CFLAGS += $(wmissing_parameter_type)
 CFLAGS += $(wold_style_declaration)
 CFLAGS += -Woverride-init -Wmissing-prototypes -Wstrict-prototypes
 
-autodepend-flags = -MMD -MF $(dir $*).$(notdir $*).d
+autodepend-flags = -MMD -MF $(dir $*).$(notdir $*).d -MT $@

Shouldn’t break anything (we have to double check of course) and it
results in a Makefile rule like this for the linker scripts:

s390x/snippets/c/flat.lds: s390x/snippets/c/flat.lds.S \
 /foo/kvm-unit-tests/lib/asm/asm-offsets.h \
 /foo/kvm-unit-tests/lib/generated/asm-offsets.h


>
>> 
>> > I don't think that would be used anywhere.
>> 
>> It’s used for the dependency tracking. See line
>> 
>> $KUT/Makefile:122
>> 
>> -include */.*.d */*/.*.d
>
> Indeed, but the target flat.lds.o isn't used anywhere, is it?

No, it’s not and it doesn’t make any sense.

> So if flat.lds.S included some other header and that changed,
> flat.lds wouldn't be rebuild.
> So you would either need to generate a rule with flat.lds as target
> or make it depend on flat.lds.o somehow.
>
>> 
>> > In the next patch you add $(asm-offsets) as a prerequisite, if the generated rule would
>> > be effective, you wouldn't need that, would you?
>> 
>> I thought, $(asm-offsets) is used to make sure that the file
>> <asm/asm-offsets.h> is generated.
>
> I was asking if this is necessary, since s390x/.flat.d contains the asm-offset headers
> as prerequisites. So if the dependency file is included, the headers will be build,
> but I guess that this is circular, in order to generate the dependencies,
> the asm-offset headers must already have been built.

Hmm - I’ll check.

Thanks for the feedback!

>> 
>> […snip]
>> 
>
-- 
Kind regards / Beste Grüße
   Marc Hartmayer

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen 
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux