On 12/11/20 1:18 PM, Thomas Huth wrote:
> On 11/12/2020 11.00, Janosch Frank wrote:
>> I've added too much to cstart64.S which is not start related
>> already. Now that I want to add even more code it's time to split
>> cstart64.S. lib.S has functions that are used in tests. macros.S
>> contains macros which are used in cstart64.S and lib.S
>>
>> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
>> ---
>> s390x/Makefile | 8 +--
>> s390x/{ => asm}/cstart64.S | 119 ++-----------------------------------
>> s390x/asm/lib.S | 65 ++++++++++++++++++++
>> s390x/asm/macros.S | 77 ++++++++++++++++++++++++
>> 4 files changed, 150 insertions(+), 119 deletions(-)
>> rename s390x/{ => asm}/cstart64.S (50%)
>> create mode 100644 s390x/asm/lib.S
>> create mode 100644 s390x/asm/macros.S
>>
>> diff --git a/s390x/Makefile b/s390x/Makefile
>> index b079a26..fb62e87 100644
>> --- a/s390x/Makefile
>> +++ b/s390x/Makefile
>> @@ -66,10 +66,10 @@ cflatobjs += lib/s390x/css_lib.o
>>
>> OBJDIRS += lib/s390x
>>
>> -cstart.o = $(TEST_DIR)/cstart64.o
>> +asmlib = $(TEST_DIR)/asm/cstart64.o $(TEST_DIR)/asm/lib.o
>>
>> FLATLIBS = $(libcflat)
>> -%.elf: %.o $(FLATLIBS) $(SRCDIR)/s390x/flat.lds $(cstart.o)
>> +%.elf: %.o $(FLATLIBS) $(SRCDIR)/s390x/flat.lds $(asmlib)
>> $(CC) $(CFLAGS) -c -o $(@:.elf=.aux.o) \
>> $(SRCDIR)/lib/auxinfo.c -DPROGNAME=\"$@\"
>> $(CC) $(LDFLAGS) -o $@ -T $(SRCDIR)/s390x/flat.lds \
>> @@ -87,7 +87,7 @@ FLATLIBS = $(libcflat)
>> $(GENPROTIMG) --host-key-document $(HOST_KEY_DOCUMENT) --no-verify --image $< -o $@
>>
>> arch_clean: asm_offsets_clean
>> - $(RM) $(TEST_DIR)/*.{o,elf,bin} $(TEST_DIR)/.*.d lib/s390x/.*.d
>> + $(RM) $(TEST_DIR)/*.{o,elf,bin} $(TEST_DIR)/.*.d lib/s390x/.*.d $(TEST_DIR)/asm/*.{o,elf,bin} $(TEST_DIR)/asm/.*.d
>>
>> generated-files = $(asm-offsets)
>> -$(tests:.elf=.o) $(cstart.o) $(cflatobjs): $(generated-files)
>> +$(tests:.elf=.o) $(asmlib) $(cflatobjs): $(generated-files)
>
> Did you check this with both, in-tree and out-of-tree builds?
> (I wonder whether that new asm directory needs some special handling for
> out-of-tree builds?)
I'm not a big fan of out-of-tree builds, so I didn't check.
To get those builds working we would need to create the asm directory in
the $testdir
>
>> diff --git a/s390x/asm/lib.S b/s390x/asm/lib.S
>> new file mode 100644
>> index 0000000..4d78ec6
>> --- /dev/null
>> +++ b/s390x/asm/lib.S
>> @@ -0,0 +1,65 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * s390x assembly library
>> + *
>> + * Copyright (c) 2019 IBM Corp.
>> + *
>> + * Authors:
>> + * Janosch Frank <frankja@xxxxxxxxxxxxx>
>> + */
>> +#include <asm/asm-offsets.h>
>> +#include <asm/sigp.h>
>> +
>> +#include "macros.S"
>> +
>> +/*
>> + * load_reset calling convention:
>> + * %r2 subcode (0 or 1)
>> + */
>> +.globl diag308_load_reset
>> +diag308_load_reset:
>
> Thinking about this twice ... this function is only used by s390x/diag308.c,
> so it's not really a library function, but rather part of a single test ...
> I think it would be cleaner to put it into a separate file instead, what do
> you think?
I don't really want to split this any further.
Moving the asm files into an own directory already improves readability
a lot for me and I don't need more files if they aren't absolutely
necessary.
>
> Thomas
>
>
