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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 01.12.2025 07:57, Penny, Zheng wrote:
> [Public]
> 
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@xxxxxxxx>
>> Sent: Tuesday, November 25, 2025 2:39 PM
>> To: Penny, Zheng <penny.zheng@xxxxxxx>; Andrew Cooper
>> <andrew.cooper3@xxxxxxxxxx>
>> Cc: Huang, Ray <Ray.Huang@xxxxxxx>; grygorii_strashko@xxxxxxxx; Roger
>> Pau Monné <roger.pau@xxxxxxxxxx>; Anthony PERARD
>> <anthony.perard@xxxxxxxxxx>; Juergen Gross <jgross@xxxxxxxx>; xen-
>> devel@xxxxxxxxxxxxxxxxxxxx
>> 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@xxxxxxx>
>>> ---
>>> 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