Linux-kselftest Archive on lore.kernel.org
 help / color / Atom feed
From: Dave Martin <Dave.Martin@arm.com>
To: shuah <shuah@kernel.org>
Cc: Cristian Marussi <cristian.marussi@arm.com>,
	amit.kachhap@arm.com, andreyknvl@google.com,
	linux-arm-kernel@lists.infradead.org,
	linux-kselftest@vger.kernel.org
Subject: Re: [PATCH v6 01/11] kselftest: arm64: extend toplevel skeleton Makefile
Date: Wed, 18 Sep 2019 11:17:07 +0100
Message-ID: <20190918101705.GP27757@arm.com> (raw)
In-Reply-To: <119aaea7-10b5-0fdf-269d-e86df07b4831@kernel.org>

On Tue, Sep 17, 2019 at 10:18:55AM -0600, shuah wrote:
> On 9/17/19 10:05 AM, Dave Martin wrote:
> >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).
> >
> 
> I am reviewing the series this week. I will provide comments in a
> day or two.
> 
> thanks,
> -- Shuah

Thanks!

From the arm64 side, I'd say we're just down to minor issues at this
point.

Cheers
---Dave

  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
2019-09-17 16:18     ` shuah
2019-09-18 10:17       ` Dave Martin [this message]
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=20190918101705.GP27757@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
	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.git