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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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:
>> > --- a/xen/arch/x86/psr.c
>> > +++ b/xen/arch/x86/psr.c
>> > @@ -19,14 +19,26 @@
>> >  #include <asm/psr.h>
>> >  
>> >  #define PSR_CMT        (1<<0)
>> > +#define PSR_CAT        (1<<1)
>> > +
>> > +struct psr_cat_socket_info {
>> > +    unsigned int cbm_len;
>> > +    unsigned int cos_max;
>> > +};
>> >  
>> >  struct psr_assoc {
>> >      uint64_t val;
>> >  };
>> >  
>> >  struct psr_cmt *__read_mostly psr_cmt;
>> > +
>> > +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).

>> >  static int cpu_callback(
>> >      struct notifier_block *nfb, unsigned long action, void *hcpu)
>> >  {
>> >      if ( action == CPU_STARTING )
>> >          psr_cpu_init();
>> > +    else if ( action == CPU_DYING )
>> > +        psr_cpu_fini();
>> 
>> Are these the right notifiers for doing things involving memory
>> allocation / freeing?
> 
> psr_cpu_fini() does not really have memory allocation/freeing but
> psr_cpu_init() does have.

Hmm, wait - where did I see this allocation? Looking again, I don't see
it now. But if there was one, then surely it would be wrong for _fini
to not free it.

> While one thing to clarify is: psr_cpu_init() will not propagate
> the error even when the memory allocation fails(instead it disables
> the CAT on the socket).
> 
> If there is still problem then perhaps I need change CPU_STARTING back
> to CPU_ONLINE and make on_selected_cpus() call on target cpu.

No, if there was any allocation needed, you should do the allocation
in CPU_UP_PREPARE and the initialization in CPU_STARTING. But it
looks like this is moot now anyway.

Jan


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