Re: [PATCH v3 02/13] libs/guest: allow fetching a specific CPUID leaf from a cpu policy

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, May 04, 2021 at 12:59:43PM +0100, Andrew Cooper wrote:
> On 30/04/2021 16:52, Roger Pau Monne wrote:
> > @@ -822,3 +825,28 @@ int xc_cpu_policy_serialise(xc_interface *xch, const 
> > xc_cpu_policy_t p,
> >      errno = 0;
> >      return 0;
> >  }
> > +
> > +int xc_cpu_policy_get_cpuid(xc_interface *xch, const xc_cpu_policy_t 
> > policy,
> > +                            uint32_t leaf, uint32_t subleaf,
> > +                            xen_cpuid_leaf_t *out)
> > +{
> > +    unsigned int nr_leaves = ARRAY_SIZE(policy->leaves);
> > +    xen_cpuid_leaf_t *tmp;
> > +    int rc;
> > +
> > +    rc = xc_cpu_policy_serialise(xch, policy, policy->leaves, &nr_leaves,
> > +                                 NULL, 0);
> > +    if ( rc )
> > +        return rc;
> 
> Sorry for not spotting this last time.
> 
> You don't need to serialise.  You can look up leaf/subleaf in O(1) time
> from cpuid_policy, which was a design goal of the structure originally.
> 
> It is probably best to adapt most of the first switch statement in
> guest_cpuid() to be a libx86 function.  The asserts aren't massively
> interesting to keep, and instead of messing around with nospec, just
> have the function return a pointer into the cpuid_policy (or NULL), and
> have a single block_speculation() in Xen.

libx86 already has array_access_nospec, so I think it's fine to just
leave the code as-is instead of adding a block_speculation in Xen and
dropping the array_access_nospec accessors?

> We'll also want a unit test
> to go with this new function to check that out-of-range leaves don't
> result in out-of-bounds reads.

Sure.

Also, whats your opinion regarding xc_cpu_policy_get_msr, should I
also split part of guest_rdmsr and place it in libx86 in order to
fetch the MSRs present in msr_policy?

Thanks, Roger.