[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v4 04/15] x86: implement data structure and CPU init flow for MBA



>>> On 05.10.17 at 06:42, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
> On 17-10-03 23:52:09, Jan Beulich wrote:
>> >>> Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx> 09/29/17 3:55 AM >>>
>> >On 17-09-28 05:00:09, Jan Beulich wrote:
>> >> >>> On 23.09.17 at 11:48, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
>> >> > @@ -1410,6 +1496,7 @@ static void psr_cpu_init(void)
>> >> >      unsigned int socket, cpu = smp_processor_id();
>> >> >      struct feat_node *feat;
>> >> >      struct cpuid_leaf regs;
>> >> > +    uint32_t ebx;
>> >> 
>> >> Is this local variable really a big help? To me it looks like it only
>> >> makes the patch larger without actually improving anything,
>> >> and without being related to the subject of the patch.
>> >> 
>> >IMHO, it can avoid the 'cpuid_count_leaf()' being repeatedly called. 
>> >Without it,
>> >we have to call 'cpuid_count_leaf()' for 2 more times.
>> 
>> Hmm, didn't you simply replace regs.b uses with ebx? Or did I overlook a 
>> place
>> where regs is being overwritten before the last of these regs.b uses (in 
>> which case
>> I think your change is fine)?
>> 
> The regs is overwritten when a feature presents. The old codes are below
> 
>     cpuid_count_leaf(PSR_CPUID_LEVEL_CAT, 0, &regs);
>     if ( regs.b & PSR_RESOURCE_TYPE_L3 )
>     {
>         cpuid_count_leaf(PSR_CPUID_LEVEL_CAT, 1, &regs); //It is overwritten 
> here.
> ......
>     }
> 
>     cpuid_count_leaf(PSR_CPUID_LEVEL_CAT, 0, &regs);  //So, we have to call 
> cpuid to get regs again.
>     if ( regs.b & PSR_RESOURCE_TYPE_L2 )
>     {
>         cpuid_count_leaf(PSR_CPUID_LEVEL_CAT, 2, &regs);
> ......
> 
> Because above reason, I defined this ebx local variable to avoid calling cpuid
> again for next feature.

I see. But then please give the variable a better name, reflecting
the data it holds.

Jan


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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.