Re: [PATCH v4 03/10] libx86: introduce helper to fetch msr entry

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.05.2021 13:04, Roger Pau Monne wrote:
> @@ -91,6 +91,21 @@ int x86_msr_copy_from_buffer(struct msr_policy *policy,
>                               const msr_entry_buffer_t msrs, uint32_t 
> nr_entries,
>                               uint32_t *err_msr);
>  
> +/**
> + * Get a MSR entry from a policy object.
> + *
> + * @param policy      The msr_policy object.
> + * @param idx         The index.
> + * @returns a pointer to the requested leaf or NULL in case of error.
> + *
> + * Do not call this function directly and instead use x86_msr_get_entry that
> + * will deal with both const and non-const policies returning a pointer with
> + * constness matching that of the input.
> + */
> +const uint64_t *_x86_msr_get_entry(const struct msr_policy *policy,
> +                                   uint32_t idx);
> +#define x86_msr_get_entry(p, i) \
> +    ((__typeof__(&(p)->platform_info.raw))_x86_msr_get_entry(p, i))
>  #endif /* !XEN_LIB_X86_MSR_H */

Just two nits: I think it would be nice to retain a blank line ahead of
the #endif. And here as well as in the CPUID counterpart you introduce,
strictly speaking, name space violations (via the leading underscore).
I realize I'm not really liked for pointing such out, but it may be
more relevant here than in pure hypervisor code, as this library code
is supposed to be usable also in userland.

Jan