[PATCH v2 0/2] x86/cpu-policy: librarify copy-in/-out

[PATCH v2 0/2] x86/cpu-policy: librarify copy-in/-out

[Jan Beulich]

1: move CPU policy library code
2: split out copy-in/-out functions

Jan

[PATCH v2 1/2] x86/cpu-policy: move CPU policy library code

[Jan Beulich]

... to a dedicated subdir of x86's private lib/ sub-tree.

For the CPU policy harnesses and libxenguest use $(lib-y) as set by the
new Makefile.common, whereas for the emulator harnesses stick to building
just cpuid.o for the time being.

Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: New.
---
 tools/fuzz/cpu-policy/Makefile                     | 7 +++++--
 tools/fuzz/x86_instruction_emulator/Makefile       | 2 +-
 tools/libs/guest/Makefile.common                   | 7 +++++--
 tools/tests/cpu-policy/Makefile                    | 7 +++++--
 tools/tests/x86_emulator/Makefile                  | 2 +-
 xen/arch/x86/arch.mk                               | 1 +
 xen/arch/x86/lib/Makefile                          | 2 ++
 xen/arch/x86/lib/cpu-policy/Makefile               | 1 +
 xen/arch/x86/lib/cpu-policy/Makefile.common        | 3 +++
 xen/{lib/x86 => arch/x86/lib/cpu-policy}/cpuid.c   | 0
 xen/{lib/x86 => arch/x86/lib/cpu-policy}/msr.c     | 0
 xen/{lib/x86 => arch/x86/lib/cpu-policy}/policy.c  | 0
 xen/{lib/x86 => arch/x86/lib/cpu-policy}/private.h | 0
 xen/lib/Makefile                                   | 2 --
 xen/lib/x86/Makefile                               | 3 ---
 15 files changed, 24 insertions(+), 13 deletions(-)
 create mode 100644 xen/arch/x86/lib/cpu-policy/Makefile
 create mode 100644 xen/arch/x86/lib/cpu-policy/Makefile.common
 rename xen/{lib/x86 => arch/x86/lib/cpu-policy}/cpuid.c (100%)
 rename xen/{lib/x86 => arch/x86/lib/cpu-policy}/msr.c (100%)
 rename xen/{lib/x86 => arch/x86/lib/cpu-policy}/policy.c (100%)
 rename xen/{lib/x86 => arch/x86/lib/cpu-policy}/private.h (100%)
 delete mode 100644 xen/lib/x86/Makefile

diff --git a/tools/fuzz/cpu-policy/Makefile b/tools/fuzz/cpu-policy/Makefile
index 6e7743e0aa12..76ecf20e8dbd 100644
--- a/tools/fuzz/cpu-policy/Makefile
+++ b/tools/fuzz/cpu-policy/Makefile
@@ -20,9 +20,12 @@ install: all
 CFLAGS += $(CFLAGS_xeninclude) -D__XEN_TOOLS__
 CFLAGS += $(APPEND_CFLAGS) -Og
 
-vpath %.c ../../../xen/lib/x86
+vpath %.c $(XEN_ROOT)/xen/arch/x86/lib/cpu-policy
 
-afl-policy-fuzzer: afl-policy-fuzzer.o msr.o cpuid.o
+lib-y :=
+include $(XEN_ROOT)/xen/arch/x86/lib/cpu-policy/Makefile.common
+
+afl-policy-fuzzer: afl-policy-fuzzer.o $(lib-y)
 	$(CC) $(CFLAGS) $^ -o $@
 
 -include $(DEPS_INCLUDE)
diff --git a/tools/fuzz/x86_instruction_emulator/Makefile b/tools/fuzz/x86_instruction_emulator/Makefile
index d104b15f4fbf..4e4877a20322 100644
--- a/tools/fuzz/x86_instruction_emulator/Makefile
+++ b/tools/fuzz/x86_instruction_emulator/Makefile
@@ -21,7 +21,7 @@ vpath %.h ../../tests/x86_emulator
 CFLAGS += -iquote ../../tests/x86_emulator
 
 # Add libx86 to the build
-vpath %.c $(XEN_ROOT)/xen/lib/x86
+vpath %.c $(XEN_ROOT)/xen/arch/x86/lib/cpu-policy
 
 x86_emulate:
 	mkdir -p $@
diff --git a/tools/libs/guest/Makefile.common b/tools/libs/guest/Makefile.common
index a026a2f662b0..b928a4a246a9 100644
--- a/tools/libs/guest/Makefile.common
+++ b/tools/libs/guest/Makefile.common
@@ -33,9 +33,12 @@ LIBELF_OBJS += libelf-dominfo.o
 OBJS-y += $(LIBELF_OBJS)
 
 ifeq ($(CONFIG_X86),y) # Add libx86 to the build
-vpath %.c ../../../xen/lib/x86
+vpath %.c $(XEN_ROOT)/xen/arch/x86/lib/cpu-policy
 
-OBJS-y                 += cpuid.o msr.o policy.o
+lib-y :=
+include $(XEN_ROOT)/xen/arch/x86/lib/cpu-policy/Makefile.common
+
+OBJS-y                 += $(lib-y)
 endif
 
 # new domain builder
diff --git a/tools/tests/cpu-policy/Makefile b/tools/tests/cpu-policy/Makefile
index 24f87e2eca2a..d8e4d222f4e4 100644
--- a/tools/tests/cpu-policy/Makefile
+++ b/tools/tests/cpu-policy/Makefile
@@ -42,11 +42,14 @@ CFLAGS += $(APPEND_CFLAGS)
 
 LDFLAGS += $(APPEND_LDFLAGS)
 
-vpath %.c ../../../xen/lib/x86
+vpath %.c $(XEN_ROOT)/xen/arch/x86/lib/cpu-policy
+
+lib-y :=
+include $(XEN_ROOT)/xen/arch/x86/lib/cpu-policy/Makefile.common
 
 %.o: Makefile
 
-test-cpu-policy: test-cpu-policy.o msr.o cpuid.o policy.o
+test-cpu-policy: test-cpu-policy.o $(lib-y)
 	$(CC) $^ -o $@ $(LDFLAGS)
 
 -include $(DEPS_INCLUDE)
diff --git a/tools/tests/x86_emulator/Makefile b/tools/tests/x86_emulator/Makefile
index 376cfe244d1c..e18725d0c303 100644
--- a/tools/tests/x86_emulator/Makefile
+++ b/tools/tests/x86_emulator/Makefile
@@ -17,7 +17,7 @@ vpath x86_emulate/%.h $(XEN_ROOT)/xen/arch/x86
 HOSTCFLAGS += -iquote $(XEN_ROOT)/xen/arch/x86
 
 # Add libx86 to the build
-vpath %.c $(XEN_ROOT)/xen/lib/x86
+vpath %.c $(XEN_ROOT)/xen/arch/x86/lib/cpu-policy
 
 CFLAGS += $(CFLAGS_xeninclude)
 
diff --git a/xen/arch/x86/arch.mk b/xen/arch/x86/arch.mk
index 0203138a819a..be6c76d2934b 100644
--- a/xen/arch/x86/arch.mk
+++ b/xen/arch/x86/arch.mk
@@ -4,6 +4,7 @@
 export XEN_IMG_OFFSET := 0x200000
 
 ARCH_LIBS-y += arch/x86/lib/lib.a
+ALL_LIBS-y += arch/x86/lib/cpu-policy/lib.a
 
 CFLAGS += -DXEN_IMG_OFFSET=$(XEN_IMG_OFFSET)
 
diff --git a/xen/arch/x86/lib/Makefile b/xen/arch/x86/lib/Makefile
index b9a65c662a56..fe9a27187992 100644
--- a/xen/arch/x86/lib/Makefile
+++ b/xen/arch/x86/lib/Makefile
@@ -6,3 +6,5 @@ lib-y += generic-hweightl.o
 lib-y += memcpy.o
 lib-y += memset.o
 lib-y += scrub-page.o
+
+obj-y += cpu-policy/
diff --git a/xen/arch/x86/lib/cpu-policy/Makefile b/xen/arch/x86/lib/cpu-policy/Makefile
new file mode 100644
index 000000000000..b12cf7836d4c
--- /dev/null
+++ b/xen/arch/x86/lib/cpu-policy/Makefile
@@ -0,0 +1 @@
+include $(srcdir)/Makefile.common
diff --git a/xen/arch/x86/lib/cpu-policy/Makefile.common b/xen/arch/x86/lib/cpu-policy/Makefile.common
new file mode 100644
index 000000000000..35475af78048
--- /dev/null
+++ b/xen/arch/x86/lib/cpu-policy/Makefile.common
@@ -0,0 +1,3 @@
+lib-y += cpuid.o
+lib-y += msr.o
+lib-y += policy.o
diff --git a/xen/lib/x86/cpuid.c b/xen/arch/x86/lib/cpu-policy/cpuid.c
similarity index 100%
rename from xen/lib/x86/cpuid.c
rename to xen/arch/x86/lib/cpu-policy/cpuid.c
diff --git a/xen/lib/x86/msr.c b/xen/arch/x86/lib/cpu-policy/msr.c
similarity index 100%
rename from xen/lib/x86/msr.c
rename to xen/arch/x86/lib/cpu-policy/msr.c
diff --git a/xen/lib/x86/policy.c b/xen/arch/x86/lib/cpu-policy/policy.c
similarity index 100%
rename from xen/lib/x86/policy.c
rename to xen/arch/x86/lib/cpu-policy/policy.c
diff --git a/xen/lib/x86/private.h b/xen/arch/x86/lib/cpu-policy/private.h
similarity index 100%
rename from xen/lib/x86/private.h
rename to xen/arch/x86/lib/cpu-policy/private.h
diff --git a/xen/lib/Makefile b/xen/lib/Makefile
index efca830d924c..850efeb4c570 100644
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -1,5 +1,3 @@
-obj-$(CONFIG_X86) += x86/
-
 lib-y += bsearch.o
 lib-y += ctors.o
 lib-y += ctype.o
diff --git a/xen/lib/x86/Makefile b/xen/lib/x86/Makefile
deleted file mode 100644
index 780ea05db158..000000000000
--- a/xen/lib/x86/Makefile
+++ /dev/null
@@ -1,3 +0,0 @@
-obj-y += cpuid.o
-obj-y += msr.o
-obj-y += policy.o

[PATCH v2 2/2] x86/cpu-policy: split out copy-in/-out functions

[Jan Beulich]

This is to aid with MGMT_HYPERCALL work, leaving the functions potentially
unreferenced (which Misra dislikes). By moving them to separate archive
members, the linker simply will not pick them up when not needed.

As the CPUID and MSR ones are always used together, put the "from" and
"to" variants of each together in one file respectively.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Re-base over moving of the entire directory.
---
 xen/arch/x86/lib/cpu-policy/Makefile.common   |   3 +-
 .../x86/lib/cpu-policy/copy-from-buffer.c     | 238 ++++++++++++++++++
 xen/arch/x86/lib/cpu-policy/copy-to-buffer.c  | 204 +++++++++++++++
 xen/arch/x86/lib/cpu-policy/cpuid.c           | 226 -----------------
 xen/arch/x86/lib/cpu-policy/msr.c             | 130 ----------
 xen/arch/x86/lib/cpu-policy/private.h         |  31 ---
 6 files changed, 444 insertions(+), 388 deletions(-)
 create mode 100644 xen/arch/x86/lib/cpu-policy/copy-from-buffer.c
 create mode 100644 xen/arch/x86/lib/cpu-policy/copy-to-buffer.c
 delete mode 100644 xen/arch/x86/lib/cpu-policy/msr.c

diff --git a/xen/arch/x86/lib/cpu-policy/Makefile.common b/xen/arch/x86/lib/cpu-policy/Makefile.common
index 35475af78048..aff662aae83b 100644
--- a/xen/arch/x86/lib/cpu-policy/Makefile.common
+++ b/xen/arch/x86/lib/cpu-policy/Makefile.common
@@ -1,3 +1,4 @@
+lib-y += copy-from-buffer.o
+lib-y += copy-to-buffer.o
 lib-y += cpuid.o
-lib-y += msr.o
 lib-y += policy.o
diff --git a/xen/arch/x86/lib/cpu-policy/copy-from-buffer.c b/xen/arch/x86/lib/cpu-policy/copy-from-buffer.c
new file mode 100644
index 000000000000..24f7bc79d2bd
--- /dev/null
+++ b/xen/arch/x86/lib/cpu-policy/copy-from-buffer.c
@@ -0,0 +1,238 @@
+#ifdef __XEN__
+
+#include <xen/errno.h>
+#include <xen/guest_access.h>
+#include <xen/nospec.h>
+#include <xen/types.h>
+
+#include <asm/msr-index.h>
+
+#define copy_from_buffer_offset copy_from_guest_offset
+
+#else /* !__XEN__ */
+
+#include <errno.h>
+#include <inttypes.h>
+#include <stdbool.h>
+#include <stddef.h>
+#include <string.h>
+
+#include <xen/asm/msr-index.h>
+
+#include <xen-tools/common-macros.h>
+
+#define array_access_nospec(a, i) (a)[(i)]
+
+/* memcpy(), but with copy_from_guest_offset()'s API. */
+#define copy_from_buffer_offset(dst, src, index, nr)    \
+({                                                      \
+    const typeof(*(src)) *src_ = (src);                 \
+    typeof(*(dst)) *dst_ = (dst);                       \
+    typeof(index) index_ = (index);                     \
+    typeof(nr) nr_ = (nr), i_;                          \
+                                                        \
+    for ( i_ = 0; i_ < nr_; i_++ )                      \
+        dst_[i_] = src_[index_ + i_];                   \
+    0;                                                  \
+})
+
+#endif /* __XEN__ */
+
+#include <xen/lib/x86/cpu-policy.h>
+
+int x86_cpuid_copy_from_buffer(struct cpu_policy *p,
+                               const cpuid_leaf_buffer_t leaves,
+                               uint32_t nr_entries, uint32_t *err_leaf,
+                               uint32_t *err_subleaf)
+{
+    unsigned int i;
+    xen_cpuid_leaf_t data;
+
+    if ( err_leaf )
+        *err_leaf = -1;
+    if ( err_subleaf )
+        *err_subleaf = -1;
+
+    /*
+     * A well formed caller is expected to pass an array with leaves in order,
+     * and without any repetitions.  However, due to per-vendor differences,
+     * and in the case of upgrade or levelled scenarios, we typically expect
+     * fewer than MAX leaves to be passed.
+     *
+     * Detecting repeated entries is prohibitively complicated, so we don't
+     * bother.  That said, one way or another if more than MAX leaves are
+     * passed, something is wrong.
+     */
+    if ( nr_entries > CPUID_MAX_SERIALISED_LEAVES )
+        return -E2BIG;
+
+    for ( i = 0; i < nr_entries; ++i )
+    {
+        struct cpuid_leaf l;
+
+        if ( copy_from_buffer_offset(&data, leaves, i, 1) )
+            return -EFAULT;
+
+        l = (struct cpuid_leaf){ data.a, data.b, data.c, data.d };
+
+        switch ( data.leaf )
+        {
+        case 0 ... ARRAY_SIZE(p->basic.raw) - 1:
+            switch ( data.leaf )
+            {
+            case 0x4:
+                if ( data.subleaf >= ARRAY_SIZE(p->cache.raw) )
+                    goto out_of_range;
+
+                array_access_nospec(p->cache.raw, data.subleaf) = l;
+                break;
+
+            case 0x7:
+                if ( data.subleaf >= ARRAY_SIZE(p->feat.raw) )
+                    goto out_of_range;
+
+                array_access_nospec(p->feat.raw, data.subleaf) = l;
+                break;
+
+            case 0xb:
+                if ( data.subleaf >= ARRAY_SIZE(p->topo.raw) )
+                    goto out_of_range;
+
+                array_access_nospec(p->topo.raw, data.subleaf) = l;
+                break;
+
+            case 0xd:
+                if ( data.subleaf >= ARRAY_SIZE(p->xstate.raw) )
+                    goto out_of_range;
+
+                array_access_nospec(p->xstate.raw, data.subleaf) = l;
+                break;
+
+            default:
+                if ( data.subleaf != XEN_CPUID_NO_SUBLEAF )
+                    goto out_of_range;
+
+                array_access_nospec(p->basic.raw, data.leaf) = l;
+                break;
+            }
+            break;
+
+        case 0x40000000:
+            if ( data.subleaf != XEN_CPUID_NO_SUBLEAF )
+                goto out_of_range;
+
+            p->hv_limit = l.a;
+            break;
+
+        case 0x40000100:
+            if ( data.subleaf != XEN_CPUID_NO_SUBLEAF )
+                goto out_of_range;
+
+            p->hv2_limit = l.a;
+            break;
+
+        case 0x80000000U ... 0x80000000U + ARRAY_SIZE(p->extd.raw) - 1:
+            if ( data.subleaf != XEN_CPUID_NO_SUBLEAF )
+                goto out_of_range;
+
+            array_access_nospec(p->extd.raw, data.leaf & 0xffff) = l;
+            break;
+
+        default:
+            goto out_of_range;
+        }
+    }
+
+    x86_cpu_policy_recalc_synth(p);
+
+    return 0;
+
+ out_of_range:
+    if ( err_leaf )
+        *err_leaf = data.leaf;
+    if ( err_subleaf )
+        *err_subleaf = data.subleaf;
+
+    return -ERANGE;
+}
+
+int x86_msr_copy_from_buffer(struct cpu_policy *p,
+                             const msr_entry_buffer_t msrs, uint32_t nr_entries,
+                             uint32_t *err_msr)
+{
+    unsigned int i;
+    xen_msr_entry_t data;
+    int rc;
+
+    if ( err_msr )
+        *err_msr = -1;
+
+    /*
+     * A well formed caller is expected to pass an array with entries in
+     * order, and without any repetitions.  However, due to per-vendor
+     * differences, and in the case of upgrade or levelled scenarios, we
+     * typically expect fewer than MAX entries to be passed.
+     *
+     * Detecting repeated entries is prohibitively complicated, so we don't
+     * bother.  That said, one way or another if more than MAX entries are
+     * passed, something is wrong.
+     */
+    if ( nr_entries > MSR_MAX_SERIALISED_ENTRIES )
+        return -E2BIG;
+
+    for ( i = 0; i < nr_entries; i++ )
+    {
+        if ( copy_from_buffer_offset(&data, msrs, i, 1) )
+            return -EFAULT;
+
+        if ( data.flags ) /* .flags MBZ */
+        {
+            rc = -EINVAL;
+            goto err;
+        }
+
+        switch ( data.idx )
+        {
+            /*
+             * Assign data.val to p->field, checking for truncation if the
+             * backing storage for field is smaller than uint64_t
+             */
+#define ASSIGN(field)                             \
+({                                                \
+    if ( (typeof(p->field))data.val != data.val ) \
+    {                                             \
+        rc = -EOVERFLOW;                          \
+        goto err;                                 \
+    }                                             \
+    p->field = data.val;                          \
+})
+
+        case MSR_INTEL_PLATFORM_INFO: ASSIGN(platform_info.raw); break;
+        case MSR_ARCH_CAPABILITIES:   ASSIGN(arch_caps.raw);     break;
+
+#undef ASSIGN
+
+        default:
+            rc = -ERANGE;
+            goto err;
+        }
+    }
+
+    return 0;
+
+ err:
+    if ( err_msr )
+        *err_msr = data.idx;
+
+    return rc;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/x86/lib/cpu-policy/copy-to-buffer.c b/xen/arch/x86/lib/cpu-policy/copy-to-buffer.c
new file mode 100644
index 000000000000..190dac746cdf
--- /dev/null
+++ b/xen/arch/x86/lib/cpu-policy/copy-to-buffer.c
@@ -0,0 +1,204 @@
+#ifdef __XEN__
+
+#include <xen/errno.h>
+#include <xen/guest_access.h>
+#include <xen/types.h>
+
+#include <asm/msr-index.h>
+
+#define copy_to_buffer_offset copy_to_guest_offset
+
+#else /* !__XEN__ */
+
+#include <errno.h>
+#include <inttypes.h>
+#include <stdbool.h>
+#include <stddef.h>
+#include <string.h>
+
+#include <xen/asm/msr-index.h>
+
+#include <xen-tools/common-macros.h>
+
+/* memcpy(), but with copy_to_guest_offset()'s API. */
+#define copy_to_buffer_offset(dst, index, src, nr)      \
+({                                                      \
+    const typeof(*(src)) *src_ = (src);                 \
+    typeof(*(dst)) *dst_ = (dst);                       \
+    typeof(index) index_ = (index);                     \
+    typeof(nr) nr_ = (nr), i_;                          \
+                                                        \
+    for ( i_ = 0; i_ < nr_; i_++ )                      \
+        dst_[index_ + i_] = src_[i_];                   \
+    0;                                                  \
+})
+
+#endif /* __XEN__ */
+
+#include <xen/lib/x86/cpu-policy.h>
+
+/*
+ * Copy a single cpuid_leaf into a provided xen_cpuid_leaf_t buffer,
+ * performing boundary checking against the buffer size.
+ */
+static int copy_leaf_to_buffer(uint32_t leaf, uint32_t subleaf,
+                               const struct cpuid_leaf *data,
+                               cpuid_leaf_buffer_t leaves,
+                               uint32_t *curr_entry, const uint32_t nr_entries)
+{
+    const xen_cpuid_leaf_t val = {
+        leaf, subleaf, data->a, data->b, data->c, data->d,
+    };
+
+    if ( *curr_entry == nr_entries )
+        return -ENOBUFS;
+
+    if ( copy_to_buffer_offset(leaves, *curr_entry, &val, 1) )
+        return -EFAULT;
+
+    ++*curr_entry;
+
+    return 0;
+}
+
+int x86_cpuid_copy_to_buffer(const struct cpu_policy *p,
+                             cpuid_leaf_buffer_t leaves, uint32_t *nr_entries_p)
+{
+    const uint32_t nr_entries = *nr_entries_p;
+    uint32_t curr_entry = 0, leaf, subleaf;
+
+#define COPY_LEAF(l, s, data)                                       \
+    ({                                                              \
+        int ret;                                                    \
+                                                                    \
+        if ( (ret = copy_leaf_to_buffer(                            \
+                  l, s, data, leaves, &curr_entry, nr_entries)) )   \
+            return ret;                                             \
+    })
+
+    /* Basic leaves. */
+    for ( leaf = 0; leaf <= MIN(p->basic.max_leaf,
+                                ARRAY_SIZE(p->basic.raw) - 1); ++leaf )
+    {
+        switch ( leaf )
+        {
+        case 0x4:
+            for ( subleaf = 0; subleaf < ARRAY_SIZE(p->cache.raw); ++subleaf )
+            {
+                COPY_LEAF(leaf, subleaf, &p->cache.raw[subleaf]);
+
+                if ( p->cache.subleaf[subleaf].type == 0 )
+                    break;
+            }
+            break;
+
+        case 0x7:
+            for ( subleaf = 0;
+                  subleaf <= MIN(p->feat.max_subleaf,
+                                 ARRAY_SIZE(p->feat.raw) - 1); ++subleaf )
+                COPY_LEAF(leaf, subleaf, &p->feat.raw[subleaf]);
+            break;
+
+        case 0xb:
+            for ( subleaf = 0; subleaf < ARRAY_SIZE(p->topo.raw); ++subleaf )
+            {
+                COPY_LEAF(leaf, subleaf, &p->topo.raw[subleaf]);
+
+                if ( p->topo.subleaf[subleaf].type == 0 )
+                    break;
+            }
+            break;
+
+        case 0xd:
+        {
+            uint64_t xstates = cpu_policy_xstates(p);
+
+            COPY_LEAF(leaf, 0, &p->xstate.raw[0]);
+            COPY_LEAF(leaf, 1, &p->xstate.raw[1]);
+
+            for ( xstates >>= 2, subleaf = 2;
+                  xstates && subleaf < ARRAY_SIZE(p->xstate.raw);
+                  xstates >>= 1, ++subleaf )
+                COPY_LEAF(leaf, subleaf, &p->xstate.raw[subleaf]);
+            break;
+        }
+
+        default:
+            COPY_LEAF(leaf, XEN_CPUID_NO_SUBLEAF, &p->basic.raw[leaf]);
+            break;
+        }
+    }
+
+    /* TODO: Port Xen and Viridian leaves to the new CPUID infrastructure. */
+    COPY_LEAF(0x40000000, XEN_CPUID_NO_SUBLEAF,
+              &(struct cpuid_leaf){ p->hv_limit });
+    COPY_LEAF(0x40000100, XEN_CPUID_NO_SUBLEAF,
+              &(struct cpuid_leaf){ p->hv2_limit });
+
+    /* Extended leaves. */
+    for ( leaf = 0; leaf <= MIN(p->extd.max_leaf & 0xffffUL,
+                                ARRAY_SIZE(p->extd.raw) - 1); ++leaf )
+        COPY_LEAF(0x80000000U | leaf, XEN_CPUID_NO_SUBLEAF, &p->extd.raw[leaf]);
+
+#undef COPY_LEAF
+
+    *nr_entries_p = curr_entry;
+
+    return 0;
+}
+
+/*
+ * Copy a single MSR into the provided msr_entry_buffer_t buffer, performing a
+ * boundary check against the buffer size.
+ */
+static int copy_msr_to_buffer(uint32_t idx, uint64_t val,
+                              msr_entry_buffer_t msrs,
+                              uint32_t *curr_entry, const uint32_t nr_entries)
+{
+    const xen_msr_entry_t ent = { .idx = idx, .val = val };
+
+    if ( *curr_entry == nr_entries )
+        return -ENOBUFS;
+
+    if ( copy_to_buffer_offset(msrs, *curr_entry, &ent, 1) )
+        return -EFAULT;
+
+    ++*curr_entry;
+
+    return 0;
+}
+
+int x86_msr_copy_to_buffer(const struct cpu_policy *p,
+                           msr_entry_buffer_t msrs, uint32_t *nr_entries_p)
+{
+    const uint32_t nr_entries = *nr_entries_p;
+    uint32_t curr_entry = 0;
+
+#define COPY_MSR(idx, val)                                      \
+    ({                                                          \
+        int ret;                                                \
+                                                                \
+        if ( (ret = copy_msr_to_buffer(                         \
+                  idx, val, msrs, &curr_entry, nr_entries)) )   \
+            return ret;                                         \
+    })
+
+    COPY_MSR(MSR_INTEL_PLATFORM_INFO, p->platform_info.raw);
+    COPY_MSR(MSR_ARCH_CAPABILITIES,   p->arch_caps.raw);
+
+#undef COPY_MSR
+
+    *nr_entries_p = curr_entry;
+
+    return 0;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/x86/lib/cpu-policy/cpuid.c b/xen/arch/x86/lib/cpu-policy/cpuid.c
index 6298d051f2a6..3162e795bc21 100644
--- a/xen/arch/x86/lib/cpu-policy/cpuid.c
+++ b/xen/arch/x86/lib/cpu-policy/cpuid.c
@@ -322,232 +322,6 @@ const uint32_t *x86_cpu_policy_lookup_deep_deps(uint32_t feature)
     return NULL;
 }
 
-/*
- * Copy a single cpuid_leaf into a provided xen_cpuid_leaf_t buffer,
- * performing boundary checking against the buffer size.
- */
-static int copy_leaf_to_buffer(uint32_t leaf, uint32_t subleaf,
-                               const struct cpuid_leaf *data,
-                               cpuid_leaf_buffer_t leaves,
-                               uint32_t *curr_entry, const uint32_t nr_entries)
-{
-    const xen_cpuid_leaf_t val = {
-        leaf, subleaf, data->a, data->b, data->c, data->d,
-    };
-
-    if ( *curr_entry == nr_entries )
-        return -ENOBUFS;
-
-    if ( copy_to_buffer_offset(leaves, *curr_entry, &val, 1) )
-        return -EFAULT;
-
-    ++*curr_entry;
-
-    return 0;
-}
-
-int x86_cpuid_copy_to_buffer(const struct cpu_policy *p,
-                             cpuid_leaf_buffer_t leaves, uint32_t *nr_entries_p)
-{
-    const uint32_t nr_entries = *nr_entries_p;
-    uint32_t curr_entry = 0, leaf, subleaf;
-
-#define COPY_LEAF(l, s, data)                                       \
-    ({                                                              \
-        int ret;                                                    \
-                                                                    \
-        if ( (ret = copy_leaf_to_buffer(                            \
-                  l, s, data, leaves, &curr_entry, nr_entries)) )   \
-            return ret;                                             \
-    })
-
-    /* Basic leaves. */
-    for ( leaf = 0; leaf <= MIN(p->basic.max_leaf,
-                                ARRAY_SIZE(p->basic.raw) - 1); ++leaf )
-    {
-        switch ( leaf )
-        {
-        case 0x4:
-            for ( subleaf = 0; subleaf < ARRAY_SIZE(p->cache.raw); ++subleaf )
-            {
-                COPY_LEAF(leaf, subleaf, &p->cache.raw[subleaf]);
-
-                if ( p->cache.subleaf[subleaf].type == 0 )
-                    break;
-            }
-            break;
-
-        case 0x7:
-            for ( subleaf = 0;
-                  subleaf <= MIN(p->feat.max_subleaf,
-                                 ARRAY_SIZE(p->feat.raw) - 1); ++subleaf )
-                COPY_LEAF(leaf, subleaf, &p->feat.raw[subleaf]);
-            break;
-
-        case 0xb:
-            for ( subleaf = 0; subleaf < ARRAY_SIZE(p->topo.raw); ++subleaf )
-            {
-                COPY_LEAF(leaf, subleaf, &p->topo.raw[subleaf]);
-
-                if ( p->topo.subleaf[subleaf].type == 0 )
-                    break;
-            }
-            break;
-
-        case 0xd:
-        {
-            uint64_t xstates = cpu_policy_xstates(p);
-
-            COPY_LEAF(leaf, 0, &p->xstate.raw[0]);
-            COPY_LEAF(leaf, 1, &p->xstate.raw[1]);
-
-            for ( xstates >>= 2, subleaf = 2;
-                  xstates && subleaf < ARRAY_SIZE(p->xstate.raw);
-                  xstates >>= 1, ++subleaf )
-                COPY_LEAF(leaf, subleaf, &p->xstate.raw[subleaf]);
-            break;
-        }
-
-        default:
-            COPY_LEAF(leaf, XEN_CPUID_NO_SUBLEAF, &p->basic.raw[leaf]);
-            break;
-        }
-    }
-
-    /* TODO: Port Xen and Viridian leaves to the new CPUID infrastructure. */
-    COPY_LEAF(0x40000000, XEN_CPUID_NO_SUBLEAF,
-              &(struct cpuid_leaf){ p->hv_limit });
-    COPY_LEAF(0x40000100, XEN_CPUID_NO_SUBLEAF,
-              &(struct cpuid_leaf){ p->hv2_limit });
-
-    /* Extended leaves. */
-    for ( leaf = 0; leaf <= MIN(p->extd.max_leaf & 0xffffUL,
-                                ARRAY_SIZE(p->extd.raw) - 1); ++leaf )
-        COPY_LEAF(0x80000000U | leaf, XEN_CPUID_NO_SUBLEAF, &p->extd.raw[leaf]);
-
-#undef COPY_LEAF
-
-    *nr_entries_p = curr_entry;
-
-    return 0;
-}
-
-int x86_cpuid_copy_from_buffer(struct cpu_policy *p,
-                               const cpuid_leaf_buffer_t leaves,
-                               uint32_t nr_entries, uint32_t *err_leaf,
-                               uint32_t *err_subleaf)
-{
-    unsigned int i;
-    xen_cpuid_leaf_t data;
-
-    if ( err_leaf )
-        *err_leaf = -1;
-    if ( err_subleaf )
-        *err_subleaf = -1;
-
-    /*
-     * A well formed caller is expected to pass an array with leaves in order,
-     * and without any repetitions.  However, due to per-vendor differences,
-     * and in the case of upgrade or levelled scenarios, we typically expect
-     * fewer than MAX leaves to be passed.
-     *
-     * Detecting repeated entries is prohibitively complicated, so we don't
-     * bother.  That said, one way or another if more than MAX leaves are
-     * passed, something is wrong.
-     */
-    if ( nr_entries > CPUID_MAX_SERIALISED_LEAVES )
-        return -E2BIG;
-
-    for ( i = 0; i < nr_entries; ++i )
-    {
-        struct cpuid_leaf l;
-
-        if ( copy_from_buffer_offset(&data, leaves, i, 1) )
-            return -EFAULT;
-
-        l = (struct cpuid_leaf){ data.a, data.b, data.c, data.d };
-
-        switch ( data.leaf )
-        {
-        case 0 ... ARRAY_SIZE(p->basic.raw) - 1:
-            switch ( data.leaf )
-            {
-            case 0x4:
-                if ( data.subleaf >= ARRAY_SIZE(p->cache.raw) )
-                    goto out_of_range;
-
-                array_access_nospec(p->cache.raw, data.subleaf) = l;
-                break;
-
-            case 0x7:
-                if ( data.subleaf >= ARRAY_SIZE(p->feat.raw) )
-                    goto out_of_range;
-
-                array_access_nospec(p->feat.raw, data.subleaf) = l;
-                break;
-
-            case 0xb:
-                if ( data.subleaf >= ARRAY_SIZE(p->topo.raw) )
-                    goto out_of_range;
-
-                array_access_nospec(p->topo.raw, data.subleaf) = l;
-                break;
-
-            case 0xd:
-                if ( data.subleaf >= ARRAY_SIZE(p->xstate.raw) )
-                    goto out_of_range;
-
-                array_access_nospec(p->xstate.raw, data.subleaf) = l;
-                break;
-
-            default:
-                if ( data.subleaf != XEN_CPUID_NO_SUBLEAF )
-                    goto out_of_range;
-
-                array_access_nospec(p->basic.raw, data.leaf) = l;
-                break;
-            }
-            break;
-
-        case 0x40000000:
-            if ( data.subleaf != XEN_CPUID_NO_SUBLEAF )
-                goto out_of_range;
-
-            p->hv_limit = l.a;
-            break;
-
-        case 0x40000100:
-            if ( data.subleaf != XEN_CPUID_NO_SUBLEAF )
-                goto out_of_range;
-
-            p->hv2_limit = l.a;
-            break;
-
-        case 0x80000000U ... 0x80000000U + ARRAY_SIZE(p->extd.raw) - 1:
-            if ( data.subleaf != XEN_CPUID_NO_SUBLEAF )
-                goto out_of_range;
-
-            array_access_nospec(p->extd.raw, data.leaf & 0xffff) = l;
-            break;
-
-        default:
-            goto out_of_range;
-        }
-    }
-
-    x86_cpu_policy_recalc_synth(p);
-
-    return 0;
-
- out_of_range:
-    if ( err_leaf )
-        *err_leaf = data.leaf;
-    if ( err_subleaf )
-        *err_subleaf = data.subleaf;
-
-    return -ERANGE;
-}
-
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/lib/cpu-policy/msr.c b/xen/arch/x86/lib/cpu-policy/msr.c
deleted file mode 100644
index e04b9ca01302..000000000000
--- a/xen/arch/x86/lib/cpu-policy/msr.c
+++ /dev/null
@@ -1,130 +0,0 @@
-#include "private.h"
-
-#include <xen/lib/x86/cpu-policy.h>
-
-/*
- * Copy a single MSR into the provided msr_entry_buffer_t buffer, performing a
- * boundary check against the buffer size.
- */
-static int copy_msr_to_buffer(uint32_t idx, uint64_t val,
-                              msr_entry_buffer_t msrs,
-                              uint32_t *curr_entry, const uint32_t nr_entries)
-{
-    const xen_msr_entry_t ent = { .idx = idx, .val = val };
-
-    if ( *curr_entry == nr_entries )
-        return -ENOBUFS;
-
-    if ( copy_to_buffer_offset(msrs, *curr_entry, &ent, 1) )
-        return -EFAULT;
-
-    ++*curr_entry;
-
-    return 0;
-}
-
-int x86_msr_copy_to_buffer(const struct cpu_policy *p,
-                           msr_entry_buffer_t msrs, uint32_t *nr_entries_p)
-{
-    const uint32_t nr_entries = *nr_entries_p;
-    uint32_t curr_entry = 0;
-
-#define COPY_MSR(idx, val)                                      \
-    ({                                                          \
-        int ret;                                                \
-                                                                \
-        if ( (ret = copy_msr_to_buffer(                         \
-                  idx, val, msrs, &curr_entry, nr_entries)) )   \
-            return ret;                                         \
-    })
-
-    COPY_MSR(MSR_INTEL_PLATFORM_INFO, p->platform_info.raw);
-    COPY_MSR(MSR_ARCH_CAPABILITIES,   p->arch_caps.raw);
-
-#undef COPY_MSR
-
-    *nr_entries_p = curr_entry;
-
-    return 0;
-}
-
-int x86_msr_copy_from_buffer(struct cpu_policy *p,
-                             const msr_entry_buffer_t msrs, uint32_t nr_entries,
-                             uint32_t *err_msr)
-{
-    unsigned int i;
-    xen_msr_entry_t data;
-    int rc;
-
-    if ( err_msr )
-        *err_msr = -1;
-
-    /*
-     * A well formed caller is expected to pass an array with entries in
-     * order, and without any repetitions.  However, due to per-vendor
-     * differences, and in the case of upgrade or levelled scenarios, we
-     * typically expect fewer than MAX entries to be passed.
-     *
-     * Detecting repeated entries is prohibitively complicated, so we don't
-     * bother.  That said, one way or another if more than MAX entries are
-     * passed, something is wrong.
-     */
-    if ( nr_entries > MSR_MAX_SERIALISED_ENTRIES )
-        return -E2BIG;
-
-    for ( i = 0; i < nr_entries; i++ )
-    {
-        if ( copy_from_buffer_offset(&data, msrs, i, 1) )
-            return -EFAULT;
-
-        if ( data.flags ) /* .flags MBZ */
-        {
-            rc = -EINVAL;
-            goto err;
-        }
-
-        switch ( data.idx )
-        {
-            /*
-             * Assign data.val to p->field, checking for truncation if the
-             * backing storage for field is smaller than uint64_t
-             */
-#define ASSIGN(field)                             \
-({                                                \
-    if ( (typeof(p->field))data.val != data.val ) \
-    {                                             \
-        rc = -EOVERFLOW;                          \
-        goto err;                                 \
-    }                                             \
-    p->field = data.val;                          \
-})
-
-        case MSR_INTEL_PLATFORM_INFO: ASSIGN(platform_info.raw); break;
-        case MSR_ARCH_CAPABILITIES:   ASSIGN(arch_caps.raw);     break;
-
-#undef ASSIGN
-
-        default:
-            rc = -ERANGE;
-            goto err;
-        }
-    }
-
-    return 0;
-
- err:
-    if ( err_msr )
-        *err_msr = data.idx;
-
-    return rc;
-}
-
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * tab-width: 4
- * indent-tabs-mode: nil
- * End:
- */
diff --git a/xen/arch/x86/lib/cpu-policy/private.h b/xen/arch/x86/lib/cpu-policy/private.h
index aedd8e482121..57f254c3e917 100644
--- a/xen/arch/x86/lib/cpu-policy/private.h
+++ b/xen/arch/x86/lib/cpu-policy/private.h
@@ -12,9 +12,6 @@
 
 #include <asm/msr.h>
 
-#define copy_to_buffer_offset copy_to_guest_offset
-#define copy_from_buffer_offset copy_from_guest_offset
-
 #else
 
 #include <errno.h>
@@ -35,34 +32,6 @@ static inline bool test_bit(unsigned int bit, const void *vaddr)
     return addr[bit / 8] & (1u << (bit % 8));
 }
 
-#define array_access_nospec(a, i) (a)[(i)]
-
-/* memcpy(), but with copy_to_guest_offset()'s API. */
-#define copy_to_buffer_offset(dst, index, src, nr)      \
-({                                                      \
-    const typeof(*(src)) *src_ = (src);                 \
-    typeof(*(dst)) *dst_ = (dst);                       \
-    typeof(index) index_ = (index);                     \
-    typeof(nr) nr_ = (nr), i_;                          \
-                                                        \
-    for ( i_ = 0; i_ < nr_; i_++ )                      \
-        dst_[index_ + i_] = src_[i_];                   \
-    0;                                                  \
-})
-
-/* memcpy(), but with copy_from_guest_offset()'s API. */
-#define copy_from_buffer_offset(dst, src, index, nr)    \
-({                                                      \
-    const typeof(*(src)) *src_ = (src);                 \
-    typeof(*(dst)) *dst_ = (dst);                       \
-    typeof(index) index_ = (index);                     \
-    typeof(nr) nr_ = (nr), i_;                          \
-                                                        \
-    for ( i_ = 0; i_ < nr_; i_++ )                      \
-        dst_[i_] = src_[index_ + i_];                   \
-    0;                                                  \
-})
-
 #endif /* __XEN__ */
 
 #endif /* XEN_LIB_X86_PRIVATE_H */

Re: [PATCH v2 1/2] x86/cpu-policy: move CPU policy library code

[Andrew Cooper]

On 07/01/2026 2:17 pm, Jan Beulich wrote:
> diff --git a/xen/arch/x86/arch.mk b/xen/arch/x86/arch.mk
> index 0203138a819a..be6c76d2934b 100644
> --- a/xen/arch/x86/arch.mk
> +++ b/xen/arch/x86/arch.mk
> @@ -4,6 +4,7 @@
>  export XEN_IMG_OFFSET := 0x200000
>  
>  ARCH_LIBS-y += arch/x86/lib/lib.a
> +ALL_LIBS-y += arch/x86/lib/cpu-policy/lib.a

This wants to extend ARCH_LIBS-y surely?  Is this a rebasing oversight?

~Andrew

Re: [PATCH v2 2/2] x86/cpu-policy: split out copy-in/-out functions

[Andrew Cooper]

On 07/01/2026 2:17 pm, Jan Beulich wrote:
> This is to aid with MGMT_HYPERCALL work, leaving the functions potentially
> unreferenced (which Misra dislikes). By moving them to separate archive
> members, the linker simply will not pick them up when not needed.
>
> As the CPUID and MSR ones are always used together, put the "from" and
> "to" variants of each together in one file respectively.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

Re: [PATCH v2 1/2] x86/cpu-policy: move CPU policy library code

[Jan Beulich]

On 02.02.2026 16:47, Andrew Cooper wrote:
> On 07/01/2026 2:17 pm, Jan Beulich wrote:
>> diff --git a/xen/arch/x86/arch.mk b/xen/arch/x86/arch.mk
>> index 0203138a819a..be6c76d2934b 100644
>> --- a/xen/arch/x86/arch.mk
>> +++ b/xen/arch/x86/arch.mk
>> @@ -4,6 +4,7 @@
>>  export XEN_IMG_OFFSET := 0x200000
>>  
>>  ARCH_LIBS-y += arch/x86/lib/lib.a
>> +ALL_LIBS-y += arch/x86/lib/cpu-policy/lib.a
> 
> This wants to extend ARCH_LIBS-y surely?  Is this a rebasing oversight?

No, this was deliberate. The functions here are different from those in
arch/x86/lib/lib.a. We don't need to fear collision with "common code"
ones. Hence I preferred to use the more "normal" placement into what's
passed to the linker.

Jan

Re: [PATCH v2 1/2] x86/cpu-policy: move CPU policy library code

[Andrew Cooper]

On 02/02/2026 4:26 pm, Jan Beulich wrote:
> On 02.02.2026 16:47, Andrew Cooper wrote:
>> On 07/01/2026 2:17 pm, Jan Beulich wrote:
>>> diff --git a/xen/arch/x86/arch.mk b/xen/arch/x86/arch.mk
>>> index 0203138a819a..be6c76d2934b 100644
>>> --- a/xen/arch/x86/arch.mk
>>> +++ b/xen/arch/x86/arch.mk
>>> @@ -4,6 +4,7 @@
>>>  export XEN_IMG_OFFSET := 0x200000
>>>  
>>>  ARCH_LIBS-y += arch/x86/lib/lib.a
>>> +ALL_LIBS-y += arch/x86/lib/cpu-policy/lib.a
>> This wants to extend ARCH_LIBS-y surely?  Is this a rebasing oversight?
> No, this was deliberate. The functions here are different from those in
> arch/x86/lib/lib.a. We don't need to fear collision with "common code"
> ones. Hence I preferred to use the more "normal" placement into what's
> passed to the linker.

I agree that we don't have the explicit ordering requirement that we
have with arch/x86/lib/lib.a.

But, it still reads as bogus to be putting arch/x86/lib/cpu-policy/lib.a
in the non-ARCH list.

What difference is there having this a little earlier in the linker
arguments?  Nothing AFAICT.

~Andrew

Re: [PATCH v2 1/2] x86/cpu-policy: move CPU policy library code

[Jan Beulich]

On 23.02.2026 20:00, Andrew Cooper wrote:
> On 02/02/2026 4:26 pm, Jan Beulich wrote:
>> On 02.02.2026 16:47, Andrew Cooper wrote:
>>> On 07/01/2026 2:17 pm, Jan Beulich wrote:
>>>> diff --git a/xen/arch/x86/arch.mk b/xen/arch/x86/arch.mk
>>>> index 0203138a819a..be6c76d2934b 100644
>>>> --- a/xen/arch/x86/arch.mk
>>>> +++ b/xen/arch/x86/arch.mk
>>>> @@ -4,6 +4,7 @@
>>>>  export XEN_IMG_OFFSET := 0x200000
>>>>  
>>>>  ARCH_LIBS-y += arch/x86/lib/lib.a
>>>> +ALL_LIBS-y += arch/x86/lib/cpu-policy/lib.a
>>> This wants to extend ARCH_LIBS-y surely?  Is this a rebasing oversight?
>> No, this was deliberate. The functions here are different from those in
>> arch/x86/lib/lib.a. We don't need to fear collision with "common code"
>> ones. Hence I preferred to use the more "normal" placement into what's
>> passed to the linker.
> 
> I agree that we don't have the explicit ordering requirement that we
> have with arch/x86/lib/lib.a.
> 
> But, it still reads as bogus to be putting arch/x86/lib/cpu-policy/lib.a
> in the non-ARCH list.
> 
> What difference is there having this a little earlier in the linker
> arguments?  Nothing AFAICT.

Indeed. The sole reason why I'd prefer things as presented is that putting
stuff in ARCH_LIBS should imo be the special case (i.e. requiring a special
reason), while putting things in ALL_LIBS should be the default.

Jan

Re: [PATCH v2 1/2] x86/cpu-policy: move CPU policy library code

[Roger Pau Monné]

On Tue, Feb 24, 2026 at 07:54:29AM +0100, Jan Beulich wrote:
> On 23.02.2026 20:00, Andrew Cooper wrote:
> > On 02/02/2026 4:26 pm, Jan Beulich wrote:
> >> On 02.02.2026 16:47, Andrew Cooper wrote:
> >>> On 07/01/2026 2:17 pm, Jan Beulich wrote:
> >>>> diff --git a/xen/arch/x86/arch.mk b/xen/arch/x86/arch.mk
> >>>> index 0203138a819a..be6c76d2934b 100644
> >>>> --- a/xen/arch/x86/arch.mk
> >>>> +++ b/xen/arch/x86/arch.mk
> >>>> @@ -4,6 +4,7 @@
> >>>>  export XEN_IMG_OFFSET := 0x200000
> >>>>  
> >>>>  ARCH_LIBS-y += arch/x86/lib/lib.a
> >>>> +ALL_LIBS-y += arch/x86/lib/cpu-policy/lib.a
> >>> This wants to extend ARCH_LIBS-y surely?  Is this a rebasing oversight?
> >> No, this was deliberate. The functions here are different from those in
> >> arch/x86/lib/lib.a. We don't need to fear collision with "common code"
> >> ones. Hence I preferred to use the more "normal" placement into what's
> >> passed to the linker.
> > 
> > I agree that we don't have the explicit ordering requirement that we
> > have with arch/x86/lib/lib.a.
> > 
> > But, it still reads as bogus to be putting arch/x86/lib/cpu-policy/lib.a
> > in the non-ARCH list.
> > 
> > What difference is there having this a little earlier in the linker
> > arguments?  Nothing AFAICT.
> 
> Indeed. The sole reason why I'd prefer things as presented is that putting
> stuff in ARCH_LIBS should imo be the special case (i.e. requiring a special
> reason), while putting things in ALL_LIBS should be the default.

I agree with Andrew that it feels weird that arch/x86/lib/lib.a is
placed in ARCH_LIBS-y and arch/x86/lib/cpu-policy/lib.a is placed in
ALL_LIBS-y.  If we want to do it that way it needs a comment
explaining why they are placed in different list, otherwise it seems
like a typo on first sight, and it's likely to confuse people in the
future.

Thanks, Roger.

Re: [PATCH v2 1/2] x86/cpu-policy: move CPU policy library code

[Jan Beulich]

On 24.02.2026 09:53, Roger Pau Monné wrote:
> On Tue, Feb 24, 2026 at 07:54:29AM +0100, Jan Beulich wrote:
>> On 23.02.2026 20:00, Andrew Cooper wrote:
>>> On 02/02/2026 4:26 pm, Jan Beulich wrote:
>>>> On 02.02.2026 16:47, Andrew Cooper wrote:
>>>>> On 07/01/2026 2:17 pm, Jan Beulich wrote:
>>>>>> diff --git a/xen/arch/x86/arch.mk b/xen/arch/x86/arch.mk
>>>>>> index 0203138a819a..be6c76d2934b 100644
>>>>>> --- a/xen/arch/x86/arch.mk
>>>>>> +++ b/xen/arch/x86/arch.mk
>>>>>> @@ -4,6 +4,7 @@
>>>>>>  export XEN_IMG_OFFSET := 0x200000
>>>>>>  
>>>>>>  ARCH_LIBS-y += arch/x86/lib/lib.a
>>>>>> +ALL_LIBS-y += arch/x86/lib/cpu-policy/lib.a
>>>>> This wants to extend ARCH_LIBS-y surely?  Is this a rebasing oversight?
>>>> No, this was deliberate. The functions here are different from those in
>>>> arch/x86/lib/lib.a. We don't need to fear collision with "common code"
>>>> ones. Hence I preferred to use the more "normal" placement into what's
>>>> passed to the linker.
>>>
>>> I agree that we don't have the explicit ordering requirement that we
>>> have with arch/x86/lib/lib.a.
>>>
>>> But, it still reads as bogus to be putting arch/x86/lib/cpu-policy/lib.a
>>> in the non-ARCH list.
>>>
>>> What difference is there having this a little earlier in the linker
>>> arguments?  Nothing AFAICT.
>>
>> Indeed. The sole reason why I'd prefer things as presented is that putting
>> stuff in ARCH_LIBS should imo be the special case (i.e. requiring a special
>> reason), while putting things in ALL_LIBS should be the default.
> 
> I agree with Andrew that it feels weird that arch/x86/lib/lib.a is
> placed in ARCH_LIBS-y and arch/x86/lib/cpu-policy/lib.a is placed in
> ALL_LIBS-y.  If we want to do it that way it needs a comment
> explaining why they are placed in different list, otherwise it seems
> like a typo on first sight, and it's likely to confuse people in the
> future.

Well, I'll (reluctantly) change then.

Jan

Re: [PATCH v2 1/2] x86/cpu-policy: move CPU policy library code

[Andrew Cooper]

On 24/02/2026 8:58 am, Jan Beulich wrote:
> On 24.02.2026 09:53, Roger Pau Monné wrote:
>> On Tue, Feb 24, 2026 at 07:54:29AM +0100, Jan Beulich wrote:
>>> On 23.02.2026 20:00, Andrew Cooper wrote:
>>>> On 02/02/2026 4:26 pm, Jan Beulich wrote:
>>>>> On 02.02.2026 16:47, Andrew Cooper wrote:
>>>>>> On 07/01/2026 2:17 pm, Jan Beulich wrote:
>>>>>>> diff --git a/xen/arch/x86/arch.mk b/xen/arch/x86/arch.mk
>>>>>>> index 0203138a819a..be6c76d2934b 100644
>>>>>>> --- a/xen/arch/x86/arch.mk
>>>>>>> +++ b/xen/arch/x86/arch.mk
>>>>>>> @@ -4,6 +4,7 @@
>>>>>>>  export XEN_IMG_OFFSET := 0x200000
>>>>>>>  
>>>>>>>  ARCH_LIBS-y += arch/x86/lib/lib.a
>>>>>>> +ALL_LIBS-y += arch/x86/lib/cpu-policy/lib.a
>>>>>> This wants to extend ARCH_LIBS-y surely?  Is this a rebasing oversight?
>>>>> No, this was deliberate. The functions here are different from those in
>>>>> arch/x86/lib/lib.a. We don't need to fear collision with "common code"
>>>>> ones. Hence I preferred to use the more "normal" placement into what's
>>>>> passed to the linker.
>>>> I agree that we don't have the explicit ordering requirement that we
>>>> have with arch/x86/lib/lib.a.
>>>>
>>>> But, it still reads as bogus to be putting arch/x86/lib/cpu-policy/lib.a
>>>> in the non-ARCH list.
>>>>
>>>> What difference is there having this a little earlier in the linker
>>>> arguments?  Nothing AFAICT.
>>> Indeed. The sole reason why I'd prefer things as presented is that putting
>>> stuff in ARCH_LIBS should imo be the special case (i.e. requiring a special
>>> reason), while putting things in ALL_LIBS should be the default.
>> I agree with Andrew that it feels weird that arch/x86/lib/lib.a is
>> placed in ARCH_LIBS-y and arch/x86/lib/cpu-policy/lib.a is placed in
>> ALL_LIBS-y.  If we want to do it that way it needs a comment
>> explaining why they are placed in different list, otherwise it seems
>> like a typo on first sight, and it's likely to confuse people in the
>> future.
> Well, I'll (reluctantly) change then.

Thanks.  With that done, Reviewed-by: Andrew Cooper
<andrew.cooper3@citrix.com>