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

On 17/09/2019 17:18, 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.

As a side question... should I rebase next v7 (after your feedback) on latest
kselftest branch (linux-kselftest-5.4-rc1 or whatever)

Cristian


> thanks,
> -- Shuah
> 


  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
2019-09-17 16:18     ` shuah
2019-09-18 10:17       ` Dave Martin
2019-09-18 10:59       ` Cristian Marussi [this message]
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=ba61ad6f-be69-6e30-d563-cfeb8503bea2@arm.com \
    --to=cristian.marussi@arm.com \
    --cc=Dave.Martin@arm.com \
    --cc=amit.kachhap@arm.com \
    --cc=andreyknvl@google.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