* [PATCH v2] selftests: kvm: fix mkdir error when building for unsupported arch @ 2024-08-19 9:30 Muhammad Usama Anjum 2024-08-19 10:01 ` Shuah Khan 2024-08-19 16:33 ` Sean Christopherson 0 siblings, 2 replies; 6+ messages in thread From: Muhammad Usama Anjum @ 2024-08-19 9:30 UTC (permalink / raw) To: Paolo Bonzini, Shuah Khan Cc: Muhammad Usama Anjum, kernel, Sean Christopherson, kvm, linux-kselftest, linux-kernel The tests are built on per architecture basis. When unsupported architecture is specified, it has no tests and TEST_GEN_PROGS is empty. The lib.mk has support for not building anything for such case. But KVM makefile doesn't handle such case correctly. It doesn't check if TEST_GEN_PROGS is empty or not and try to create directory by mkdir. Hence mkdir generates the error. mkdir: missing operand Try 'mkdir --help' for more information. This can be easily fixed by checking if TEST_GEN_PROGS isn't empty before calling mkdir. Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Sean Christopherson <seanjc@google.com> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> --- Changes since v1: - Instead of ignoring error, check TEST_GEN_PROGS's validity first --- tools/testing/selftests/kvm/Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile index 48d32c5aa3eb7..9f8ed82ff1d65 100644 --- a/tools/testing/selftests/kvm/Makefile +++ b/tools/testing/selftests/kvm/Makefile @@ -317,7 +317,9 @@ $(LIBKVM_S_OBJ): $(OUTPUT)/%.o: %.S $(GEN_HDRS) $(LIBKVM_STRING_OBJ): $(OUTPUT)/%.o: %.c $(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c -ffreestanding $< -o $@ +ifneq ($(strip $(TEST_GEN_PROGS)),) $(shell mkdir -p $(sort $(dir $(TEST_GEN_PROGS)))) +endif $(SPLIT_TEST_GEN_OBJ): $(GEN_HDRS) $(TEST_GEN_PROGS): $(LIBKVM_OBJS) $(TEST_GEN_PROGS_EXTENDED): $(LIBKVM_OBJS) -- 2.39.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] selftests: kvm: fix mkdir error when building for unsupported arch 2024-08-19 9:30 [PATCH v2] selftests: kvm: fix mkdir error when building for unsupported arch Muhammad Usama Anjum @ 2024-08-19 10:01 ` Shuah Khan 2024-08-19 16:33 ` Sean Christopherson 1 sibling, 0 replies; 6+ messages in thread From: Shuah Khan @ 2024-08-19 10:01 UTC (permalink / raw) To: Muhammad Usama Anjum, Paolo Bonzini, Shuah Khan Cc: kernel, Sean Christopherson, kvm, linux-kselftest, linux-kernel, Shuah Khan On 8/19/24 03:30, Muhammad Usama Anjum wrote: > The tests are built on per architecture basis. When unsupported > architecture is specified, it has no tests and TEST_GEN_PROGS is empty. > The lib.mk has support for not building anything for such case. But KVM > makefile doesn't handle such case correctly. It doesn't check if > TEST_GEN_PROGS is empty or not and try to create directory by mkdir. > Hence mkdir generates the error. > > mkdir: missing operand > Try 'mkdir --help' for more information. > > This can be easily fixed by checking if TEST_GEN_PROGS isn't empty > before calling mkdir. > > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Sean Christopherson <seanjc@google.com> > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> > --- > Changes since v1: > - Instead of ignoring error, check TEST_GEN_PROGS's validity first > --- > tools/testing/selftests/kvm/Makefile | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile > index 48d32c5aa3eb7..9f8ed82ff1d65 100644 > --- a/tools/testing/selftests/kvm/Makefile > +++ b/tools/testing/selftests/kvm/Makefile > @@ -317,7 +317,9 @@ $(LIBKVM_S_OBJ): $(OUTPUT)/%.o: %.S $(GEN_HDRS) > $(LIBKVM_STRING_OBJ): $(OUTPUT)/%.o: %.c > $(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c -ffreestanding $< -o $@ > > +ifneq ($(strip $(TEST_GEN_PROGS)),) > $(shell mkdir -p $(sort $(dir $(TEST_GEN_PROGS)))) > +endif > $(SPLIT_TEST_GEN_OBJ): $(GEN_HDRS) > $(TEST_GEN_PROGS): $(LIBKVM_OBJS) > $(TEST_GEN_PROGS_EXTENDED): $(LIBKVM_OBJS) Looks good to me. Reviewed-by: Shuah Khan <skhan@linuxfoundation.org> thanks, -- Shuah ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] selftests: kvm: fix mkdir error when building for unsupported arch 2024-08-19 9:30 [PATCH v2] selftests: kvm: fix mkdir error when building for unsupported arch Muhammad Usama Anjum 2024-08-19 10:01 ` Shuah Khan @ 2024-08-19 16:33 ` Sean Christopherson 2024-08-19 22:15 ` Oliver Upton 2024-08-20 7:26 ` Muhammad Usama Anjum 1 sibling, 2 replies; 6+ messages in thread From: Sean Christopherson @ 2024-08-19 16:33 UTC (permalink / raw) To: Muhammad Usama Anjum Cc: Paolo Bonzini, Shuah Khan, kernel, kvm, linux-kselftest, linux-kernel, Marc Zyngier, Oliver Upton, Christian Borntraeger, Janosch Frank, Claudio Imbrenda, Anup Patel +KVM arch maintainers On Mon, Aug 19, 2024, Muhammad Usama Anjum wrote: > The tests are built on per architecture basis. When unsupported > architecture is specified, it has no tests and TEST_GEN_PROGS is empty. > The lib.mk has support for not building anything for such case. But KVM > makefile doesn't handle such case correctly. It doesn't check if > TEST_GEN_PROGS is empty or not and try to create directory by mkdir. > Hence mkdir generates the error. > > mkdir: missing operand > Try 'mkdir --help' for more information. > > This can be easily fixed by checking if TEST_GEN_PROGS isn't empty > before calling mkdir. > > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Sean Christopherson <seanjc@google.com> > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> > --- > Changes since v1: > - Instead of ignoring error, check TEST_GEN_PROGS's validity first > --- > tools/testing/selftests/kvm/Makefile | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile > index 48d32c5aa3eb7..9f8ed82ff1d65 100644 > --- a/tools/testing/selftests/kvm/Makefile > +++ b/tools/testing/selftests/kvm/Makefile > @@ -317,7 +317,9 @@ $(LIBKVM_S_OBJ): $(OUTPUT)/%.o: %.S $(GEN_HDRS) > $(LIBKVM_STRING_OBJ): $(OUTPUT)/%.o: %.c > $(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c -ffreestanding $< -o $@ > > +ifneq ($(strip $(TEST_GEN_PROGS)),) > $(shell mkdir -p $(sort $(dir $(TEST_GEN_PROGS)))) > +endif This just suppresses an error, it doesn't fix the underlying problem. E.g. there are other weird side effects, such as an above mkdir creating the $(ARCH) directory even though it shouldn't exist in the end. It's also very opaque, e.g. without a comment or the context of the changelog, I'd have no idea what purpose the above serves. Rather than bury the effective "is this arch supported" check in the middle of the Makefile, what if we wrap the "real" makefile and include it only for supported architectures, and provide dummy targets for everything else? E.g. --- # SPDX-License-Identifier: GPL-2.0-only top_srcdir = ../../../.. include $(top_srcdir)/scripts/subarch.include ARCH ?= $(SUBARCH) ifeq ($(ARCH),$(filter $(ARCH),arm64 s390 riscv x86 x86_64)) ifeq ($(ARCH),x86) ARCH_DIR := x86_64 else ifeq ($(ARCH),arm64) ARCH_DIR := aarch64 else ifeq ($(ARCH),s390) ARCH_DIR := s390x else ARCH_DIR := $(ARCH) endif include Makefile.kvm else all: clean: endif --- And other KVM maintainers, the big question is: if we do the above, would now be a decent time to bite the bullet and switch to the kernel's canonical arch paths, i.e. arm64, s390, and x86? I feel like if we're ever going to get away from using aarch64, x86_64, and s390x, this is as about a good of an opportunity as we're going to get. The annoying x86_64=>x86 alias still needs to be handled to avoid breaking explicit ARCH=x86_64 builds (which apparently are allowed, *sigh*), but we can ditch ARCH_DIR and the KVM selftests dirs match tools' include paths. --- # SPDX-License-Identifier: GPL-2.0-only top_srcdir = ../../../.. include $(top_srcdir)/scripts/subarch.include ARCH ?= $(SUBARCH) ifeq ($(ARCH),$(filter $(ARCH),arm64 s390 riscv x86 x86_64)) # Top-level selftests allows ARCH=x86_64 :-( ifeq ($(ARCH),x86_64) ARCH := x86 endif include Makefile.kvm else all: clean: endif --- If no one objects or has a better idea, I'll post a series to do the above. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] selftests: kvm: fix mkdir error when building for unsupported arch 2024-08-19 16:33 ` Sean Christopherson @ 2024-08-19 22:15 ` Oliver Upton 2024-08-20 6:40 ` Marc Zyngier 2024-08-20 7:26 ` Muhammad Usama Anjum 1 sibling, 1 reply; 6+ messages in thread From: Oliver Upton @ 2024-08-19 22:15 UTC (permalink / raw) To: Sean Christopherson Cc: Muhammad Usama Anjum, Paolo Bonzini, Shuah Khan, kernel, kvm, linux-kselftest, linux-kernel, Marc Zyngier, Christian Borntraeger, Janosch Frank, Claudio Imbrenda, Anup Patel On Mon, Aug 19, 2024 at 09:33:17AM -0700, Sean Christopherson wrote: > And other KVM maintainers, the big question is: if we do the above, would now be > a decent time to bite the bullet and switch to the kernel's canonical arch paths, > i.e. arm64, s390, and x86? I feel like if we're ever going to get away from > using aarch64, x86_64, and s390x, this is as about a good of an opportunity as > we're going to get. I'm pretty much indifferent on the matter, but I won't complain if you send out a change for this. -- Thanks, Oliver ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] selftests: kvm: fix mkdir error when building for unsupported arch 2024-08-19 22:15 ` Oliver Upton @ 2024-08-20 6:40 ` Marc Zyngier 0 siblings, 0 replies; 6+ messages in thread From: Marc Zyngier @ 2024-08-20 6:40 UTC (permalink / raw) To: Sean Christopherson, Oliver Upton Cc: Muhammad Usama Anjum, Paolo Bonzini, Shuah Khan, kernel, kvm, linux-kselftest, linux-kernel, Christian Borntraeger, Janosch Frank, Claudio Imbrenda, Anup Patel On Mon, 19 Aug 2024 23:15:44 +0100, Oliver Upton <oliver.upton@linux.dev> wrote: > > On Mon, Aug 19, 2024 at 09:33:17AM -0700, Sean Christopherson wrote: > > And other KVM maintainers, the big question is: if we do the above, would now be > > a decent time to bite the bullet and switch to the kernel's canonical arch paths, > > i.e. arm64, s390, and x86? I feel like if we're ever going to get away from > > using aarch64, x86_64, and s390x, this is as about a good of an opportunity as > > we're going to get. > > I'm pretty much indifferent on the matter, but I won't complain if you > send out a change for this. Same here. Call it arm64 or aargh64, same difference. Whatever we change, some people will moan anyway. M. -- Without deviation from the norm, progress is not possible. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] selftests: kvm: fix mkdir error when building for unsupported arch 2024-08-19 16:33 ` Sean Christopherson 2024-08-19 22:15 ` Oliver Upton @ 2024-08-20 7:26 ` Muhammad Usama Anjum 1 sibling, 0 replies; 6+ messages in thread From: Muhammad Usama Anjum @ 2024-08-20 7:26 UTC (permalink / raw) To: Sean Christopherson Cc: Usama.Anjum, Paolo Bonzini, Shuah Khan, kernel, kvm, linux-kselftest, linux-kernel, Marc Zyngier, Oliver Upton, Christian Borntraeger, Janosch Frank, Claudio Imbrenda, Anup Patel On 8/19/24 9:33 PM, Sean Christopherson wrote: > +KVM arch maintainers > > On Mon, Aug 19, 2024, Muhammad Usama Anjum wrote: >> The tests are built on per architecture basis. When unsupported >> architecture is specified, it has no tests and TEST_GEN_PROGS is empty. >> The lib.mk has support for not building anything for such case. But KVM >> makefile doesn't handle such case correctly. It doesn't check if >> TEST_GEN_PROGS is empty or not and try to create directory by mkdir. >> Hence mkdir generates the error. >> >> mkdir: missing operand >> Try 'mkdir --help' for more information. >> >> This can be easily fixed by checking if TEST_GEN_PROGS isn't empty >> before calling mkdir. >> >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> Cc: Sean Christopherson <seanjc@google.com> >> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> >> --- >> Changes since v1: >> - Instead of ignoring error, check TEST_GEN_PROGS's validity first >> --- >> tools/testing/selftests/kvm/Makefile | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile >> index 48d32c5aa3eb7..9f8ed82ff1d65 100644 >> --- a/tools/testing/selftests/kvm/Makefile >> +++ b/tools/testing/selftests/kvm/Makefile >> @@ -317,7 +317,9 @@ $(LIBKVM_S_OBJ): $(OUTPUT)/%.o: %.S $(GEN_HDRS) >> $(LIBKVM_STRING_OBJ): $(OUTPUT)/%.o: %.c >> $(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c -ffreestanding $< -o $@ >> >> +ifneq ($(strip $(TEST_GEN_PROGS)),) >> $(shell mkdir -p $(sort $(dir $(TEST_GEN_PROGS)))) >> +endif > This just suppresses an error, it doesn't fix the underlying problem. E.g. there > are other weird side effects, such as an above mkdir creating the $(ARCH) directory > even though it shouldn't exist in the end. > > It's also very opaque, e.g. without a comment or the context of the changelog, > I'd have no idea what purpose the above serves. > > Rather than bury the effective "is this arch supported" check in the middle of > the Makefile, what if we wrap the "real" makefile and include it only for > supported architectures, and provide dummy targets for everything else? > > E.g. > > --- > # SPDX-License-Identifier: GPL-2.0-only > top_srcdir = ../../../.. > include $(top_srcdir)/scripts/subarch.include > ARCH ?= $(SUBARCH) > > ifeq ($(ARCH),$(filter $(ARCH),arm64 s390 riscv x86 x86_64)) > ifeq ($(ARCH),x86) > ARCH_DIR := x86_64 > else ifeq ($(ARCH),arm64) > ARCH_DIR := aarch64 > else ifeq ($(ARCH),s390) > ARCH_DIR := s390x > else > ARCH_DIR := $(ARCH) > endif > > include Makefile.kvm > else > all: > clean: > endif > --- > > And other KVM maintainers, the big question is: if we do the above, would now be > a decent time to bite the bullet and switch to the kernel's canonical arch paths, > i.e. arm64, s390, and x86? I feel like if we're ever going to get away from > using aarch64, x86_64, and s390x, this is as about a good of an opportunity as > we're going to get. > > The annoying x86_64=>x86 alias still needs to be handled to avoid breaking explicit > ARCH=x86_64 builds (which apparently are allowed, *sigh*), but we can ditch ARCH_DIR > and the KVM selftests dirs match tools' include paths. > > --- > # SPDX-License-Identifier: GPL-2.0-only > top_srcdir = ../../../.. > include $(top_srcdir)/scripts/subarch.include > ARCH ?= $(SUBARCH) > > ifeq ($(ARCH),$(filter $(ARCH),arm64 s390 riscv x86 x86_64)) > # Top-level selftests allows ARCH=x86_64 🙁 > ifeq ($(ARCH),x86_64) > ARCH := x86 > endif > include Makefile.kvm > else > all: > clean: > endif > --- > > If no one objects or has a better idea, I'll post a series to do the above. I didn't had enough knowledge to attempt a better fix. Thank you. -- BR, Muhammad Usama Anjum ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-08-20 7:27 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-08-19 9:30 [PATCH v2] selftests: kvm: fix mkdir error when building for unsupported arch Muhammad Usama Anjum 2024-08-19 10:01 ` Shuah Khan 2024-08-19 16:33 ` Sean Christopherson 2024-08-19 22:15 ` Oliver Upton 2024-08-20 6:40 ` Marc Zyngier 2024-08-20 7:26 ` Muhammad Usama Anjum
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox