| Message ID | 20190819132830.9056-8-jarkko.sakkinen@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix the reported SGX selftest makefile issues | expand |
On Mon, Aug 19, 2019 at 04:28:30PM +0300, Jarkko Sakkinen wrote: > Recurse into a list of subdirectories defined by SUBDIRS when running > x86 selftests. Override run_tests, install, emit_tests and clean > targets to implement this behaviour. The code looks good (which doesn't say much, my Makefile knowledge is garbage), but the result is a bit odd. x86/sgx doesn't implement emit_tests (or at least test_sgx doesn't show up), while the rest of the x86 tests don't support install. AFAICT the Makefile is itself weird; it's definitely different than other selftests Makfiles. I don't see any reason to delay this patch, but it feels like something here needs to be cleaned up. > A possible alternative would be to add "x86/sgx" to TARGETS. However, > this would be problematic because detecting 64-bit build would have > to duplicated. > > The implementation is derived from the makefiles of powerpc and sparc64 > selftests. > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > --- > tools/testing/selftests/x86/Makefile | 44 ++++++++++++++++++++++++++++ > 1 file changed, 44 insertions(+) > > diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile > index fa07d526fe39..80571bac8ed5 100644 > --- a/tools/testing/selftests/x86/Makefile > +++ b/tools/testing/selftests/x86/Makefile > @@ -10,6 +10,8 @@ CAN_BUILD_I386 := $(shell ./check_cc.sh $(CC) trivial_32bit_program.c -m32) > CAN_BUILD_X86_64 := $(shell ./check_cc.sh $(CC) trivial_64bit_program.c) > CAN_BUILD_WITH_NOPIE := $(shell ./check_cc.sh $(CC) trivial_program.c -no-pie) > > +SUBDIRS := sgx > + > TARGETS_C_BOTHBITS := single_step_syscall sysret_ss_attrs syscall_nt test_mremap_vdso \ > check_initial_reg_state sigreturn iopl mpx-mini-test ioperm \ > protection_keys test_vdso test_vsyscall mov_ss_trap \ > @@ -59,6 +61,48 @@ endif > > ifeq ($(CAN_BUILD_X86_64),1) > all: all_64 > + @for DIR in $(SUBDIRS); do \ > + BUILD_TARGET=$(OUTPUT)/$$DIR; \ > + mkdir $$BUILD_TARGET -p; \ > + make OUTPUT=$$BUILD_TARGET -C $$DIR $@; \ > + done > + > +DEFAULT_RUN_TESTS := $(RUN_TESTS) > +override define RUN_TESTS > + $(DEFAULT_RUN_TESTS) > + @for TARGET in $(SUBDIRS); do \ > + BUILD_TARGET=$(OUTPUT)/$$TARGET; \ > + $(MAKE) OUTPUT=$$BUILD_TARGET -C $$TARGET run_tests; \ > + done; > +endef > + > +DEFAULT_INSTALL_RULE := $(INSTALL_RULE) > +override define INSTALL_RULE > + $(DEFAULT_INSTALL_RULE) > + @for TARGET in $(SUBDIRS); do \ > + BUILD_TARGET=$(OUTPUT)/$$TARGET; \ > + $(MAKE) OUTPUT=$$BUILD_TARGET -C $$TARGET install; \ > + done; > +endef > + > +DEFAULT_EMIT_TESTS := $(EMIT_TESTS) > +override define EMIT_TESTS > + $(DEFAULT_EMIT_TESTS) > + @for TARGET in $(SUBDIRS); do \ > + BUILD_TARGET=$(OUTPUT)/$$TARGET; \ > + $(MAKE) OUTPUT=$$BUILD_TARGET -s -C $$TARGET emit_tests; \ > + done; > +endef > + > +DEFAULT_CLEAN := $(CLEAN) > +override define CLEAN > + $(DEFAULT_CLEAN) > + @for TARGET in $(SUBDIRS); do \ > + BUILD_TARGET=$(OUTPUT)/$$TARGET; \ > + $(MAKE) OUTPUT=$$BUILD_TARGET -C $$TARGET clean; \ > + done; > +endef > + > TEST_PROGS += $(BINARIES_64) > EXTRA_CFLAGS += -DCAN_BUILD_64 > $(foreach t,$(TARGETS_C_64BIT_ALL),$(eval $(call gen-target-rule-64,$(t)))) > -- > 2.20.1 >
On Wed, 2019-08-21 at 20:42 -0700, Sean Christopherson wrote: > On Mon, Aug 19, 2019 at 04:28:30PM +0300, Jarkko Sakkinen wrote: > > Recurse into a list of subdirectories defined by SUBDIRS when running > > x86 selftests. Override run_tests, install, emit_tests and clean > > targets to implement this behaviour. > > The code looks good (which doesn't say much, my Makefile knowledge is > garbage), but the result is a bit odd. x86/sgx doesn't implement > emit_tests (or at least test_sgx doesn't show up), while the rest of the > x86 tests don't support install. > > AFAICT the Makefile is itself weird; it's definitely different than other > selftests Makfiles. I don't see any reason to delay this patch, but it > feels like something here needs to be cleaned up. Lets fix emit_tests before sending v22. /Jarkko
On Thu, 2019-08-22 at 17:59 +0300, Jarkko Sakkinen wrote: > On Wed, 2019-08-21 at 20:42 -0700, Sean Christopherson wrote: > > On Mon, Aug 19, 2019 at 04:28:30PM +0300, Jarkko Sakkinen wrote: > > > Recurse into a list of subdirectories defined by SUBDIRS when running > > > x86 selftests. Override run_tests, install, emit_tests and clean > > > targets to implement this behaviour. > > > > The code looks good (which doesn't say much, my Makefile knowledge is > > garbage), but the result is a bit odd. x86/sgx doesn't implement > > emit_tests (or at least test_sgx doesn't show up), while the rest of the > > x86 tests don't support install. > > > > AFAICT the Makefile is itself weird; it's definitely different than other > > selftests Makfiles. I don't see any reason to delay this patch, but it > > feels like something here needs to be cleaned up. > > Lets fix emit_tests before sending v22. Selftest patches have been squahed. Further improvements are welcome. Thanks for the review! /Jarkko
On Thu, 2019-08-22 at 18:02 +0300, Jarkko Sakkinen wrote: > On Thu, 2019-08-22 at 17:59 +0300, Jarkko Sakkinen wrote: > > On Wed, 2019-08-21 at 20:42 -0700, Sean Christopherson wrote: > > > On Mon, Aug 19, 2019 at 04:28:30PM +0300, Jarkko Sakkinen wrote: > > > > Recurse into a list of subdirectories defined by SUBDIRS when running > > > > x86 selftests. Override run_tests, install, emit_tests and clean > > > > targets to implement this behaviour. > > > > > > The code looks good (which doesn't say much, my Makefile knowledge is > > > garbage), but the result is a bit odd. x86/sgx doesn't implement > > > emit_tests (or at least test_sgx doesn't show up), while the rest of the > > > x86 tests don't support install. > > > > > > AFAICT the Makefile is itself weird; it's definitely different than other > > > selftests Makfiles. I don't see any reason to delay this patch, but it > > > feels like something here needs to be cleaned up. > > > > Lets fix emit_tests before sending v22. > > Selftest patches have been squahed. Further improvements are welcome. > Thanks for the review! At least my use case is satisfied with these fixes: $ ls output/target/usr/lib/kselftests/x86 test_sgx I can build initramfs now with the selftest bundled. Oddly enough none of the other x86 selftests get included. /Jarkko
On Thu, 2019-08-22 at 19:13 +0300, Jarkko Sakkinen wrote: > On Thu, 2019-08-22 at 18:02 +0300, Jarkko Sakkinen wrote: > > On Thu, 2019-08-22 at 17:59 +0300, Jarkko Sakkinen wrote: > > > On Wed, 2019-08-21 at 20:42 -0700, Sean Christopherson wrote: > > > > On Mon, Aug 19, 2019 at 04:28:30PM +0300, Jarkko Sakkinen wrote: > > > > > Recurse into a list of subdirectories defined by SUBDIRS when running > > > > > x86 selftests. Override run_tests, install, emit_tests and clean > > > > > targets to implement this behaviour. > > > > > > > > The code looks good (which doesn't say much, my Makefile knowledge is > > > > garbage), but the result is a bit odd. x86/sgx doesn't implement > > > > emit_tests (or at least test_sgx doesn't show up), while the rest of the > > > > x86 tests don't support install. > > > > > > > > AFAICT the Makefile is itself weird; it's definitely different than other > > > > selftests Makfiles. I don't see any reason to delay this patch, but it > > > > feels like something here needs to be cleaned up. > > > > > > Lets fix emit_tests before sending v22. > > > > Selftest patches have been squahed. Further improvements are welcome. > > Thanks for the review! > > At least my use case is satisfied with these fixes: > > $ ls output/target/usr/lib/kselftests/x86 > test_sgx > > I can build initramfs now with the selftest bundled. Oddly enough none of the > other x86 selftests get included. # ls /usr/lib/kselftests/x86 test_sgx # /usr/lib/kselftests/x86/test_sgx Binary size 24576 (0x6000), SIGSTRUCT size 1808 Loading the enclave. Input: 0x1122334455667788 Output: 0x1122334455667788 This is a big improvement for me :-) Do not have to upload selftest anymore with minicom or scp. /Jarkko
diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile index fa07d526fe39..80571bac8ed5 100644 --- a/tools/testing/selftests/x86/Makefile +++ b/tools/testing/selftests/x86/Makefile @@ -10,6 +10,8 @@ CAN_BUILD_I386 := $(shell ./check_cc.sh $(CC) trivial_32bit_program.c -m32) CAN_BUILD_X86_64 := $(shell ./check_cc.sh $(CC) trivial_64bit_program.c) CAN_BUILD_WITH_NOPIE := $(shell ./check_cc.sh $(CC) trivial_program.c -no-pie) +SUBDIRS := sgx + TARGETS_C_BOTHBITS := single_step_syscall sysret_ss_attrs syscall_nt test_mremap_vdso \ check_initial_reg_state sigreturn iopl mpx-mini-test ioperm \ protection_keys test_vdso test_vsyscall mov_ss_trap \ @@ -59,6 +61,48 @@ endif ifeq ($(CAN_BUILD_X86_64),1) all: all_64 + @for DIR in $(SUBDIRS); do \ + BUILD_TARGET=$(OUTPUT)/$$DIR; \ + mkdir $$BUILD_TARGET -p; \ + make OUTPUT=$$BUILD_TARGET -C $$DIR $@; \ + done + +DEFAULT_RUN_TESTS := $(RUN_TESTS) +override define RUN_TESTS + $(DEFAULT_RUN_TESTS) + @for TARGET in $(SUBDIRS); do \ + BUILD_TARGET=$(OUTPUT)/$$TARGET; \ + $(MAKE) OUTPUT=$$BUILD_TARGET -C $$TARGET run_tests; \ + done; +endef + +DEFAULT_INSTALL_RULE := $(INSTALL_RULE) +override define INSTALL_RULE + $(DEFAULT_INSTALL_RULE) + @for TARGET in $(SUBDIRS); do \ + BUILD_TARGET=$(OUTPUT)/$$TARGET; \ + $(MAKE) OUTPUT=$$BUILD_TARGET -C $$TARGET install; \ + done; +endef + +DEFAULT_EMIT_TESTS := $(EMIT_TESTS) +override define EMIT_TESTS + $(DEFAULT_EMIT_TESTS) + @for TARGET in $(SUBDIRS); do \ + BUILD_TARGET=$(OUTPUT)/$$TARGET; \ + $(MAKE) OUTPUT=$$BUILD_TARGET -s -C $$TARGET emit_tests; \ + done; +endef + +DEFAULT_CLEAN := $(CLEAN) +override define CLEAN + $(DEFAULT_CLEAN) + @for TARGET in $(SUBDIRS); do \ + BUILD_TARGET=$(OUTPUT)/$$TARGET; \ + $(MAKE) OUTPUT=$$BUILD_TARGET -C $$TARGET clean; \ + done; +endef + TEST_PROGS += $(BINARIES_64) EXTRA_CFLAGS += -DCAN_BUILD_64 $(foreach t,$(TARGETS_C_64BIT_ALL),$(eval $(call gen-target-rule-64,$(t))))
Recurse into a list of subdirectories defined by SUBDIRS when running x86 selftests. Override run_tests, install, emit_tests and clean targets to implement this behaviour. A possible alternative would be to add "x86/sgx" to TARGETS. However, this would be problematic because detecting 64-bit build would have to duplicated. The implementation is derived from the makefiles of powerpc and sparc64 selftests. Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> --- tools/testing/selftests/x86/Makefile | 44 ++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+)