[Qemu-devel] [PATCH 0/5] s390x: misc fixes

[Qemu-devel] [PATCH 0/5] s390x: misc fixes

[Jens Freimann]

Conny, Christian, Alex,

here are some fixes for s390:

* Patch 1 fixes our s390 bios makefile to avoid floating point operations
* Patch 2 adds virtualization specific registers to our gdb code
* Patch 3 fixes a possible endless loop in our sclp event facility
* Patch 4 fixes printing of vector register contents    
* Patch 5 renames a function to match with the Principles of Operation 

Christian Borntraeger (2):
  pc-bios/s390-ccw: avoid floating point operations
  s390x/info registers: print vector registers properly

David Hildenbrand (3):
  s390x/gdb: expose virtualization specific registers
  s390x: set missing parent for hotplug and quiesce events
  s390x: rename io_subsystem_reset -> subsystem_reset

 configure                  |  2 +-
 gdb-xml/s390-virt.xml      | 18 ++++++++++
 hw/s390x/event-facility.c  | 20 +++++------
 hw/s390x/s390-virtio-ccw.c |  2 +-
 pc-bios/s390-ccw/Makefile  |  3 +-
 target-s390x/cpu.h         |  2 +-
 target-s390x/gdbstub.c     | 82 ++++++++++++++++++++++++++++++++++++++++++++++
 target-s390x/misc_helper.c |  4 +--
 target-s390x/translate.c   |  2 +-
 9 files changed, 118 insertions(+), 17 deletions(-)
 create mode 100644 gdb-xml/s390-virt.xml

-- 
2.3.9

[Qemu-devel] [PATCH 1/5] pc-bios/s390-ccw: avoid floating point operations

[Jens Freimann]

From: Christian Borntraeger <borntraeger@de.ibm.com>

Some gcc versions (e.g. Fedora 22 gcc 5.1.1) seem to use floating
point registers for spilling and filling of general purpose registers.
As the BIOS does not activate the AFP register setting of CR0 this can
cause data exception program checks.
Disallow floating point in the BIOS as a simple solution.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 pc-bios/s390-ccw/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
index 746603a..15e4232 100644
--- a/pc-bios/s390-ccw/Makefile
+++ b/pc-bios/s390-ccw/Makefile
@@ -10,7 +10,8 @@ $(call set-vpath, $(SRC_PATH)/pc-bios/s390-ccw)
 .PHONY : all clean build-all
 
 OBJECTS = start.o main.o bootmap.o sclp-ascii.o virtio.o
-CFLAGS += -fPIE -fno-stack-protector -ffreestanding -fno-delete-null-pointer-checks
+CFLAGS += -fPIE -fno-stack-protector -ffreestanding
+CFLAGS += -fno-delete-null-pointer-checks -msoft-float
 LDFLAGS += -Wl,-pie -nostdlib
 
 build-all: s390-ccw.img
-- 
2.3.9

[Qemu-devel] [PATCH 2/5] s390x/gdb: expose virtualization specific registers

[Jens Freimann]

From: David Hildenbrand <dahi@linux.vnet.ibm.com>

Let's expose some virtual/fake registers as virtualization specific
registers.

Signed-off-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 configure              |  2 +-
 gdb-xml/s390-virt.xml  | 18 +++++++++++
 target-s390x/gdbstub.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 101 insertions(+), 1 deletion(-)
 create mode 100644 gdb-xml/s390-virt.xml

diff --git a/configure b/configure
index f14454e..2d2a498 100755
--- a/configure
+++ b/configure
@@ -5457,7 +5457,7 @@ case "$target_name" in
     echo "TARGET_ABI32=y" >> $config_target_mak
   ;;
   s390x)
-    gdb_xml_files="s390x-core64.xml s390-acr.xml s390-fpr.xml s390-vx.xml s390-cr.xml"
+    gdb_xml_files="s390x-core64.xml s390-acr.xml s390-fpr.xml s390-vx.xml s390-cr.xml s390-virt.xml"
   ;;
   tilegx)
   ;;
diff --git a/gdb-xml/s390-virt.xml b/gdb-xml/s390-virt.xml
new file mode 100644
index 0000000..e2e9a7a
--- /dev/null
+++ b/gdb-xml/s390-virt.xml
@@ -0,0 +1,18 @@
+<?xml version="1.0"?>
+<!-- Copyright 2015 IBM Corp.
+
+     This work is licensed under the terms of the GNU GPL, version 2 or
+     (at your option) any later version. See the COPYING file in the
+     top-level directory. -->
+
+<!DOCTYPE feature SYSTEM "gdb-target.dtd">
+<feature name="org.gnu.gdb.s390.virt">
+  <reg name="ckc" bitsize="64" type="uint64" group="system"/>
+  <reg name="cputm" bitsize="64" type="uint64" group="system"/>
+  <reg name="last_break" bitsize="64" type="code_ptr" group="system"/>
+  <reg name="prefix" bitsize="64" type="data_ptr" group="system"/>
+  <reg name="pp" bitsize="64" type="uint64" group="system"/>
+  <reg name="pfault_token" bitsize="64" type="uint64" group="system"/>
+  <reg name="pfault_select" bitsize="64" type="uint64" group="system"/>
+  <reg name="pfault_compare" bitsize="64" type="uint64" group="system"/>
+</feature>
diff --git a/target-s390x/gdbstub.c b/target-s390x/gdbstub.c
index 0c39a3c..a05d1cd 100644
--- a/target-s390x/gdbstub.c
+++ b/target-s390x/gdbstub.c
@@ -205,6 +205,82 @@ static int cpu_write_c_reg(CPUS390XState *env, uint8_t *mem_buf, int n)
         return 0;
     }
 }
+
+/* the values represent the positions in s390-virt.xml */
+#define S390_VIRT_CKC_REGNUM    0
+#define S390_VIRT_CPUTM_REGNUM  1
+#define S390_VIRT_BEA_REGNUM    2
+#define S390_VIRT_PREFIX_REGNUM 3
+#define S390_VIRT_PP_REGNUM     4
+#define S390_VIRT_PFT_REGNUM    5
+#define S390_VIRT_PFS_REGNUM    6
+#define S390_VIRT_PFC_REGNUM    7
+/* total number of registers in s390-virt.xml */
+#define S390_NUM_VIRT_REGS 8
+
+static int cpu_read_virt_reg(CPUS390XState *env, uint8_t *mem_buf, int n)
+{
+    switch (n) {
+    case S390_VIRT_CKC_REGNUM:
+        return gdb_get_regl(mem_buf, env->ckc);
+    case S390_VIRT_CPUTM_REGNUM:
+        return gdb_get_regl(mem_buf, env->cputm);
+    case S390_VIRT_BEA_REGNUM:
+        return gdb_get_regl(mem_buf, env->gbea);
+    case S390_VIRT_PREFIX_REGNUM:
+        return gdb_get_regl(mem_buf, env->psa);
+    case S390_VIRT_PP_REGNUM:
+        return gdb_get_regl(mem_buf, env->pp);
+    case S390_VIRT_PFT_REGNUM:
+        return gdb_get_regl(mem_buf, env->pfault_token);
+    case S390_VIRT_PFS_REGNUM:
+        return gdb_get_regl(mem_buf, env->pfault_select);
+    case S390_VIRT_PFC_REGNUM:
+        return gdb_get_regl(mem_buf, env->pfault_compare);
+    default:
+        return 0;
+    }
+}
+
+static int cpu_write_virt_reg(CPUS390XState *env, uint8_t *mem_buf, int n)
+{
+    switch (n) {
+    case S390_VIRT_CKC_REGNUM:
+        env->ckc = ldtul_p(mem_buf);
+        cpu_synchronize_post_init(ENV_GET_CPU(env));
+        return 8;
+    case S390_VIRT_CPUTM_REGNUM:
+        env->cputm = ldtul_p(mem_buf);
+        cpu_synchronize_post_init(ENV_GET_CPU(env));
+        return 8;
+    case S390_VIRT_BEA_REGNUM:
+        env->gbea = ldtul_p(mem_buf);
+        cpu_synchronize_post_init(ENV_GET_CPU(env));
+        return 8;
+    case S390_VIRT_PREFIX_REGNUM:
+        env->psa = ldtul_p(mem_buf);
+        cpu_synchronize_post_init(ENV_GET_CPU(env));
+        return 8;
+    case S390_VIRT_PP_REGNUM:
+        env->pp = ldtul_p(mem_buf);
+        cpu_synchronize_post_init(ENV_GET_CPU(env));
+        return 8;
+    case S390_VIRT_PFT_REGNUM:
+        env->pfault_token = ldtul_p(mem_buf);
+        cpu_synchronize_post_init(ENV_GET_CPU(env));
+        return 8;
+    case S390_VIRT_PFS_REGNUM:
+        env->pfault_select = ldtul_p(mem_buf);
+        cpu_synchronize_post_init(ENV_GET_CPU(env));
+        return 8;
+    case S390_VIRT_PFC_REGNUM:
+        env->pfault_compare = ldtul_p(mem_buf);
+        cpu_synchronize_post_init(ENV_GET_CPU(env));
+        return 8;
+    default:
+        return 0;
+    }
+}
 #endif
 
 void s390_cpu_gdb_init(CPUState *cs)
@@ -225,5 +301,11 @@ void s390_cpu_gdb_init(CPUState *cs)
     gdb_register_coprocessor(cs, cpu_read_c_reg,
                              cpu_write_c_reg,
                              S390_NUM_C_REGS, "s390-cr.xml", 0);
+
+    if (kvm_enabled()) {
+        gdb_register_coprocessor(cs, cpu_read_virt_reg,
+                                 cpu_write_virt_reg,
+                                 S390_NUM_VIRT_REGS, "s390-virt.xml", 0);
+    }
 #endif
 }
-- 
2.3.9

[Qemu-devel] [PATCH 3/5] s390x: set missing parent for hotplug and quiesce events

[Jens Freimann]

From: David Hildenbrand <dahi@linux.vnet.ibm.com>

Existing code missed to set a parent for the quiesce and hotplug event.
While this didn't matter in practise, new introspection APIs basically now
do an object_unref(object_new(T)), which loops forever.

When trying to remove the event facility bus, the code tries to
unparent all childs on the bus, so they are properly deleted and therefore removed.
As object_unparent() on these child devices doesn't work, as there is no parent,
we loop forever.

Let's fix this by adding the event facility as a parent. Also switch from
object_initialize to object_new, so the only valid reference is in fact the
parent property. This makes it more obvious when the device (state) is actually
gone (and how the reference counting works).

Signed-off-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 hw/s390x/event-facility.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index ef2a051..907b485 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -27,8 +27,6 @@ typedef struct SCLPEventsBus {
 struct SCLPEventFacility {
     SysBusDevice parent_obj;
     SCLPEventsBus sbus;
-    SCLPEvent quiesce_event;
-    SCLPEvent cpu_hotplug_event;
     /* guest' receive mask */
     unsigned int receive_mask;
 };
@@ -347,19 +345,21 @@ static void init_event_facility(Object *obj)
 {
     SCLPEventFacility *event_facility = EVENT_FACILITY(obj);
     DeviceState *sdev = DEVICE(obj);
+    Object *new;
 
     /* Spawn a new bus for SCLP events */
     qbus_create_inplace(&event_facility->sbus, sizeof(event_facility->sbus),
                         TYPE_SCLP_EVENTS_BUS, sdev, NULL);
 
-    object_initialize(&event_facility->quiesce_event, sizeof(SCLPEvent),
-                      TYPE_SCLP_QUIESCE);
-    qdev_set_parent_bus(DEVICE(&event_facility->quiesce_event),
-                        &event_facility->sbus.qbus);
-    object_initialize(&event_facility->cpu_hotplug_event, sizeof(SCLPEvent),
-                      TYPE_SCLP_CPU_HOTPLUG);
-    qdev_set_parent_bus(DEVICE(&event_facility->cpu_hotplug_event),
-                        &event_facility->sbus.qbus);
+    new = object_new(TYPE_SCLP_QUIESCE);
+    object_property_add_child(obj, TYPE_SCLP_QUIESCE, new, NULL);
+    object_unref(new);
+    qdev_set_parent_bus(DEVICE(new), &event_facility->sbus.qbus);
+
+    new = object_new(TYPE_SCLP_CPU_HOTPLUG);
+    object_property_add_child(obj, TYPE_SCLP_CPU_HOTPLUG, new, NULL);
+    object_unref(new);
+    qdev_set_parent_bus(DEVICE(new), &event_facility->sbus.qbus);
     /* the facility will automatically realize the devices via the bus */
 }
 
-- 
2.3.9

[Qemu-devel] [PATCH 4/5] s390x/info registers: print vector registers properly

[Jens Freimann]

From: Christian Borntraeger <borntraeger@de.ibm.com>

We want

F12=0000000000000000 F13=0000000000000000 F14=0000000000000000 F15=0000000000000000
V00=00000000000000000000000000000000 V01=00000000000000000000000000000000

instead of
F12=0000000000000000 F13=0000000000000000 F14=0000000000000000 F15=0000000000000000
V00=00000000000000000000000000000000
V01=00000000000000000000000000000000 V02=00000000000000000000000000000000

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 target-s390x/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index 2bca33a..1151515 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -124,7 +124,7 @@ void s390_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
     for (i = 0; i < 32; i++) {
         cpu_fprintf(f, "V%02d=%016" PRIx64 "%016" PRIx64, i,
                     env->vregs[i][0].ll, env->vregs[i][1].ll);
-        cpu_fprintf(f, (i % 2) ? " " : "\n");
+        cpu_fprintf(f, (i % 2) ? "\n" : " ");
     }
 
 #ifndef CONFIG_USER_ONLY
-- 
2.3.9

[Qemu-devel] [PATCH 5/5] s390x: rename io_subsystem_reset -> subsystem_reset

[Jens Freimann]

From: David Hildenbrand <dahi@linux.vnet.ibm.com>

According to the Pop:
"Subsystem reset operates only on those elements in the configuration
which are not CPUs".

As this is what we actually do, let's simply rename the function.

Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 hw/s390x/s390-virtio-ccw.c | 2 +-
 target-s390x/cpu.h         | 2 +-
 target-s390x/misc_helper.c | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index c53ebc1..6195f13 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -35,7 +35,7 @@ typedef struct S390CcwMachineState {
     bool dea_key_wrap;
 } S390CcwMachineState;
 
-void io_subsystem_reset(void)
+void subsystem_reset(void)
 {
     DeviceState *css, *sclp, *flic, *diag288;
 
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 5acd54c..843904b 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -568,7 +568,7 @@ void cpu_unlock(void);
 typedef struct SubchDev SubchDev;
 
 #ifndef CONFIG_USER_ONLY
-extern void io_subsystem_reset(void);
+extern void subsystem_reset(void);
 SubchDev *css_find_subch(uint8_t m, uint8_t cssid, uint8_t ssid,
                          uint16_t schid);
 bool css_subch_visible(SubchDev *sch);
diff --git a/target-s390x/misc_helper.c b/target-s390x/misc_helper.c
index 8eac0e1..3a19e32 100644
--- a/target-s390x/misc_helper.c
+++ b/target-s390x/misc_helper.c
@@ -128,7 +128,7 @@ static int modified_clear_reset(S390CPU *cpu)
         run_on_cpu(t, s390_do_cpu_full_reset, t);
     }
     cmma_reset(cpu);
-    io_subsystem_reset();
+    subsystem_reset();
     scc->load_normal(CPU(cpu));
     cpu_synchronize_all_post_reset();
     resume_all_vcpus();
@@ -146,7 +146,7 @@ static int load_normal_reset(S390CPU *cpu)
         run_on_cpu(t, s390_do_cpu_reset, t);
     }
     cmma_reset(cpu);
-    io_subsystem_reset();
+    subsystem_reset();
     scc->initial_cpu_reset(CPU(cpu));
     scc->load_normal(CPU(cpu));
     cpu_synchronize_all_post_reset();
-- 
2.3.9

Re: [Qemu-devel] [PATCH 0/5] s390x: misc fixes

[Christian Borntraeger]

Am 01.10.2015 um 10:49 schrieb Jens Freimann:
> Conny, Christian, Alex,
> 
> here are some fixes for s390:
> 
> * Patch 1 fixes our s390 bios makefile to avoid floating point operations
> * Patch 2 adds virtualization specific registers to our gdb code
> * Patch 3 fixes a possible endless loop in our sclp event facility
> * Patch 4 fixes printing of vector register contents    
> * Patch 5 renames a function to match with the Principles of Operation 
> 
> Christian Borntraeger (2):
>   pc-bios/s390-ccw: avoid floating point operations
>   s390x/info registers: print vector registers properly
> 
> David Hildenbrand (3):
>   s390x/gdb: expose virtualization specific registers
>   s390x: set missing parent for hotplug and quiesce events
>   s390x: rename io_subsystem_reset -> subsystem_reset
> 
>  configure                  |  2 +-
>  gdb-xml/s390-virt.xml      | 18 ++++++++++
>  hw/s390x/event-facility.c  | 20 +++++------
>  hw/s390x/s390-virtio-ccw.c |  2 +-
>  pc-bios/s390-ccw/Makefile  |  3 +-
>  target-s390x/cpu.h         |  2 +-
>  target-s390x/gdbstub.c     | 82 ++++++++++++++++++++++++++++++++++++++++++++++
>  target-s390x/misc_helper.c |  4 +--
>  target-s390x/translate.c   |  2 +-
>  9 files changed, 118 insertions(+), 17 deletions(-)
>  create mode 100644 gdb-xml/s390-virt.xml
> 

Thanks. applied to
git://github.com/borntraeger/qemu.git s390-next

Re: [Qemu-devel] [PATCH 3/5] s390x: set missing parent for hotplug and quiesce events

[Christian Borntraeger]

Am 01.10.2015 um 10:49 schrieb Jens Freimann:
> From: David Hildenbrand <dahi@linux.vnet.ibm.com>
> 
> Existing code missed to set a parent for the quiesce and hotplug event.
> While this didn't matter in practise, new introspection APIs basically now
> do an object_unref(object_new(T)), which loops forever.
> 
> When trying to remove the event facility bus, the code tries to
> unparent all childs on the bus, so they are properly deleted and therefore removed.
> As object_unparent() on these child devices doesn't work, as there is no parent,
> we loop forever.
> 
> Let's fix this by adding the event facility as a parent. Also switch from
> object_initialize to object_new, so the only valid reference is in fact the
> parent property. This makes it more obvious when the device (state) is actually
> gone (and how the reference counting works).

Markus, I applied this one to s390-next and it should make "qdev: Protect 
device-list-properties against broken devices" unnecessary for the sclp devices.
Depending on who is upstream first, I will add a revert of your changes in event-facility.c
and sclp.c or you can then drop the hunks.

Christian


> 
> Signed-off-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
> ---
>  hw/s390x/event-facility.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
> index ef2a051..907b485 100644
> --- a/hw/s390x/event-facility.c
> +++ b/hw/s390x/event-facility.c
> @@ -27,8 +27,6 @@ typedef struct SCLPEventsBus {
>  struct SCLPEventFacility {
>      SysBusDevice parent_obj;
>      SCLPEventsBus sbus;
> -    SCLPEvent quiesce_event;
> -    SCLPEvent cpu_hotplug_event;
>      /* guest' receive mask */
>      unsigned int receive_mask;
>  };
> @@ -347,19 +345,21 @@ static void init_event_facility(Object *obj)
>  {
>      SCLPEventFacility *event_facility = EVENT_FACILITY(obj);
>      DeviceState *sdev = DEVICE(obj);
> +    Object *new;
> 
>      /* Spawn a new bus for SCLP events */
>      qbus_create_inplace(&event_facility->sbus, sizeof(event_facility->sbus),
>                          TYPE_SCLP_EVENTS_BUS, sdev, NULL);
> 
> -    object_initialize(&event_facility->quiesce_event, sizeof(SCLPEvent),
> -                      TYPE_SCLP_QUIESCE);
> -    qdev_set_parent_bus(DEVICE(&event_facility->quiesce_event),
> -                        &event_facility->sbus.qbus);
> -    object_initialize(&event_facility->cpu_hotplug_event, sizeof(SCLPEvent),
> -                      TYPE_SCLP_CPU_HOTPLUG);
> -    qdev_set_parent_bus(DEVICE(&event_facility->cpu_hotplug_event),
> -                        &event_facility->sbus.qbus);
> +    new = object_new(TYPE_SCLP_QUIESCE);
> +    object_property_add_child(obj, TYPE_SCLP_QUIESCE, new, NULL);
> +    object_unref(new);
> +    qdev_set_parent_bus(DEVICE(new), &event_facility->sbus.qbus);
> +
> +    new = object_new(TYPE_SCLP_CPU_HOTPLUG);
> +    object_property_add_child(obj, TYPE_SCLP_CPU_HOTPLUG, new, NULL);
> +    object_unref(new);
> +    qdev_set_parent_bus(DEVICE(new), &event_facility->sbus.qbus);
>      /* the facility will automatically realize the devices via the bus */
>  }
>