Re: [Qemu-devel] [PATCH v6 12/17] target-s390x: Add S390 CPU class initialization routines
On Wed, May 06, 2015 at 10:02:22AM +0200, Michael Mueller wrote:
> On Tue, 5 May 2015 11:34:06 -0300
> Eduardo Habkost wrote:
>
> > On Mon, Apr 27, 2015 at 04:53:26PM +0200, Michael Mueller wrote:
> > > This patch provides routines to dynamically update the previously defined
> > > S390 CPU classes in the current host context. The main function performing
> > > this process is s390_setup_cpu_classes(). It takes the current host
> > > context
> > > and a facility list mask as parameter to setup the classes accordingly. It
> > > basically performs the following sub-tasks:
> > >
> > > - Update of CPU classes with accelerator specific host and QEMU properties
> > > - Mark adequate CPU class as default CPU class to be used for CPU model
> > > 'host'
> > > - Invalidate CPU classes not supported by this hosting machine
> > > - Define machine type aliases to latest GA number of a processor model
> > > - Define aliases for common CPU model names
> > > - Set CPU model alias 'host' to default CPU class
> > >
> > > Forthermore the patch provides the following routines:
> > >
> > > - cpu_desc_avail(), s390 specific stub indicating that list_cpus() can run
> > > - s390_setup_cpu_aliases(), adds cu model aliases
> > > - s390_cpu_classes_initialized(), test if CPU classes have been
> > > initialized
> > > - s390_fac_list_mask_by_machine(), returns facility list mask by machine
> > > - s390_current_fac_list_mask(), returns facility list mask of current
> > > machine
> > >
> > > Signed-off-by: Michael Mueller
> > > Acked-by: Christian Borntraeger
> > > ---
> > [...]
> > > +/**
> > > + * s390_setup_cpu_classes:
> > > + * @mode: the accelerator mode
> > > + * @prop: the machine property structure's address
> > > + *
> > > + * This function validates the defined cpu classes against the given
> > > + * machine properties @prop. Only cpu classes that are runnable on the
> > > + * current host will be set active. In addition the corresponding
> > > + * cpuid, ibc value and the active set of facilities will be
> > > + * initialized. Depending on @mode, the function porforms operations
> > > + * on the current or the temporary accelerator properies.
> > > + *
> > > + * Since: 2.4
> > > + */
> > > +void s390_setup_cpu_classes(S390AccelMode mode, S390MachineProps *prop,
> > > +uint64_t *fac_list_mask)
> > > +{
> >
> > Can't you replace the S390AccelMode arguments everywhere with simply an
> > AccelState pointer? That's the kind of thing that should have been
> > easier to implement using the accel QOM stuff.
>
> Would just make sense in conjunction with an AccelId indexed array
> in the CPU class but see my concerns below.
>
> >
> > If you still need to save accel-specific data somewhere (like the
> > is_active, is_host and fac_list arrays), maybe it can be indexed using
> > the AccelId enum you have introduced, instead of S390AccelMode?
>
> I had an AccelId indexed array in a previous version of the patch but
> dismissed it in favor to this AccelMode index approach for the following
> reasons:
>
> a) There is just one accelerator active and and a second set of values is
>used for the query-cpu-definitions case. Using the AcceldId index would
>instantly double the required memory being used for no reason. The size
>of the second dimension in uint64_t
> fac_list[ACCEL_MODE_MAX][FAC_LIST_CPU_S390_SIZE_UINT64];
>is architecturally allowed to grow up to 2KB.
>
> b) The information stored has more dimensions than just the accelerator,
>it also contains the selected machine (s390-virtio) which is represented
>by means of qemu_s390_fac_list_mask[] which currently is identical for
>all machines but that will change as the implementation progresses.
>
> So AccelMode (current, tmp) might also not fully express the semantics.
Right. So the data depends on (cpu_model, accel, machine), but we need to cache
the results for very few (just 2) accel+machine combinations at any given
moment. AccelMode makes more sense to me, now.
--
Eduardo
Re: [Qemu-devel] [PATCH v6 12/17] target-s390x: Add S390 CPU class initialization routines
On Tue, 5 May 2015 11:34:06 -0300
Eduardo Habkost wrote:
> On Mon, Apr 27, 2015 at 04:53:26PM +0200, Michael Mueller wrote:
> > This patch provides routines to dynamically update the previously defined
> > S390 CPU classes in the current host context. The main function performing
> > this process is s390_setup_cpu_classes(). It takes the current host context
> > and a facility list mask as parameter to setup the classes accordingly. It
> > basically performs the following sub-tasks:
> >
> > - Update of CPU classes with accelerator specific host and QEMU properties
> > - Mark adequate CPU class as default CPU class to be used for CPU model
> > 'host'
> > - Invalidate CPU classes not supported by this hosting machine
> > - Define machine type aliases to latest GA number of a processor model
> > - Define aliases for common CPU model names
> > - Set CPU model alias 'host' to default CPU class
> >
> > Forthermore the patch provides the following routines:
> >
> > - cpu_desc_avail(), s390 specific stub indicating that list_cpus() can run
> > - s390_setup_cpu_aliases(), adds cu model aliases
> > - s390_cpu_classes_initialized(), test if CPU classes have been initialized
> > - s390_fac_list_mask_by_machine(), returns facility list mask by machine
> > - s390_current_fac_list_mask(), returns facility list mask of current
> > machine
> >
> > Signed-off-by: Michael Mueller
> > Acked-by: Christian Borntraeger
> > ---
> [...]
> > +/**
> > + * s390_setup_cpu_classes:
> > + * @mode: the accelerator mode
> > + * @prop: the machine property structure's address
> > + *
> > + * This function validates the defined cpu classes against the given
> > + * machine properties @prop. Only cpu classes that are runnable on the
> > + * current host will be set active. In addition the corresponding
> > + * cpuid, ibc value and the active set of facilities will be
> > + * initialized. Depending on @mode, the function porforms operations
> > + * on the current or the temporary accelerator properies.
> > + *
> > + * Since: 2.4
> > + */
> > +void s390_setup_cpu_classes(S390AccelMode mode, S390MachineProps *prop,
> > +uint64_t *fac_list_mask)
> > +{
>
> Can't you replace the S390AccelMode arguments everywhere with simply an
> AccelState pointer? That's the kind of thing that should have been
> easier to implement using the accel QOM stuff.
Would just make sense in conjunction with an AccelId indexed array
in the CPU class but see my concerns below.
>
> If you still need to save accel-specific data somewhere (like the
> is_active, is_host and fac_list arrays), maybe it can be indexed using
> the AccelId enum you have introduced, instead of S390AccelMode?
I had an AccelId indexed array in a previous version of the patch but
dismissed it in favor to this AccelMode index approach for the following
reasons:
a) There is just one accelerator active and and a second set of values is
used for the query-cpu-definitions case. Using the AcceldId index would
instantly double the required memory being used for no reason. The size
of the second dimension in uint64_t
fac_list[ACCEL_MODE_MAX][FAC_LIST_CPU_S390_SIZE_UINT64];
is architecturally allowed to grow up to 2KB.
b) The information stored has more dimensions than just the accelerator,
it also contains the selected machine (s390-virtio) which is represented
by means of qemu_s390_fac_list_mask[] which currently is identical for
all machines but that will change as the implementation progresses.
So AccelMode (current, tmp) might also not fully express the semantics.
Michael
>
> > +GSList *list;
> > +ParmAddrAddrModeMask parm = {
> > +.mode = mode,
> > +.prop = prop,
> > +.mask = fac_list_mask,
> > +.host_cc = NULL,
> > +};
> > +
> > +list = object_class_get_list(TYPE_S390_CPU, false);
> > +list = g_slist_sort(list, s390_cpu_class_asc_order_compare);
> > +
> > +g_slist_foreach(list, (GFunc) s390_update_cpu_class, (gpointer) &parm);
> > +g_slist_foreach(list, (GFunc) s390_mark_host_cpu_class, (gpointer)
> > &parm);
> > +g_slist_foreach(list, (GFunc) s390_disable_not_supported_cpu_class,
> > &parm);
> > +
> > +g_slist_free(list);
> > +}
> > +
> [...]
>
Re: [Qemu-devel] [PATCH v6 12/17] target-s390x: Add S390 CPU class initialization routines
On Mon, Apr 27, 2015 at 04:53:26PM +0200, Michael Mueller wrote:
> This patch provides routines to dynamically update the previously defined
> S390 CPU classes in the current host context. The main function performing
> this process is s390_setup_cpu_classes(). It takes the current host context
> and a facility list mask as parameter to setup the classes accordingly. It
> basically performs the following sub-tasks:
>
> - Update of CPU classes with accelerator specific host and QEMU properties
> - Mark adequate CPU class as default CPU class to be used for CPU model 'host'
> - Invalidate CPU classes not supported by this hosting machine
> - Define machine type aliases to latest GA number of a processor model
> - Define aliases for common CPU model names
> - Set CPU model alias 'host' to default CPU class
>
> Forthermore the patch provides the following routines:
>
> - cpu_desc_avail(), s390 specific stub indicating that list_cpus() can run
> - s390_setup_cpu_aliases(), adds cu model aliases
> - s390_cpu_classes_initialized(), test if CPU classes have been initialized
> - s390_fac_list_mask_by_machine(), returns facility list mask by machine
> - s390_current_fac_list_mask(), returns facility list mask of current machine
>
> Signed-off-by: Michael Mueller
> Acked-by: Christian Borntraeger
> ---
[...]
> +/**
> + * s390_setup_cpu_classes:
> + * @mode: the accelerator mode
> + * @prop: the machine property structure's address
> + *
> + * This function validates the defined cpu classes against the given
> + * machine properties @prop. Only cpu classes that are runnable on the
> + * current host will be set active. In addition the corresponding
> + * cpuid, ibc value and the active set of facilities will be
> + * initialized. Depending on @mode, the function porforms operations
> + * on the current or the temporary accelerator properies.
> + *
> + * Since: 2.4
> + */
> +void s390_setup_cpu_classes(S390AccelMode mode, S390MachineProps *prop,
> +uint64_t *fac_list_mask)
> +{
Can't you replace the S390AccelMode arguments everywhere with simply an
AccelState pointer? That's the kind of thing that should have been
easier to implement using the accel QOM stuff.
If you still need to save accel-specific data somewhere (like the
is_active, is_host and fac_list arrays), maybe it can be indexed using
the AccelId enum you have introduced, instead of S390AccelMode?
> +GSList *list;
> +ParmAddrAddrModeMask parm = {
> +.mode = mode,
> +.prop = prop,
> +.mask = fac_list_mask,
> +.host_cc = NULL,
> +};
> +
> +list = object_class_get_list(TYPE_S390_CPU, false);
> +list = g_slist_sort(list, s390_cpu_class_asc_order_compare);
> +
> +g_slist_foreach(list, (GFunc) s390_update_cpu_class, (gpointer) &parm);
> +g_slist_foreach(list, (GFunc) s390_mark_host_cpu_class, (gpointer)
> &parm);
> +g_slist_foreach(list, (GFunc) s390_disable_not_supported_cpu_class,
> &parm);
> +
> +g_slist_free(list);
> +}
> +
[...]
--
Eduardo
[Qemu-devel] [PATCH v6 12/17] target-s390x: Add S390 CPU class initialization routines
This patch provides routines to dynamically update the previously defined
S390 CPU classes in the current host context. The main function performing
this process is s390_setup_cpu_classes(). It takes the current host context
and a facility list mask as parameter to setup the classes accordingly. It
basically performs the following sub-tasks:
- Update of CPU classes with accelerator specific host and QEMU properties
- Mark adequate CPU class as default CPU class to be used for CPU model 'host'
- Invalidate CPU classes not supported by this hosting machine
- Define machine type aliases to latest GA number of a processor model
- Define aliases for common CPU model names
- Set CPU model alias 'host' to default CPU class
Forthermore the patch provides the following routines:
- cpu_desc_avail(), s390 specific stub indicating that list_cpus() can run
- s390_setup_cpu_aliases(), adds cu model aliases
- s390_cpu_classes_initialized(), test if CPU classes have been initialized
- s390_fac_list_mask_by_machine(), returns facility list mask by machine
- s390_current_fac_list_mask(), returns facility list mask of current machine
Signed-off-by: Michael Mueller
Acked-by: Christian Borntraeger
---
target-s390x/cpu-models.c | 501 ++
target-s390x/cpu-models.h | 29 +++
target-s390x/cpu.c| 17 +-
target-s390x/kvm.c| 5 +-
4 files changed, 550 insertions(+), 2 deletions(-)
diff --git a/target-s390x/cpu-models.c b/target-s390x/cpu-models.c
index b840ebe..3743f38 100644
--- a/target-s390x/cpu-models.c
+++ b/target-s390x/cpu-models.c
@@ -13,6 +13,11 @@
#include "qemu-common.h"
#include "cpu-models.h"
#include "gen-facilities.h"
+#include "qemu/error-report.h"
+#ifndef CONFIG_USER_ONLY
+#include "exec/cpu-common.h"
+#include "hw/boards.h"
+#endif
#define S390_PROC_DEF(_name, _cpu_id, _desc)\
static void \
@@ -90,6 +95,39 @@ S390_PROC_DEF("2827-ga2", CPU_S390_2827_GA2, "IBM
zEnterprise EC12 GA2")
S390_PROC_DEF("2828-ga1", CPU_S390_2828_GA1, "IBM zEnterprise BC12 GA1")
S390_PROC_DEF("2964-ga1", CPU_S390_2964_GA1, "IBM z13 GA1")
+/* some types for calls to g_list_foreach() with parameters */
+typedef struct ParmBoolShortShort {
+bool valid;
+unsigned short type;
+union {
+unsigned short class;
+unsigned short gen;
+unsigned short ga;
+};
+} ParmBoolShortShort;
+
+typedef struct ParmAddrAddrModeMask {
+S390MachineProps *prop;
+S390CPUClass *host_cc;
+S390AccelMode mode;
+uint64_t *mask;
+} ParmAddrAddrModeMask;
+
+/* compare order of two cpu classes for ascending sort */
+gint s390_cpu_class_asc_order_compare(gconstpointer a, gconstpointer b)
+{
+S390CPUClass *cc_a = S390_CPU_CLASS((ObjectClass *) a);
+S390CPUClass *cc_b = S390_CPU_CLASS((ObjectClass *) b);
+
+if (cc_a->mach.order < cc_b->mach.order) {
+return -1;
+}
+if (cc_a->mach.order > cc_b->mach.order) {
+return 1;
+}
+return 0;
+}
+
static gint s390_cpu_compare_name(gconstpointer a, gconstpointer b)
{
char *aname = strdup_cpu_name((S390CPUClass *) a);
@@ -169,3 +207,466 @@ int set_s390_cpu_alias(const char *name, const char
*model)
s390_cpu_aliases = g_slist_append(s390_cpu_aliases, alias);
return 0;
}
+
+/* return machine class for specific machine type */
+static void s390_machine_class_test_cpu_class(gpointer data, gpointer
user_data)
+{
+S390CPUClass *cc = S390_CPU_CLASS((ObjectClass *) data);
+ParmBoolShortShort *parm = user_data;
+
+if (parm->valid || is_cpu_class_none(cc) || parm->type != cc->proc.type) {
+return;
+}
+
+parm->class = cc->mach.class;
+parm->valid = true;
+}
+
+/* return machine class by machine type */
+static unsigned short machine_class(unsigned short type, void *user_data)
+{
+GSList *list = object_class_get_list(TYPE_S390_CPU, false);
+ParmBoolShortShort parm_class, *parm = user_data;
+
+if (parm->type != type) {
+parm->class = 0;
+}
+if (!parm->class) {
+parm_class.type = type;
+parm_class.class = 0;
+parm_class.valid = false;
+g_slist_foreach(list, (GFunc) s390_machine_class_test_cpu_class,
+&parm_class);
+g_slist_free(list);
+if (parm_class.valid) {
+parm->class = parm_class.class;
+}
+}
+parm->type = type;
+
+return parm->class;
+}
+
+/* return CMOS generation for specific machine type */
+static void s390_machine_class_test_cpu_gen(gpointer data, gpointer user_data)
+{
+S390CPUClass *cc = S390_CPU_CLASS((ObjectClass *) data);
+ParmBoolShortShort *parm = user_data;
+
+if (parm->valid) {
+return;
+}
+
+if (parm->type == cc->proc.type) {
+parm->gen = cc->proc.gen;
+parm->valid = true;
+}
+}
+
+/* return CMOS generation by machine type */
+static uint16_t
