Re: [Xen-devel] [PATCH v12 16/23] x86: L2 CAT: implement CPU init flow.

["Jan Beulich" <jbeulich@xxxxxxxx>]

>>> Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx> 06/30/17 9:28 AM >>>
>On 17-06-30 00:58:47, Jan Beulich wrote:
>> >>> Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx> 06/14/17 3:26 AM >>>
>> > @@ -1407,6 +1424,19 @@ static void psr_cpu_init(void)
>> >          info->feat_init = true;
>> >      }
>> >  
>> > +    cpuid_count_leaf(PSR_CPUID_LEVEL_CAT, 0, ®s);
>> > +    if ( regs.b & PSR_RESOURCE_TYPE_L2 )
>> > +    {
>> > +        cpuid_count_leaf(PSR_CPUID_LEVEL_CAT, 2, ®s);
>> > +
>> > +        feat = feat_l2_cat;
>> > +        feat_l2_cat = NULL;
>> > +        feat_props[PSR_SOCKET_L2_CAT] = &l2_cat_props;
>> > +        cat_init_feature(®s, feat, info, PSR_SOCKET_L2_CAT);
>> > +
>> > +        info->feat_init = true;
>> 
>> This recurring setting of feat_init starts looking suspicious here. Why can't
>> this be done once at the end of the function, outside of any if()-s?
>> 
>I am afraid there is no any feature found through CPUID so I set feat_init in
>every statement that a feature is found.

Well, even if no feature is available, you're done with initializing by the time
you finish this function.

>> > --- a/xen/include/asm-x86/psr.h
>> > +++ b/xen/include/asm-x86/psr.h
>> > @@ -23,6 +23,7 @@
>> >  
>> >  /* Resource Type Enumeration */
>> >  #define PSR_RESOURCE_TYPE_L3            0x2
>> > +#define PSR_RESOURCE_TYPE_L2            0x4
>> 
>> These are used in psr.c only afaics, so shouldn't be put in a header.
>> 
>PSR_RESOURCE_TYPE_L3 is used in sysctl.c too. For L2, I just want to keep it
>same place as L3.

I must have overlooked that one - of course they should stay together.

Jan



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