From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47238) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bZfZM-0003ky-D7 for qemu-devel@nongnu.org; Tue, 16 Aug 2016 10:42:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bZfZI-0000ny-3H for qemu-devel@nongnu.org; Tue, 16 Aug 2016 10:42:19 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:60188) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bZfZH-0000nk-R0 for qemu-devel@nongnu.org; Tue, 16 Aug 2016 10:42:16 -0400 Received: from pps.filterd (m0098404.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.11/8.16.0.11) with SMTP id u7GEdSL8112968 for ; Tue, 16 Aug 2016 10:42:13 -0400 Received: from e06smtp15.uk.ibm.com (e06smtp15.uk.ibm.com [195.75.94.111]) by mx0a-001b2d01.pphosted.com with ESMTP id 24symqfevc-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Tue, 16 Aug 2016 10:42:13 -0400 Received: from localhost by e06smtp15.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 16 Aug 2016 15:42:10 +0100 Received: from b06cxnps4075.portsmouth.uk.ibm.com (d06relay12.portsmouth.uk.ibm.com [9.149.109.197]) by d06dlp01.portsmouth.uk.ibm.com (Postfix) with ESMTP id 9025C17D8024 for ; Tue, 16 Aug 2016 15:43:49 +0100 (BST) Received: from d06av01.portsmouth.uk.ibm.com (d06av01.portsmouth.uk.ibm.com [9.149.37.212]) by b06cxnps4075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id u7GEg7a262259340 for ; Tue, 16 Aug 2016 14:42:07 GMT Received: from d06av01.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av01.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id u7GEg6fJ010607 for ; Tue, 16 Aug 2016 08:42:06 -0600 Date: Tue, 16 Aug 2016 16:42:05 +0200 From: David Hildenbrand In-Reply-To: <239fec14-6cbb-3679-b66a-bb237343af02@de.ibm.com> References: <1470670378-53732-1-git-send-email-dahi@linux.vnet.ibm.com> <1470670378-53732-5-git-send-email-dahi@linux.vnet.ibm.com> <239fec14-6cbb-3679-b66a-bb237343af02@de.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-Id: <20160816164205.3bd1654a@thinkpad-w530> Subject: Re: [Qemu-devel] [Patch v2 04/29] s390x/cpumodel: introduce CPU features List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Christian Borntraeger Cc: qemu-devel@nongnu.org, ehabkost@redhat.com, jdenemar@redhat.com, imammedo@redhat.com, cornelia.huck@de.ibm.com, fiuczy@linux.vnet.ibm.com, mimu@linux.vnet.ibm.com > On 08/08/2016 05:32 PM, David Hildenbrand wrote: > > In general this this very good. Mostly bike shedding and naming. Thanks! > > > +/* indexed by feature number for easy lookup */ > > +static const S390FeatDef s390_features[] = { > > + FEAT_INIT("n3", S390_FEAT_TYPE_STFL, 0, "Instructions marked as n3"), > > /proc/cpuinfo calls this "esan3", so use the same? Agreed. > > > + FEAT_INIT("zarch", S390_FEAT_TYPE_STFL, 1, "z/Architecture architectural mode"), > > + FEAT_INIT("dateh", S390_FEAT_TYPE_STFL, 3, "DAT-enhancement facility"), > > + FEAT_INIT("idtes", S390_FEAT_TYPE_STFL, 4, "IDTE selective TLB segment-table clearing"), > > + FEAT_INIT("idter", S390_FEAT_TYPE_STFL, 5, "IDTE selective TLB region-table clearing"), > > + FEAT_INIT("asnlxr", S390_FEAT_TYPE_STFL, 6, "ASN-and-LX reuse facility"), > > + FEAT_INIT("stfle", S390_FEAT_TYPE_STFL, 7, "Store-facility-list-extended facility"), > > + FEAT_INIT("edat", S390_FEAT_TYPE_STFL, 8, "Enhanced-DAT facility"), > > + FEAT_INIT("srs", S390_FEAT_TYPE_STFL, 9, "Sense-running-status facility"), > > + FEAT_INIT("csske", S390_FEAT_TYPE_STFL, 10, "Conditional-SSKE facility"), > > + FEAT_INIT("ctop", S390_FEAT_TYPE_STFL, 11, "Configuration-topology facility"), > > + FEAT_INIT("ipter", S390_FEAT_TYPE_STFL, 13, "IPTE-range facility"), > > + FEAT_INIT("nonqks", S390_FEAT_TYPE_STFL, 14, "Nonquiescing key-setting facility"), > > + FEAT_INIT("etf2", S390_FEAT_TYPE_STFL, 16, "Extended-translation facility 2"), > > + FEAT_INIT("msa-base", S390_FEAT_TYPE_STFL, 17, "Message-security-assist facility (excluding subfunctions)"), > > Hmm, same for all other msa variants below. Why not just "msa" (as /proc/cpuinfo)? Does "base" > indicate that we have the facility bit/instruction,but no subfunction like SHA? Yes, as the description says "excluding subfunctions". msa is a feature group, containing all introduced subfunctions. msa-base is just the STFL bit. As discussed a while ago, we need this, because we could have some variance at that point int the future. > > > + FEAT_INIT("ldisp", S390_FEAT_TYPE_STFL, 18, "Long-displacement facility"), > > + FEAT_INIT("ldisphp", S390_FEAT_TYPE_STFL, 19, "Long-displacement facility has high performance"), > > + FEAT_INIT("hfpm", S390_FEAT_TYPE_STFL, 20, "HFP-multiply-add/subtract facility"), > > + FEAT_INIT("eimm", S390_FEAT_TYPE_STFL, 21, "Extended-immediate facility"), > > + FEAT_INIT("etf3", S390_FEAT_TYPE_STFL, 22, "Extended-translation facility 3"), > > + FEAT_INIT("hfpue", S390_FEAT_TYPE_STFL, 23, "HFP-unnormalized-extension facility"), > > + FEAT_INIT("etf2eh", S390_FEAT_TYPE_STFL, 24, "ETF2-enhancement facility"), > > + FEAT_INIT("stckf", S390_FEAT_TYPE_STFL, 25, "Store-clock-fast facility"), > > + FEAT_INIT("parseh", S390_FEAT_TYPE_STFL, 26, "Parsing-enhancement facility"), > > + FEAT_INIT("mvcos", S390_FEAT_TYPE_STFL, 27, "Move-with-optional-specification facility"), > > + FEAT_INIT("tods-base", S390_FEAT_TYPE_STFL, 28, "TOD-clock-steering facility (excluding subfunctions)"), > > + FEAT_INIT("etf3eh", S390_FEAT_TYPE_STFL, 30, "ETF3-enhancement facility"), > > + FEAT_INIT("ectg", S390_FEAT_TYPE_STFL, 31, "Extract-CPU-time facility"), > > + FEAT_INIT("csst", S390_FEAT_TYPE_STFL, 32, "Compare-and-swap-and-store facility"), > > + FEAT_INIT("csst2", S390_FEAT_TYPE_STFL, 33, "Compare-and-swap-and-store facility 2"), > > + FEAT_INIT("ginste", S390_FEAT_TYPE_STFL, 34, "General-instructions-extension facility"), > > + FEAT_INIT("exrl", S390_FEAT_TYPE_STFL, 35, "Execute-extensions facility"), > > + FEAT_INIT("emon", S390_FEAT_TYPE_STFL, 36, "Enhanced-monitor facility"), > > + FEAT_INIT("fpe", S390_FEAT_TYPE_STFL, 37, "Floating-point extension facility"), > > + FEAT_INIT("sprogp", S390_FEAT_TYPE_STFL, 40, "Set-program-parameters facility"), > > + FEAT_INIT("fpseh", S390_FEAT_TYPE_STFL, 41, "Floating-point-support-enhancement facilities"), > > + FEAT_INIT("dfp", S390_FEAT_TYPE_STFL, 42, "DFP (decimal-floating-point) facility"), > > + FEAT_INIT("dfphp", S390_FEAT_TYPE_STFL, 43, "DFP (decimal-floating-point) facility has high performance"), > > + FEAT_INIT("pfpo", S390_FEAT_TYPE_STFL, 44, "PFPO instruction"), > > + FEAT_INIT("gen11e", S390_FEAT_TYPE_STFL, 45, "Various facilities introduced with z196"), > > Hmm, not sure if gen11e is the best name. > > It is for > "The distinct-operands, fast-BCR-serialization, high- > word, and population-count facilities, the > interlocked-access facility 1, and the load/store-on- > condition facility 1 are installed in the z/Architecture > architectural mode." > > which is certainly too long,even for the comment. So what about > "stfle45" instead of "gen11e" ? well, it at least tells you that it is a an enhancement part of hw generation 11. But I don't have a strong opinion on that. These should usually never be presented to the user either way (because contained in the base models). David