On 04/09/2019 12:47, Dave Martin wrote:
> On Mon, Sep 02, 2019 at 12:29:22pm +0100, Cristian Marussi wrote:
>> Add a new arm64-specific empty subsystem amongst TARGETS of KSFT build
>> framework; keep these new arm64 KSFT testcases separated into distinct
>
> Nit: this isn't true any more, since the tags tests already added the
> arm64 subsystem here.
Ok
>
>> subdirs inside tools/testing/selftests/arm64/ depending on the specific
>> subsystem targeted.
>>
>> Add into toplevel arm64 KSFT Makefile a mechanism to guess the effective
>> location of Kernel headers as installed by KSFT framework.
>
> This:
>
>> Merge with
>>
>> commit 9ce1263033cd ("selftests, arm64: add a selftest for passing
>> tagged pointers to kernel")
>>
>> while moving such KSFT tags tests inside their own subdirectory
>> (arm64/tags).
>
> ...could be put under the tearoff, but it doesn't really belong in the
> commit message IMHO.
>
> I suggest rewriting the commit message to reflect the current
> situation (but it can be kept brief).
>
> Basically, what this patch now seems to do is to prepare for adding
> more arm64 tests, by moving the tags tests into their own subdirectory
> and extending the existing skeleton Makefile as appropriate.
>
Ok
>> Signed-off-by: Cristian Marussi <cristian.marussi@xxxxxxx>
>> ---
>> 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
>> ---
>
> [...]
>
>> diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile
>> index a61b2e743e99..5dbb0ffdfc9a 100644
>> --- a/tools/testing/selftests/arm64/Makefile
>> +++ b/tools/testing/selftests/arm64/Makefile
>> @@ -1,11 +1,69 @@
>> # SPDX-License-Identifier: GPL-2.0
>> +# Copyright (C) 2019 ARM Limited
>
> Change of copyright? This isn't pure Arm IP upstream IIUC.
>
> Maybe just drop it: Makefiles don't usually contain significant IP, so
> many have no copyright message anyway.
>
Right. I'll drop.
>> -# ARCH can be overridden by the user for cross compiling
>> -ARCH ?= $(shell uname -m 2>/dev/null || echo not)
>> +# When ARCH not overridden for crosscompiling, lookup machine
>> +ARCH ?= $(shell uname -m)
>> +ARCH := $(shell echo $(ARCH) | sed -e s/aarch64/arm64/)
>>
>> -ifneq (,$(filter $(ARCH),aarch64 arm64))
>> -TEST_GEN_PROGS := tags_test
>> -TEST_PROGS := run_tags_test.sh
>> +ifeq ("x$(ARCH)", "xarm64")
>> +SUBDIRS := tags
>> +else
>> +SUBDIRS :=
>> endif
>>
>> -include ../lib.mk
>> +CFLAGS := -Wall -O2 -g
>> +
>> +# A proper top_srcdir is needed by KSFT(lib.mk)
>> +top_srcdir = ../../../../..
>> +
>> +# 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
>
> Looking at this, can we just pass the directory in from the toplevel
> "all" rule instead of guessing?
>
Do you mean toplevel in KSFT ?
I think it's how should be done at the end, but I was trying to keep this series on
arm64/ lands only. (also maybe I'm missing something obvious in KSFT handling of this
situation....even though many other KSFT use built CFLAGS like: -I../../../usr/include
or similar)
> Maybe don't churn this for now though. It's something that could be
> looked at later.
>
Ok. I'll leave here and fix it to avoid relative paths...which could be problematic
when exported to lower level Makefiles.
Cheers
Cristian
> [...]
>
> Apart from the comments above, the patch looks reasonable to me.
>
> Cheers
> ---Dave
>
