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

From: Jan Beulich <jbeulich@suse.com>
To: Roger Pau Monne <roger.pau@citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>, Wei Liu <wl@xen.org>,
	Ian Jackson <iwj@xenproject.org>,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH v4 03/10] libx86: introduce helper to fetch msr entry
Date: Mon, 17 May 2021 17:43:33 +0200	[thread overview]
Message-ID: <035cc783-6083-f141-d4a3-db7a6adc36f5@suse.com> (raw)
In-Reply-To: <20210507110422.24608-4-roger.pau@citrix.com>

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