[PATCH 0/9] xen: beginnings of moving library-like code into an archive

[PATCH 0/9] xen: beginnings of moving library-like code into an archive

[Jan Beulich]

In a few cases we link in library-like functions when they're not
actually needed. While we could use Kconfig options for each one
of them, I think the better approach for such generic code is to
build it always (thus making sure a build issue can't be introduced
for these in any however exotic configuration) and then put it into
an archive, for the linker to pick up as needed. The series here
presents a first few tiny steps towards such a goal.

Not that we can't use thin archives yet, due to our tool chain
(binutils) baseline being too low.

The first patch actually isn't directly related to the rest of the
series, except that - to avoid undue redundancy - I ran into the
issue addressed there while (originally) making patch 3 convert to
using $(call if_changed,ld) "on the fly". IOW it's a full
(contextual and functional) prereq to the series.

Further almost immediate steps I'd like to take if the approach
meets no opposition are
- split and move the rest of common/lib.c,
- split and move common/string.c, dropping the need for all the
  __HAVE_ARCH_* (implying possible per-arch archives then need to
  be specified ahead of lib/lib.a on the linker command lines),
- move common/libelf/ and common/libfdt/.

1: build: use if_changed more consistently (and correctly) for prelink*.o
2: lib: split _ctype[] into its own object, under lib/
3: lib: collect library files in an archive
4: lib: move list sorting code
5: lib: move parse_size_and_unit()
6: lib: move init_constructors()
7: lib: move rbtree code
8: lib: move bsearch code
9: lib: move sort code

Jan

[PATCH 1/9] build: use if_changed more consistently (and correctly) for prelink*.o

[Jan Beulich]

Switch to $(call if_changed,ld) where possible; presumably not doing so
in e321576f4047 ("xen/build: start using if_changed") right away was an
oversight, as it did for Arm in (just) one case. It failed to add
prelink.o to $(targets), though, causing - judging from the observed
behavior on x86 - undue rebuilds of the final binary (because of
prelink.o getting rebuild for $(cmd_prelink.o) being empty, in turn
because of .prelink.o.cmd not getting read) during "make install-xen".

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
 xen/arch/arm/Makefile |  4 +++-
 xen/arch/x86/Makefile | 18 ++++++++++--------
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 51173d97127e..296c5e68bbc3 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -95,12 +95,14 @@ prelink_lto.o: $(ALL_OBJS)
 
 # Link it with all the binary objects
 prelink.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o
-	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
+	$(call if_changed,ld)
 else
 prelink.o: $(ALL_OBJS) FORCE
 	$(call if_changed,ld)
 endif
 
+targets += prelink.o
+
 $(TARGET)-syms: prelink.o xen.lds
 	$(LD) $(XEN_LDFLAGS) -T xen.lds -N prelink.o \
 	    $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 74152f2a0dad..9b368632fb43 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -136,19 +136,21 @@ prelink_lto.o: $(ALL_OBJS)
 	$(LD_LTO) -r -o $@ $^
 
 # Link it with all the binary objects
-prelink.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o $(EFI_OBJS-y)
-	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
+prelink.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o $(EFI_OBJS-y) FORCE
+	$(call if_changed,ld)
 
-prelink-efi.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o
-	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
+prelink-efi.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o FORCE
+	$(call if_changed,ld)
 else
-prelink.o: $(ALL_OBJS) $(EFI_OBJS-y)
-	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
+prelink.o: $(ALL_OBJS) $(EFI_OBJS-y) FORCE
+	$(call if_changed,ld)
 
-prelink-efi.o: $(ALL_OBJS)
-	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
+prelink-efi.o: $(ALL_OBJS) FORCE
+	$(call if_changed,ld)
 endif
 
+targets += prelink.o prelink-efi.o
+
 $(TARGET)-syms: prelink.o xen.lds
 	$(LD) $(XEN_LDFLAGS) -T xen.lds -N prelink.o $(build_id_linker) \
 	    $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0
-- 
2.22.0

[PATCH 2/9] lib: split _ctype[] into its own object, under lib/

[Jan Beulich]

This is, besides for tidying, in preparation of then starting to use an
archive rather than an object file for generic library code which
arch-es (or even specific configurations within a single arch) may or
may not need.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
 xen/Makefile     |  3 ++-
 xen/Rules.mk     |  2 +-
 xen/common/lib.c | 29 -----------------------------
 xen/lib/Makefile |  1 +
 xen/lib/ctype.c  | 38 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 42 insertions(+), 31 deletions(-)
 create mode 100644 xen/lib/ctype.c

diff --git a/xen/Makefile b/xen/Makefile
index bf0c804d4352..73bdc326c549 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -331,6 +331,7 @@ _clean: delete-unfresh-files
 	$(MAKE) $(clean) include
 	$(MAKE) $(clean) common
 	$(MAKE) $(clean) drivers
+	$(MAKE) $(clean) lib
 	$(MAKE) $(clean) xsm
 	$(MAKE) $(clean) crypto
 	$(MAKE) $(clean) arch/arm
@@ -414,7 +415,7 @@ include/asm-$(TARGET_ARCH)/asm-offsets.h: arch/$(TARGET_ARCH)/asm-offsets.s
 	  echo ""; \
 	  echo "#endif") <$< >$@
 
-SUBDIRS = xsm arch/$(TARGET_ARCH) common drivers test
+SUBDIRS = xsm arch/$(TARGET_ARCH) common drivers lib test
 define all_sources
     ( find include/asm-$(TARGET_ARCH) -name '*.h' -print; \
       find include -name 'asm-*' -prune -o -name '*.h' -print; \
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 891c94e6ad00..333e19bec343 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -36,7 +36,7 @@ TARGET := $(BASEDIR)/xen
 # Note that link order matters!
 ALL_OBJS-y               += $(BASEDIR)/common/built_in.o
 ALL_OBJS-y               += $(BASEDIR)/drivers/built_in.o
-ALL_OBJS-$(CONFIG_X86)   += $(BASEDIR)/lib/built_in.o
+ALL_OBJS-y               += $(BASEDIR)/lib/built_in.o
 ALL_OBJS-y               += $(BASEDIR)/xsm/built_in.o
 ALL_OBJS-y               += $(BASEDIR)/arch/$(TARGET_ARCH)/built_in.o
 ALL_OBJS-$(CONFIG_CRYPTO)   += $(BASEDIR)/crypto/built_in.o
diff --git a/xen/common/lib.c b/xen/common/lib.c
index b2b799da44c5..a224efa8f6e8 100644
--- a/xen/common/lib.c
+++ b/xen/common/lib.c
@@ -1,37 +1,8 @@
-
-#include <xen/ctype.h>
 #include <xen/lib.h>
 #include <xen/types.h>
 #include <xen/init.h>
 #include <asm/byteorder.h>
 
-/* for ctype.h */
-const unsigned char _ctype[] = {
-    _C,_C,_C,_C,_C,_C,_C,_C,                        /* 0-7 */
-    _C,_C|_S,_C|_S,_C|_S,_C|_S,_C|_S,_C,_C,         /* 8-15 */
-    _C,_C,_C,_C,_C,_C,_C,_C,                        /* 16-23 */
-    _C,_C,_C,_C,_C,_C,_C,_C,                        /* 24-31 */
-    _S|_SP,_P,_P,_P,_P,_P,_P,_P,                    /* 32-39 */
-    _P,_P,_P,_P,_P,_P,_P,_P,                        /* 40-47 */
-    _D,_D,_D,_D,_D,_D,_D,_D,                        /* 48-55 */
-    _D,_D,_P,_P,_P,_P,_P,_P,                        /* 56-63 */
-    _P,_U|_X,_U|_X,_U|_X,_U|_X,_U|_X,_U|_X,_U,      /* 64-71 */
-    _U,_U,_U,_U,_U,_U,_U,_U,                        /* 72-79 */
-    _U,_U,_U,_U,_U,_U,_U,_U,                        /* 80-87 */
-    _U,_U,_U,_P,_P,_P,_P,_P,                        /* 88-95 */
-    _P,_L|_X,_L|_X,_L|_X,_L|_X,_L|_X,_L|_X,_L,      /* 96-103 */
-    _L,_L,_L,_L,_L,_L,_L,_L,                        /* 104-111 */
-    _L,_L,_L,_L,_L,_L,_L,_L,                        /* 112-119 */
-    _L,_L,_L,_P,_P,_P,_P,_C,                        /* 120-127 */
-    0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,                /* 128-143 */
-    0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,                /* 144-159 */
-    _S|_SP,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,   /* 160-175 */
-    _P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,       /* 176-191 */
-    _U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,       /* 192-207 */
-    _U,_U,_U,_U,_U,_U,_U,_P,_U,_U,_U,_U,_U,_U,_U,_L,       /* 208-223 */
-    _L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,       /* 224-239 */
-    _L,_L,_L,_L,_L,_L,_L,_P,_L,_L,_L,_L,_L,_L,_L,_L};      /* 240-255 */
-
 /*
  * A couple of 64 bit operations ported from FreeBSD.
  * The code within the '#if BITS_PER_LONG == 32' block below, and no other
diff --git a/xen/lib/Makefile b/xen/lib/Makefile
index 7019ca00e8fd..53b1da025e0d 100644
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -1 +1,2 @@
+obj-y += ctype.o
 obj-$(CONFIG_X86) += x86/
diff --git a/xen/lib/ctype.c b/xen/lib/ctype.c
new file mode 100644
index 000000000000..7b233a335fdf
--- /dev/null
+++ b/xen/lib/ctype.c
@@ -0,0 +1,38 @@
+#include <xen/ctype.h>
+
+/* for ctype.h */
+const unsigned char _ctype[] = {
+    _C,_C,_C,_C,_C,_C,_C,_C,                        /* 0-7 */
+    _C,_C|_S,_C|_S,_C|_S,_C|_S,_C|_S,_C,_C,         /* 8-15 */
+    _C,_C,_C,_C,_C,_C,_C,_C,                        /* 16-23 */
+    _C,_C,_C,_C,_C,_C,_C,_C,                        /* 24-31 */
+    _S|_SP,_P,_P,_P,_P,_P,_P,_P,                    /* 32-39 */
+    _P,_P,_P,_P,_P,_P,_P,_P,                        /* 40-47 */
+    _D,_D,_D,_D,_D,_D,_D,_D,                        /* 48-55 */
+    _D,_D,_P,_P,_P,_P,_P,_P,                        /* 56-63 */
+    _P,_U|_X,_U|_X,_U|_X,_U|_X,_U|_X,_U|_X,_U,      /* 64-71 */
+    _U,_U,_U,_U,_U,_U,_U,_U,                        /* 72-79 */
+    _U,_U,_U,_U,_U,_U,_U,_U,                        /* 80-87 */
+    _U,_U,_U,_P,_P,_P,_P,_P,                        /* 88-95 */
+    _P,_L|_X,_L|_X,_L|_X,_L|_X,_L|_X,_L|_X,_L,      /* 96-103 */
+    _L,_L,_L,_L,_L,_L,_L,_L,                        /* 104-111 */
+    _L,_L,_L,_L,_L,_L,_L,_L,                        /* 112-119 */
+    _L,_L,_L,_P,_P,_P,_P,_C,                        /* 120-127 */
+    0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,                /* 128-143 */
+    0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,                /* 144-159 */
+    _S|_SP,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,   /* 160-175 */
+    _P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,       /* 176-191 */
+    _U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,       /* 192-207 */
+    _U,_U,_U,_U,_U,_U,_U,_P,_U,_U,_U,_U,_U,_U,_U,_L,       /* 208-223 */
+    _L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,       /* 224-239 */
+    _L,_L,_L,_L,_L,_L,_L,_P,_L,_L,_L,_L,_L,_L,_L,_L};      /* 240-255 */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
2.22.0

[PATCH 3/9] lib: collect library files in an archive

[Jan Beulich]

In order to (subsequently) drop odd things like CONFIG_NEEDS_LIST_SORT
just to avoid bloating binaries when only some arch-es and/or
configurations need generic library routines, combine objects under lib/
into an archive, which the linker then can pick the necessary objects
out of.

Note that we can't use thin archives just yet, until we've raised the
minimum required binutils version suitably.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
 xen/Rules.mk          | 33 +++++++++++++++++++++++++++------
 xen/arch/arm/Makefile |  6 +++---
 xen/arch/x86/Makefile |  8 ++++----
 xen/lib/Makefile      |  3 ++-
 4 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 333e19bec343..e59c7f213f77 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -41,12 +41,16 @@ ALL_OBJS-y               += $(BASEDIR)/xsm/built_in.o
 ALL_OBJS-y               += $(BASEDIR)/arch/$(TARGET_ARCH)/built_in.o
 ALL_OBJS-$(CONFIG_CRYPTO)   += $(BASEDIR)/crypto/built_in.o
 
+ALL_LIBS-y               := $(BASEDIR)/lib/lib.a
+
 # Initialise some variables
+lib-y :=
 targets :=
 CFLAGS-y :=
 AFLAGS-y :=
 
 ALL_OBJS := $(ALL_OBJS-y)
+ALL_LIBS := $(ALL_LIBS-y)
 
 SPECIAL_DATA_SECTIONS := rodata $(foreach a,1 2 4 8 16, \
                                             $(foreach w,1 2 4, \
@@ -60,7 +64,14 @@ include Makefile
 # ---------------------------------------------------------------------------
 
 quiet_cmd_ld = LD      $@
-cmd_ld = $(LD) $(XEN_LDFLAGS) -r -o $@ $(real-prereqs)
+cmd_ld = $(LD) $(XEN_LDFLAGS) -r -o $@ $(filter-out %.a,$(real-prereqs)) \
+               --start-group $(filter %.a,$(real-prereqs)) --end-group
+
+# Archive
+# ---------------------------------------------------------------------------
+
+quiet_cmd_ar = AR      $@
+cmd_ar = rm -f $@; $(AR) cPrs $@ $(real-prereqs)
 
 # Objcopy
 # ---------------------------------------------------------------------------
@@ -86,6 +97,10 @@ obj-y    := $(patsubst %/, %/built_in.o, $(obj-y))
 # tell kbuild to descend
 subdir-obj-y := $(filter %/built_in.o, $(obj-y))
 
+# Libraries are always collected in one lib file.
+# Filter out objects already built-in
+lib-y := $(filter-out $(obj-y), $(sort $(lib-y)))
+
 $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS-y += -DINIT_SECTIONS_ONLY
 
 ifeq ($(CONFIG_COVERAGE),y)
@@ -129,19 +144,25 @@ include $(BASEDIR)/arch/$(TARGET_ARCH)/Rules.mk
 c_flags += $(CFLAGS-y)
 a_flags += $(CFLAGS-y) $(AFLAGS-y)
 
-built_in.o: $(obj-y) $(extra-y)
+built_in.o: $(obj-y) $(if $(strip $(lib-y)),lib.a) $(extra-y)
 ifeq ($(obj-y),)
 	$(CC) $(c_flags) -c -x c /dev/null -o $@
 else
 ifeq ($(CONFIG_LTO),y)
-	$(LD_LTO) -r -o $@ $(filter-out $(extra-y),$^)
+	$(LD_LTO) -r -o $@ $(filter-out lib.a $(extra-y),$^)
 else
-	$(LD) $(XEN_LDFLAGS) -r -o $@ $(filter-out $(extra-y),$^)
+	$(LD) $(XEN_LDFLAGS) -r -o $@ $(filter-out lib.a $(extra-y),$^)
 endif
 endif
 
+lib.a: $(lib-y) FORCE
+	$(call if_changed,ar)
+
 targets += built_in.o
-targets += $(filter-out $(subdir-obj-y), $(obj-y)) $(extra-y)
+ifneq ($(strip $(lib-y)),)
+targets += lib.a
+endif
+targets += $(filter-out $(subdir-obj-y), $(obj-y) $(lib-y)) $(extra-y)
 targets += $(MAKECMDGOALS)
 
 built_in_bin.o: $(obj-bin-y) $(extra-y)
@@ -155,7 +176,7 @@ endif
 PHONY += FORCE
 FORCE:
 
-%/built_in.o: FORCE
+%/built_in.o %/lib.a: FORCE
 	$(MAKE) -f $(BASEDIR)/Rules.mk -C $* built_in.o
 
 %/built_in_bin.o: FORCE
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 296c5e68bbc3..612a83b315c8 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -90,14 +90,14 @@ endif
 
 ifeq ($(CONFIG_LTO),y)
 # Gather all LTO objects together
-prelink_lto.o: $(ALL_OBJS)
-	$(LD_LTO) -r -o $@ $^
+prelink_lto.o: $(ALL_OBJS) $(ALL_LIBS)
+	$(LD_LTO) -r -o $@ $(filter-out %.a,$^) --start-group $(filter %.a,$^) --end-group
 
 # Link it with all the binary objects
 prelink.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o
 	$(call if_changed,ld)
 else
-prelink.o: $(ALL_OBJS) FORCE
+prelink.o: $(ALL_OBJS) $(ALL_LIBS) FORCE
 	$(call if_changed,ld)
 endif
 
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 9b368632fb43..8f2180485b2b 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -132,8 +132,8 @@ EFI_OBJS-$(XEN_BUILD_EFI) := efi/relocs-dummy.o
 
 ifeq ($(CONFIG_LTO),y)
 # Gather all LTO objects together
-prelink_lto.o: $(ALL_OBJS)
-	$(LD_LTO) -r -o $@ $^
+prelink_lto.o: $(ALL_OBJS) $(ALL_LIBS)
+	$(LD_LTO) -r -o $@ $(filter-out %.a,$^) --start-group $(filter %.a,$^) --end-group
 
 # Link it with all the binary objects
 prelink.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o $(EFI_OBJS-y) FORCE
@@ -142,10 +142,10 @@ prelink.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o $
 prelink-efi.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o FORCE
 	$(call if_changed,ld)
 else
-prelink.o: $(ALL_OBJS) $(EFI_OBJS-y) FORCE
+prelink.o: $(ALL_OBJS) $(ALL_LIBS) $(EFI_OBJS-y) FORCE
 	$(call if_changed,ld)
 
-prelink-efi.o: $(ALL_OBJS) FORCE
+prelink-efi.o: $(ALL_OBJS) $(ALL_LIBS) FORCE
 	$(call if_changed,ld)
 endif
 
diff --git a/xen/lib/Makefile b/xen/lib/Makefile
index 53b1da025e0d..b8814361d63e 100644
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -1,2 +1,3 @@
-obj-y += ctype.o
 obj-$(CONFIG_X86) += x86/
+
+lib-y += ctype.o
-- 
2.22.0

[PATCH 4/9] lib: move list sorting code

[Jan Beulich]

Build the source file always, as by putting it into an archive it still
won't be linked into final binaries when not needed. This way possible
build breakage will be easier to notice, and it's more consistent with
us unconditionally building other library kind of code (e.g. sort() or
bsearch()).

While moving the source file, take the opportunity and drop the
pointless EXPORT_SYMBOL().

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
 xen/arch/arm/Kconfig                        | 4 +---
 xen/common/Kconfig                          | 3 ---
 xen/common/Makefile                         | 1 -
 xen/lib/Makefile                            | 1 +
 xen/{common/list_sort.c => lib/list-sort.c} | 2 --
 5 files changed, 2 insertions(+), 9 deletions(-)
 rename xen/{common/list_sort.c => lib/list-sort.c} (98%)

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 277738826581..cb7e2523b6de 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -56,9 +56,7 @@ config HVM
         def_bool y
 
 config NEW_VGIC
-	bool
-	prompt "Use new VGIC implementation"
-	select NEEDS_LIST_SORT
+	bool "Use new VGIC implementation"
 	---help---
 
 	This is an alternative implementation of the ARM GIC interrupt
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 15e3b79ff57f..87e99d4ba2f7 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -66,9 +66,6 @@ config HAS_SCHED_GRANULARITY
 config NEEDS_LIBELF
 	bool
 
-config NEEDS_LIST_SORT
-	bool
-
 menu "Speculative hardening"
 
 config SPECULATIVE_HARDEN_ARRAY
diff --git a/xen/common/Makefile b/xen/common/Makefile
index b3b60a1ba25b..958ad8c7d946 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -21,7 +21,6 @@ obj-y += keyhandler.o
 obj-$(CONFIG_KEXEC) += kexec.o
 obj-$(CONFIG_KEXEC) += kimage.o
 obj-y += lib.o
-obj-$(CONFIG_NEEDS_LIST_SORT) += list_sort.o
 obj-$(CONFIG_LIVEPATCH) += livepatch.o livepatch_elf.o
 obj-$(CONFIG_MEM_ACCESS) += mem_access.o
 obj-y += memory.o
diff --git a/xen/lib/Makefile b/xen/lib/Makefile
index b8814361d63e..764f3624b5f9 100644
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -1,3 +1,4 @@
 obj-$(CONFIG_X86) += x86/
 
 lib-y += ctype.o
+lib-y += list-sort.o
diff --git a/xen/common/list_sort.c b/xen/lib/list-sort.c
similarity index 98%
rename from xen/common/list_sort.c
rename to xen/lib/list-sort.c
index af2b2f6519f1..f8d8bbf28178 100644
--- a/xen/common/list_sort.c
+++ b/xen/lib/list-sort.c
@@ -15,7 +15,6 @@
  * this program; If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include <xen/lib.h>
 #include <xen/list.h>
 
 #define MAX_LIST_LENGTH_BITS 20
@@ -154,4 +153,3 @@ void list_sort(void *priv, struct list_head *head,
 
 	merge_and_restore_back_links(priv, cmp, head, part[max_lev], list);
 }
-EXPORT_SYMBOL(list_sort);
-- 
2.22.0

[PATCH 5/9] lib: move parse_size_and_unit()

[Jan Beulich]

... into its own CU, to build it into an archive.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
 xen/common/lib.c     | 39 ----------------------------------
 xen/lib/Makefile     |  1 +
 xen/lib/parse-size.c | 50 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 51 insertions(+), 39 deletions(-)
 create mode 100644 xen/lib/parse-size.c

diff --git a/xen/common/lib.c b/xen/common/lib.c
index a224efa8f6e8..6cfa332142a5 100644
--- a/xen/common/lib.c
+++ b/xen/common/lib.c
@@ -423,45 +423,6 @@ uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
 #endif
 }
 
-unsigned long long parse_size_and_unit(const char *s, const char **ps)
-{
-    unsigned long long ret;
-    const char *s1;
-
-    ret = simple_strtoull(s, &s1, 0);
-
-    switch ( *s1 )
-    {
-    case 'T': case 't':
-        ret <<= 10;
-        /* fallthrough */
-    case 'G': case 'g':
-        ret <<= 10;
-        /* fallthrough */
-    case 'M': case 'm':
-        ret <<= 10;
-        /* fallthrough */
-    case 'K': case 'k':
-        ret <<= 10;
-        /* fallthrough */
-    case 'B': case 'b':
-        s1++;
-        break;
-    case '%':
-        if ( ps )
-            break;
-        /* fallthrough */
-    default:
-        ret <<= 10; /* default to kB */
-        break;
-    }
-
-    if ( ps != NULL )
-        *ps = s1;
-
-    return ret;
-}
-
 typedef void (*ctor_func_t)(void);
 extern const ctor_func_t __ctors_start[], __ctors_end[];
 
diff --git a/xen/lib/Makefile b/xen/lib/Makefile
index 764f3624b5f9..99f857540c99 100644
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -2,3 +2,4 @@ obj-$(CONFIG_X86) += x86/
 
 lib-y += ctype.o
 lib-y += list-sort.o
+lib-y += parse-size.o
diff --git a/xen/lib/parse-size.c b/xen/lib/parse-size.c
new file mode 100644
index 000000000000..ec980cadfff3
--- /dev/null
+++ b/xen/lib/parse-size.c
@@ -0,0 +1,50 @@
+#include <xen/lib.h>
+
+unsigned long long parse_size_and_unit(const char *s, const char **ps)
+{
+    unsigned long long ret;
+    const char *s1;
+
+    ret = simple_strtoull(s, &s1, 0);
+
+    switch ( *s1 )
+    {
+    case 'T': case 't':
+        ret <<= 10;
+        /* fallthrough */
+    case 'G': case 'g':
+        ret <<= 10;
+        /* fallthrough */
+    case 'M': case 'm':
+        ret <<= 10;
+        /* fallthrough */
+    case 'K': case 'k':
+        ret <<= 10;
+        /* fallthrough */
+    case 'B': case 'b':
+        s1++;
+        break;
+    case '%':
+        if ( ps )
+            break;
+        /* fallthrough */
+    default:
+        ret <<= 10; /* default to kB */
+        break;
+    }
+
+    if ( ps != NULL )
+        *ps = s1;
+
+    return ret;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
2.22.0

[PATCH 6/9] lib: move init_constructors()

[Jan Beulich]

... into its own CU, for being unrelated to other things in
common/lib.c. For now it gets compiled into built_in.o rather than
lib.a, as it gets used unconditionally by Arm's as well as x86'es
{,__}start_xen(). But this could be changed in principle, the more that
there typically aren't any constructors anyway. Then again it's just
__init code anyway.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
 xen/common/lib.c | 14 --------------
 xen/lib/Makefile |  1 +
 xen/lib/ctors.c  | 25 +++++++++++++++++++++++++
 3 files changed, 26 insertions(+), 14 deletions(-)
 create mode 100644 xen/lib/ctors.c

diff --git a/xen/common/lib.c b/xen/common/lib.c
index 6cfa332142a5..f5ca179a0af4 100644
--- a/xen/common/lib.c
+++ b/xen/common/lib.c
@@ -1,6 +1,5 @@
 #include <xen/lib.h>
 #include <xen/types.h>
-#include <xen/init.h>
 #include <asm/byteorder.h>
 
 /*
@@ -423,19 +422,6 @@ uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
 #endif
 }
 
-typedef void (*ctor_func_t)(void);
-extern const ctor_func_t __ctors_start[], __ctors_end[];
-
-void __init init_constructors(void)
-{
-    const ctor_func_t *f;
-    for ( f = __ctors_start; f < __ctors_end; ++f )
-        (*f)();
-
-    /* Putting this here seems as good (or bad) as any other place. */
-    BUILD_BUG_ON(sizeof(size_t) != sizeof(ssize_t));
-}
-
 /*
  * Local variables:
  * mode: C
diff --git a/xen/lib/Makefile b/xen/lib/Makefile
index 99f857540c99..ba1fb7bcdee2 100644
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -1,3 +1,4 @@
+obj-y += ctors.o
 obj-$(CONFIG_X86) += x86/
 
 lib-y += ctype.o
diff --git a/xen/lib/ctors.c b/xen/lib/ctors.c
new file mode 100644
index 000000000000..5bdc591cd50a
--- /dev/null
+++ b/xen/lib/ctors.c
@@ -0,0 +1,25 @@
+#include <xen/init.h>
+#include <xen/lib.h>
+
+typedef void (*ctor_func_t)(void);
+extern const ctor_func_t __ctors_start[], __ctors_end[];
+
+void __init init_constructors(void)
+{
+    const ctor_func_t *f;
+    for ( f = __ctors_start; f < __ctors_end; ++f )
+        (*f)();
+
+    /* Putting this here seems as good (or bad) as any other place. */
+    BUILD_BUG_ON(sizeof(size_t) != sizeof(ssize_t));
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
2.22.0

[PATCH 7/9] lib: move rbtree code

[Jan Beulich]

Build this code into an archive, which results in not linking it into
x86 final binaries. This saves about 1.5k of dead code.

While moving the source file, take the opportunity and drop the
pointless EXPORT_SYMBOL().

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
 xen/common/Makefile          | 1 -
 xen/lib/Makefile             | 1 +
 xen/{common => lib}/rbtree.c | 9 +--------
 3 files changed, 2 insertions(+), 9 deletions(-)
 rename xen/{common => lib}/rbtree.c (98%)

diff --git a/xen/common/Makefile b/xen/common/Makefile
index 958ad8c7d946..46406dccdfd4 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -33,7 +33,6 @@ obj-y += preempt.o
 obj-y += random.o
 obj-y += rangeset.o
 obj-y += radix-tree.o
-obj-y += rbtree.o
 obj-y += rcupdate.o
 obj-y += rwlock.o
 obj-y += shutdown.o
diff --git a/xen/lib/Makefile b/xen/lib/Makefile
index ba1fb7bcdee2..b469d2dff7b8 100644
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_X86) += x86/
 lib-y += ctype.o
 lib-y += list-sort.o
 lib-y += parse-size.o
+lib-y += rbtree.o
diff --git a/xen/common/rbtree.c b/xen/lib/rbtree.c
similarity index 98%
rename from xen/common/rbtree.c
rename to xen/lib/rbtree.c
index 9f5498a89d4e..95e045d52461 100644
--- a/xen/common/rbtree.c
+++ b/xen/lib/rbtree.c
@@ -25,7 +25,7 @@
 #include <xen/rbtree.h>
 
 /*
- * red-black trees properties:  http://en.wikipedia.org/wiki/Rbtree 
+ * red-black trees properties:  http://en.wikipedia.org/wiki/Rbtree
  *
  *  1) A node is either red or black
  *  2) The root is black
@@ -223,7 +223,6 @@ void rb_insert_color(struct rb_node *node, struct rb_root *root)
 		}
 	}
 }
-EXPORT_SYMBOL(rb_insert_color);
 
 static void __rb_erase_color(struct rb_node *parent, struct rb_root *root)
 {
@@ -467,7 +466,6 @@ void rb_erase(struct rb_node *node, struct rb_root *root)
 	if (rebalance)
 		__rb_erase_color(rebalance, root);
 }
-EXPORT_SYMBOL(rb_erase);
 
 /*
  * This function returns the first node (in sort order) of the tree.
@@ -483,7 +481,6 @@ struct rb_node *rb_first(const struct rb_root *root)
 		n = n->rb_left;
 	return n;
 }
-EXPORT_SYMBOL(rb_first);
 
 struct rb_node *rb_last(const struct rb_root *root)
 {
@@ -496,7 +493,6 @@ struct rb_node *rb_last(const struct rb_root *root)
 		n = n->rb_right;
 	return n;
 }
-EXPORT_SYMBOL(rb_last);
 
 struct rb_node *rb_next(const struct rb_node *node)
 {
@@ -528,7 +524,6 @@ struct rb_node *rb_next(const struct rb_node *node)
 
 	return parent;
 }
-EXPORT_SYMBOL(rb_next);
 
 struct rb_node *rb_prev(const struct rb_node *node)
 {
@@ -557,7 +552,6 @@ struct rb_node *rb_prev(const struct rb_node *node)
 
 	return parent;
 }
-EXPORT_SYMBOL(rb_prev);
 
 void rb_replace_node(struct rb_node *victim, struct rb_node *new,
 		     struct rb_root *root)
@@ -574,4 +568,3 @@ void rb_replace_node(struct rb_node *victim, struct rb_node *new,
 	/* Copy the pointers/colour from the victim to the replacement */
 	*new = *victim;
 }
-EXPORT_SYMBOL(rb_replace_node);
-- 
2.22.0

[PATCH 8/9] lib: move bsearch code

[Jan Beulich]

Build this code into an archive, which results in not linking it into
x86 final binaries. This saves a little bit of dead code.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
 xen/common/Makefile           | 1 -
 xen/lib/Makefile              | 1 +
 xen/{common => lib}/bsearch.c | 0
 3 files changed, 1 insertion(+), 1 deletion(-)
 rename xen/{common => lib}/bsearch.c (100%)

diff --git a/xen/common/Makefile b/xen/common/Makefile
index 46406dccdfd4..149466b473a8 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -1,6 +1,5 @@
 obj-$(CONFIG_ARGO) += argo.o
 obj-y += bitmap.o
-obj-y += bsearch.o
 obj-$(CONFIG_HYPFS_CONFIG) += config_data.o
 obj-$(CONFIG_CORE_PARKING) += core_parking.o
 obj-y += cpu.o
diff --git a/xen/lib/Makefile b/xen/lib/Makefile
index b469d2dff7b8..122eeb3d327b 100644
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -1,6 +1,7 @@
 obj-y += ctors.o
 obj-$(CONFIG_X86) += x86/
 
+lib-y += bsearch.o
 lib-y += ctype.o
 lib-y += list-sort.o
 lib-y += parse-size.o
diff --git a/xen/common/bsearch.c b/xen/lib/bsearch.c
similarity index 100%
rename from xen/common/bsearch.c
rename to xen/lib/bsearch.c
-- 
2.22.0

[PATCH 9/9] lib: move sort code

[Jan Beulich]

Build this code into an archive, to parallel bsearch().

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
 xen/common/Makefile        | 1 -
 xen/lib/Makefile           | 1 +
 xen/{common => lib}/sort.c | 0
 3 files changed, 1 insertion(+), 1 deletion(-)
 rename xen/{common => lib}/sort.c (100%)

diff --git a/xen/common/Makefile b/xen/common/Makefile
index 149466b473a8..efcfd4c34ecf 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -36,7 +36,6 @@ obj-y += rcupdate.o
 obj-y += rwlock.o
 obj-y += shutdown.o
 obj-y += softirq.o
-obj-y += sort.o
 obj-y += smp.o
 obj-y += spinlock.o
 obj-y += stop_machine.o
diff --git a/xen/lib/Makefile b/xen/lib/Makefile
index 122eeb3d327b..33ff322b1655 100644
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -6,3 +6,4 @@ lib-y += ctype.o
 lib-y += list-sort.o
 lib-y += parse-size.o
 lib-y += rbtree.o
+lib-y += sort.o
diff --git a/xen/common/sort.c b/xen/lib/sort.c
similarity index 100%
rename from xen/common/sort.c
rename to xen/lib/sort.c
-- 
2.22.0

Re: [PATCH 1/9] build: use if_changed more consistently (and correctly) for prelink*.o

[Roger Pau Monné]

On Mon, Sep 14, 2020 at 12:15:39PM +0200, Jan Beulich wrote:
> Switch to $(call if_changed,ld) where possible; presumably not doing so
> in e321576f4047 ("xen/build: start using if_changed") right away was an
> oversight, as it did for Arm in (just) one case. It failed to add
> prelink.o to $(targets), though, causing - judging from the observed
> behavior on x86 - undue rebuilds of the final binary (because of
> prelink.o getting rebuild for $(cmd_prelink.o) being empty, in turn
> because of .prelink.o.cmd not getting read) during "make install-xen".

I'm not sure I follow why prelink.o needs to be added to targets, does
this offer some kind of protection against rebuilds when doing make
install?

The switch to if_changed LGTM.

Thanks, Roger.

Re: [PATCH 1/9] build: use if_changed more consistently (and correctly) for prelink*.o

[Jan Beulich]

On 15.09.2020 13:56, Roger Pau Monné wrote:
> On Mon, Sep 14, 2020 at 12:15:39PM +0200, Jan Beulich wrote:
>> Switch to $(call if_changed,ld) where possible; presumably not doing so
>> in e321576f4047 ("xen/build: start using if_changed") right away was an
>> oversight, as it did for Arm in (just) one case. It failed to add
>> prelink.o to $(targets), though, causing - judging from the observed
>> behavior on x86 - undue rebuilds of the final binary (because of
>> prelink.o getting rebuild for $(cmd_prelink.o) being empty, in turn
>> because of .prelink.o.cmd not getting read) during "make install-xen".
> 
> I'm not sure I follow why prelink.o needs to be added to targets, does
> this offer some kind of protection against rebuilds when doing make
> install?

In a way, but (as I view it) not really. It is the use of ...

> The switch to if_changed LGTM.

... if_changed which requires this. .*.cmd files will only be loaded
for anything explicitly or implicitly listed as a target. While .o
coming from $(obj-y) get added there automatically, prelink.o is not
something that could be recognized as needing adding, hence the
"manual" insertion.

Without .prelink.o.cmd loaded, $(if_changed ) will always arrange
for it to get re-built, because it then will consider the command
used to build the file to have changed (as the stored one appears to
be empty).

Jan

Re: [PATCH 1/9] build: use if_changed more consistently (and correctly) for prelink*.o

[Roger Pau Monné]

On Tue, Sep 15, 2020 at 02:26:34PM +0200, Jan Beulich wrote:
> On 15.09.2020 13:56, Roger Pau Monné wrote:
> > On Mon, Sep 14, 2020 at 12:15:39PM +0200, Jan Beulich wrote:
> >> Switch to $(call if_changed,ld) where possible; presumably not doing so
> >> in e321576f4047 ("xen/build: start using if_changed") right away was an
> >> oversight, as it did for Arm in (just) one case. It failed to add
> >> prelink.o to $(targets), though, causing - judging from the observed
> >> behavior on x86 - undue rebuilds of the final binary (because of
> >> prelink.o getting rebuild for $(cmd_prelink.o) being empty, in turn
> >> because of .prelink.o.cmd not getting read) during "make install-xen".
> > 
> > I'm not sure I follow why prelink.o needs to be added to targets, does
> > this offer some kind of protection against rebuilds when doing make
> > install?
> 
> In a way, but (as I view it) not really. It is the use of ...
> 
> > The switch to if_changed LGTM.
> 
> ... if_changed which requires this. .*.cmd files will only be loaded
> for anything explicitly or implicitly listed as a target. While .o
> coming from $(obj-y) get added there automatically, prelink.o is not
> something that could be recognized as needing adding, hence the
> "manual" insertion.

This seems very prone to mistakes, as you have to remember that
whatever object that uses if_changed should also be part of the
targets set, or else it will get rebuild unconditionally.

I think adding some of the above reasoning to the commit message would
be helpful IMO.

> Without .prelink.o.cmd loaded, $(if_changed ) will always arrange
> for it to get re-built, because it then will consider the command
> used to build the file to have changed (as the stored one appears to
> be empty).

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.

Re: [PATCH 1/9] build: use if_changed more consistently (and correctly) for prelink*.o

[Jan Beulich]

On 15.09.2020 15:46, Roger Pau Monné wrote:
> On Tue, Sep 15, 2020 at 02:26:34PM +0200, Jan Beulich wrote:
>> On 15.09.2020 13:56, Roger Pau Monné wrote:
>>> On Mon, Sep 14, 2020 at 12:15:39PM +0200, Jan Beulich wrote:
>>>> Switch to $(call if_changed,ld) where possible; presumably not doing so
>>>> in e321576f4047 ("xen/build: start using if_changed") right away was an
>>>> oversight, as it did for Arm in (just) one case. It failed to add
>>>> prelink.o to $(targets), though, causing - judging from the observed
>>>> behavior on x86 - undue rebuilds of the final binary (because of
>>>> prelink.o getting rebuild for $(cmd_prelink.o) being empty, in turn
>>>> because of .prelink.o.cmd not getting read) during "make install-xen".
>>>
>>> I'm not sure I follow why prelink.o needs to be added to targets, does
>>> this offer some kind of protection against rebuilds when doing make
>>> install?
>>
>> In a way, but (as I view it) not really. It is the use of ...
>>
>>> The switch to if_changed LGTM.
>>
>> ... if_changed which requires this. .*.cmd files will only be loaded
>> for anything explicitly or implicitly listed as a target. While .o
>> coming from $(obj-y) get added there automatically, prelink.o is not
>> something that could be recognized as needing adding, hence the
>> "manual" insertion.
> 
> This seems very prone to mistakes, as you have to remember that
> whatever object that uses if_changed should also be part of the
> targets set, or else it will get rebuild unconditionally.

The use of $(if_changed ...) has further similar pitfalls: One may
not forget to add FORCE to the dependencies, and there may not be
blanks after the comma (where one would usually put one). But I think
the benefits of this construct are so significant that this extra
care that's needed is well justified. Plus this is actually mentioned
(at least in passing) in docs/misc/xen-makefiles/makefile.rst.

> I think adding some of the above reasoning to the commit message would
> be helpful IMO.

Well, I've merely re-explained what I think the commit message
already says.

>> Without .prelink.o.cmd loaded, $(if_changed ) will always arrange
>> for it to get re-built, because it then will consider the command
>> used to build the file to have changed (as the stored one appears to
>> be empty).
> 
> Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks.

Jan

Ping: [PATCH 1/9] build: use if_changed more consistently (and correctly) for prelink*.o

[Jan Beulich]

On 14.09.2020 12:15, Jan Beulich wrote:
> Switch to $(call if_changed,ld) where possible; presumably not doing so
> in e321576f4047 ("xen/build: start using if_changed") right away was an
> oversight, as it did for Arm in (just) one case. It failed to add
> prelink.o to $(targets), though, causing - judging from the observed
> behavior on x86 - undue rebuilds of the final binary (because of
> prelink.o getting rebuild for $(cmd_prelink.o) being empty, in turn
> because of .prelink.o.cmd not getting read) during "make install-xen".
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
>  xen/arch/arm/Makefile |  4 +++-
>  xen/arch/x86/Makefile | 18 ++++++++++--------
>  2 files changed, 13 insertions(+), 9 deletions(-)

May I ask for an Arm-side ack (or otherwise) here, please?

Jan

> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 51173d97127e..296c5e68bbc3 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -95,12 +95,14 @@ prelink_lto.o: $(ALL_OBJS)
>  
>  # Link it with all the binary objects
>  prelink.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o
> -	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
> +	$(call if_changed,ld)
>  else
>  prelink.o: $(ALL_OBJS) FORCE
>  	$(call if_changed,ld)
>  endif
>  
> +targets += prelink.o
> +
>  $(TARGET)-syms: prelink.o xen.lds
>  	$(LD) $(XEN_LDFLAGS) -T xen.lds -N prelink.o \
>  	    $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0
> diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
> index 74152f2a0dad..9b368632fb43 100644
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -136,19 +136,21 @@ prelink_lto.o: $(ALL_OBJS)
>  	$(LD_LTO) -r -o $@ $^
>  
>  # Link it with all the binary objects
> -prelink.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o $(EFI_OBJS-y)
> -	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
> +prelink.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o $(EFI_OBJS-y) FORCE
> +	$(call if_changed,ld)
>  
> -prelink-efi.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o
> -	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
> +prelink-efi.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o FORCE
> +	$(call if_changed,ld)
>  else
> -prelink.o: $(ALL_OBJS) $(EFI_OBJS-y)
> -	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
> +prelink.o: $(ALL_OBJS) $(EFI_OBJS-y) FORCE
> +	$(call if_changed,ld)
>  
> -prelink-efi.o: $(ALL_OBJS)
> -	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
> +prelink-efi.o: $(ALL_OBJS) FORCE
> +	$(call if_changed,ld)
>  endif
>  
> +targets += prelink.o prelink-efi.o
> +
>  $(TARGET)-syms: prelink.o xen.lds
>  	$(LD) $(XEN_LDFLAGS) -T xen.lds -N prelink.o $(build_id_linker) \
>  	    $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0
> 

Re: Ping: [PATCH 1/9] build: use if_changed more consistently (and correctly) for prelink*.o

[Julien Grall]

Hi Jan,

On 21/09/2020 11:17, Jan Beulich wrote:
> On 14.09.2020 12:15, Jan Beulich wrote:
>> Switch to $(call if_changed,ld) where possible; presumably not doing so
>> in e321576f4047 ("xen/build: start using if_changed") right away was an
>> oversight, as it did for Arm in (just) one case. It failed to add
>> prelink.o to $(targets), though, causing - judging from the observed
>> behavior on x86 - undue rebuilds of the final binary (because of
>> prelink.o getting rebuild for $(cmd_prelink.o) being empty, in turn
>> because of .prelink.o.cmd not getting read) during "make install-xen".
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>>   xen/arch/arm/Makefile |  4 +++-
>>   xen/arch/x86/Makefile | 18 ++++++++++--------
>>   2 files changed, 13 insertions(+), 9 deletions(-)
> 
> May I ask for an Arm-side ack (or otherwise) here, please?

Acked-by: Julien Grall <jgrall@amazon.com>

Cheers,

> 
> Jan
> 
>> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
>> index 51173d97127e..296c5e68bbc3 100644
>> --- a/xen/arch/arm/Makefile
>> +++ b/xen/arch/arm/Makefile
>> @@ -95,12 +95,14 @@ prelink_lto.o: $(ALL_OBJS)
>>   
>>   # Link it with all the binary objects
>>   prelink.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o
>> -	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
>> +	$(call if_changed,ld)
>>   else
>>   prelink.o: $(ALL_OBJS) FORCE
>>   	$(call if_changed,ld)
>>   endif
>>   
>> +targets += prelink.o
>> +
>>   $(TARGET)-syms: prelink.o xen.lds
>>   	$(LD) $(XEN_LDFLAGS) -T xen.lds -N prelink.o \
>>   	    $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0
>> diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
>> index 74152f2a0dad..9b368632fb43 100644
>> --- a/xen/arch/x86/Makefile
>> +++ b/xen/arch/x86/Makefile
>> @@ -136,19 +136,21 @@ prelink_lto.o: $(ALL_OBJS)
>>   	$(LD_LTO) -r -o $@ $^
>>   
>>   # Link it with all the binary objects
>> -prelink.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o $(EFI_OBJS-y)
>> -	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
>> +prelink.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o $(EFI_OBJS-y) FORCE
>> +	$(call if_changed,ld)
>>   
>> -prelink-efi.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o
>> -	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
>> +prelink-efi.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o FORCE
>> +	$(call if_changed,ld)
>>   else
>> -prelink.o: $(ALL_OBJS) $(EFI_OBJS-y)
>> -	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
>> +prelink.o: $(ALL_OBJS) $(EFI_OBJS-y) FORCE
>> +	$(call if_changed,ld)
>>   
>> -prelink-efi.o: $(ALL_OBJS)
>> -	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
>> +prelink-efi.o: $(ALL_OBJS) FORCE
>> +	$(call if_changed,ld)
>>   endif
>>   
>> +targets += prelink.o prelink-efi.o
>> +
>>   $(TARGET)-syms: prelink.o xen.lds
>>   	$(LD) $(XEN_LDFLAGS) -T xen.lds -N prelink.o $(build_id_linker) \
>>   	    $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0
>>
> 

-- 
Julien Grall

Re: Ping: [PATCH 1/9] build: use if_changed more consistently (and correctly) for prelink*.o

[Jan Beulich]

On 21.09.2020 13:39, Julien Grall wrote:
> On 21/09/2020 11:17, Jan Beulich wrote:
>> On 14.09.2020 12:15, Jan Beulich wrote:
>>> Switch to $(call if_changed,ld) where possible; presumably not doing so
>>> in e321576f4047 ("xen/build: start using if_changed") right away was an
>>> oversight, as it did for Arm in (just) one case. It failed to add
>>> prelink.o to $(targets), though, causing - judging from the observed
>>> behavior on x86 - undue rebuilds of the final binary (because of
>>> prelink.o getting rebuild for $(cmd_prelink.o) being empty, in turn
>>> because of .prelink.o.cmd not getting read) during "make install-xen".
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> ---
>>>   xen/arch/arm/Makefile |  4 +++-
>>>   xen/arch/x86/Makefile | 18 ++++++++++--------
>>>   2 files changed, 13 insertions(+), 9 deletions(-)
>>
>> May I ask for an Arm-side ack (or otherwise) here, please?
> 
> Acked-by: Julien Grall <jgrall@amazon.com>

Thanks. On the Arm side this is actually addressing a (minor) bug,
so I wonder whether I should queue this up for backporting. Do you
have an opinion either way?

Jan

Re: Ping: [PATCH 1/9] build: use if_changed more consistently (and correctly) for prelink*.o

[Julien Grall]

Hi Jan,

On 22/09/2020 09:28, Jan Beulich wrote:
> On 21.09.2020 13:39, Julien Grall wrote:
>> On 21/09/2020 11:17, Jan Beulich wrote:
>>> On 14.09.2020 12:15, Jan Beulich wrote:
>>>> Switch to $(call if_changed,ld) where possible; presumably not doing so
>>>> in e321576f4047 ("xen/build: start using if_changed") right away was an
>>>> oversight, as it did for Arm in (just) one case. It failed to add
>>>> prelink.o to $(targets), though, causing - judging from the observed
>>>> behavior on x86 - undue rebuilds of the final binary (because of
>>>> prelink.o getting rebuild for $(cmd_prelink.o) being empty, in turn
>>>> because of .prelink.o.cmd not getting read) during "make install-xen".
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>> ---
>>>>    xen/arch/arm/Makefile |  4 +++-
>>>>    xen/arch/x86/Makefile | 18 ++++++++++--------
>>>>    2 files changed, 13 insertions(+), 9 deletions(-)
>>>
>>> May I ask for an Arm-side ack (or otherwise) here, please?
>>
>> Acked-by: Julien Grall <jgrall@amazon.com>
> 
> Thanks. On the Arm side this is actually addressing a (minor) bug,

Just to confirm, the bug is: Xen will be rebuilt when it is not 
necessary, right?

Cheers,

-- 
Julien Grall

Re: Ping: [PATCH 1/9] build: use if_changed more consistently (and correctly) for prelink*.o

[Jan Beulich]

On 22.09.2020 11:24, Julien Grall wrote:
> On 22/09/2020 09:28, Jan Beulich wrote:
>> On 21.09.2020 13:39, Julien Grall wrote:
>>> On 21/09/2020 11:17, Jan Beulich wrote:
>>>> On 14.09.2020 12:15, Jan Beulich wrote:
>>>>> Switch to $(call if_changed,ld) where possible; presumably not doing so
>>>>> in e321576f4047 ("xen/build: start using if_changed") right away was an
>>>>> oversight, as it did for Arm in (just) one case. It failed to add
>>>>> prelink.o to $(targets), though, causing - judging from the observed
>>>>> behavior on x86 - undue rebuilds of the final binary (because of
>>>>> prelink.o getting rebuild for $(cmd_prelink.o) being empty, in turn
>>>>> because of .prelink.o.cmd not getting read) during "make install-xen".
>>>>>
>>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>>> ---
>>>>>    xen/arch/arm/Makefile |  4 +++-
>>>>>    xen/arch/x86/Makefile | 18 ++++++++++--------
>>>>>    2 files changed, 13 insertions(+), 9 deletions(-)
>>>>
>>>> May I ask for an Arm-side ack (or otherwise) here, please?
>>>
>>> Acked-by: Julien Grall <jgrall@amazon.com>
>>
>> Thanks. On the Arm side this is actually addressing a (minor) bug,
> 
> Just to confirm, the bug is: Xen will be rebuilt when it is not 
> necessary, right?

Yes. When building as non-root but installing as root, this would
typically involve an owner change of some of the involved files.
That's how I did notice the issue on x86 (after switching to
if_changed) in the first place.

Jan

Re: Ping: [PATCH 1/9] build: use if_changed more consistently (and correctly) for prelink*.o

[Julien Grall]

Hi Jan,

On 22/09/2020 11:55, Jan Beulich wrote:
> On 22.09.2020 11:24, Julien Grall wrote:
>> On 22/09/2020 09:28, Jan Beulich wrote:
>>> On 21.09.2020 13:39, Julien Grall wrote:
>>>> On 21/09/2020 11:17, Jan Beulich wrote:
>>>>> On 14.09.2020 12:15, Jan Beulich wrote:
>>>>>> Switch to $(call if_changed,ld) where possible; presumably not doing so
>>>>>> in e321576f4047 ("xen/build: start using if_changed") right away was an
>>>>>> oversight, as it did for Arm in (just) one case. It failed to add
>>>>>> prelink.o to $(targets), though, causing - judging from the observed
>>>>>> behavior on x86 - undue rebuilds of the final binary (because of
>>>>>> prelink.o getting rebuild for $(cmd_prelink.o) being empty, in turn
>>>>>> because of .prelink.o.cmd not getting read) during "make install-xen".
>>>>>>
>>>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>>>> ---
>>>>>>     xen/arch/arm/Makefile |  4 +++-
>>>>>>     xen/arch/x86/Makefile | 18 ++++++++++--------
>>>>>>     2 files changed, 13 insertions(+), 9 deletions(-)
>>>>>
>>>>> May I ask for an Arm-side ack (or otherwise) here, please?
>>>>
>>>> Acked-by: Julien Grall <jgrall@amazon.com>
>>>
>>> Thanks. On the Arm side this is actually addressing a (minor) bug,
>>
>> Just to confirm, the bug is: Xen will be rebuilt when it is not
>> necessary, right?
> 
> Yes. When building as non-root but installing as root, this would
> typically involve an owner change of some of the involved files.
> That's how I did notice the issue on x86 (after switching to
> if_changed) in the first place.

Thanks for the explanation. I think it would be fine to backport.
@Stefano, what do you think?

Cheers,

-- 
Julien Grall

Re: Ping: [PATCH 1/9] build: use if_changed more consistently (and correctly) for prelink*.o

[Stefano Stabellini]

On Tue, 22 Sep 2020, Julien Grall wrote:
> Hi Jan,
> 
> On 22/09/2020 11:55, Jan Beulich wrote:
> > On 22.09.2020 11:24, Julien Grall wrote:
> > > On 22/09/2020 09:28, Jan Beulich wrote:
> > > > On 21.09.2020 13:39, Julien Grall wrote:
> > > > > On 21/09/2020 11:17, Jan Beulich wrote:
> > > > > > On 14.09.2020 12:15, Jan Beulich wrote:
> > > > > > > Switch to $(call if_changed,ld) where possible; presumably not
> > > > > > > doing so
> > > > > > > in e321576f4047 ("xen/build: start using if_changed") right away
> > > > > > > was an
> > > > > > > oversight, as it did for Arm in (just) one case. It failed to add
> > > > > > > prelink.o to $(targets), though, causing - judging from the
> > > > > > > observed
> > > > > > > behavior on x86 - undue rebuilds of the final binary (because of
> > > > > > > prelink.o getting rebuild for $(cmd_prelink.o) being empty, in
> > > > > > > turn
> > > > > > > because of .prelink.o.cmd not getting read) during "make
> > > > > > > install-xen".
> > > > > > > 
> > > > > > > Signed-off-by: Jan Beulich <jbeulich@suse.com>
> > > > > > > ---
> > > > > > >     xen/arch/arm/Makefile |  4 +++-
> > > > > > >     xen/arch/x86/Makefile | 18 ++++++++++--------
> > > > > > >     2 files changed, 13 insertions(+), 9 deletions(-)
> > > > > > 
> > > > > > May I ask for an Arm-side ack (or otherwise) here, please?
> > > > > 
> > > > > Acked-by: Julien Grall <jgrall@amazon.com>
> > > > 
> > > > Thanks. On the Arm side this is actually addressing a (minor) bug,
> > > 
> > > Just to confirm, the bug is: Xen will be rebuilt when it is not
> > > necessary, right?
> > 
> > Yes. When building as non-root but installing as root, this would
> > typically involve an owner change of some of the involved files.
> > That's how I did notice the issue on x86 (after switching to
> > if_changed) in the first place.
> 
> Thanks for the explanation. I think it would be fine to backport.
> @Stefano, what do you think?

I am OK with that

Re: [PATCH 8/9] lib: move bsearch code

[Andrew Cooper]

On 14/09/2020 11:18, Jan Beulich wrote:
> Build this code into an archive, which results in not linking it into
> x86 final binaries. This saves a little bit of dead code.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

This wants to be an extern inline in the header file just like in stdlib.h.

The implementation is trivial, and much faster when the compiler can
inline the cmp() function pointer.

I doubt we actually need out-of-line implementation (except perhaps for
CONFIG_UBSAN builds or so).

~Andrew

Re: [PATCH 5/9] lib: move parse_size_and_unit()

[Andrew Cooper]

On 14/09/2020 11:17, Jan Beulich wrote:
> ... into its own CU, to build it into an archive.

CU?

Irrespective, it seems very weird to carve this out, seeing as it is
called from a number of core locations, and depends on other core
functions which aren't split out.

~Andrew

Re: [PATCH 5/9] lib: move parse_size_and_unit()

[Jan Beulich]

On 22.09.2020 21:41, Andrew Cooper wrote:
> On 14/09/2020 11:17, Jan Beulich wrote:
>> ... into its own CU, to build it into an archive.
> 
> CU?

Compilation Unit - we've been using this acronym in a number of
cases, I think.

> Irrespective, it seems very weird to carve this out, seeing as it is
> called from a number of core locations, and depends on other core
> functions which aren't split out.

As said in the cover letter, the goal is to get rid of common/lib.c
as a whole. It's a bad file name for _anything_ to live in, as from
its name you can't really derive what may or may not be (or belong)
in there.

Depending on other core functions isn't a problem at all for stuff
living in archives. It being called "from a number of core
locations" isn't a convincing argument either, as all of those could
potentially be inside some #ifdef CONFIG_*. However, if it is
believed that this would better live in an object file than in an
archive, I can easily move it from lib-y to obj-y.

Jan

Re: [PATCH 8/9] lib: move bsearch code

[Jan Beulich]

On 22.09.2020 21:34, Andrew Cooper wrote:
> On 14/09/2020 11:18, Jan Beulich wrote:
>> Build this code into an archive, which results in not linking it into
>> x86 final binaries. This saves a little bit of dead code.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> This wants to be an extern inline in the header file just like in stdlib.h.

I can move it there, but why "extern" rather than "static"? We're
not at risk of conflicting with a C library implementation.

> The implementation is trivial, and much faster when the compiler can
> inline the cmp() function pointer.
> 
> I doubt we actually need out-of-line implementation (except perhaps for
> CONFIG_UBSAN builds or so).

The only references are in Arm code; I don't know enough of UBSAN to
see why uses may appear there.

Jan