[PATCH v2.1 0/2] tools: fix build dependencies upon generated header(s)

[PATCH v2.1 0/2] tools: fix build dependencies upon generated header(s)

[Jan Beulich]

1: tools: fix build dependency upon generated header(s)
2: x86emul: fix test harness and fuzzer build dependencies

I've decided to split the original patch, in the hope that at least
the main bug fix (patch 1) can go in. Andrew, I've dropped your
ack, for you to reconsider whether you're still willing to give it in
light of Ian's reservations (against what is now patch 2) of
whether instead we should go any of the other possible routes.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2.1 1/2] tools: fix build dependency upon generated header(s)

[Jan Beulich]

Commit fd35f32b4b ("tools/x86emul: Use struct cpuid_policy in the
userspace test harnesses") didn't account for the dependencies of
cpuid-autogen.h to potentially change between incremental builds.
Putting the make invocation to produce the header together with the
directory tree creation therefore does not work. Introduce a separate
goal.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2.1: Split controversial parts from (hopefully) non-controversial ones.

--- a/tools/include/Makefile
+++ b/tools/include/Makefile
@@ -4,8 +4,9 @@ include $(XEN_ROOT)/tools/Rules.mk
 # Relative to $(XEN_ROOT)/xen/xsm/flask
 FLASK_H_DEPEND := policy/initial_sids
 
-.PHONY: all build
-all build: xen-foreign xen/.dir xen-xsm/.dir
+.PHONY: all all-y build
+all build: all-y xen-foreign xen/.dir xen-xsm/.dir
+all-y:
 
 .PHONY: xen-foreign
 xen-foreign:
@@ -27,10 +28,12 @@ ifeq ($(CONFIG_X86),y)
 	for f in $(filter-out %autogen.h,$(patsubst $(XEN_ROOT)/xen/include/xen/lib/x86/%,%,Makefile $(wildcard $(XEN_ROOT)/xen/include/xen/lib/x86/*.h))); do \
 		ln -sf $(XEN_ROOT)/xen/include/xen/lib/x86/$$f xen/lib/x86/$$f; \
 	done
-	$(MAKE) -C xen/lib/x86 all XEN_ROOT=$(XEN_ROOT)
 endif
 	touch $@
 
+all-$(CONFIG_X86): xen/.dir
+	$(MAKE) -C xen/lib/x86 all XEN_ROOT=$(XEN_ROOT)
+
 # Not xen/xsm as that clashes with link to
 # $(XEN_ROOT)/xen/include/public/xsm above.
 xen-xsm/.dir: $(XEN_ROOT)/xen/xsm/flask/policy/mkflask.sh \





_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2.1 2/2] x86emul: fix test harness and fuzzer build dependencies

[Jan Beulich]

Commit fd35f32b4b ("tools/x86emul: Use struct cpuid_policy in the
userspace test harnesses") didn't account for the dependencies of
cpuid-autogen.h to potentially change between incremental builds.
In particular the harness has a "run" goal which is supposed to be
usable independently of the rest of the tools sub-tree building, and
both the harness and the fuzzer code are also supposed to be buildable
independently. Therefore they need to recursively invoke make to re-
build the generated header if needed, but only when these rules did not
get invoked recursively themselves.

Finally cpuid.o did not have any dependencies added for it.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
v2.1: Split controversial parts from (hopefully) non-controversial ones.
v2: Guard $(MAKE) invocations by $(MAKELEVEL) checks.

--- a/tools/fuzz/x86_instruction_emulator/Makefile
+++ b/tools/fuzz/x86_instruction_emulator/Makefile
@@ -26,13 +26,15 @@ GCOV_FLAGS := --coverage
 	$(CC) -c $(CFLAGS) $(GCOV_FLAGS) $< -o $@
 
 x86.h := $(addprefix $(XEN_ROOT)/tools/include/xen/asm/,\
-                     x86-vendors.h x86-defns.h msr-index.h)
+                     x86-vendors.h x86-defns.h msr-index.h) \
+         $(addprefix $(XEN_ROOT)/tools/include/xen/lib/x86/, \
+                     cpuid.h cpuid-autogen.h)
 x86_emulate.h := x86-emulate.h x86_emulate/x86_emulate.h $(x86.h)
 
 # x86-emulate.c will be implicit for both
 x86-emulate.o x86-emulate-cov.o: x86_emulate/x86_emulate.c $(x86_emulate.h)
 
-fuzz-emul.o fuzz-emulate-cov.o wrappers.o: $(x86_emulate.h)
+fuzz-emul.o fuzz-emulate-cov.o cpuid.o wrappers.o: $(x86_emulate.h)
 
 x86-insn-fuzzer.a: fuzz-emul.o x86-emulate.o cpuid.o
 	$(AR) rc $@ $^
@@ -43,6 +45,11 @@ afl-harness: afl-harness.o fuzz-emul.o x
 afl-harness-cov: afl-harness-cov.o fuzz-emul-cov.o x86-emulate-cov.o cpuid.o wrappers.o
 	$(CC) $(CFLAGS) $(GCOV_FLAGS) $^ -o $@
 
+ifeq ($(MAKELEVEL),0)
+$(XEN_ROOT)/tools/include/xen/lib/x86/cpuid-autogen.h: FORCE
+	$(MAKE) -C $(XEN_ROOT)/tools/include build
+endif
+
 # Common targets
 .PHONY: all
 all: x86-insn-fuzz-all
@@ -60,6 +67,9 @@ install: all
 
 .PHONY: uninstall
 
+.PHONY: FORCE
+FORCE:
+
 .PHONY: afl
 afl: afl-harness
 
--- a/tools/tests/x86_emulator/Makefile
+++ b/tools/tests/x86_emulator/Makefile
@@ -11,6 +11,9 @@ all:
 run: $(TARGET)
 	./$(TARGET)
 
+.PHONY: FORCE
+FORCE:
+
 # Add libx86 to the build
 vpath %.c $(XEN_ROOT)/xen/lib/x86
 
@@ -208,13 +211,20 @@ $(call cc-option-add,HOSTCFLAGS-x86_64,H
 HOSTCFLAGS += $(CFLAGS_xeninclude) -I. $(HOSTCFLAGS-$(XEN_COMPILE_ARCH))
 
 x86.h := $(addprefix $(XEN_ROOT)/tools/include/xen/asm/,\
-                     x86-vendors.h x86-defns.h msr-index.h)
+                     x86-vendors.h x86-defns.h msr-index.h) \
+         $(addprefix $(XEN_ROOT)/tools/include/xen/lib/x86/, \
+                     cpuid.h cpuid-autogen.h)
 x86_emulate.h := x86-emulate.h x86_emulate/x86_emulate.h $(x86.h)
 
-x86-emulate.o test_x86_emulator.o evex-disp8.o wrappers.o: %.o: %.c $(x86_emulate.h)
+x86-emulate.o cpuid.o test_x86_emulator.o evex-disp8.o wrappers.o: %.o: %.c $(x86_emulate.h)
 	$(HOSTCC) $(HOSTCFLAGS) -c -g -o $@ $<
 
 x86-emulate.o: x86_emulate/x86_emulate.c
 x86-emulate.o: HOSTCFLAGS += -D__XEN_TOOLS__
 
 test_x86_emulator.o: $(addsuffix .h,$(TESTCASES)) $(addsuffix -opmask.h,$(OPMASK))
+
+ifeq ($(MAKELEVEL),0)
+$(XEN_ROOT)/tools/include/xen/lib/x86/cpuid-autogen.h: FORCE
+	$(MAKE) -C $(XEN_ROOT)/tools/include build
+endif





_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2.1 1/2] tools: fix build dependency upon generated header(s)

[Wei Liu]

On Fri, Jan 25, 2019 at 12:33:49AM -0700, Jan Beulich wrote:
> Commit fd35f32b4b ("tools/x86emul: Use struct cpuid_policy in the
> userspace test harnesses") didn't account for the dependencies of
> cpuid-autogen.h to potentially change between incremental builds.
> Putting the make invocation to produce the header together with the
> directory tree creation therefore does not work. Introduce a separate
> goal.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2.1 2/2] x86emul: fix test harness and fuzzer build dependencies

[Wei Liu]

On Fri, Jan 25, 2019 at 12:34:18AM -0700, Jan Beulich wrote:
> Commit fd35f32b4b ("tools/x86emul: Use struct cpuid_policy in the
> userspace test harnesses") didn't account for the dependencies of
> cpuid-autogen.h to potentially change between incremental builds.
> In particular the harness has a "run" goal which is supposed to be
> usable independently of the rest of the tools sub-tree building, and
> both the harness and the fuzzer code are also supposed to be buildable
> independently. Therefore they need to recursively invoke make to re-
> build the generated header if needed, but only when these rules did not
> get invoked recursively themselves.
> 
> Finally cpuid.o did not have any dependencies added for it.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> v2.1: Split controversial parts from (hopefully) non-controversial ones.
> v2: Guard $(MAKE) invocations by $(MAKELEVEL) checks.
> 
> --- a/tools/fuzz/x86_instruction_emulator/Makefile
> +++ b/tools/fuzz/x86_instruction_emulator/Makefile
> @@ -26,13 +26,15 @@ GCOV_FLAGS := --coverage
>  	$(CC) -c $(CFLAGS) $(GCOV_FLAGS) $< -o $@
>  
>  x86.h := $(addprefix $(XEN_ROOT)/tools/include/xen/asm/,\
> -                     x86-vendors.h x86-defns.h msr-index.h)
> +                     x86-vendors.h x86-defns.h msr-index.h) \
> +         $(addprefix $(XEN_ROOT)/tools/include/xen/lib/x86/, \
> +                     cpuid.h cpuid-autogen.h)
>  x86_emulate.h := x86-emulate.h x86_emulate/x86_emulate.h $(x86.h)
>  
>  # x86-emulate.c will be implicit for both
>  x86-emulate.o x86-emulate-cov.o: x86_emulate/x86_emulate.c $(x86_emulate.h)
>  
> -fuzz-emul.o fuzz-emulate-cov.o wrappers.o: $(x86_emulate.h)
> +fuzz-emul.o fuzz-emulate-cov.o cpuid.o wrappers.o: $(x86_emulate.h)
>  
>  x86-insn-fuzzer.a: fuzz-emul.o x86-emulate.o cpuid.o
>  	$(AR) rc $@ $^
> @@ -43,6 +45,11 @@ afl-harness: afl-harness.o fuzz-emul.o x
>  afl-harness-cov: afl-harness-cov.o fuzz-emul-cov.o x86-emulate-cov.o cpuid.o wrappers.o
>  	$(CC) $(CFLAGS) $(GCOV_FLAGS) $^ -o $@
>  
> +ifeq ($(MAKELEVEL),0)
> +$(XEN_ROOT)/tools/include/xen/lib/x86/cpuid-autogen.h: FORCE
> +	$(MAKE) -C $(XEN_ROOT)/tools/include build
> +endif
> +

I'm not a big fan of this.  But Andrew and you are the maintainers of
this facility so I won't block this patch.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2.1 1/2] tools: fix build dependency upon generated header(s)

[Juergen Gross]

On 25/01/2019 08:33, Jan Beulich wrote:
> Commit fd35f32b4b ("tools/x86emul: Use struct cpuid_policy in the
> userspace test harnesses") didn't account for the dependencies of
> cpuid-autogen.h to potentially change between incremental builds.
> Putting the make invocation to produce the header together with the
> directory tree creation therefore does not work. Introduce a separate
> goal.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Release-acked-by: Juergen Gross <jgross@suse.com>


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2.1 2/2] x86emul: fix test harness and fuzzer build dependencies

[Juergen Gross]

On 25/01/2019 08:34, Jan Beulich wrote:
> Commit fd35f32b4b ("tools/x86emul: Use struct cpuid_policy in the
> userspace test harnesses") didn't account for the dependencies of
> cpuid-autogen.h to potentially change between incremental builds.
> In particular the harness has a "run" goal which is supposed to be
> usable independently of the rest of the tools sub-tree building, and
> both the harness and the fuzzer code are also supposed to be buildable
> independently. Therefore they need to recursively invoke make to re-
> build the generated header if needed, but only when these rules did not
> get invoked recursively themselves.
> 
> Finally cpuid.o did not have any dependencies added for it.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

Release-acked-by: Juergen Gross <jgross@suse.com>


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel