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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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.

> You cared to provide feedback, and I'm happy to make the change.

find_msr() would be a better name.  As for entries and nr_entries,
suggestions welcome.  I couldn't think of anything better for the low
level helpers.

~Andrew