Re: [PATCH 3/3] tools/xg: Clean up xend-style overrides for CPU policies

[Anthony PERARD <anthony.perard@xxxxxxxxx>]

On Wed, Feb 07, 2024 at 05:39:57PM +0000, Alejandro Vallejo wrote:
> diff --git a/tools/libs/guest/xg_cpuid_x86.c b/tools/libs/guest/xg_cpuid_x86.c
> index 5699a26b946e..cee0be80ba5b 100644
> --- a/tools/libs/guest/xg_cpuid_x86.c
> +++ b/tools/libs/guest/xg_cpuid_x86.c
> @@ -772,49 +616,45 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t 
> domid, bool restore,
>               * apic_id_size values greater than 7.  Limit the value to
>               * 7 for now.
>               */
> -            if ( p->policy.extd.nc < 0x7f )
> +            if ( cur->policy.extd.nc < 0x7f )
>              {
> -                if ( p->policy.extd.apic_id_size != 0 && 
> p->policy.extd.apic_id_size < 0x7 )
> -                    p->policy.extd.apic_id_size++;
> +                if ( cur->policy.extd.apic_id_size != 0 && 
> cur->policy.extd.apic_id_size < 0x7 )
> +                    cur->policy.extd.apic_id_size++;
>  
> -                p->policy.extd.nc = (p->policy.extd.nc << 1) | 1;
> +                cur->policy.extd.nc = (cur->policy.extd.nc << 1) | 1;
>              }
>              break;
>          }
>      }
>  
> -    nr_leaves = ARRAY_SIZE(p->leaves);
> -    rc = x86_cpuid_copy_to_buffer(&p->policy, p->leaves, &nr_leaves);
> -    if ( rc )
> +    if ( xend || msr )
>      {
> -        ERROR("Failed to serialise CPUID (%d = %s)", -rc, strerror(-rc));
> -        goto out;
> +        // The overrides are over the serialised form of the policy

Comments should use /* */

> +        if ( (rc = xc_cpu_policy_serialise(xch, cur)) )
> +            goto out;
> +
> +        if ( (rc = xc_cpuid_xend_policy(xch, domid, xend, host, def, cur)) )
> +            goto out;
> +        if ( (rc = xc_msr_policy(xch, domid, msr, host, def, cur)) )
> +            goto out;

What if `xend` is set, but `msr` isn't? Looks like there's going to be a
segv in xc_msr_policy() because it doesn't check that `msr` is actually
set.


Thanks,

-- 
Anthony PERARD