From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chao Peng Subject: Re: [PATCH v7 03/14] x86: detect and initialize Intel CAT feature Date: Tue, 19 May 2015 15:40:19 +0800 Message-ID: <20150519074019.GD8239@pengc-linux.bj.intel.com> References: <1431075415-21917-1-git-send-email-chao.p.peng@linux.intel.com> <1431075415-21917-4-git-send-email-chao.p.peng@linux.intel.com> <555A065F020000780007B2DE@mail.emea.novell.com> Reply-To: Chao Peng Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <555A065F020000780007B2DE@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: wei.liu2@citrix.com, Ian.Campbell@citrix.com, stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com, Ian.Jackson@eu.citrix.com, xen-devel@lists.xen.org, will.auld@intel.com, keir@xen.org, dgdegra@tycho.nsa.gov List-Id: xen-devel@lists.xenproject.org On Mon, May 18, 2015 at 02:33:51PM +0100, Jan Beulich wrote: > >>> On 08.05.15 at 10:56, wrote: > > --- a/xen/arch/x86/psr.c > > +++ b/xen/arch/x86/psr.c > > @@ -19,14 +19,26 @@ > > #include > > > > #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; > > + cpuid_count(PSR_CPUID_LEVEL_CAT, 0, &eax, &ebx, &ecx, &edx); > > + if ( ebx & PSR_RESOURCE_TYPE_L3 ) > > + { > > + cpuid_count(PSR_CPUID_LEVEL_CAT, 1, &eax, &ebx, &ecx, &edx); > > + info = cat_socket_info + socket; > > + info->cbm_len = (eax & 0x1f) + 1; > > + info->cos_max = min(opt_cos_max, edx & 0xffff); > > > Is opt_cos_max being zero (or even one) going to result in a useful / > working environment? I.e. shouldn't you rather disable CAT in that > case? OK, I will disable CAT in init_psr_cat() for this case. > > > +static void cat_cpu_fini(void) > > +{ > > + unsigned int cpu = smp_processor_id(); > > + unsigned int socket = cpu_to_socket(cpu); > > This is the only use of "cpu", i.e. the variable is pretty pointless. > > > 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. 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. Chao