Re: [PATCH 2/2] x86/cpuid: Alter the policy logic for leaf 0xb to be multi-invocation

["Jan Beulich" <JBeulich@suse.com>]

>>> On 27.06.18 at 17:58, <andrew.cooper3@citrix.com> wrote:
> On 27/06/18 15:58, Jan Beulich wrote:
>>>>> On 27.06.18 at 15:55, <andrew.cooper3@citrix.com> wrote:
>>> @@ -316,6 +319,33 @@ static void __init calculate_raw_policy(void)
>>>              cpuid_count_leaf(7, i, &p->feat.raw[i]);
>>>      }
>>>  
>>> +    if ( p->basic.max_leaf >= 0xb )
>>> +    {
>>> +        union {
>>> +            struct cpuid_leaf l;
>>> +            struct cpuid_topo_leaf t;
>>> +        } u;
>>> +
>>> +        for ( i = 0; i < ARRAY_SIZE(p->topo.raw); ++i )
>>> +        {
>>> +            cpuid_count_leaf(0xb, i, &u.l);
>>> +
>>> +            if ( u.t.type == 0 )
>>> +                break;
>>> +
>>> +            p->topo.subleaf[i] = u.t;
>>> +        }
>>> +
>>> +        /*
>>> +         * The choice of CPUID_GUEST_NR_TOPO is per the manual.  It may need
>>> +         * to grow for future harware.
>> Missing d.
> 
> Where?  I'm afraid that after repeated re-reads, I can't spot any issue.

hardware

>>> @@ -108,7 +109,11 @@ struct cpuid_policy
>>>              uint64_t :64, :64; /* Leaf 0x9 - DCA */
>>>  
>>>              /* Leaf 0xa - Intel PMU. */
>>> -            uint8_t pmu_version;
>>> +            uint8_t pmu_version, _pmu[15];
>>> +
>>> +            uint64_t :64, :64; /* Leaf 0xb - Topology. */
>>> +            uint64_t :64, :64; /* Leaf 0xc - rsvd */
>>> +            uint64_t :64, :64; /* Leaf 0xd - XSTATE. */
>> I don't understand why you add the latter two lines, neither in general
>> nor in the particular context of this patch.
> 
> This is part of reducing the effort for people extending the CPUID
> leaves, by keeping the .basic union in line with max_leaf.
> 
> There are further non-subleaf leaves beyond this point (0x15/0x16) and
> I've noticed several mistakes with newer submitted series.  I'd much
> rather do this myself once now, than attempt to explain it to others
> during code review.

Ah, I see. If you make the commit message say so,
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan



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