Re: [Qemu-devel] [RFC 06/28] s390x/cpumodel: introduce CPU feature group definitions

2016-06-22 Thread Eduardo Habkost 
On Wed, Jun 22, 2016 at 08:19:44AM +0200, David Hildenbrand wrote:
> > On 22.06.2016 15:02, David Hildenbrand wrote:
> > > Let's use the generated groups to create feature group representations for
> > > the user. These groups can later be used to enable/disable multiple
> > > features in one shot and will be used to reduce the amount of reported
> > > features to the user if all subfeatures are in place.
> > > 
> > > Acked-by: Cornelia Huck 
> > > Signed-off-by: David Hildenbrand 
> > > ---
> > >  target-s390x/cpu_features.c | 44 
> > > +++-
> > >  target-s390x/cpu_features.h | 23 +++
> > >  2 files changed, 66 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/target-s390x/cpu_features.c b/target-s390x/cpu_features.c
> > > index c78a189..6ec2bfc 100644
> > > --- a/target-s390x/cpu_features.c
> > > +++ b/target-s390x/cpu_features.c
> > > @@ -12,6 +12,7 @@
> > >  
> > >  #include "qemu/osdep.h"
> > >  #include "cpu_features.h"
> > > +#include "gen-features.h"
> > >  
> > >  #define FEAT_INIT(_name, _type, _bit, _desc) \
> > >  {\
> > > @@ -321,14 +322,55 @@ void s390_add_from_feat_block(S390FeatBitmap 
> > > features, S390FeatType type,
> > >  }
> > >  }
> > >  
> > > -void s390_feat_bitmap_to_ascii(const S390FeatBitmap bitmap, void *opaque,
> > > +void s390_feat_bitmap_to_ascii(const S390FeatBitmap features, void 
> > > *opaque,
> > > void (*fn)(const char *name, void 
> > > *opaque))
> > >  {
> > > +S390FeatBitmap bitmap, tmp;
> > > +S390FeatGroup group;
> > >  S390Feat feat;
> > >  
> > > +bitmap_copy(bitmap, features, S390_FEAT_MAX);
> > > +
> > > +/* process whole groups first */
> > > +for (group = 0; group < S390_FEAT_GROUP_MAX; group++) {
> > > +const S390FeatGroupDef *def = s390_feat_group_def(group);
> > > +
> > > +bitmap_and(tmp, bitmap, def->feat, S390_FEAT_MAX);
> > > +if (bitmap_equal(tmp, def->feat, S390_FEAT_MAX)) {
> > > +bitmap_andnot(bitmap, bitmap, def->feat, S390_FEAT_MAX);
> > > +(*fn)(def->name, opaque);  
> > 
> > Maybe simply write
> >fn(dev->name, opaque);
> > instead?
> 
> Will that work? As fn is a pointer I am not a 100% sure. If so, I'll change 
> it!

Both are 100% equivalent. Actually, the expression denoting the
function to be called has to be a function pointer, but (*fn)
happens to be magically converted back to a function pointer
because the spec says so.

-- 
Eduardo

Re: [Qemu-devel] [RFC 06/28] s390x/cpumodel: introduce CPU feature group definitions

2016-06-22 Thread David Hildenbrand 
> On 21.06.2016 15:02, David Hildenbrand wrote:
> > Let's use the generated groups to create feature group representations for
> > the user. These groups can later be used to enable/disable multiple
> > features in one shot and will be used to reduce the amount of reported
> > features to the user if all subfeatures are in place.
> > 
> > Acked-by: Cornelia Huck 
> > Signed-off-by: David Hildenbrand 
> > ---
> >  target-s390x/cpu_features.c | 44 
> > +++-
> >  target-s390x/cpu_features.h | 23 +++
> >  2 files changed, 66 insertions(+), 1 deletion(-)
> > 
> > diff --git a/target-s390x/cpu_features.c b/target-s390x/cpu_features.c
> > index c78a189..6ec2bfc 100644
> > --- a/target-s390x/cpu_features.c
> > +++ b/target-s390x/cpu_features.c
> > @@ -12,6 +12,7 @@
> >  
> >  #include "qemu/osdep.h"
> >  #include "cpu_features.h"
> > +#include "gen-features.h"
> >  
> >  #define FEAT_INIT(_name, _type, _bit, _desc) \
> >  {\
> > @@ -321,14 +322,55 @@ void s390_add_from_feat_block(S390FeatBitmap 
> > features, S390FeatType type,
> >  }
> >  }
> >  
> > -void s390_feat_bitmap_to_ascii(const S390FeatBitmap bitmap, void *opaque,
> > +void s390_feat_bitmap_to_ascii(const S390FeatBitmap features, void *opaque,
> > void (*fn)(const char *name, void *opaque))
> >  {
> > +S390FeatBitmap bitmap, tmp;
> > +S390FeatGroup group;
> >  S390Feat feat;
> >  
> > +bitmap_copy(bitmap, features, S390_FEAT_MAX);
> > +
> > +/* process whole groups first */
> > +for (group = 0; group < S390_FEAT_GROUP_MAX; group++) {
> > +const S390FeatGroupDef *def = s390_feat_group_def(group);
> > +
> > +bitmap_and(tmp, bitmap, def->feat, S390_FEAT_MAX);
> > +if (bitmap_equal(tmp, def->feat, S390_FEAT_MAX)) {
> > +bitmap_andnot(bitmap, bitmap, def->feat, S390_FEAT_MAX);
> > +(*fn)(def->name, opaque);  
> 
> Maybe simply write
>fn(dev->name, opaque);
> instead?

Will that work? As fn is a pointer I am not a 100% sure. If so, I'll change it!

Thanks!

David

Re: [Qemu-devel] [RFC 06/28] s390x/cpumodel: introduce CPU feature group definitions

2016-06-21 Thread Thomas Huth 
On 21.06.2016 15:02, David Hildenbrand wrote:
> Let's use the generated groups to create feature group representations for
> the user. These groups can later be used to enable/disable multiple
> features in one shot and will be used to reduce the amount of reported
> features to the user if all subfeatures are in place.
> 
> Acked-by: Cornelia Huck 
> Signed-off-by: David Hildenbrand 
> ---
>  target-s390x/cpu_features.c | 44 +++-
>  target-s390x/cpu_features.h | 23 +++
>  2 files changed, 66 insertions(+), 1 deletion(-)
> 
> diff --git a/target-s390x/cpu_features.c b/target-s390x/cpu_features.c
> index c78a189..6ec2bfc 100644
> --- a/target-s390x/cpu_features.c
> +++ b/target-s390x/cpu_features.c
> @@ -12,6 +12,7 @@
>  
>  #include "qemu/osdep.h"
>  #include "cpu_features.h"
> +#include "gen-features.h"
>  
>  #define FEAT_INIT(_name, _type, _bit, _desc) \
>  {\
> @@ -321,14 +322,55 @@ void s390_add_from_feat_block(S390FeatBitmap features, 
> S390FeatType type,
>  }
>  }
>  
> -void s390_feat_bitmap_to_ascii(const S390FeatBitmap bitmap, void *opaque,
> +void s390_feat_bitmap_to_ascii(const S390FeatBitmap features, void *opaque,
> void (*fn)(const char *name, void *opaque))
>  {
> +S390FeatBitmap bitmap, tmp;
> +S390FeatGroup group;
>  S390Feat feat;
>  
> +bitmap_copy(bitmap, features, S390_FEAT_MAX);
> +
> +/* process whole groups first */
> +for (group = 0; group < S390_FEAT_GROUP_MAX; group++) {
> +const S390FeatGroupDef *def = s390_feat_group_def(group);
> +
> +bitmap_and(tmp, bitmap, def->feat, S390_FEAT_MAX);
> +if (bitmap_equal(tmp, def->feat, S390_FEAT_MAX)) {
> +bitmap_andnot(bitmap, bitmap, def->feat, S390_FEAT_MAX);
> +(*fn)(def->name, opaque);

Maybe simply write
   fn(dev->name, opaque);
instead?

 Thomas

[Qemu-devel] [RFC 06/28] s390x/cpumodel: introduce CPU feature group definitions

2016-06-21 Thread David Hildenbrand 
Let's use the generated groups to create feature group representations for
the user. These groups can later be used to enable/disable multiple
features in one shot and will be used to reduce the amount of reported
features to the user if all subfeatures are in place.

Acked-by: Cornelia Huck 
Signed-off-by: David Hildenbrand 
---
 target-s390x/cpu_features.c | 44 +++-
 target-s390x/cpu_features.h | 23 +++
 2 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/target-s390x/cpu_features.c b/target-s390x/cpu_features.c
index c78a189..6ec2bfc 100644
--- a/target-s390x/cpu_features.c
+++ b/target-s390x/cpu_features.c
@@ -12,6 +12,7 @@
 
 #include "qemu/osdep.h"
 #include "cpu_features.h"
+#include "gen-features.h"
 
 #define FEAT_INIT(_name, _type, _bit, _desc) \
 {\
@@ -321,14 +322,55 @@ void s390_add_from_feat_block(S390FeatBitmap features, 
S390FeatType type,
 }
 }
 
-void s390_feat_bitmap_to_ascii(const S390FeatBitmap bitmap, void *opaque,
+void s390_feat_bitmap_to_ascii(const S390FeatBitmap features, void *opaque,
void (*fn)(const char *name, void *opaque))
 {
+S390FeatBitmap bitmap, tmp;
+S390FeatGroup group;
 S390Feat feat;
 
+bitmap_copy(bitmap, features, S390_FEAT_MAX);
+
+/* process whole groups first */
+for (group = 0; group < S390_FEAT_GROUP_MAX; group++) {
+const S390FeatGroupDef *def = s390_feat_group_def(group);
+
+bitmap_and(tmp, bitmap, def->feat, S390_FEAT_MAX);
+if (bitmap_equal(tmp, def->feat, S390_FEAT_MAX)) {
+bitmap_andnot(bitmap, bitmap, def->feat, S390_FEAT_MAX);
+(*fn)(def->name, opaque);
+}
+}
+
+/* report leftovers as separate features */
 feat = find_first_bit(bitmap, S390_FEAT_MAX);
 while (feat < S390_FEAT_MAX) {
 (*fn)(s390_feat_def(feat)->name, opaque);
 feat = find_next_bit(bitmap, S390_FEAT_MAX, feat + 1);
 };
 }
+
+#define FEAT_GROUP_INIT(_name, _group, _desc)\
+{\
+.name = _name,   \
+.desc = _desc,   \
+.feat = { S390_FEAT_GROUP_LIST_ ## _group }, \
+}
+
+/* indexed by feature group number for easy lookup */
+static const S390FeatGroupDef s390_feature_groups[] = {
+FEAT_GROUP_INIT("plo", PLO, "Perform-locked-operation facility"),
+FEAT_GROUP_INIT("tods", TOD_CLOCK_STEERING, "Tod-clock-steering facility"),
+FEAT_GROUP_INIT("gen13ptff", GEN13_PTFF, "PTFF enhancements introduced 
with z13"),
+FEAT_GROUP_INIT("msa", MSA, "Message-security-assist facility"),
+FEAT_GROUP_INIT("msa1", MSA_EXT_1, "Message-security-assist-extension 1 
facility"),
+FEAT_GROUP_INIT("msa2", MSA_EXT_2, "Message-security-assist-extension 2 
facility"),
+FEAT_GROUP_INIT("msa3", MSA_EXT_3, "Message-security-assist-extension 3 
facility"),
+FEAT_GROUP_INIT("msa4", MSA_EXT_4, "Message-security-assist-extension 4 
facility"),
+FEAT_GROUP_INIT("msa5", MSA_EXT_5, "Message-security-assist-extension 5 
facility"),
+};
+
+const S390FeatGroupDef *s390_feat_group_def(S390FeatGroup group)
+{
+return _feature_groups[group];
+}
diff --git a/target-s390x/cpu_features.h b/target-s390x/cpu_features.h
index 485446c..e40a636 100644
--- a/target-s390x/cpu_features.h
+++ b/target-s390x/cpu_features.h
@@ -273,6 +273,29 @@ void s390_add_from_feat_block(S390FeatBitmap features, 
S390FeatType type,
 void s390_feat_bitmap_to_ascii(const S390FeatBitmap features, void *opaque,
void (*fn)(const char *name, void *opaque));
 
+/* static groups that will never change */
+typedef enum {
+S390_FEAT_GROUP_PLO,
+S390_FEAT_GROUP_TOD_CLOCK_STEERING,
+S390_FEAT_GROUP_GEN13_PTFF_ENH,
+S390_FEAT_GROUP_MSA,
+S390_FEAT_GROUP_MSA_EXT_1,
+S390_FEAT_GROUP_MSA_EXT_2,
+S390_FEAT_GROUP_MSA_EXT_3,
+S390_FEAT_GROUP_MSA_EXT_4,
+S390_FEAT_GROUP_MSA_EXT_5,
+S390_FEAT_GROUP_MAX,
+} S390FeatGroup;
+
+/* Definition of a CPU feature group */
+typedef struct {
+const char *name;   /* name exposed to the user */
+const char *desc;   /* description exposed to the user */
+S390FeatBitmap feat;/* features contained in the group */
+} S390FeatGroupDef;
+
+const S390FeatGroupDef *s390_feat_group_def(S390FeatGroup group);
+
 #define BE_BIT_NR(BIT) (BIT ^ (BITS_PER_LONG - 1))
 #define BE_BIT(BIT) (1ULL < BE_BIT_NR(BIT))
 
-- 
2.6.6