On 12.12.2025 05:01, Penny Zheng wrote: > There are some cpuid library functions only referenced in > XEN_DOMCTL-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
At this point of the series there's no MGMT_HYPERCALLS yet (see also my reply to the cover letter). > For file cpupolicy-clr.c to contain cpupolicy clearing library function: > - x86_cpu_policy_clear_out_of_range_leaves > - zero_leaves > For file cpuid-cp2buf.c to contain cpuid copy-to-buffer library function: > - x86_cpuid_copy_to_buffer > - copy_leaf_to_buffer > For file cpuid-cpfrbuf.c to contain cpuid copy-from-buffer library function: > - x86_cpuid_copy_from_buffer > Sunmmerize all needed cpuid-library object file under a new variable > CPUID_OBJS in Makefile. > > Suggested-by: Jan Beulich <[email protected]> > Signed-off-by: Penny Zheng <[email protected]> > --- > v4 -> v5: > - library-fy cpuid-releated functions > --- > tools/fuzz/cpu-policy/Makefile | 4 +- > tools/fuzz/x86_instruction_emulator/Makefile | 11 +- > tools/libs/guest/Makefile.common | 4 +- > tools/tests/cpu-policy/Makefile | 3 +- > tools/tests/x86_emulator/Makefile | 3 +- > xen/lib/Makefile | 5 + > xen/lib/x86/cpuid-cp2buf.c | 123 ++++++++ > xen/lib/x86/cpuid-cpfrbuf.c | 129 +++++++++ > xen/lib/x86/cpuid.c | 286 ------------------- > xen/lib/x86/cpupolicy-clr.c | 73 +++++ > 10 files changed, 346 insertions(+), 295 deletions(-) > create mode 100644 xen/lib/x86/cpuid-cp2buf.c > create mode 100644 xen/lib/x86/cpuid-cpfrbuf.c > create mode 100644 xen/lib/x86/cpupolicy-clr.c This looks to be doing at least three things in one go. If all of them would be simple (including them being a reasonably small diff), that may be fine. But the diffstat above says otherwise, so I may I ask that this be split in three, maybe even four pieces (one per function moving to a new file, and maybe one doing prep work in the Makefile-s touched)? The filenames also aren't very descriptive. cp-from-buffer.c, cp-to-buffer.c, and cp-clear.c maybe? Albeit the last one is where I'm the least convinced that splitting out and making a library function is actually a good idea. Note how I also didn't mention that function as a possible candidate for library- fying. I'll try to not forget to bring this up with the x86 maintainers later in the day. > --- a/tools/libs/guest/Makefile.common > +++ b/tools/libs/guest/Makefile.common > @@ -35,7 +35,9 @@ 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 > +CPUID_OBJS := cpuid.o cpuid-cp2buf.o cpuid-cpfrbuf.o cpupolicy-clr.o > +OBJS-y += $(CPUID_OBJS) > +OBJS-y += msr.o policy.o > endif Why the mismatched padding on the := line? > --- a/xen/lib/Makefile > +++ b/xen/lib/Makefile > @@ -45,3 +45,8 @@ lib-$(CONFIG_X86) += xxhash64.o > lib32-y := divmod.o > lib32-$(CONFIG_64BIT) := > lib-y += $(lib32-y) > + > +libx86-y := x86/cpuid-cp2buf.o > +libx86-y += x86/cpuid-cpfrbuf.o > +libx86-y += x86/cpupolicy-clr.o > +lib-$(CONFIG_X86) += $(libx86-y) Why the intermediate libx86-y? And why is this not being done in xen/lib/x86/Makefile anyway? Jan
