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

["Roger Pau Monné" <roger.pau@citrix.com>]

On Mon, May 17, 2021 at 05:43:33PM +0200, Jan Beulich wrote:
> 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 guess another option would be to name the function
x86_msr_get_entry_const, and keep the x86_msr_get_entry macro as-is.

Does that seem better?

Thanks, Roger.