Linux-kselftest Archive on lore.kernel.org
 help / color / Atom feed
From: Dave Martin <Dave.Martin@arm.com>
To: Cristian Marussi <cristian.marussi@arm.com>
Cc: linux-kselftest@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, shuah@kernel.org,
	amit.kachhap@arm.com, andreyknvl@google.com
Subject: Re: [PATCH v6 01/11] kselftest: arm64: extend toplevel skeleton Makefile
Date: Tue, 17 Sep 2019 17:05:34 +0100
Message-ID: <20190917160534.GK27757@arm.com> (raw)
In-Reply-To: <20190910123111.33478-2-cristian.marussi@arm.com>

On Tue, Sep 10, 2019 at 01:31:01pm +0100, Cristian Marussi wrote:
> Modify KSFT arm64 toplevel Makefile to maintain arm64 kselftests organized
> by subsystem, keeping them into distinct subdirectories under arm64 custom
> KSFT directory: tools/testing/selftests/arm64/
> 
> Add to such toplevel Makefile a mechanism to guess the effective location
> of Kernel headers as installed by KSFT framework.
> 
> Fit existing arm64 tags kselftest into this new schema moving them into
> their own subdirectory (arm64/tags).
> 
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
> Based on:
> commit 9ce1263033cd ("selftests, arm64: add a selftest for passing
> 		     tagged pointers to kernel")
> ---
> v5 --> v6
> - using realpath to avoid passing down relative paths
> - fix commit msg & Copyright
> - removed unneded Makefile export
> - added SUBTARGETS specification, to allow building specific only some
>   arm64 test subsystems
> v4 --> v5
> - rebased on arm64/for-next/core
> - merged this patch with KSFT arm64 tags patch, while moving the latter
>   into its own subdir
> - moved kernel header includes search mechanism from KSFT arm64
>   SIGNAL Makefile
> - export proper top_srcdir ENV for lib.mk
> v3 --> v4
> - comment reword
> - simplified documentation in README
> - dropped README about standalone
> ---
>  tools/testing/selftests/Makefile              |  1 +
>  tools/testing/selftests/arm64/Makefile        | 63 +++++++++++++++++--
>  tools/testing/selftests/arm64/README          | 25 ++++++++
>  tools/testing/selftests/arm64/tags/Makefile   |  6 ++
>  .../arm64/{ => tags}/run_tags_test.sh         |  0
>  .../selftests/arm64/{ => tags}/tags_test.c    |  0
>  6 files changed, 91 insertions(+), 4 deletions(-)
>  create mode 100644 tools/testing/selftests/arm64/README
>  create mode 100644 tools/testing/selftests/arm64/tags/Makefile
>  rename tools/testing/selftests/arm64/{ => tags}/run_tags_test.sh (100%)
>  rename tools/testing/selftests/arm64/{ => tags}/tags_test.c (100%)
> 
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index 25b43a8c2b15..1722dae9381a 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0
>  TARGETS = android
> +TARGETS += arm64
>  TARGETS += bpf
>  TARGETS += breakpoints
>  TARGETS += capabilities
> diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile
> index a61b2e743e99..cbb2a5a9e3fc 100644
> --- a/tools/testing/selftests/arm64/Makefile
> +++ b/tools/testing/selftests/arm64/Makefile
> @@ -1,11 +1,66 @@
>  # SPDX-License-Identifier: GPL-2.0
>  
> -# ARCH can be overridden by the user for cross compiling
> +# When ARCH not overridden for crosscompiling, lookup machine
>  ARCH ?= $(shell uname -m 2>/dev/null || echo not)
>  
>  ifneq (,$(filter $(ARCH),aarch64 arm64))
> -TEST_GEN_PROGS := tags_test
> -TEST_PROGS := run_tags_test.sh
> +SUBTARGETS ?= tags
> +else
> +SUBTARGETS :=
>  endif
>  
> -include ../lib.mk
> +CFLAGS := -Wall -O2 -g
> +
> +# A proper top_srcdir is needed by KSFT(lib.mk)
> +top_srcdir = $(realpath ../../../../)
> +
> +# Additional include paths needed by kselftest.h and local headers
> +CFLAGS += -I$(top_srcdir)/tools/testing/selftests/
> +
> +# Guessing where the Kernel headers could have been installed
> +# depending on ENV config
> +ifeq ($(KBUILD_OUTPUT),)
> +khdr_dir = $(top_srcdir)/usr/include
> +else
> +# the KSFT preferred location when KBUILD_OUTPUT is set
> +khdr_dir = $(KBUILD_OUTPUT)/kselftest/usr/include
> +endif

I still tend to think that for now we should just do what all the other
tests do.

Most tests use

	CFLAGS += -I../../../../usr/include/

in their Makefiles.

For us, the test Makefiles are nested one level deeper, so I guess
we would put

	CFLAGS += -I../../../../../usr/include/

in each.


This will break in some cases, but only in the same cases where
kselftest is already broken.

Ideally we would fix this globally, but can that instead be done
independently of this series?

Fixing only arm64, by pasting some arbitrary logic from
selftests/Makefile doesn't seem like a future-proof approach.


Or did I miss something?

> +
> +CFLAGS += -I$(khdr_dir)
> +
> +export CFLAGS
> +export top_srcdir
> +
> +all:
> +	@for DIR in $(SUBTARGETS); do				\
> +		BUILD_TARGET=$(OUTPUT)/$$DIR;			\
> +		mkdir -p $$BUILD_TARGET;			\
> +		make OUTPUT=$$BUILD_TARGET -C $$DIR $@;		\
> +	done
> +
> +install: all
> +	@for DIR in $(SUBTARGETS); do				\
> +		BUILD_TARGET=$(OUTPUT)/$$DIR;			\
> +		make OUTPUT=$$BUILD_TARGET -C $$DIR $@;		\
> +	done
> +
> +run_tests: all
> +	@for DIR in $(SUBTARGETS); do				\
> +		BUILD_TARGET=$(OUTPUT)/$$DIR;			\
> +		make OUTPUT=$$BUILD_TARGET -C $$DIR $@;		\
> +	done
> +
> +# Avoid any output on non arm64 on emit_tests

This comment can be dropped: the whole file does nothing for
non-arm64, and it achieves it in the same way as other arch-specific
Makefiles, so it's odd to have the comment here specifically (?)


With or without the above changes, I'm happy to give

Reviewed-by: Dave Martin <Dave.Martin@arm.com>

(but Shuah or someone will need to give a view on how this integrates
with kselftest overall).

[...]

Cheers
---Dave

  parent reply index

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-10 12:31 [PATCH v6 00/11] Add arm64/signal initial kselftest support Cristian Marussi
2019-09-10 12:31 ` [PATCH v6 01/11] kselftest: arm64: extend toplevel skeleton Makefile Cristian Marussi
2019-09-17 13:42   ` Anders Roxell
2019-09-17 15:17     ` Cristian Marussi
2019-09-17 15:29       ` shuah
2019-09-17 15:58         ` Cristian Marussi
2019-09-17 16:16           ` shuah
2019-09-17 16:05   ` Dave Martin [this message]
2019-09-17 16:18     ` shuah
2019-09-18 10:17       ` Dave Martin
2019-09-18 10:59       ` Cristian Marussi
2019-10-07 18:22     ` Cristian Marussi
2019-09-10 12:31 ` [PATCH v6 02/11] kselftest: arm64: mangle_pstate_invalid_compat_toggle and common utils Cristian Marussi
2019-09-17 16:05   ` Dave Martin
2019-09-26 11:00     ` Cristian Marussi
2019-09-10 12:31 ` [PATCH v6 03/11] kselftest: arm64: mangle_pstate_invalid_daif_bits Cristian Marussi
2019-09-10 12:31 ` [PATCH v6 04/11] kselftest: arm64: mangle_pstate_invalid_mode_el[123][ht] Cristian Marussi
2019-09-10 12:31 ` [PATCH v6 05/11] kselftest: arm64: mangle_pstate_ssbs_regs Cristian Marussi
2019-09-17 16:05   ` Dave Martin
2019-10-07 18:23     ` Cristian Marussi
2019-10-08 15:07       ` Dave Martin
2019-09-10 12:31 ` [PATCH v6 06/11] kselftest: arm64: fake_sigreturn_bad_magic Cristian Marussi
2019-09-17 16:06   ` Dave Martin
2019-10-07 18:23     ` Cristian Marussi
2019-09-10 12:31 ` [PATCH v6 07/11] kselftest: arm64: fake_sigreturn_bad_size_for_magic0 Cristian Marussi
2019-09-10 12:31 ` [PATCH v6 08/11] kselftest: arm64: fake_sigreturn_missing_fpsimd Cristian Marussi
2019-09-10 12:31 ` [PATCH v6 09/11] kselftest: arm64: fake_sigreturn_duplicated_fpsimd Cristian Marussi
2019-09-17 16:06   ` Dave Martin
2019-09-10 12:31 ` [PATCH v6 10/11] kselftest: arm64: fake_sigreturn_bad_size Cristian Marussi
2019-09-10 12:31 ` [PATCH v6 11/11] kselftest: arm64: fake_sigreturn_misaligned_sp Cristian Marussi

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190917160534.GK27757@arm.com \
    --to=dave.martin@arm.com \
    --cc=amit.kachhap@arm.com \
    --cc=andreyknvl@google.com \
    --cc=cristian.marussi@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=shuah@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-kselftest Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-kselftest/0 linux-kselftest/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-kselftest linux-kselftest/ https://lore.kernel.org/linux-kselftest \
		linux-kselftest@vger.kernel.org linux-kselftest@archiver.kernel.org
	public-inbox-index linux-kselftest

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kselftest


AGPL code for this site: git clone https://public-inbox.org/ public-inbox