From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chao Peng Subject: Re: [PATCH 1/6] x86: detect and initialize Intel CAT feature Date: Tue, 17 Mar 2015 16:48:30 +0800 Message-ID: <20150317084830.GB5371@pengc-linux.bj.intel.com> References: <1426241605-4114-1-git-send-email-chao.p.peng@linux.intel.com> <1426241605-4114-2-git-send-email-chao.p.peng@linux.intel.com> <5506ECEA020000780006A4CD@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: <5506ECEA020000780006A4CD@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, Mar 16, 2015 at 01:47:06PM +0000, Jan Beulich wrote: > >>> On 13.03.15 at 11:13, wrote: > > @@ -1112,6 +1117,12 @@ The following resources are available: > > total/local memory bandwidth. Follow the same options with Cache Monitoring > > Technology. > > > > +* Cache Alllocation Technology (Broadwell and later). Information regarding > > + the cache allocation. > > + * `cat` instructs Xen to enable/disable Cache Allocation Technology. > > + * `socket_num` indicates socket number. Detecte automatically at boot time > > + if not specified(0). Useful for CPU hot-plug case. > > While saying something, at least for me what is being said doesn't at > all make clear what "socket number" here is: The number of a specific > socket? The number of sockets? Yet something else? Without knowing > what it means by _just_ reading this description, people won't know > what to use the command line option for. I agree, the name is a little confusing. Or perhaps: max_socket? E.g. `max_socket` indicates the maximum number of available sockets for CAT feature detection. All the sockets up to max_socket will be checked for CAT feature. The value is normally detected at boot time automatically if not specified(0). While the value may need to be specified manually for CPU hot-plug scenario in which case the maximum socket number detected at boot time may be not correct. > > > -static void __init init_psr_cmt(unsigned int rmid_max) > > +static void __init psr_cmt_init(unsigned int rmid_max) > > Is this renaming really an integral part of this patch? OK, I will move it to separate one or just keep it unchanged. > > > + on_selected_cpus(cpumask_of(cpu), do_cat_cpu_init, info, 0); > > Hmm, using an IPI here seems odd. Is there a reason to can't hook > this onto CPU_STARTING instead of CPU_ONLINE? Looks like CPU_STARTING is what I need, IPI then is unnecessary. > > > +static unsigned int get_max_socket(void) > > +{ > > + unsigned int cpu, max_apicid = boot_cpu_physical_apicid; > > + > > + for_each_present_cpu(cpu) > > + if (max_apicid < x86_cpu_to_apicid[cpu]) > > Coding style. > > > + max_apicid = x86_cpu_to_apicid[cpu]; > > + > > + return apicid_to_socket(max_apicid); > > Since when is the socket with the highest numbered APIC ID > the highest numbered socket? I think the whole function needs > to act on socket numbers only. Then perhaps looping cpus and checking against cpu_to_socket(cpu) directly make sense. Thanks Jan. Chao