[PATCH v4 02/24] xen: consolidate cpuid library

Penny Zheng posted 24 patches 3 weeks, 1 day ago
There is a newer version of this series
[PATCH v4 02/24] xen: consolidate cpuid library
Posted by Penny Zheng 3 weeks, 1 day ago
There are some cpuid library functions only referenced in
XEN_DOMCTL_get{,set}_cpu_policy-case, and shall be wrapped with
CONFIG_MGMT_HYPERCALLS later, otherwise they will become unreachable when
MGMT_HYPERCALLS=n, and hence violate Misra 2.1
- x86_cpu_policy_clear_out_of_range_leaves
  - zero_leaves
- x86_cpuid_copy_to_buffer
  - copy_leaf_to_buffer
- x86_cpuid_copy_from_buffer
We seperate these functions by moving other functions to a new file named
cpuid-generic.c, and modify related Makefile-s to retain same behavior.

Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
---
v3 -> v4:
- new commit
---
 tools/fuzz/cpu-policy/Makefile               |   2 +-
 tools/fuzz/x86_instruction_emulator/Makefile |  10 +-
 tools/libs/guest/Makefile.common             |   2 +-
 tools/tests/cpu-policy/Makefile              |   2 +-
 tools/tests/x86_emulator/Makefile            |   2 +-
 xen/lib/x86/Makefile                         |   1 +
 xen/lib/x86/cpuid-generic.c                  | 273 +++++++++++++++++++
 xen/lib/x86/cpuid.c                          | 260 ------------------
 8 files changed, 283 insertions(+), 269 deletions(-)
 create mode 100644 xen/lib/x86/cpuid-generic.c

diff --git a/tools/fuzz/cpu-policy/Makefile b/tools/fuzz/cpu-policy/Makefile
index 6e7743e0aa..9d4627887b 100644
--- a/tools/fuzz/cpu-policy/Makefile
+++ b/tools/fuzz/cpu-policy/Makefile
@@ -22,7 +22,7 @@ CFLAGS += $(APPEND_CFLAGS) -Og
 
 vpath %.c ../../../xen/lib/x86
 
-afl-policy-fuzzer: afl-policy-fuzzer.o msr.o cpuid.o
+afl-policy-fuzzer: afl-policy-fuzzer.o msr.o cpuid.o cpuid-generic.o
 	$(CC) $(CFLAGS) $^ -o $@
 
 -include $(DEPS_INCLUDE)
diff --git a/tools/fuzz/x86_instruction_emulator/Makefile b/tools/fuzz/x86_instruction_emulator/Makefile
index 459743f4d9..6e2363a06b 100644
--- a/tools/fuzz/x86_instruction_emulator/Makefile
+++ b/tools/fuzz/x86_instruction_emulator/Makefile
@@ -25,7 +25,7 @@ x86_emulate/%.h: x86_emulate ;
 	ln -nsf $< $@
 
 CFLAGS += $(CFLAGS_xeninclude) -D__XEN_TOOLS__ -iquote .
-cpuid.o: CFLAGS += -iquote $(XEN_ROOT)/xen/lib/x86
+cpuid.o cpuid-generic.o: CFLAGS += -iquote $(XEN_ROOT)/xen/lib/x86
 
 GCOV_FLAGS := --coverage
 %-cov.o: %.c
@@ -49,16 +49,16 @@ $(filter x86_emulate/%.o,$(OBJS)): x86_emulate/%.o: x86_emulate/%.c $(private.h)
 $(patsubst %.o,%-cov.o,$(filter x86_emulate/%.o,$(OBJS))): x86_emulate/%-cov.o: x86_emulate/%.c $(private.h)
 	$(CC) $(CPPFLAGS) $(CFLAGS) $(CFLAGS_$*.o) $(GCOV_FLAGS) -c -o $@ $< $(APPEND_CFLAGS)
 
-x86-insn-fuzzer.a: $(OBJS) cpuid.o
+x86-insn-fuzzer.a: $(OBJS) cpuid.o cpuid-generic.o
 	$(AR) rc $@ $^
 
-afl-harness: afl-harness.o $(OBJS) cpuid.o wrappers.o
+afl-harness: afl-harness.o $(OBJS) cpuid.o cpuid-generic.o wrappers.o
 	$(CC) $(CFLAGS) $(addprefix -Wl$(comma)--wrap=,$(WRAPPED)) $^ -o $@
 
-afl-harness-cov: afl-harness-cov.o $(patsubst %.o,%-cov.o,$(OBJS)) cpuid.o wrappers.o
+afl-harness-cov: afl-harness-cov.o $(patsubst %.o,%-cov.o,$(OBJS)) cpuid.o cpuid-generic.o wrappers.o
 	$(CC) $(CFLAGS) $(GCOV_FLAGS) $(addprefix -Wl$(comma)--wrap=,$(WRAPPED)) $^ -o $@
 
-libfuzzer-harness: $(OBJS) cpuid.o wrappers.o
+libfuzzer-harness: $(OBJS) cpuid.o cpuid-generic.o wrappers.o
 	$(CC) $(CFLAGS) $(LIB_FUZZING_ENGINE) -fsanitize=fuzzer $(addprefix -Wl$(comma)--wrap=,$(WRAPPED)) $^ -o $@
 
 # Common targets
diff --git a/tools/libs/guest/Makefile.common b/tools/libs/guest/Makefile.common
index a026a2f662..da3c21e67e 100644
--- a/tools/libs/guest/Makefile.common
+++ b/tools/libs/guest/Makefile.common
@@ -35,7 +35,7 @@ OBJS-y += $(LIBELF_OBJS)
 ifeq ($(CONFIG_X86),y) # Add libx86 to the build
 vpath %.c ../../../xen/lib/x86
 
-OBJS-y                 += cpuid.o msr.o policy.o
+OBJS-y                 += cpuid.o cpuid-generic.o msr.o policy.o
 endif
 
 # new domain builder
diff --git a/tools/tests/cpu-policy/Makefile b/tools/tests/cpu-policy/Makefile
index 24f87e2eca..2f946b8a5e 100644
--- a/tools/tests/cpu-policy/Makefile
+++ b/tools/tests/cpu-policy/Makefile
@@ -46,7 +46,7 @@ vpath %.c ../../../xen/lib/x86
 
 %.o: Makefile
 
-test-cpu-policy: test-cpu-policy.o msr.o cpuid.o policy.o
+test-cpu-policy: test-cpu-policy.o msr.o cpuid.o cpuid-generic.o policy.o
 	$(CC) $^ -o $@ $(LDFLAGS)
 
 -include $(DEPS_INCLUDE)
diff --git a/tools/tests/x86_emulator/Makefile b/tools/tests/x86_emulator/Makefile
index fefa29a06c..eb79abd5b4 100644
--- a/tools/tests/x86_emulator/Makefile
+++ b/tools/tests/x86_emulator/Makefile
@@ -248,7 +248,7 @@ xop.h avx512f.h avx512fp16.h: simd-fma.c
 
 endif # 32-bit override
 
-OBJS := x86-emulate.o cpuid.o test_x86_emulator.o evex-disp8.o predicates.o wrappers.o
+OBJS := x86-emulate.o cpuid.o cpuid-generic.o test_x86_emulator.o evex-disp8.o predicates.o wrappers.o
 OBJS += x86_emulate/0f01.o x86_emulate/0fae.o x86_emulate/0fc7.o
 OBJS += x86_emulate/blk.o x86_emulate/decode.o x86_emulate/fpu.o x86_emulate/util.o
 
diff --git a/xen/lib/x86/Makefile b/xen/lib/x86/Makefile
index 780ea05db1..ac6d4369f3 100644
--- a/xen/lib/x86/Makefile
+++ b/xen/lib/x86/Makefile
@@ -1,3 +1,4 @@
 obj-y += cpuid.o
+obj-y += cpuid-generic.o
 obj-y += msr.o
 obj-y += policy.o
diff --git a/xen/lib/x86/cpuid-generic.c b/xen/lib/x86/cpuid-generic.c
new file mode 100644
index 0000000000..465bdee35a
--- /dev/null
+++ b/xen/lib/x86/cpuid-generic.c
@@ -0,0 +1,273 @@
+#include "private.h"
+
+#include <xen/lib/x86/cpu-policy.h>
+
+unsigned int x86_cpuid_lookup_vendor(uint32_t ebx, uint32_t ecx, uint32_t edx)
+{
+    switch ( ebx )
+    {
+    case X86_VENDOR_INTEL_EBX:
+        if ( ecx == X86_VENDOR_INTEL_ECX &&
+             edx == X86_VENDOR_INTEL_EDX )
+            return X86_VENDOR_INTEL;
+        break;
+
+    case X86_VENDOR_AMD_EBX:
+        if ( ecx == X86_VENDOR_AMD_ECX &&
+             edx == X86_VENDOR_AMD_EDX )
+            return X86_VENDOR_AMD;
+        break;
+
+    case X86_VENDOR_CENTAUR_EBX:
+        if ( ecx == X86_VENDOR_CENTAUR_ECX &&
+             edx == X86_VENDOR_CENTAUR_EDX )
+            return X86_VENDOR_CENTAUR;
+        break;
+
+    case X86_VENDOR_SHANGHAI_EBX:
+        if ( ecx == X86_VENDOR_SHANGHAI_ECX &&
+             edx == X86_VENDOR_SHANGHAI_EDX )
+            return X86_VENDOR_SHANGHAI;
+        break;
+
+    case X86_VENDOR_HYGON_EBX:
+        if ( ecx == X86_VENDOR_HYGON_ECX &&
+             edx == X86_VENDOR_HYGON_EDX )
+            return X86_VENDOR_HYGON;
+        break;
+    }
+
+    return X86_VENDOR_UNKNOWN;
+}
+
+const char *x86_cpuid_vendor_to_str(unsigned int vendor)
+{
+    switch ( vendor )
+    {
+    case X86_VENDOR_INTEL:    return "Intel";
+    case X86_VENDOR_AMD:      return "AMD";
+    case X86_VENDOR_CENTAUR:  return "Centaur";
+    case X86_VENDOR_SHANGHAI: return "Shanghai";
+    case X86_VENDOR_HYGON:    return "Hygon";
+    default:                  return "Unknown";
+    }
+}
+
+void x86_cpu_policy_to_featureset(
+    const struct cpu_policy *p, uint32_t fs[FEATURESET_NR_ENTRIES])
+{
+    fs[FEATURESET_1d]        = p->basic._1d;
+    fs[FEATURESET_1c]        = p->basic._1c;
+    fs[FEATURESET_e1d]       = p->extd.e1d;
+    fs[FEATURESET_e1c]       = p->extd.e1c;
+    fs[FEATURESET_Da1]       = p->xstate.Da1;
+    fs[FEATURESET_7b0]       = p->feat._7b0;
+    fs[FEATURESET_7c0]       = p->feat._7c0;
+    fs[FEATURESET_e7d]       = p->extd.e7d;
+    fs[FEATURESET_e8b]       = p->extd.e8b;
+    fs[FEATURESET_7d0]       = p->feat._7d0;
+    fs[FEATURESET_7a1]       = p->feat._7a1;
+    fs[FEATURESET_e21a]      = p->extd.e21a;
+    fs[FEATURESET_7b1]       = p->feat._7b1;
+    fs[FEATURESET_7d2]       = p->feat._7d2;
+    fs[FEATURESET_7c1]       = p->feat._7c1;
+    fs[FEATURESET_7d1]       = p->feat._7d1;
+    fs[FEATURESET_m10Al]     = p->arch_caps.lo;
+    fs[FEATURESET_m10Ah]     = p->arch_caps.hi;
+    fs[FEATURESET_e21c]      = p->extd.e21c;
+}
+
+void x86_cpu_featureset_to_policy(
+    const uint32_t fs[FEATURESET_NR_ENTRIES], struct cpu_policy *p)
+{
+    p->basic._1d             = fs[FEATURESET_1d];
+    p->basic._1c             = fs[FEATURESET_1c];
+    p->extd.e1d              = fs[FEATURESET_e1d];
+    p->extd.e1c              = fs[FEATURESET_e1c];
+    p->xstate.Da1            = fs[FEATURESET_Da1];
+    p->feat._7b0             = fs[FEATURESET_7b0];
+    p->feat._7c0             = fs[FEATURESET_7c0];
+    p->extd.e7d              = fs[FEATURESET_e7d];
+    p->extd.e8b              = fs[FEATURESET_e8b];
+    p->feat._7d0             = fs[FEATURESET_7d0];
+    p->feat._7a1             = fs[FEATURESET_7a1];
+    p->extd.e21a             = fs[FEATURESET_e21a];
+    p->feat._7b1             = fs[FEATURESET_7b1];
+    p->feat._7d2             = fs[FEATURESET_7d2];
+    p->feat._7c1             = fs[FEATURESET_7c1];
+    p->feat._7d1             = fs[FEATURESET_7d1];
+    p->arch_caps.lo          = fs[FEATURESET_m10Al];
+    p->arch_caps.hi          = fs[FEATURESET_m10Ah];
+    p->extd.e21c             = fs[FEATURESET_e21c];
+}
+
+void x86_cpu_policy_recalc_synth(struct cpu_policy *p)
+{
+    p->x86_vendor = x86_cpuid_lookup_vendor(
+        p->basic.vendor_ebx, p->basic.vendor_ecx, p->basic.vendor_edx);
+}
+
+void x86_cpu_policy_fill_native(struct cpu_policy *p)
+{
+    unsigned int i;
+
+    cpuid_leaf(0, &p->basic.raw[0]);
+    for ( i = 1; i <= MIN(p->basic.max_leaf,
+                          ARRAY_SIZE(p->basic.raw) - 1); ++i )
+    {
+        switch ( i )
+        {
+        case 0x4: case 0x7: case 0xb: case 0xd:
+            /* Multi-invocation leaves.  Deferred. */
+            continue;
+        }
+
+        cpuid_leaf(i, &p->basic.raw[i]);
+    }
+
+    if ( p->basic.max_leaf >= 4 )
+    {
+        for ( i = 0; i < ARRAY_SIZE(p->cache.raw); ++i )
+        {
+            union {
+                struct cpuid_leaf l;
+                struct cpuid_cache_leaf c;
+            } u;
+
+            cpuid_count_leaf(4, i, &u.l);
+
+            if ( u.c.type == 0 )
+                break;
+
+            p->cache.subleaf[i] = u.c;
+        }
+
+        /*
+         * The choice of CPUID_GUEST_NR_CACHE is arbitrary.  It is expected
+         * that it will eventually need increasing for future hardware.
+         */
+#ifdef __XEN__
+        if ( i == ARRAY_SIZE(p->cache.raw) )
+            printk(XENLOG_WARNING
+                   "CPUID: Insufficient Leaf 4 space for this hardware\n");
+#endif
+    }
+
+    if ( p->basic.max_leaf >= 7 )
+    {
+        cpuid_count_leaf(7, 0, &p->feat.raw[0]);
+
+        for ( i = 1; i <= MIN(p->feat.max_subleaf,
+                              ARRAY_SIZE(p->feat.raw) - 1); ++i )
+            cpuid_count_leaf(7, i, &p->feat.raw[i]);
+    }
+
+    if ( p->basic.max_leaf >= 0xb )
+    {
+        union {
+            struct cpuid_leaf l;
+            struct cpuid_topo_leaf t;
+        } u;
+
+        for ( i = 0; i < ARRAY_SIZE(p->topo.raw); ++i )
+        {
+            cpuid_count_leaf(0xb, i, &u.l);
+
+            if ( u.t.type == 0 )
+                break;
+
+            p->topo.subleaf[i] = u.t;
+        }
+
+        /*
+         * The choice of CPUID_GUEST_NR_TOPO is per the manual.  It may need
+         * to grow for future hardware.
+         */
+#ifdef __XEN__
+        if ( i == ARRAY_SIZE(p->topo.raw) &&
+             (cpuid_count_leaf(0xb, i, &u.l), u.t.type != 0) )
+            printk(XENLOG_WARNING
+                   "CPUID: Insufficient Leaf 0xb space for this hardware\n");
+#endif
+    }
+
+    if ( p->basic.max_leaf >= 0xd )
+    {
+        uint64_t xstates;
+
+        cpuid_count_leaf(0xd, 0, &p->xstate.raw[0]);
+        cpuid_count_leaf(0xd, 1, &p->xstate.raw[1]);
+
+        xstates = cpu_policy_xstates(p);
+
+        /* This logic will probably need adjusting when XCR0[63] gets used. */
+        BUILD_BUG_ON(ARRAY_SIZE(p->xstate.raw) > 63);
+
+        for ( i = 2; i < min_t(unsigned int, 63,
+                               ARRAY_SIZE(p->xstate.raw)); ++i )
+        {
+            if ( xstates & (1ULL << i) )
+                cpuid_count_leaf(0xd, i, &p->xstate.raw[i]);
+        }
+    }
+
+    /* Extended leaves. */
+    cpuid_leaf(0x80000000U, &p->extd.raw[0]);
+    for ( i = 1; i <= MIN(p->extd.max_leaf & 0xffffU,
+                          ARRAY_SIZE(p->extd.raw) - 1); ++i )
+        cpuid_leaf(0x80000000U + i, &p->extd.raw[i]);
+
+    /* Don't report leaves from possible lower level hypervisor, for now. */
+    p->hv_limit = 0;
+    p->hv2_limit = 0;
+
+#ifdef __XEN__
+    /* TODO MSR_PLATFORM_INFO */
+
+    if ( p->feat.arch_caps )
+        rdmsrl(MSR_ARCH_CAPABILITIES, p->arch_caps.raw);
+#endif
+
+    x86_cpu_policy_recalc_synth(p);
+}
+
+const uint32_t *x86_cpu_policy_lookup_deep_deps(uint32_t feature)
+{
+    static const uint32_t deep_features[] = INIT_DEEP_FEATURES;
+    static const struct {
+        uint32_t feature;
+        uint32_t fs[FEATURESET_NR_ENTRIES];
+    } deep_deps[] = INIT_DEEP_DEPS;
+    unsigned int start = 0, end = ARRAY_SIZE(deep_deps);
+
+    BUILD_BUG_ON(ARRAY_SIZE(deep_deps) != NR_DEEP_DEPS);
+
+    /* Fast early exit. */
+    if ( !test_bit(feature, deep_features) )
+        return NULL;
+
+    /* deep_deps[] is sorted.  Perform a binary search. */
+    while ( start < end )
+    {
+        unsigned int mid = start + ((end - start) / 2);
+
+        if ( deep_deps[mid].feature > feature )
+            end = mid;
+        else if ( deep_deps[mid].feature < feature )
+            start = mid + 1;
+        else
+            return deep_deps[mid].fs;
+    }
+
+    return NULL;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/lib/x86/cpuid.c b/xen/lib/x86/cpuid.c
index 6298d051f2..5de1e2ca74 100644
--- a/xen/lib/x86/cpuid.c
+++ b/xen/lib/x86/cpuid.c
@@ -9,235 +9,6 @@ static void zero_leaves(struct cpuid_leaf *l,
         memset(&l[first], 0, sizeof(*l) * (last - first + 1));
 }
 
-unsigned int x86_cpuid_lookup_vendor(uint32_t ebx, uint32_t ecx, uint32_t edx)
-{
-    switch ( ebx )
-    {
-    case X86_VENDOR_INTEL_EBX:
-        if ( ecx == X86_VENDOR_INTEL_ECX &&
-             edx == X86_VENDOR_INTEL_EDX )
-            return X86_VENDOR_INTEL;
-        break;
-
-    case X86_VENDOR_AMD_EBX:
-        if ( ecx == X86_VENDOR_AMD_ECX &&
-             edx == X86_VENDOR_AMD_EDX )
-            return X86_VENDOR_AMD;
-        break;
-
-    case X86_VENDOR_CENTAUR_EBX:
-        if ( ecx == X86_VENDOR_CENTAUR_ECX &&
-             edx == X86_VENDOR_CENTAUR_EDX )
-            return X86_VENDOR_CENTAUR;
-        break;
-
-    case X86_VENDOR_SHANGHAI_EBX:
-        if ( ecx == X86_VENDOR_SHANGHAI_ECX &&
-             edx == X86_VENDOR_SHANGHAI_EDX )
-            return X86_VENDOR_SHANGHAI;
-        break;
-
-    case X86_VENDOR_HYGON_EBX:
-        if ( ecx == X86_VENDOR_HYGON_ECX &&
-             edx == X86_VENDOR_HYGON_EDX )
-            return X86_VENDOR_HYGON;
-        break;
-    }
-
-    return X86_VENDOR_UNKNOWN;
-}
-
-const char *x86_cpuid_vendor_to_str(unsigned int vendor)
-{
-    switch ( vendor )
-    {
-    case X86_VENDOR_INTEL:    return "Intel";
-    case X86_VENDOR_AMD:      return "AMD";
-    case X86_VENDOR_CENTAUR:  return "Centaur";
-    case X86_VENDOR_SHANGHAI: return "Shanghai";
-    case X86_VENDOR_HYGON:    return "Hygon";
-    default:                  return "Unknown";
-    }
-}
-
-void x86_cpu_policy_to_featureset(
-    const struct cpu_policy *p, uint32_t fs[FEATURESET_NR_ENTRIES])
-{
-    fs[FEATURESET_1d]        = p->basic._1d;
-    fs[FEATURESET_1c]        = p->basic._1c;
-    fs[FEATURESET_e1d]       = p->extd.e1d;
-    fs[FEATURESET_e1c]       = p->extd.e1c;
-    fs[FEATURESET_Da1]       = p->xstate.Da1;
-    fs[FEATURESET_7b0]       = p->feat._7b0;
-    fs[FEATURESET_7c0]       = p->feat._7c0;
-    fs[FEATURESET_e7d]       = p->extd.e7d;
-    fs[FEATURESET_e8b]       = p->extd.e8b;
-    fs[FEATURESET_7d0]       = p->feat._7d0;
-    fs[FEATURESET_7a1]       = p->feat._7a1;
-    fs[FEATURESET_e21a]      = p->extd.e21a;
-    fs[FEATURESET_7b1]       = p->feat._7b1;
-    fs[FEATURESET_7d2]       = p->feat._7d2;
-    fs[FEATURESET_7c1]       = p->feat._7c1;
-    fs[FEATURESET_7d1]       = p->feat._7d1;
-    fs[FEATURESET_m10Al]     = p->arch_caps.lo;
-    fs[FEATURESET_m10Ah]     = p->arch_caps.hi;
-    fs[FEATURESET_e21c]      = p->extd.e21c;
-}
-
-void x86_cpu_featureset_to_policy(
-    const uint32_t fs[FEATURESET_NR_ENTRIES], struct cpu_policy *p)
-{
-    p->basic._1d             = fs[FEATURESET_1d];
-    p->basic._1c             = fs[FEATURESET_1c];
-    p->extd.e1d              = fs[FEATURESET_e1d];
-    p->extd.e1c              = fs[FEATURESET_e1c];
-    p->xstate.Da1            = fs[FEATURESET_Da1];
-    p->feat._7b0             = fs[FEATURESET_7b0];
-    p->feat._7c0             = fs[FEATURESET_7c0];
-    p->extd.e7d              = fs[FEATURESET_e7d];
-    p->extd.e8b              = fs[FEATURESET_e8b];
-    p->feat._7d0             = fs[FEATURESET_7d0];
-    p->feat._7a1             = fs[FEATURESET_7a1];
-    p->extd.e21a             = fs[FEATURESET_e21a];
-    p->feat._7b1             = fs[FEATURESET_7b1];
-    p->feat._7d2             = fs[FEATURESET_7d2];
-    p->feat._7c1             = fs[FEATURESET_7c1];
-    p->feat._7d1             = fs[FEATURESET_7d1];
-    p->arch_caps.lo          = fs[FEATURESET_m10Al];
-    p->arch_caps.hi          = fs[FEATURESET_m10Ah];
-    p->extd.e21c             = fs[FEATURESET_e21c];
-}
-
-void x86_cpu_policy_recalc_synth(struct cpu_policy *p)
-{
-    p->x86_vendor = x86_cpuid_lookup_vendor(
-        p->basic.vendor_ebx, p->basic.vendor_ecx, p->basic.vendor_edx);
-}
-
-void x86_cpu_policy_fill_native(struct cpu_policy *p)
-{
-    unsigned int i;
-
-    cpuid_leaf(0, &p->basic.raw[0]);
-    for ( i = 1; i <= MIN(p->basic.max_leaf,
-                          ARRAY_SIZE(p->basic.raw) - 1); ++i )
-    {
-        switch ( i )
-        {
-        case 0x4: case 0x7: case 0xb: case 0xd:
-            /* Multi-invocation leaves.  Deferred. */
-            continue;
-        }
-
-        cpuid_leaf(i, &p->basic.raw[i]);
-    }
-
-    if ( p->basic.max_leaf >= 4 )
-    {
-        for ( i = 0; i < ARRAY_SIZE(p->cache.raw); ++i )
-        {
-            union {
-                struct cpuid_leaf l;
-                struct cpuid_cache_leaf c;
-            } u;
-
-            cpuid_count_leaf(4, i, &u.l);
-
-            if ( u.c.type == 0 )
-                break;
-
-            p->cache.subleaf[i] = u.c;
-        }
-
-        /*
-         * The choice of CPUID_GUEST_NR_CACHE is arbitrary.  It is expected
-         * that it will eventually need increasing for future hardware.
-         */
-#ifdef __XEN__
-        if ( i == ARRAY_SIZE(p->cache.raw) )
-            printk(XENLOG_WARNING
-                   "CPUID: Insufficient Leaf 4 space for this hardware\n");
-#endif
-    }
-
-    if ( p->basic.max_leaf >= 7 )
-    {
-        cpuid_count_leaf(7, 0, &p->feat.raw[0]);
-
-        for ( i = 1; i <= MIN(p->feat.max_subleaf,
-                              ARRAY_SIZE(p->feat.raw) - 1); ++i )
-            cpuid_count_leaf(7, i, &p->feat.raw[i]);
-    }
-
-    if ( p->basic.max_leaf >= 0xb )
-    {
-        union {
-            struct cpuid_leaf l;
-            struct cpuid_topo_leaf t;
-        } u;
-
-        for ( i = 0; i < ARRAY_SIZE(p->topo.raw); ++i )
-        {
-            cpuid_count_leaf(0xb, i, &u.l);
-
-            if ( u.t.type == 0 )
-                break;
-
-            p->topo.subleaf[i] = u.t;
-        }
-
-        /*
-         * The choice of CPUID_GUEST_NR_TOPO is per the manual.  It may need
-         * to grow for future hardware.
-         */
-#ifdef __XEN__
-        if ( i == ARRAY_SIZE(p->topo.raw) &&
-             (cpuid_count_leaf(0xb, i, &u.l), u.t.type != 0) )
-            printk(XENLOG_WARNING
-                   "CPUID: Insufficient Leaf 0xb space for this hardware\n");
-#endif
-    }
-
-    if ( p->basic.max_leaf >= 0xd )
-    {
-        uint64_t xstates;
-
-        cpuid_count_leaf(0xd, 0, &p->xstate.raw[0]);
-        cpuid_count_leaf(0xd, 1, &p->xstate.raw[1]);
-
-        xstates = cpu_policy_xstates(p);
-
-        /* This logic will probably need adjusting when XCR0[63] gets used. */
-        BUILD_BUG_ON(ARRAY_SIZE(p->xstate.raw) > 63);
-
-        for ( i = 2; i < min_t(unsigned int, 63,
-                               ARRAY_SIZE(p->xstate.raw)); ++i )
-        {
-            if ( xstates & (1ULL << i) )
-                cpuid_count_leaf(0xd, i, &p->xstate.raw[i]);
-        }
-    }
-
-    /* Extended leaves. */
-    cpuid_leaf(0x80000000U, &p->extd.raw[0]);
-    for ( i = 1; i <= MIN(p->extd.max_leaf & 0xffffU,
-                          ARRAY_SIZE(p->extd.raw) - 1); ++i )
-        cpuid_leaf(0x80000000U + i, &p->extd.raw[i]);
-
-    /* Don't report leaves from possible lower level hypervisor, for now. */
-    p->hv_limit = 0;
-    p->hv2_limit = 0;
-
-#ifdef __XEN__
-    /* TODO MSR_PLATFORM_INFO */
-
-    if ( p->feat.arch_caps )
-        rdmsrl(MSR_ARCH_CAPABILITIES, p->arch_caps.raw);
-#endif
-
-    x86_cpu_policy_recalc_synth(p);
-}
-
 void x86_cpu_policy_clear_out_of_range_leaves(struct cpu_policy *p)
 {
     unsigned int i;
@@ -291,37 +62,6 @@ void x86_cpu_policy_clear_out_of_range_leaves(struct cpu_policy *p)
                 ARRAY_SIZE(p->extd.raw) - 1);
 }
 
-const uint32_t *x86_cpu_policy_lookup_deep_deps(uint32_t feature)
-{
-    static const uint32_t deep_features[] = INIT_DEEP_FEATURES;
-    static const struct {
-        uint32_t feature;
-        uint32_t fs[FEATURESET_NR_ENTRIES];
-    } deep_deps[] = INIT_DEEP_DEPS;
-    unsigned int start = 0, end = ARRAY_SIZE(deep_deps);
-
-    BUILD_BUG_ON(ARRAY_SIZE(deep_deps) != NR_DEEP_DEPS);
-
-    /* Fast early exit. */
-    if ( !test_bit(feature, deep_features) )
-        return NULL;
-
-    /* deep_deps[] is sorted.  Perform a binary search. */
-    while ( start < end )
-    {
-        unsigned int mid = start + ((end - start) / 2);
-
-        if ( deep_deps[mid].feature > feature )
-            end = mid;
-        else if ( deep_deps[mid].feature < feature )
-            start = mid + 1;
-        else
-            return deep_deps[mid].fs;
-    }
-
-    return NULL;
-}
-
 /*
  * Copy a single cpuid_leaf into a provided xen_cpuid_leaf_t buffer,
  * performing boundary checking against the buffer size.
-- 
2.34.1
Re: [PATCH v4 02/24] xen: consolidate cpuid library
Posted by Grygorii Strashko 2 weeks, 3 days ago

On 21.11.25 12:57, Penny Zheng wrote:
> There are some cpuid library functions only referenced in
> XEN_DOMCTL_get{,set}_cpu_policy-case, and shall be wrapped with
> CONFIG_MGMT_HYPERCALLS later, otherwise they will become unreachable when
> MGMT_HYPERCALLS=n, and hence violate Misra 2.1
> - x86_cpu_policy_clear_out_of_range_leaves
>    - zero_leaves
> - x86_cpuid_copy_to_buffer
>    - copy_leaf_to_buffer
> - x86_cpuid_copy_from_buffer
> We seperate these functions by moving other functions to a new file named
> cpuid-generic.c, and modify related Makefile-s to retain same behavior.
> 
> Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
> ---
> v3 -> v4:
> - new commit
> ---
>   tools/fuzz/cpu-policy/Makefile               |   2 +-
>   tools/fuzz/x86_instruction_emulator/Makefile |  10 +-
>   tools/libs/guest/Makefile.common             |   2 +-
>   tools/tests/cpu-policy/Makefile              |   2 +-
>   tools/tests/x86_emulator/Makefile            |   2 +-
>   xen/lib/x86/Makefile                         |   1 +
>   xen/lib/x86/cpuid-generic.c                  | 273 +++++++++++++++++++
>   xen/lib/x86/cpuid.c                          | 260 ------------------
>   8 files changed, 283 insertions(+), 269 deletions(-)
>   create mode 100644 xen/lib/x86/cpuid-generic.c

It seems this patch is not required prerequisite, at least definitely not for Patch 3.

In general, i think it can be removed from this series and sent as follow up patch.

-- 
Best regards,
-grygorii
RE: [PATCH v4 02/24] xen: consolidate cpuid library
Posted by Penny, Zheng 1 week, 5 days ago
[Public]

> -----Original Message-----
> From: Grygorii Strashko <grygorii_strashko@epam.com>
> Sent: Thursday, November 27, 2025 5:45 AM
> To: Penny, Zheng <penny.zheng@amd.com>; xen-devel@lists.xenproject.org
> Cc: Huang, Ray <Ray.Huang@amd.com>; Jan Beulich <jbeulich@suse.com>;
> Andrew Cooper <andrew.cooper3@citrix.com>; Roger Pau Monné
> <roger.pau@citrix.com>; Anthony PERARD <anthony.perard@vates.tech>; Juergen
> Gross <jgross@suse.com>
> Subject: Re: [PATCH v4 02/24] xen: consolidate cpuid library
>
>
>
> On 21.11.25 12:57, Penny Zheng wrote:
> > There are some cpuid library functions only referenced in
> > XEN_DOMCTL_get{,set}_cpu_policy-case, and shall be wrapped with
> > CONFIG_MGMT_HYPERCALLS later, otherwise they will become unreachable
> > when MGMT_HYPERCALLS=n, and hence violate Misra 2.1
> > - x86_cpu_policy_clear_out_of_range_leaves
> >    - zero_leaves
> > - x86_cpuid_copy_to_buffer
> >    - copy_leaf_to_buffer
> > - x86_cpuid_copy_from_buffer
> > We seperate these functions by moving other functions to a new file
> > named cpuid-generic.c, and modify related Makefile-s to retain same behavior.
> >
> > Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
> > ---
> > v3 -> v4:
> > - new commit
> > ---
> >   tools/fuzz/cpu-policy/Makefile               |   2 +-
> >   tools/fuzz/x86_instruction_emulator/Makefile |  10 +-
> >   tools/libs/guest/Makefile.common             |   2 +-
> >   tools/tests/cpu-policy/Makefile              |   2 +-
> >   tools/tests/x86_emulator/Makefile            |   2 +-
> >   xen/lib/x86/Makefile                         |   1 +
> >   xen/lib/x86/cpuid-generic.c                  | 273 +++++++++++++++++++
> >   xen/lib/x86/cpuid.c                          | 260 ------------------
> >   8 files changed, 283 insertions(+), 269 deletions(-)
> >   create mode 100644 xen/lib/x86/cpuid-generic.c
>
> It seems this patch is not required prerequisite, at least definitely not for Patch 3.
>

It is the pre-requisite for commit " xen/x86: wrap x86-specific domctl-op with CONFIG_MGMT_HYPERCALLS ", We want to guard new cpuid.o with MGMT_HYPERCALLS there. Without the split, fwis, I could not think a better way to avoid functions like x86_cpuid_copy_to{,from}_buffer becoming unreachable when MGMT_HYPERCALLS=n.

> In general, i think it can be removed from this series and sent as follow up patch.
>
> --
> Best regards,
> -grygorii

Re: [PATCH v4 02/24] xen: consolidate cpuid library
Posted by Grygorii Strashko 1 week, 4 days ago

On 01.12.25 08:34, Penny, Zheng wrote:
> [Public]
> 
>> -----Original Message-----
>> From: Grygorii Strashko <grygorii_strashko@epam.com>
>> Sent: Thursday, November 27, 2025 5:45 AM
>> To: Penny, Zheng <penny.zheng@amd.com>; xen-devel@lists.xenproject.org
>> Cc: Huang, Ray <Ray.Huang@amd.com>; Jan Beulich <jbeulich@suse.com>;
>> Andrew Cooper <andrew.cooper3@citrix.com>; Roger Pau Monné
>> <roger.pau@citrix.com>; Anthony PERARD <anthony.perard@vates.tech>; Juergen
>> Gross <jgross@suse.com>
>> Subject: Re: [PATCH v4 02/24] xen: consolidate cpuid library
>>
>>
>>
>> On 21.11.25 12:57, Penny Zheng wrote:
>>> There are some cpuid library functions only referenced in
>>> XEN_DOMCTL_get{,set}_cpu_policy-case, and shall be wrapped with
>>> CONFIG_MGMT_HYPERCALLS later, otherwise they will become unreachable
>>> when MGMT_HYPERCALLS=n, and hence violate Misra 2.1
>>> - x86_cpu_policy_clear_out_of_range_leaves
>>>     - zero_leaves
>>> - x86_cpuid_copy_to_buffer
>>>     - copy_leaf_to_buffer
>>> - x86_cpuid_copy_from_buffer
>>> We seperate these functions by moving other functions to a new file
>>> named cpuid-generic.c, and modify related Makefile-s to retain same behavior.
>>>
>>> Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
>>> ---
>>> v3 -> v4:
>>> - new commit
>>> ---
>>>    tools/fuzz/cpu-policy/Makefile               |   2 +-
>>>    tools/fuzz/x86_instruction_emulator/Makefile |  10 +-
>>>    tools/libs/guest/Makefile.common             |   2 +-
>>>    tools/tests/cpu-policy/Makefile              |   2 +-
>>>    tools/tests/x86_emulator/Makefile            |   2 +-
>>>    xen/lib/x86/Makefile                         |   1 +
>>>    xen/lib/x86/cpuid-generic.c                  | 273 +++++++++++++++++++
>>>    xen/lib/x86/cpuid.c                          | 260 ------------------
>>>    8 files changed, 283 insertions(+), 269 deletions(-)
>>>    create mode 100644 xen/lib/x86/cpuid-generic.c
>>
>> It seems this patch is not required prerequisite, at least definitely not for Patch 3.
>>
> 
> It is the pre-requisite for commit " xen/x86: wrap x86-specific domctl-op with CONFIG_MGMT_HYPERCALLS ", We want to guard new cpuid.o with MGMT_HYPERCALLS there. Without the split, fwis, I could not think a better way to avoid functions like x86_cpuid_copy_to{,from}_buffer becoming unreachable when MGMT_HYPERCALLS=n.

I think, confusion here is that this patch:
- new
- placed before patch 3
which introduces false assumption that all patches after it are dependent  on it.
Effectively blocking possible merge of Patch 3, for example.

Could it be moved up in this series before patch you've mentioned?

> 
>> In general, i think it can be removed from this series and sent as follow up patch.

-- 
Best regards,
-grygorii


Re: [PATCH v4 02/24] xen: consolidate cpuid library
Posted by Jan Beulich 2 weeks, 4 days ago
On 21.11.2025 11:57, Penny Zheng wrote:
> There are some cpuid library functions only referenced in
> XEN_DOMCTL_get{,set}_cpu_policy-case, and shall be wrapped with
> CONFIG_MGMT_HYPERCALLS later, otherwise they will become unreachable when
> MGMT_HYPERCALLS=n, and hence violate Misra 2.1
> - x86_cpu_policy_clear_out_of_range_leaves
>   - zero_leaves
> - x86_cpuid_copy_to_buffer
>   - copy_leaf_to_buffer
> - x86_cpuid_copy_from_buffer
> We seperate these functions by moving other functions to a new file named
> cpuid-generic.c, and modify related Makefile-s to retain same behavior.
> 
> Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
> ---
> v3 -> v4:
> - new commit
> ---
>  tools/fuzz/cpu-policy/Makefile               |   2 +-
>  tools/fuzz/x86_instruction_emulator/Makefile |  10 +-
>  tools/libs/guest/Makefile.common             |   2 +-
>  tools/tests/cpu-policy/Makefile              |   2 +-
>  tools/tests/x86_emulator/Makefile            |   2 +-
>  xen/lib/x86/Makefile                         |   1 +
>  xen/lib/x86/cpuid-generic.c                  | 273 +++++++++++++++++++
>  xen/lib/x86/cpuid.c                          | 260 ------------------
>  8 files changed, 283 insertions(+), 269 deletions(-)
>  create mode 100644 xen/lib/x86/cpuid-generic.c

Andrew - what's your take on such a split? Personally I'm not overly
happy to see related functions be scattered across two files. The
separation also feels pretty random, posing the risk that later some
of the code may need to move back.

Penny, I also don't think "consolidate" is what is happening here.
Perhaps "split" would be getting closer?

Jan
RE: [PATCH v4 02/24] xen: consolidate cpuid library
Posted by Penny, Zheng 1 week, 5 days ago
[Public]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Tuesday, November 25, 2025 2:39 PM
> To: Penny, Zheng <penny.zheng@amd.com>; Andrew Cooper
> <andrew.cooper3@citrix.com>
> Cc: Huang, Ray <Ray.Huang@amd.com>; grygorii_strashko@epam.com; Roger
> Pau Monné <roger.pau@citrix.com>; Anthony PERARD
> <anthony.perard@vates.tech>; Juergen Gross <jgross@suse.com>; xen-
> devel@lists.xenproject.org
> Subject: Re: [PATCH v4 02/24] xen: consolidate cpuid library
>
> On 21.11.2025 11:57, Penny Zheng wrote:
> > There are some cpuid library functions only referenced in
> > XEN_DOMCTL_get{,set}_cpu_policy-case, and shall be wrapped with
> > CONFIG_MGMT_HYPERCALLS later, otherwise they will become unreachable
> > when MGMT_HYPERCALLS=n, and hence violate Misra 2.1
> > - x86_cpu_policy_clear_out_of_range_leaves
> >   - zero_leaves
> > - x86_cpuid_copy_to_buffer
> >   - copy_leaf_to_buffer
> > - x86_cpuid_copy_from_buffer
> > We seperate these functions by moving other functions to a new file
> > named cpuid-generic.c, and modify related Makefile-s to retain same behavior.
> >
> > Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
> > ---
> > v3 -> v4:
> > - new commit
> > ---
> >  tools/fuzz/cpu-policy/Makefile               |   2 +-
> >  tools/fuzz/x86_instruction_emulator/Makefile |  10 +-
> >  tools/libs/guest/Makefile.common             |   2 +-
> >  tools/tests/cpu-policy/Makefile              |   2 +-
> >  tools/tests/x86_emulator/Makefile            |   2 +-
> >  xen/lib/x86/Makefile                         |   1 +
> >  xen/lib/x86/cpuid-generic.c                  | 273 +++++++++++++++++++
> >  xen/lib/x86/cpuid.c                          | 260 ------------------
> >  8 files changed, 283 insertions(+), 269 deletions(-)  create mode
> > 100644 xen/lib/x86/cpuid-generic.c
>
> Andrew - what's your take on such a split? Personally I'm not overly happy to see
> related functions be scattered across two files. The separation also feels pretty
> random, posing the risk that later some of the code may need to move back.
>

Right now, I could not think a better way to guard x86_cpuid_copy_from{,to}_buffer with MGMT_HYPERCALLS without split, any better suggestion? Or maybe I could add up some explanations on the file cpuid_generic.c head note to explain the diffs between itself and cpuid.c, something like:
```
The difference between cpuid.c and cpuid_generic.c is that the former contains library functions that has only been referenced in management hypercalls, such as sysctl, domctl, etc. See comment for MGMT_HYPERCALLS.
```

> Penny, I also don't think "consolidate" is what is happening here.
> Perhaps "split" would be getting closer?
>
> Jan
Re: [PATCH v4 02/24] xen: consolidate cpuid library
Posted by Jan Beulich 1 week, 5 days ago
On 01.12.2025 07:57, Penny, Zheng wrote:
> [Public]
> 
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: Tuesday, November 25, 2025 2:39 PM
>> To: Penny, Zheng <penny.zheng@amd.com>; Andrew Cooper
>> <andrew.cooper3@citrix.com>
>> Cc: Huang, Ray <Ray.Huang@amd.com>; grygorii_strashko@epam.com; Roger
>> Pau Monné <roger.pau@citrix.com>; Anthony PERARD
>> <anthony.perard@vates.tech>; Juergen Gross <jgross@suse.com>; xen-
>> devel@lists.xenproject.org
>> Subject: Re: [PATCH v4 02/24] xen: consolidate cpuid library
>>
>> On 21.11.2025 11:57, Penny Zheng wrote:
>>> There are some cpuid library functions only referenced in
>>> XEN_DOMCTL_get{,set}_cpu_policy-case, and shall be wrapped with
>>> CONFIG_MGMT_HYPERCALLS later, otherwise they will become unreachable
>>> when MGMT_HYPERCALLS=n, and hence violate Misra 2.1
>>> - x86_cpu_policy_clear_out_of_range_leaves
>>>   - zero_leaves
>>> - x86_cpuid_copy_to_buffer
>>>   - copy_leaf_to_buffer
>>> - x86_cpuid_copy_from_buffer
>>> We seperate these functions by moving other functions to a new file
>>> named cpuid-generic.c, and modify related Makefile-s to retain same behavior.
>>>
>>> Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
>>> ---
>>> v3 -> v4:
>>> - new commit
>>> ---
>>>  tools/fuzz/cpu-policy/Makefile               |   2 +-
>>>  tools/fuzz/x86_instruction_emulator/Makefile |  10 +-
>>>  tools/libs/guest/Makefile.common             |   2 +-
>>>  tools/tests/cpu-policy/Makefile              |   2 +-
>>>  tools/tests/x86_emulator/Makefile            |   2 +-
>>>  xen/lib/x86/Makefile                         |   1 +
>>>  xen/lib/x86/cpuid-generic.c                  | 273 +++++++++++++++++++
>>>  xen/lib/x86/cpuid.c                          | 260 ------------------
>>>  8 files changed, 283 insertions(+), 269 deletions(-)  create mode
>>> 100644 xen/lib/x86/cpuid-generic.c
>>
>> Andrew - what's your take on such a split? Personally I'm not overly happy to see
>> related functions be scattered across two files. The separation also feels pretty
>> random, posing the risk that later some of the code may need to move back.
>>
> 
> Right now, I could not think a better way to guard x86_cpuid_copy_from{,to}_buffer with MGMT_HYPERCALLS without split, any better suggestion? Or maybe I could add up some explanations on the file cpuid_generic.c head note to explain the diffs between itself and cpuid.c, something like:
> ```
> The difference between cpuid.c and cpuid_generic.c is that the former contains library functions that has only been referenced in management hypercalls, such as sysctl, domctl, etc. See comment for MGMT_HYPERCALLS.
> ```

If one of the files is to have only MGMT_HYPERCALLS related stuff (and if, prior
to that, using #ifdef-ary in the existing file was proven unwieldy), then imo
the more "natural" split would be to have a separate cpuid-mgmt.c file, where
then from its name alone it already becomes halfway clear what it to live there.

Another option might be to properly library-fy the copy-in and copy-out functions,
one per source file, and then referenced by lib-y (or lib-$(CONFIG_...)) from the
Makefile.

Jan

RE: [PATCH v4 02/24] xen: consolidate cpuid library
Posted by Penny, Zheng 1 week, 4 days ago
[Public]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Monday, December 1, 2025 4:23 PM
> To: Penny, Zheng <penny.zheng@amd.com>
> Cc: Huang, Ray <Ray.Huang@amd.com>; grygorii_strashko@epam.com; Roger
> Pau Monné <roger.pau@citrix.com>; Anthony PERARD
> <anthony.perard@vates.tech>; Juergen Gross <jgross@suse.com>; xen-
> devel@lists.xenproject.org; Andrew Cooper <andrew.cooper3@citrix.com>
> Subject: Re: [PATCH v4 02/24] xen: consolidate cpuid library
>
> On 01.12.2025 07:57, Penny, Zheng wrote:
> > [Public]
> >
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@suse.com>
> >> Sent: Tuesday, November 25, 2025 2:39 PM
> >> To: Penny, Zheng <penny.zheng@amd.com>; Andrew Cooper
> >> <andrew.cooper3@citrix.com>
> >> Cc: Huang, Ray <Ray.Huang@amd.com>; grygorii_strashko@epam.com; Roger
> >> Pau Monné <roger.pau@citrix.com>; Anthony PERARD
> >> <anthony.perard@vates.tech>; Juergen Gross <jgross@suse.com>; xen-
> >> devel@lists.xenproject.org
> >> Subject: Re: [PATCH v4 02/24] xen: consolidate cpuid library
> >>
> >> On 21.11.2025 11:57, Penny Zheng wrote:
> >>> There are some cpuid library functions only referenced in
> >>> XEN_DOMCTL_get{,set}_cpu_policy-case, and shall be wrapped with
> >>> CONFIG_MGMT_HYPERCALLS later, otherwise they will become unreachable
> >>> when MGMT_HYPERCALLS=n, and hence violate Misra 2.1
> >>> - x86_cpu_policy_clear_out_of_range_leaves
> >>>   - zero_leaves
> >>> - x86_cpuid_copy_to_buffer
> >>>   - copy_leaf_to_buffer
> >>> - x86_cpuid_copy_from_buffer
> >>> We seperate these functions by moving other functions to a new file
> >>> named cpuid-generic.c, and modify related Makefile-s to retain same behavior.
> >>>
> >>> Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
> >>> ---
> >>> v3 -> v4:
> >>> - new commit
> >>> ---
> >>>  tools/fuzz/cpu-policy/Makefile               |   2 +-
> >>>  tools/fuzz/x86_instruction_emulator/Makefile |  10 +-
> >>>  tools/libs/guest/Makefile.common             |   2 +-
> >>>  tools/tests/cpu-policy/Makefile              |   2 +-
> >>>  tools/tests/x86_emulator/Makefile            |   2 +-
> >>>  xen/lib/x86/Makefile                         |   1 +
> >>>  xen/lib/x86/cpuid-generic.c                  | 273 +++++++++++++++++++
> >>>  xen/lib/x86/cpuid.c                          | 260 ------------------
> >>>  8 files changed, 283 insertions(+), 269 deletions(-)  create mode
> >>> 100644 xen/lib/x86/cpuid-generic.c
> >>
> >> Andrew - what's your take on such a split? Personally I'm not overly
> >> happy to see related functions be scattered across two files. The
> >> separation also feels pretty random, posing the risk that later some of the code
> may need to move back.
> >>
> >
> > Right now, I could not think a better way to guard
> x86_cpuid_copy_from{,to}_buffer with MGMT_HYPERCALLS without split, any
> better suggestion? Or maybe I could add up some explanations on the file
> cpuid_generic.c head note to explain the diffs between itself and cpuid.c, something
> like:
> > ```
> > The difference between cpuid.c and cpuid_generic.c is that the former contains
> library functions that has only been referenced in management hypercalls, such as
> sysctl, domctl, etc. See comment for MGMT_HYPERCALLS.
> > ```
>
> If one of the files is to have only MGMT_HYPERCALLS related stuff (and if, prior to
> that, using #ifdef-ary in the existing file was proven unwieldy), then imo the more
> "natural" split would be to have a separate cpuid-mgmt.c file, where then from its
> name alone it already becomes halfway clear what it to live there.
>
> Another option might be to properly library-fy the copy-in and copy-out functions,
> one per source file, and then referenced by lib-y (or lib-$(CONFIG_...)) from the
> Makefile.
>

Thx!
I'd prefer second option, we need to library-fy the copy-in, copy-out, and trim-cpuid-leaves functions. Maybe it is fragmentated, but could avoid future movement in option one

> Jan
RE: [PATCH v4 02/24] xen: consolidate cpuid library
Posted by Penny, Zheng 1 week, 6 days ago
[Public]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Tuesday, November 25, 2025 2:39 PM
> To: Penny, Zheng <penny.zheng@amd.com>; Andrew Cooper
> <andrew.cooper3@citrix.com>
> Cc: Huang, Ray <Ray.Huang@amd.com>; grygorii_strashko@epam.com; Roger
> Pau Monné <roger.pau@citrix.com>; Anthony PERARD
> <anthony.perard@vates.tech>; Juergen Gross <jgross@suse.com>; xen-
> devel@lists.xenproject.org
> Subject: Re: [PATCH v4 02/24] xen: consolidate cpuid library
>
> On 21.11.2025 11:57, Penny Zheng wrote:
> > There are some cpuid library functions only referenced in
> > XEN_DOMCTL_get{,set}_cpu_policy-case, and shall be wrapped with
> > CONFIG_MGMT_HYPERCALLS later, otherwise they will become unreachable
> > when MGMT_HYPERCALLS=n, and hence violate Misra 2.1
> > - x86_cpu_policy_clear_out_of_range_leaves
> >   - zero_leaves
> > - x86_cpuid_copy_to_buffer
> >   - copy_leaf_to_buffer
> > - x86_cpuid_copy_from_buffer
> > We seperate these functions by moving other functions to a new file
> > named cpuid-generic.c, and modify related Makefile-s to retain same behavior.
> >
> > Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
> > ---
> > v3 -> v4:
> > - new commit
> > ---
> >  tools/fuzz/cpu-policy/Makefile               |   2 +-
> >  tools/fuzz/x86_instruction_emulator/Makefile |  10 +-
> >  tools/libs/guest/Makefile.common             |   2 +-
> >  tools/tests/cpu-policy/Makefile              |   2 +-
> >  tools/tests/x86_emulator/Makefile            |   2 +-
> >  xen/lib/x86/Makefile                         |   1 +
> >  xen/lib/x86/cpuid-generic.c                  | 273 +++++++++++++++++++
> >  xen/lib/x86/cpuid.c                          | 260 ------------------
> >  8 files changed, 283 insertions(+), 269 deletions(-)  create mode
> > 100644 xen/lib/x86/cpuid-generic.c
>
> Andrew - what's your take on such a split? Personally I'm not overly happy to see
> related functions be scattered across two files. The separation also feels pretty
> random, posing the risk that later some of the code may need to move back.
>
> Penny, I also don't think "consolidate" is what is happening here.
> Perhaps "split" would be getting closer?
>

I'll change it to split, thx

> Jan