Re: [Qemu-devel] [Patch v2 04/29] s390x/cpumodel: introduce CPU features

[Christian Borntraeger]

On 08/08/2016 05:32 PM, David Hildenbrand wrote:

In general this this very good. Mostly bike shedding and naming.

> +/* 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?

> +    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?

> +    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" ? 


> +    FEAT_INIT("cmpsceh", S390_FEAT_TYPE_STFL, 47, "CMPSC-enhancement 
> facility"),
> +    FEAT_INIT("dfpzc", S390_FEAT_TYPE_STFL, 48, "Decimal-floating-point 
> zoned-conversion facility"),
> +    FEAT_INIT("gen12e", S390_FEAT_TYPE_STFL, 49, "Various facilities 
> introduced with zEC12"),

Same here.

> +    FEAT_INIT("cte", S390_FEAT_TYPE_STFL, 50, "Constrained 
> transactional-execution facility"),
> +    FEAT_INIT("ltlbc", S390_FEAT_TYPE_STFL, 51, "Local-TLB-clearing 
> facility"),
> +    FEAT_INIT("iacc2", S390_FEAT_TYPE_STFL, 52, "Interlocked-access facility 
> 2"),
> +    FEAT_INIT("gen13e", S390_FEAT_TYPE_STFL, 53, "Various facilities 
> introduced with z13"),
and here