Re: [PATCH v5 02/24] xen/x86: library-fy cpuid-releated functions

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.12.2025 13:14, Jan Beulich wrote:
> 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 <jbeulich@xxxxxxxx>
>> Signed-off-by: Penny Zheng <Penny.Zheng@xxxxxxx>
>> ---
>> 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.

Having discussed this, we thought we might as well aim for proper library
fication (longer term, i.e. not something we'd ask you to do). Therefore
splitting by function is generally okay, but the filenames really want to be
sufficiently descriptive. For x86_cpu_policy_clear_out_of_range_leaves() this
may very well mean cp-clear-out-of-range-leaves.c (not sure whether
shortening to e.g. cp-clear-oor-leaves.c would make sense).

Jan