Re: [PATCH RFC 0/9] x86: Merge cpuid and msr policy

From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Xen-devel <xen-devel@lists.xenproject.org>,
	Jan Beulich <JBeulich@suse.com>, Wei Liu <wl@xen.org>
Subject: Re: [PATCH RFC 0/9] x86: Merge cpuid and msr policy
Date: Thu, 30 Mar 2023 13:07:29 +0200	[thread overview]
Message-ID: <ZCVtcR5u/14/WmCU@Air-de-Roger> (raw)
In-Reply-To: <20230329205137.323253-1-andrew.cooper3@citrix.com>

On Wed, Mar 29, 2023 at 09:51:28PM +0100, Andrew Cooper wrote:
> tl;dr to add MSR_ARCH_CAPS features sensibly, cpu_{featureset<->policy}() need
> to not operate on objects of differing lifetimes, so structs
> {cpuid,msr}_policy need merging and cpu_policy is the obvious name.

So the problem is that there's a chance we might get a cpu_policy
object that contains a valid (allocated) cpuid object, but not an msr
one?

Or the problem is rather with the domain struct containing separate
cpuid/msr fields not encapsulated in the cpu_policy struct?

I don't think the current set of cpu_policy operations permit you to
operate on incomplete objects anyway.

I assume you are worried about the usage of x86_msr_copy_from_buffer()
for example that could load data into the MSR_ARCH_CAPS field for the
msr object without checking that the corresponding CPUID bit is set?

> But this does mean that we now have
> 
>   cpu_policy->basic.$X
>   cpu_policy->feat.$Y
>   cpu_policy->arch_caps.$Z

I'm not sure I like the fact that we now can't differentiate between
policy fields related to MSRs or CPUID leafs.

Isn't there a chance we might in the future get some name space
collision by us having decided to unify both?

Thanks, Roger.