Linux-kselftest Archive on lore.kernel.org
 help / color / Atom feed
From: shuah <shuah@kernel.org>
To: Cristian Marussi <cristian.marussi@arm.com>,
	Anders Roxell <anders.roxell@linaro.org>
Cc: linux-kselftest@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, andreyknvl@google.com,
	dave.martin@arm.com, amit.kachhap@arm.com,
	shuah <shuah@kernel.org>
Subject: Re: [PATCH v6 01/11] kselftest: arm64: extend toplevel skeleton Makefile
Date: Tue, 17 Sep 2019 10:16:41 -0600
Message-ID: <a572c088-eb8a-9efd-cb7b-978254006f97@kernel.org> (raw)
In-Reply-To: <883054a8-7d56-9f8b-ed35-892582bf7599@arm.com>

On 9/17/19 9:58 AM, Cristian Marussi wrote:
> On 17/09/2019 16:29, shuah wrote:
>> On 9/17/19 9:17 AM, Cristian Marussi wrote:
>>> Hi Anders
>>>
>>> thanks for the review.
>>>
>>> On 17/09/2019 14:42, Anders Roxell wrote:
>>>> On 2019-09-10 13:31, 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
>>>>> +
>>>>> +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
>>>>> +emit_tests: all
>>>>> +	@for DIR in $(SUBTARGETS); do				\
>>>>> +		BUILD_TARGET=$(OUTPUT)/$$DIR;			\
>>>>> +		make OUTPUT=$$BUILD_TARGET -C $$DIR $@;		\
>>>>> +	done
>>>>> +
>>>>> +clean:
>>>>> +	@for DIR in $(SUBTARGETS); do				\
>>>>> +		BUILD_TARGET=$(OUTPUT)/$$DIR;			\
>>>>> +		make OUTPUT=$$BUILD_TARGET -C $$DIR $@;		\
>>>>> +	done
>>>>> +
>>>>> +.PHONY: all clean install run_tests emit_tests
>>>>> diff --git a/tools/testing/selftests/arm64/README b/tools/testing/selftests/arm64/README
>>>>> new file mode 100644
>>>>> index 000000000000..cc1e51796fee
>>>>> --- /dev/null
>>>>> +++ b/tools/testing/selftests/arm64/README
>>>>> @@ -0,0 +1,25 @@
>>>>> +KSelfTest ARM64
>>>>> +===============
>>>>> +
>>>>> +- These tests are arm64 specific and so not built or run but just skipped
>>>>> +  completely when env-variable ARCH is found to be different than 'arm64'
>>>>> +  and `uname -m` reports other than 'aarch64'.
>>>>> +
>>>>> +- Holding true the above, ARM64 KSFT tests can be run within the KSelfTest
>>>>> +  framework using standard Linux top-level-makefile targets:
>>>>> +
>>>>> +      $ make TARGETS=arm64 kselftest-clean
>>>>> +      $ make TARGETS=arm64 kselftest
>>>>> +
>>>>> +      or
>>>>> +
>>>>> +      $ make -C tools/testing/selftests TARGETS=arm64 \
>>>>> +		INSTALL_PATH=<your-installation-path> install
>>>>> +
>>>>> +      or, alternatively, only specific arm64/ subtargets can be picked:
>>>>> +
>>>>> +      $ make -C tools/testing/selftests TARGETS=arm64 SUBTARGETS="tags signal" \
>>>>> +		INSTALL_PATH=<your-installation-path> install
>>>>> +
>>>>> +   Further details on building and running KFST can be found in:
>>>>> +     Documentation/dev-tools/kselftest.rst
>>>>> diff --git a/tools/testing/selftests/arm64/tags/Makefile b/tools/testing/selftests/arm64/tags/Makefile
>>>>> new file mode 100644
>>>>> index 000000000000..dcc8b0467b68
>>>>> --- /dev/null
>>>>> +++ b/tools/testing/selftests/arm64/tags/Makefile
>>>>> @@ -0,0 +1,6 @@
>>>>> +# SPDX-License-Identifier: GPL-2.0
>>>>> +
>>>>> +TEST_GEN_PROGS := tags_test
>>>>
>>>> This should be TEST_GEN_FILES, since its used by run_tags_test.sh.
>>>> If its TEST_GEN_PROGS it will be added to the script run_kselftest.sh,
>>>> and I don't think thats the intent, even though it looked like that
>>>> before.
>>>>
>>>
>>> In fact I saw the tags tests running twice (via ./tags_test and via ./run_tags_test.sh) when called
>>> via run_kselftest.sh....but since it was already like that in the original patch so I did not want to
>>> fix it in the context of this series (where tags tests are simply relocated into their own directory)
>>>
>>> I could add a separate fix on top of this series if it could make sense.
>>>
>>
>> We are still in review phase I would think. It would make sense to fix
>> the original patch and not as a separate fix patch.
> 
> The original code for:
> 
>>>>> +++ b/tools/testing/selftests/arm64/tags/Makefile
>>>>> @@ -0,0 +1,6 @@
>>>>> +# SPDX-License-Identifier: GPL-2.0
>>>>> +
>>>>> +TEST_GEN_PROGS := tags_test
> 
> has not been introduced in this series (under review), and that's merged already I think:
> 
> https://lore.kernel.org/linux-kselftest/0999c80cd639b78ae27c0674069d552833227564.1561386715.git.andreyknvl@google.com/
> 
> This patch only moves the original tags tests (introduced with the above commit) from arm64/ into their own arm64/tags/
> directory and integrate with the new arm64/signal tests by this series.
> 
> Here I have just moved down the original code including the bug, that's why I'm saying I could push a fix on top of this series.
> 
> I thought I had to keep the two series distinct give that I'm integrating someone else commit (and eventually fix later): but if
> not I can alternatively fix the above tags tests issue in the next v7 02/11 within this series.
> 

Thanks. Yeah if it is already in a tree then, let's fix it in a separate
patch and add Fixes tag to make it easier to track them.

thanks,
-- Shuah


  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 [this message]
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
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=a572c088-eb8a-9efd-cb7b-978254006f97@kernel.org \
    --to=shuah@kernel.org \
    --cc=amit.kachhap@arm.com \
    --cc=anders.roxell@linaro.org \
    --cc=andreyknvl@google.com \
    --cc=cristian.marussi@arm.com \
    --cc=dave.martin@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kselftest@vger.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