From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47210) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eoAEe-0008Ci-6K for qemu-devel@nongnu.org; Tue, 20 Feb 2018 10:53:41 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eoAEZ-0002nr-7V for qemu-devel@nongnu.org; Tue, 20 Feb 2018 10:53:40 -0500 Date: Tue, 20 Feb 2018 16:53:23 +0100 From: Cornelia Huck Message-ID: <20180220165323.02898d8a.cohuck@redhat.com> In-Reply-To: <20180220150713.6056-1-pasic@linux.vnet.ibm.com> References: <20180220150713.6056-1-pasic@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [qemu-s390x] [PATCH 1/1] 390x/cpumodel: document S390FeatDef.bit not applicable List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Halil Pasic Cc: David Hildenbrand , qemu-devel@nongnu.org, Alexander Graf , Christian Borntraeger , qemu-s390x@nongnu.org, Richard Henderson On Tue, 20 Feb 2018 16:07:13 +0100 Halil Pasic wrote: > The 'bit' field of the 'S390FeatDef' structure is not applicable to all > it's instances. Currently a this field is not applicable, and remains s/it's/its/ s/a this/this/ > 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, > as it's a perfectly legit bit value, and as usually the value of the bit > field is ought to be unique for each feature. > > Let's document this, and hopefully reduce the potential for confusion. > > Signed-off-by: Halil Pasic > --- > > Hi! > > This may be an overkill. A comment where the misc features > are defined would do to, but I think this is nicer. So > I decided to try it with this approach first. Is there likely to be anything else than FEAT_MISC _not_ using .bit? If not, would it be better to at a comment to the FEAT_MISC definition? > > --- > target/s390x/cpu_features.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c > index a5619f2893..34fddfe78b 100644 > --- a/target/s390x/cpu_features.c > +++ b/target/s390x/cpu_features.c > @@ -23,6 +23,13 @@ > .desc = _desc, \ > } > > +/* > + * For some feature types (e.g. S390_FEAT_TYPE_MISC) S390FeatDef.bit > + * is not applicable, as there is no corresponding feature block. See > + * s390_fill_feat_block() and it's usages. > + */ > +#define FEAT_BIT_NA -1 > + > /* 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 +130,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("dateh2", S390_FEAT_TYPE_MISC, FEAT_BIT_NA, "DAT-enhancement facility 2"), > + FEAT_INIT("cmm", S390_FEAT_TYPE_MISC, FEAT_BIT_NA, "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)"),