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

[Chao Peng <chao.p.peng@xxxxxxxxxxxxxxx>]

On Wed, May 20, 2015 at 10:06:32AM +0100, Jan Beulich wrote:
> >>> 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.

Then I will remove cat_socket_init.

> 
> >> 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?

I agree, such systems should be rare, or even doesn't exist in the real
world. But as we are trying to support it, then I have to make it working.

Chao

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