Re: [PATCH 09/21] libs/guest: allow fetching a specific CPUID leaf from a cpu policy

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

On Thu, Apr 01, 2021 at 03:47:20PM +0100, Andrew Cooper wrote:
> On 23/03/2021 09:58, Roger Pau Monne wrote:
> > diff --git a/tools/libs/guest/xg_cpuid_x86.c 
> > b/tools/libs/guest/xg_cpuid_x86.c
> > index 48351f1c4c6..a1e1bf10d5c 100644
> > --- a/tools/libs/guest/xg_cpuid_x86.c
> > +++ b/tools/libs/guest/xg_cpuid_x86.c
> > @@ -883,3 +883,45 @@ 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, nr_msrs, i;
> > +    xen_cpuid_leaf_t *leaves;
> > +    int rc = xc_cpu_policy_get_size(xch, &nr_leaves, &nr_msrs);
> > +
> > +    if ( rc )
> > +    {
> > +        PERROR("Failed to obtain policy info size");
> > +        return -1;
> > +    }
> > +
> > +    leaves = calloc(nr_leaves, sizeof(*leaves));
> > +    if ( !leaves )
> > +    {
> > +        PERROR("Failed to allocate resources");
> > +        errno = ENOMEM;
> > +        return -1;
> > +    }
> > +
> > +    rc = xc_cpu_policy_serialise(xch, policy, leaves, &nr_leaves, NULL, 0);
> > +    if ( rc )
> > +        goto out;
> > +
> > +    for ( i = 0; i < nr_leaves; i++ )
> > +        if ( leaves[i].leaf == leaf && leaves[i].subleaf == subleaf )
> > +        {
> > +            *out = leaves[i];
> > +            goto out;
> > +        }
> 
> Please adapt find_leaf(), probably by dropping xc_xend_cpuid and passing
> in leaf/subleaf parameters.
> 
> Serialised leaves are sorted and there are plenty of them, so a log
> search is better.
> 
> How frequent is this call going to be for the same policy?  With the
> arrays embedded in a policy, they're still around, and serialise is an
> expensive operation.
> 
> I wonder if it makes sense to try and keep both forms in sync, so we can
> avoid redundant calls like this?

I think we would then need to call xc_cpu_policy_serialise whenever we
update the CPUID policy in order to re-generate the full serialized
version, as I expect changes to a specific leaf can affect others
leaves, and thus the helpers should not try to update the serialized
version manually.

Hence I'm not sure it's worth to try to keep both versions in sync, as
it seems easier to load the policy, modify the policy object and then
serialize back only when needed to upload to Xen or to export.

Thanks, Roger.