Re: [qemu-s390x] [PATCH v2 1/1] 390x/cpumodel: document S390FeatDef.bit not applicable

[David Hildenbrand]

On 21.02.2018 17:56, Halil Pasic wrote:
> The 'bit' field of the 'S390FeatDef' structure is not applicable to all
> its instances. Currently this field is not applicable, and remains
> unused, iff the feature is of type S390_FEAT_TYPE_MISC. Having the value 0
> specified for multiple such feature definition  was a little confusing,

s/  / /

> as it's a perfectly legit bit value, and as the value of the bit
> field is usually ought to be unique for each feature of a given
> feature type.
> 
> Let us introduce a specialized macro for defining features of type
> S390_FEAT_TYPE_MISC so, that one does not have to specify neither bit nor
> type (as the later is implied).

s/later is implied/latter is implicit/

> 
> Signed-off-by: Halil Pasic <address@hidden>
> ---
> 
> v1 -> v2
> * Specialized feature initializer macro for type MISC that does not
>   require a bit value instead of defining a 'not a bit number' (that
>   is extremal) bit number.
> ---
>  target/s390x/cpu_features.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
> index a5619f2893..3b9e2745e9 100644
> --- a/target/s390x/cpu_features.c
> +++ b/target/s390x/cpu_features.c
> @@ -23,6 +23,10 @@
>          .desc = _desc,                               \
>      }
>  
> +/* S390FeatDef.bit is not applicable as there is no feature block. */
> +#define FEAT_INIT_MISC(_name, _desc)                 \
> +            FEAT_INIT(_name, S390_FEAT_TYPE_MISC, 0, _desc)
> +
>  /* indexed by feature number for easy lookup */
>  static const S390FeatDef s390_features[] = {
>      FEAT_INIT("esan3", S390_FEAT_TYPE_STFL, 0, "Instructions marked as n3"),
> @@ -123,8 +127,8 @@ static const S390FeatDef s390_features[] = {
>      FEAT_INIT("ib", S390_FEAT_TYPE_SCLP_CPU, 42, "SIE: Intervention bypass 
> facility"),
>      FEAT_INIT("cei", S390_FEAT_TYPE_SCLP_CPU, 43, "SIE: 
> Conditional-external-interception facility"),
>  
> -    FEAT_INIT("dateh2", S390_FEAT_TYPE_MISC, 0, "DAT-enhancement facility 
> 2"),
> -    FEAT_INIT("cmm", S390_FEAT_TYPE_MISC, 0, 
> "Collaborative-memory-management facility"),
> +    FEAT_INIT_MISC("dateh2", "DAT-enhancement facility 2"),
> +    FEAT_INIT_MISC("cmm", "Collaborative-memory-management facility"),
>  
>      FEAT_INIT("plo-cl", S390_FEAT_TYPE_PLO, 0, "PLO Compare and load (32 bit 
> in general registers)"),
>      FEAT_INIT("plo-clg", S390_FEAT_TYPE_PLO, 1, "PLO Compare and load (64 
> bit in parameter list)"),
> 

Reviewed-by: David Hildenbrand <address@hidden>

-- 

Thanks,

David / dhildenb