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?)
> 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?
Thomas
