Re: [Xen-devel] [PATCH v2 04/10] x86/cpuid: Handle leaf 0x4 in guest_cpuid()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.03.17 at 14:24, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 13/03/17 13:05, Jan Beulich wrote:
>>>>> On 13.03.17 at 13:51, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> On 13/03/17 12:03, Jan Beulich wrote:
>>>>>>> On 10.03.17 at 17:27, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>>> @@ -242,6 +243,25 @@ static void __init calculate_raw_policy(void)
>>>>>          cpuid_leaf(i, &p->basic.raw[i]);
>>>>>      }
>>>>>  
>>>>> +    if ( p->basic.max_leaf >= 4 )
>>>>> +    {
>>>>> +        for ( i = 0; i < ARRAY_SIZE(p->cache.raw); ++i )
>>>>> +        {
>>>>> +            cpuid_count_leaf(4, i, &p->cache.raw[i]);
>>>>> +
>>>>> +            if ( p->cache.subleaf[i].type == 0 )
>>>>> +                break;
>>>>> +        }
>>>>> +
>>>>> +        /*
>>>>> +         * The choice of CPUID_GUEST_NR_CACHE is arbitrary.  It is 
>>>>> expected
>>>>> +         * that it will eventually need increasing for future hardware.
>>>>> +         */
>>>>> +        if ( i == ARRAY_SIZE(p->cache.raw) )
>>>>> +            printk(XENLOG_WARNING
>>>>> +                   "CPUID: Insufficient Leaf 4 space for this 
>>>>> hardware\n");
>>>>> +    }
>>>> As expressed before (perhaps in the context of another patch),
>>>> the warning may be logged prematurely, which I'd prefer to be
>>>> avoided.
>>> How would you like it then?  You previously indicated that it probably
>>> want a problem leaving it like this, which is why I did.
>> And I continue to be of that opinion: It's probably not a problem to
>> leave it as is (hence the R-b), but I'd prefer if the warning was only
>> issued when we actually can't fit at least one sub-leaf.
> 
> Is this delta ok?

Yes, thanks. R-b stands, if in doubt.

Jan

> diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
> index b70fc0b..7caf6af 100644
> --- a/xen/arch/x86/cpuid.c
> +++ b/xen/arch/x86/cpuid.c
> @@ -251,10 +251,17 @@ static void __init calculate_raw_policy(void)
>      {
>          for ( i = 0; i < ARRAY_SIZE(p->cache.raw); ++i )
>          {
> -            cpuid_count_leaf(4, i, &p->cache.raw[i]);
> +            union {
> +                struct cpuid_leaf l;
> +                struct cpuid_cache_leaf c;
> +            } u;
>  
> -            if ( p->cache.subleaf[i].type == 0 )
> +            cpuid_count_leaf(4, i, &u.l);
> +
> +            if ( u.c.type == 0 )
>                  break;
> +
> +            p->cache.subleaf[i] = u.c;
>          }
>  
>          /*
> diff --git a/xen/include/asm-x86/cpuid.h b/xen/include/asm-x86/cpuid.h
> index 816f06d..ac25908 100644
> --- a/xen/include/asm-x86/cpuid.h
> +++ b/xen/include/asm-x86/cpuid.h
> @@ -118,7 +118,7 @@ struct cpuid_policy
>      /* Structured cache leaf: 0x00000004[xx] */
>      union {
>          struct cpuid_leaf raw[CPUID_GUEST_NR_CACHE];
> -        struct {
> +        struct cpuid_cache_leaf {
>              uint32_t type:5,
>                  :27, :32, :32, :32;
>          } subleaf[CPUID_GUEST_NR_CACHE];




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