Re: [PATCH v3 03/13] libs/guest: allow fetching a specific MSR entry from a cpu policy

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.05.2021 19:11, Andrew Cooper wrote:
> On 04/05/2021 12:58, Roger Pau Monné wrote:
>> On Tue, May 04, 2021 at 01:40:11PM +0200, Jan Beulich wrote:
>>> On 04.05.2021 12:56, Roger Pau Monné wrote:
>>>> On Mon, May 03, 2021 at 12:41:29PM +0200, Jan Beulich wrote:
>>>>> On 30.04.2021 17:52, Roger Pau Monne wrote:
>>>>>> --- a/tools/libs/guest/xg_cpuid_x86.c
>>>>>> +++ b/tools/libs/guest/xg_cpuid_x86.c
>>>>>> @@ -850,3 +850,45 @@ int xc_cpu_policy_get_cpuid(xc_interface *xch, 
>>>>>> const xc_cpu_policy_t policy,
>>>>>>      *out = *tmp;
>>>>>>      return 0;
>>>>>>  }
>>>>>> +
>>>>>> +static int compare_entries(const void *l, const void *r)
>>>>>> +{
>>>>>> +    const xen_msr_entry_t *lhs = l;
>>>>>> +    const xen_msr_entry_t *rhs = r;
>>>>>> +
>>>>>> +    if ( lhs->idx == rhs->idx )
>>>>>> +        return 0;
>>>>>> +    return lhs->idx < rhs->idx ? -1 : 1;
>>>>>> +}
>>>>>> +
>>>>>> +static xen_msr_entry_t *find_entry(xen_msr_entry_t *entries,
>>>>>> +                                   unsigned int nr_entries, unsigned 
>>>>>> int index)
>>>>>> +{
>>>>>> +    const xen_msr_entry_t key = { index };
>>>>>> +
>>>>>> +    return bsearch(&key, entries, nr_entries, sizeof(*entries), 
>>>>>> compare_entries);
>>>>>> +}
>>>>> Isn't "entries" / "entry" a little too generic a name here, considering
>>>>> the CPUID equivalents use "leaves" / "leaf"? (Noticed really while looking
>>>>> at patch 7.)
>>>> Would you be fine with naming the function find_msr and leaving the
>>>> rest of the parameters names as-is?
>>> Yes. But recall I'm not the maintainer of this code anyway.
> 
> This file in particular has been entirely within the x86 remit for
> multiple years now, as have the other cpuid bits in misc/ and libxl.

Well, it's somewhere in the middle imo: Its maintainers have kind of
permanently delegated to the x86 maintainers. Which still doesn't
make it part of "X86 ARCHITECTURE" until such time as the file gets
actually named there. Note how e.g. tools/misc/xen-cpuid.c already is.

Jan