Re: [Xen-devel] [PATCH v7 03/14] x86: detect and initialize Intel CAT feature

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 20.05.15 at 05:21, <chao.p.peng@xxxxxxxxxxxxxxx> wrote:
> On Tue, May 19, 2015 at 11:22:54AM +0100, Jan Beulich wrote:
>> >>> On 19.05.15 at 11:33, <chao.p.peng@xxxxxxxxxxxxxxx> wrote:
>> > On Tue, May 19, 2015 at 09:42:07AM +0100, Jan Beulich wrote:
>> >> >>> On 19.05.15 at 09:40, <chao.p.peng@xxxxxxxxxxxxxxx> wrote:
>> >> > On Mon, May 18, 2015 at 02:33:51PM +0100, Jan Beulich wrote:
>> >> >> >>> On 08.05.15 at 10:56, <chao.p.peng@xxxxxxxxxxxxxxx> wrote:
>> >> >> > +static unsigned long *__read_mostly cat_socket_init_bitmap;
>> >> >> > +static unsigned long *__read_mostly cat_socket_enable_bitmap;
>> >> >> 
>> >> >> Didn't we agree to fold these two into one? Apart from that the
>> >> >> _bitmap name tag doesn't seem very useful...
>> >> > 
>> >> > Looks like this?
>> >> > 
>> >> > static unsigned long *__read_mostly cat_socket_init,
>> >> >                      *__read_mostly cat_socket_enable;
>> >> 
>> >> Yes, except that you will want to drop one (or make clear why you
>> >> need both).
>> > 
>> > As said in the previous version, cat_socket_enable is the actual CAT
>> > enabled status.
>> > 
>> > And cat_socket_init is used for performance optimization purpose only.
>> > It indicate if the socket has been initialized, so that initialization
>> > happens only once for each socket, but not __cpus_in_socket__ times.
>> > 
>> > There are three possibilities:
>> > 1)  Not initialized;
>> > 2)  Initialized, CAT disabled;
>> > 3)  Initialized, CAT enabled;
>> > 
>> > So it's not possible to use only one bit to represent three values.
>> 
>> Why? The two bits get set together, and get cleared together. Even
>> if they have formally different meaning, there is - afaict - no case
>> where their values will differ (except for the brief period between
>> setting one and then the other).
> 
> Do you want to try the failure initialization(both because CAT is not
> even supported on the machine or fails to allocate cos_to_cbm) on and
> on?

I don't see any harm - all you save is a cpuid_count() invocation.

>> But again - only the allocation/freeing should be done in the
>> alternative named notifications, not the actual data setup (as that
>> iirc needs to run on the CPU).
> 
> I know this is the general rules, but for this case, I don't think it's
> going to work to allocate memory in CPU_UP_PREPARE while do the
> initialization later(in CPU_STARTING).
> 
> As said, the allocation has dependency on the result of the
> initialization. E.g. before
> 
> info->cos_to_cbm = xzalloc_array(struct psr_cat_cbm,
>                                  info->cos_max + 1UL);
> 
> info->cos_max must have been known.

Ah, yes. Otoh, considering that everywhere else we expect
symmetry across CPUs, do you really think there are going to be
systems with different CPUs having different cos_max values?

Jan


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