Re: [Xen-devel] [PATCH 1/2] libx86: Helper for clearing out-of-range CPUID leaves

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.06.19 at 21:51, <andrew.cooper3@xxxxxxxxxx> wrote:
> @@ -163,6 +170,58 @@ void x86_cpuid_policy_fill_native(struct cpuid_policy *p)
>      recalculate_synth(p);
>  }
>  
> +void x86_cpuid_policy_clear_out_of_range_leaves(struct cpuid_policy *p)
> +{
> +    unsigned int i;
> +
> +    zero_leaves(p->basic.raw, p->basic.max_leaf + 1,
> +                ARRAY_SIZE(p->basic.raw) - 1);
> +
> +    if ( p->basic.max_leaf < 4 )
> +        memset(p->cache.raw, 0, sizeof(p->cache.raw));
> +    else
> +    {
> +        for ( i = 0; (i < ARRAY_SIZE(p->cache.raw) &&
> +                      p->cache.subleaf[i].type); ++i )
> +            ;
> +
> +        zero_leaves(p->cache.raw, i + 1,
> +                    ARRAY_SIZE(p->cache.raw) - 1);

Do you really want "i + 1" here? Wouldn't it be better to fully zap
subleaf i as well, when its type is zero? Same for leaf 0xb then.

> +    }
> +
> +    if ( p->basic.max_leaf < 7 )
> +        memset(p->feat.raw, 0, sizeof(p->feat.raw));
> +    else
> +        zero_leaves(p->feat.raw, p->feat.max_subleaf + 1,
> +                    ARRAY_SIZE(p->feat.raw) - 1);
> +
> +    if ( p->basic.max_leaf < 0xb )
> +        memset(p->topo.raw, 0, sizeof(p->topo.raw));
> +    else
> +    {
> +        for ( i = 0; (i < ARRAY_SIZE(p->topo.raw) &&
> +                      p->topo.subleaf[i].type); ++i )
> +            ;
> +
> +        zero_leaves(p->topo.raw, i + 1,
> +                    ARRAY_SIZE(p->topo.raw) - 1);
> +    }
> +
> +    if ( p->basic.max_leaf < 0xd || !cpuid_policy_xstates(p) )
> +        memset(p->xstate.raw, 0, sizeof(p->xstate.raw));
> +    else
> +    {
> +        /* First two leaves always valid.  Rest depend on xstates. */
> +        i = max(2, 64 - __builtin_clzll(cpuid_policy_xstates(p)));
> +
> +        zero_leaves(p->xstate.raw, i,
> +                    ARRAY_SIZE(p->xstate.raw) - 1);
> +    }

In x86_cpuid_policy_fill_native() you're using 63 as the loop
bound, guaranteeing to ignore bit 63 in case it gains a meaning.
Here and in x86_cpuid_copy_to_buffer() you don't. I'm slightly
worried that these code sequences will need changing (with no
way to easily notice) when CPUID_GUEST_NR_XSTATE gets
increased. But I'm not going to insist - for now the code is fine
as is (afaict).

Having reached the end of the patch and seeing the title of
patch 2 - is there no need to use this function anywhere
outside the fuzzing harness?

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel