From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCH v8 02/13] x86: detect and initialize Intel CAT feature Date: Thu, 28 May 2015 13:54:39 +0100 Message-ID: <55672C2F020000780007E960@mail.emea.novell.com> References: <1432197704-20816-1-git-send-email-chao.p.peng@linux.intel.com> <1432197704-20816-3-git-send-email-chao.p.peng@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1432197704-20816-3-git-send-email-chao.p.peng@linux.intel.com> Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Chao Peng Cc: wei.liu2@citrix.com, Ian.Campbell@citrix.com, stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com, dario.faggioli@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 21.05.15 at 10:41, wrote: > --- a/xen/arch/x86/psr.c > +++ b/xen/arch/x86/psr.c > @@ -19,14 +19,25 @@ > #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_enable; > +static struct psr_cat_socket_info *__read_mostly cat_socket_info; > + > static unsigned int __initdata opt_psr; > static unsigned int __initdata opt_rmid_max = 255; > +static unsigned int opt_cos_max = 255; opt_* variables should generally be either __initdata or __read_mostly (the latter in this case). > @@ -194,16 +209,86 @@ void psr_ctxt_switch_to(struct domain *d) > } > } > > +static void cat_cpu_init(void) > +{ > + unsigned int eax, ebx, ecx, edx; > + struct psr_cat_socket_info *info; > + unsigned int socket; > + unsigned int cpu = smp_processor_id(); > + const struct cpuinfo_x86 *c = cpu_data + cpu; > + > + if ( !cpu_has(c, X86_FEATURE_CAT) ) > + return; > + > + socket = cpu_to_socket(cpu); > + if ( test_bit(socket, cat_socket_enable) ) > + return; > + > + cpuid_count(PSR_CPUID_LEVEL_CAT, 0, &eax, &ebx, &ecx, &edx); While one would hope that X86_FEATURE_CAT implies the respective CPUID leaf being available, I think explicitly checking this should still be done just like is the case elsewhere. > + 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); > + > + set_bit(socket, cat_socket_enable); > + printk(XENLOG_INFO "CAT: enabled on socket %u, cos_max:%u, cbm_len:%u\n", > + socket, info->cos_max, info->cbm_len); > + } > +} > + > +static void cat_cpu_fini(unsigned int cpu) > +{ > + unsigned int socket = cpu_to_socket(cpu); > + > + if ( !socket_cpumask[socket] || cpumask_empty(socket_cpumask[socket]) ) > + clear_bit(socket, cat_socket_enable); > +} This being called from the CPU_DEAD notification, you now depend on cpu_smpboot_free) to run ahead of you. Which isn't the case afaict, and even if it happened to be that way you shouldn't rely on it without explicitly enforcing ordering between the two by setting the priority of on of them to a non-default value. > +static void __init init_psr_cat(void) > +{ > + if ( opt_cos_max < 1 ) > + { > + printk(XENLOG_INFO "CAT: disabled, cos_max is too small\n"); > + return; > + } Is opt_cos_max == 1 really useful for anything? > static int cpu_callback( > struct notifier_block *nfb, unsigned long action, void *hcpu) > { > + unsigned int cpu = (unsigned long)hcpu; > + > if ( action == CPU_STARTING ) > psr_cpu_init(); > + else if ( action == CPU_DEAD ) > + psr_cpu_fini(cpu); switch() Jan