Re: [Xen-devel] [PATCH 4/6] x86/cpuid: Handle leaf 0x80000000 in guest_cpuid()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.01.17 at 20:40, <andrew.cooper3@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/cpuid.c
> +++ b/xen/arch/x86/cpuid.c
> @@ -163,6 +163,24 @@ static void recalculate_xstate(struct cpuid_policy *p)
>      }
>  }
>  
> +static void recalculate_common(struct cpuid_policy *p)
> +{
> +    switch ( p->x86_vendor )
> +    {
> +    case X86_VENDOR_INTEL:
> +        p->extd.vendor_ebx = 0;
> +        p->extd.vendor_ecx = 0;
> +        p->extd.vendor_edx = 0;
> +        break;
> +
> +    case X86_VENDOR_AMD:
> +        p->extd.vendor_ebx = p->basic.vendor_ebx;
> +        p->extd.vendor_ecx = p->basic.vendor_ecx;
> +        p->extd.vendor_edx = p->basic.vendor_edx;
> +        break;
> +    }
> +}

I find the word "common" in the name here not very indicative
of what the function does, especially with ...

> @@ -227,12 +245,12 @@ static void __init calculate_host_policy(void)
>          min_t(uint32_t, p->basic.max_leaf,   ARRAY_SIZE(p->basic.raw) - 1);
>      p->feat.max_subleaf =
>          min_t(uint32_t, p->feat.max_subleaf, ARRAY_SIZE(p->feat.raw) - 1);
> -    p->extd.max_leaf =
> -        min_t(uint32_t, p->extd.max_leaf,
> -              0x80000000u + ARRAY_SIZE(p->extd.raw) - 1);
> +    p->extd.max_leaf = 0x80000000 | min_t(uint32_t, p->extd.max_leaf & 
> 0xffff,
> +                                          ARRAY_SIZE(p->extd.raw) - 1);
>  
>      cpuid_featureset_to_policy(boot_cpu_data.x86_capability, p);
>      recalculate_xstate(p);
> +    recalculate_common(p);
>  }

... the neighboring call here (which is quite a bit more specific).
Of course possible alternatives depend on what further uses of
this function you do (or do not) plan, but by the name of the
other function here it could be recalculate_extd_vendor().

> @@ -901,9 +925,21 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
>          return cpuid_hypervisor_leaves(v, leaf, subleaf, res);
>  
>      case 0x80000000 ... 0x80000000 + CPUID_GUEST_NR_EXTD - 1:
> -        if ( leaf > p->extd.max_leaf )
> +        ASSERT((p->extd.max_leaf & 0xffff) < ARRAY_SIZE(p->extd.raw));
> +        if ( (leaf & 0xffff) > min_t(uint32_t, p->extd.max_leaf & 0xffff,
> +                                     ARRAY_SIZE(p->extd.raw) - 1) )
>              return;
> -        goto legacy;
> +
> +        switch ( leaf )
> +        {
> +        default:
> +            goto legacy;
> +
> +        case 0x80000000:
> +            *res = p->extd.raw[leaf & 0xffff];

I take it that the plan is to have further leaves and up here, or else
the array index could simply be literal zero.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel