[Qemu-devel] [PATCH 0/3] s390: ipl device, cpu reset handler and cpu model support

[Qemu-devel] [PATCH 0/3] s390: ipl device, cpu reset handler and cpu model support

[Jens Freimann]

Hi Alex,

here's a reworked version of the ipl-device patch, the cpu reset handler
and the patch to query cpu definitions via QMP.

Patch 1: Creates a new ipl device and moves ipl code from s390_virtio.c
Patch 2: Adds a cpu reset handler to 
Patch 3  Allow to query cpu types via commandline -? argument as well
         as via qmp


Christian Borntraeger (1):
  s390: Move IPL code into a separate device

Jens Freimann (1):
  s390: Add CPU reset handler

Viktor Mihajlovski (1):
  S390: Enable -cpu help and QMP query-cpu-definitions

 hw/s390-virtio.c       | 104 +++++------------------------
 hw/s390x/Makefile.objs |   1 +
 hw/s390x/ipl.c         | 174 +++++++++++++++++++++++++++++++++++++++++++++++++
 target-s390x/cpu.c     |  58 ++++++++++++++++-
 target-s390x/cpu.h     |   3 +
 target-s390x/kvm.c     |   9 ++-
 6 files changed, 257 insertions(+), 92 deletions(-)
 create mode 100644 hw/s390x/ipl.c

-- 
1.7.12.4

[Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device

[Jens Freimann]

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

Lets move the code to setup IPL for external kernel
or via the zipl rom into a separate file. This allows to

- define a reboot handler, setting up the PSW appropriately
- enhance the boot code to IPL disks that contain a bootmap that
  was created with zipl under LPAR or z/VM (future patch)
- reuse that code for several machines (e.g. virtio-ccw and virtio-s390)
- allow different machines to provide different defaults

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>

---
v2 -> v3:
* changed include <sysemu.h> -> "sysemu.h"
* make S390IPLState non-anonymous struct
* add QOM cast macro S390_IPL(dev)
* remove trailing whitespace

v1 -> v2:
* get rid of ipl.h
* move defines to ipl.c and make s390_ipl_cpu static
---
---
 hw/s390-virtio.c       |  98 +++-------------------------
 hw/s390x/Makefile.objs |   1 +
 hw/s390x/ipl.c         | 174 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 185 insertions(+), 88 deletions(-)
 create mode 100644 hw/s390x/ipl.c

diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
index ca1bb09..a350430 100644
--- a/hw/s390-virtio.c
+++ b/hw/s390-virtio.c
@@ -25,7 +25,6 @@
 #include "boards.h"
 #include "monitor.h"
 #include "loader.h"
-#include "elf.h"
 #include "hw/virtio.h"
 #include "hw/sysbus.h"
 #include "kvm.h"
@@ -48,17 +47,6 @@
 #define KVM_S390_VIRTIO_RESET           1
 #define KVM_S390_VIRTIO_SET_STATUS      2
 
-#define KERN_IMAGE_START                0x010000UL
-#define KERN_PARM_AREA                  0x010480UL
-#define INITRD_START                    0x800000UL
-#define INITRD_PARM_START               0x010408UL
-#define INITRD_PARM_SIZE                0x010410UL
-#define PARMFILE_START                  0x001000UL
-
-#define ZIPL_START			0x009000UL
-#define ZIPL_LOAD_ADDR			0x009000UL
-#define ZIPL_FILENAME			"s390-zipl.rom"
-
 #define MAX_BLK_DEVS                    10
 
 static VirtIOS390Bus *s390_bus;
@@ -156,15 +144,10 @@ static void s390_init(QEMUMachineInitArgs *args)
 {
     ram_addr_t my_ram_size = args->ram_size;
     const char *cpu_model = args->cpu_model;
-    const char *kernel_filename = args->kernel_filename;
-    const char *kernel_cmdline = args->kernel_cmdline;
-    const char *initrd_filename = args->initrd_filename;
     CPUS390XState *env = NULL;
+    DeviceState *dev;
     MemoryRegion *sysmem = get_system_memory();
     MemoryRegion *ram = g_new(MemoryRegion, 1);
-    ram_addr_t kernel_size = 0;
-    ram_addr_t initrd_offset;
-    ram_addr_t initrd_size = 0;
     int shift = 0;
     uint8_t *storage_keys;
     void *virtio_region;
@@ -185,6 +168,15 @@ static void s390_init(QEMUMachineInitArgs *args)
     /* get a BUS */
     s390_bus = s390_virtio_bus_init(&my_ram_size);
     s390_sclp_init();
+    dev  = qdev_create(NULL, "s390-ipl");
+    if (args->kernel_filename) {
+        qdev_prop_set_string(dev, "kernel", args->kernel_filename);
+    }
+    if (args->initrd_filename) {
+        qdev_prop_set_string(dev, "initrd", args->initrd_filename);
+    }
+    qdev_prop_set_string(dev, "cmdline", args->kernel_cmdline);
+    qdev_init_nofail(dev);
 
     /* allocate RAM */
     memory_region_init_ram(ram, "s390.ram", my_ram_size);
@@ -225,76 +217,6 @@ static void s390_init(QEMUMachineInitArgs *args)
         tmp_env->storage_keys = storage_keys;
     }
 
-    /* One CPU has to run */
-    s390_add_running_cpu(env);
-
-    if (kernel_filename) {
-
-        kernel_size = load_elf(kernel_filename, NULL, NULL, NULL, NULL,
-                               NULL, 1, ELF_MACHINE, 0);
-        if (kernel_size == -1UL) {
-            kernel_size = load_image_targphys(kernel_filename, 0, ram_size);
-        }
-        if (kernel_size == -1UL) {
-            fprintf(stderr, "qemu: could not load kernel '%s'\n",
-                    kernel_filename);
-            exit(1);
-        }
-        /*
-         * we can not rely on the ELF entry point, since up to 3.2 this
-         * value was 0x800 (the SALIPL loader) and it wont work. For
-         * all (Linux) cases 0x10000 (KERN_IMAGE_START) should be fine.
-         */
-        env->psw.addr = KERN_IMAGE_START;
-        env->psw.mask = 0x0000000180000000ULL;
-    } else {
-        ram_addr_t bios_size = 0;
-        char *bios_filename;
-
-        /* Load zipl bootloader */
-        if (bios_name == NULL) {
-            bios_name = ZIPL_FILENAME;
-        }
-
-        bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
-        bios_size = load_image_targphys(bios_filename, ZIPL_LOAD_ADDR, 4096);
-        g_free(bios_filename);
-
-        if ((long)bios_size < 0) {
-            hw_error("could not load bootloader '%s'\n", bios_name);
-        }
-
-        if (bios_size > 4096) {
-            hw_error("stage1 bootloader is > 4k\n");
-        }
-
-        env->psw.addr = ZIPL_START;
-        env->psw.mask = 0x0000000180000000ULL;
-    }
-
-    if (initrd_filename) {
-        initrd_offset = INITRD_START;
-        while (kernel_size + 0x100000 > initrd_offset) {
-            initrd_offset += 0x100000;
-        }
-        initrd_size = load_image_targphys(initrd_filename, initrd_offset,
-                                          ram_size - initrd_offset);
-        if (initrd_size == -1UL) {
-            fprintf(stderr, "qemu: could not load initrd '%s'\n",
-                    initrd_filename);
-            exit(1);
-        }
-
-        /* we have to overwrite values in the kernel image, which are "rom" */
-        stq_p(rom_ptr(INITRD_PARM_START), initrd_offset);
-        stq_p(rom_ptr(INITRD_PARM_SIZE), initrd_size);
-    }
-
-    if (rom_ptr(KERN_PARM_AREA)) {
-        /* we have to overwrite values in the kernel image, which are "rom" */
-        memcpy(rom_ptr(KERN_PARM_AREA), kernel_cmdline,
-               strlen(kernel_cmdline) + 1);
-    }
 
     /* Create VirtIO network adapters */
     for(i = 0; i < nb_nics; i++) {
diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
index 096dfcd..4a5a5d8 100644
--- a/hw/s390x/Makefile.objs
+++ b/hw/s390x/Makefile.objs
@@ -4,3 +4,4 @@ obj-y := $(addprefix ../,$(obj-y))
 obj-y += sclp.o
 obj-y += event-facility.o
 obj-y += sclpquiesce.o sclpconsole.o
+obj-y += ipl.o
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
new file mode 100644
index 0000000..7670079
--- /dev/null
+++ b/hw/s390x/ipl.c
@@ -0,0 +1,174 @@
+/*
+ * bootloader support
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ *  Christian Borntraeger <borntraeger@de.ibm.com>
+ *
+ * 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.
+ *
+ */
+
+#include "sysemu.h"
+#include "cpu.h"
+#include "elf.h"
+#include "hw/loader.h"
+#include "hw/sysbus.h"
+
+#define KERN_IMAGE_START                0x010000UL
+#define KERN_PARM_AREA                  0x010480UL
+#define INITRD_START                    0x800000UL
+#define INITRD_PARM_START               0x010408UL
+#define INITRD_PARM_SIZE                0x010410UL
+#define PARMFILE_START                  0x001000UL
+#define ZIPL_FILENAME                   "s390-zipl.rom"
+#define ZIPL_IMAGE_START                0x009000UL
+#define IPL_PSW_MASK                    0x0000000180000000ULL
+
+#define TYPE_S390_IPL "s390-ipl"
+#define S390_IPL(obj) \
+    OBJECT_CHECK(S390IPLState, (obj), TYPE_S390_IPL)
+#if 0
+#define S390_IPL_CLASS(klass) \
+    OBJECT_CLASS_CHECK(S390IPLState, (klass), TYPE_S390_IPL)
+#define S390_IPL_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(S390IPLState, (obj), TYPE_S390_IPL)
+#endif
+
+typedef struct S390IPLClass {
+    /*< private >*/
+    SysBusDeviceClass parent_class;
+    /*< public >*/
+
+    void (*parent_reset) (SysBusDevice *dev);
+} S390IPLClass;
+
+typedef struct S390IPLState {
+    /*< private >*/
+    SysBusDevice parent_obj;
+    /*< public >*/
+
+    char *kernel;
+    char *initrd;
+    char *cmdline;
+} S390IPLState;
+
+
+static void s390_ipl_cpu(uint64_t pswaddr)
+{
+    CPUS390XState *env = qemu_get_cpu(0);
+    env->psw.addr = pswaddr;
+    env->psw.mask = IPL_PSW_MASK;
+    s390_add_running_cpu(env);
+}
+
+static int s390_ipl_init(SysBusDevice *dev)
+{
+    S390IPLState *ipl = S390_IPL(dev);
+    ram_addr_t kernel_size = 0;
+
+    if (!ipl->kernel) {
+        ram_addr_t bios_size = 0;
+        char *bios_filename;
+
+        /* Load zipl bootloader */
+        if (bios_name == NULL) {
+            bios_name = ZIPL_FILENAME;
+        }
+
+        bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
+        bios_size = load_image_targphys(bios_filename, ZIPL_IMAGE_START, 4096);
+        g_free(bios_filename);
+
+        if ((long)bios_size < 0) {
+            hw_error("could not load bootloader '%s'\n", bios_name);
+        }
+
+        if (bios_size > 4096) {
+            hw_error("stage1 bootloader is > 4k\n");
+        }
+        return 0;
+    } else {
+        kernel_size = load_elf(ipl->kernel, NULL, NULL, NULL, NULL,
+                               NULL, 1, ELF_MACHINE, 0);
+        if (kernel_size == -1UL) {
+            kernel_size = load_image_targphys(ipl->kernel, 0, ram_size);
+        }
+        if (kernel_size == -1UL) {
+            fprintf(stderr, "could not load kernel '%s'\n", ipl->kernel);
+            return -1;
+        }
+        /* we have to overwrite values in the kernel image, which are "rom" */
+        strcpy(rom_ptr(KERN_PARM_AREA), ipl->cmdline);
+    }
+    if (ipl->initrd) {
+        ram_addr_t initrd_offset, initrd_size;
+
+        initrd_offset = INITRD_START;
+        while (kernel_size + 0x100000 > initrd_offset) {
+            initrd_offset += 0x100000;
+        }
+        initrd_size = load_image_targphys(ipl->initrd, initrd_offset,
+                                          ram_size - initrd_offset);
+        if (initrd_size == -1UL) {
+            fprintf(stderr, "qemu: could not load initrd '%s'\n", ipl->initrd);
+            exit(1);
+        }
+
+        /* we have to overwrite values in the kernel image, which are "rom" */
+        stq_p(rom_ptr(INITRD_PARM_START), initrd_offset);
+        stq_p(rom_ptr(INITRD_PARM_SIZE), initrd_size);
+    }
+
+    return 0;
+}
+
+static Property s390_ipl_properties[] = {
+    DEFINE_PROP_STRING("kernel", S390IPLState, kernel),
+    DEFINE_PROP_STRING("initrd", S390IPLState, initrd),
+    DEFINE_PROP_STRING("cmdline", S390IPLState, cmdline),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void s390_ipl_reset(DeviceState *dev)
+{
+    S390IPLState *ipl = S390_IPL(dev);
+
+    if (ipl->kernel) {
+        /*
+         * we can not rely on the ELF entry point, since up to 3.2 this
+         * value was 0x800 (the SALIPL loader) and it wont work. For
+         * all (Linux) cases 0x10000 (KERN_IMAGE_START) should be fine.
+         */
+        return s390_ipl_cpu(KERN_IMAGE_START);
+    } else {
+        return s390_ipl_cpu(ZIPL_IMAGE_START);
+    }
+}
+
+static void s390_ipl_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+
+    k->init = s390_ipl_init;
+    dc->props = s390_ipl_properties;
+    dc->reset = s390_ipl_reset;
+    dc->no_user = 1;
+}
+
+static TypeInfo s390_ipl_info = {
+    .class_init = s390_ipl_class_init,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .name  = "s390-ipl",
+    .instance_size  = sizeof(S390IPLState),
+};
+
+static void s390_ipl_register_types(void)
+{
+    type_register_static(&s390_ipl_info);
+}
+
+type_init(s390_ipl_register_types)
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device

[Alexander Graf]

On 18.12.2012, at 18:50, Jens Freimann wrote:

> From: Christian Borntraeger <borntraeger@de.ibm.com>
> 
> Lets move the code to setup IPL for external kernel
> or via the zipl rom into a separate file. This allows to
> 
> - define a reboot handler, setting up the PSW appropriately
> - enhance the boot code to IPL disks that contain a bootmap that
>  was created with zipl under LPAR or z/VM (future patch)
> - reuse that code for several machines (e.g. virtio-ccw and virtio-s390)
> - allow different machines to provide different defaults
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
> 
> ---
> v2 -> v3:
> * changed include <sysemu.h> -> "sysemu.h"
> * make S390IPLState non-anonymous struct
> * add QOM cast macro S390_IPL(dev)
> * remove trailing whitespace
> 
> v1 -> v2:
> * get rid of ipl.h
> * move defines to ipl.c and make s390_ipl_cpu static
> ---
> ---
> hw/s390-virtio.c       |  98 +++-------------------------
> hw/s390x/Makefile.objs |   1 +
> hw/s390x/ipl.c         | 174 +++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 185 insertions(+), 88 deletions(-)
> create mode 100644 hw/s390x/ipl.c
> 
> diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
> index ca1bb09..a350430 100644
> --- a/hw/s390-virtio.c
> +++ b/hw/s390-virtio.c
> @@ -25,7 +25,6 @@
> #include "boards.h"
> #include "monitor.h"
> #include "loader.h"
> -#include "elf.h"
> #include "hw/virtio.h"
> #include "hw/sysbus.h"
> #include "kvm.h"
> @@ -48,17 +47,6 @@
> #define KVM_S390_VIRTIO_RESET           1
> #define KVM_S390_VIRTIO_SET_STATUS      2
> 
> -#define KERN_IMAGE_START                0x010000UL
> -#define KERN_PARM_AREA                  0x010480UL
> -#define INITRD_START                    0x800000UL
> -#define INITRD_PARM_START               0x010408UL
> -#define INITRD_PARM_SIZE                0x010410UL
> -#define PARMFILE_START                  0x001000UL
> -
> -#define ZIPL_START			0x009000UL
> -#define ZIPL_LOAD_ADDR			0x009000UL
> -#define ZIPL_FILENAME			"s390-zipl.rom"
> -
> #define MAX_BLK_DEVS                    10
> 
> static VirtIOS390Bus *s390_bus;
> @@ -156,15 +144,10 @@ static void s390_init(QEMUMachineInitArgs *args)
> {
>     ram_addr_t my_ram_size = args->ram_size;
>     const char *cpu_model = args->cpu_model;
> -    const char *kernel_filename = args->kernel_filename;
> -    const char *kernel_cmdline = args->kernel_cmdline;
> -    const char *initrd_filename = args->initrd_filename;
>     CPUS390XState *env = NULL;
> +    DeviceState *dev;
>     MemoryRegion *sysmem = get_system_memory();
>     MemoryRegion *ram = g_new(MemoryRegion, 1);
> -    ram_addr_t kernel_size = 0;
> -    ram_addr_t initrd_offset;
> -    ram_addr_t initrd_size = 0;
>     int shift = 0;
>     uint8_t *storage_keys;
>     void *virtio_region;
> @@ -185,6 +168,15 @@ static void s390_init(QEMUMachineInitArgs *args)
>     /* get a BUS */
>     s390_bus = s390_virtio_bus_init(&my_ram_size);
>     s390_sclp_init();
> +    dev  = qdev_create(NULL, "s390-ipl");
> +    if (args->kernel_filename) {
> +        qdev_prop_set_string(dev, "kernel", args->kernel_filename);
> +    }
> +    if (args->initrd_filename) {
> +        qdev_prop_set_string(dev, "initrd", args->initrd_filename);
> +    }
> +    qdev_prop_set_string(dev, "cmdline", args->kernel_cmdline);
> +    qdev_init_nofail(dev);
> 
>     /* allocate RAM */
>     memory_region_init_ram(ram, "s390.ram", my_ram_size);
> @@ -225,76 +217,6 @@ static void s390_init(QEMUMachineInitArgs *args)
>         tmp_env->storage_keys = storage_keys;
>     }
> 
> -    /* One CPU has to run */
> -    s390_add_running_cpu(env);
> -
> -    if (kernel_filename) {
> -
> -        kernel_size = load_elf(kernel_filename, NULL, NULL, NULL, NULL,
> -                               NULL, 1, ELF_MACHINE, 0);
> -        if (kernel_size == -1UL) {
> -            kernel_size = load_image_targphys(kernel_filename, 0, ram_size);
> -        }
> -        if (kernel_size == -1UL) {
> -            fprintf(stderr, "qemu: could not load kernel '%s'\n",
> -                    kernel_filename);
> -            exit(1);
> -        }
> -        /*
> -         * we can not rely on the ELF entry point, since up to 3.2 this
> -         * value was 0x800 (the SALIPL loader) and it wont work. For
> -         * all (Linux) cases 0x10000 (KERN_IMAGE_START) should be fine.
> -         */
> -        env->psw.addr = KERN_IMAGE_START;
> -        env->psw.mask = 0x0000000180000000ULL;
> -    } else {
> -        ram_addr_t bios_size = 0;
> -        char *bios_filename;
> -
> -        /* Load zipl bootloader */
> -        if (bios_name == NULL) {
> -            bios_name = ZIPL_FILENAME;
> -        }
> -
> -        bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> -        bios_size = load_image_targphys(bios_filename, ZIPL_LOAD_ADDR, 4096);
> -        g_free(bios_filename);
> -
> -        if ((long)bios_size < 0) {
> -            hw_error("could not load bootloader '%s'\n", bios_name);
> -        }
> -
> -        if (bios_size > 4096) {
> -            hw_error("stage1 bootloader is > 4k\n");
> -        }
> -
> -        env->psw.addr = ZIPL_START;
> -        env->psw.mask = 0x0000000180000000ULL;
> -    }
> -
> -    if (initrd_filename) {
> -        initrd_offset = INITRD_START;
> -        while (kernel_size + 0x100000 > initrd_offset) {
> -            initrd_offset += 0x100000;
> -        }
> -        initrd_size = load_image_targphys(initrd_filename, initrd_offset,
> -                                          ram_size - initrd_offset);
> -        if (initrd_size == -1UL) {
> -            fprintf(stderr, "qemu: could not load initrd '%s'\n",
> -                    initrd_filename);
> -            exit(1);
> -        }
> -
> -        /* we have to overwrite values in the kernel image, which are "rom" */
> -        stq_p(rom_ptr(INITRD_PARM_START), initrd_offset);
> -        stq_p(rom_ptr(INITRD_PARM_SIZE), initrd_size);
> -    }
> -
> -    if (rom_ptr(KERN_PARM_AREA)) {
> -        /* we have to overwrite values in the kernel image, which are "rom" */
> -        memcpy(rom_ptr(KERN_PARM_AREA), kernel_cmdline,
> -               strlen(kernel_cmdline) + 1);
> -    }
> 
>     /* Create VirtIO network adapters */
>     for(i = 0; i < nb_nics; i++) {
> diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
> index 096dfcd..4a5a5d8 100644
> --- a/hw/s390x/Makefile.objs
> +++ b/hw/s390x/Makefile.objs
> @@ -4,3 +4,4 @@ obj-y := $(addprefix ../,$(obj-y))
> obj-y += sclp.o
> obj-y += event-facility.o
> obj-y += sclpquiesce.o sclpconsole.o
> +obj-y += ipl.o
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> new file mode 100644
> index 0000000..7670079
> --- /dev/null
> +++ b/hw/s390x/ipl.c
> @@ -0,0 +1,174 @@
> +/*
> + * bootloader support
> + *
> + * Copyright IBM, Corp. 2012
> + *
> + * Authors:
> + *  Christian Borntraeger <borntraeger@de.ibm.com>
> + *
> + * 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.
> + *
> + */
> +
> +#include "sysemu.h"
> +#include "cpu.h"
> +#include "elf.h"
> +#include "hw/loader.h"
> +#include "hw/sysbus.h"
> +
> +#define KERN_IMAGE_START                0x010000UL
> +#define KERN_PARM_AREA                  0x010480UL
> +#define INITRD_START                    0x800000UL
> +#define INITRD_PARM_START               0x010408UL
> +#define INITRD_PARM_SIZE                0x010410UL
> +#define PARMFILE_START                  0x001000UL
> +#define ZIPL_FILENAME                   "s390-zipl.rom"
> +#define ZIPL_IMAGE_START                0x009000UL
> +#define IPL_PSW_MASK                    0x0000000180000000ULL

I actually meant something along the lines of

#define IPL_PSW_MASK                    (PSW_MASK_32 | PSW_MASK_64)

but I'll just change it accordingly while applying the patch :).

> +
> +#define TYPE_S390_IPL "s390-ipl"
> +#define S390_IPL(obj) \
> +    OBJECT_CHECK(S390IPLState, (obj), TYPE_S390_IPL)
> +#if 0
> +#define S390_IPL_CLASS(klass) \
> +    OBJECT_CLASS_CHECK(S390IPLState, (klass), TYPE_S390_IPL)
> +#define S390_IPL_GET_CLASS(obj) \
> +    OBJECT_GET_CLASS(S390IPLState, (obj), TYPE_S390_IPL)
> +#endif
> +
> +typedef struct S390IPLClass {
> +    /*< private >*/
> +    SysBusDeviceClass parent_class;
> +    /*< public >*/
> +
> +    void (*parent_reset) (SysBusDevice *dev);
> +} S390IPLClass;
> +
> +typedef struct S390IPLState {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +    /*< public >*/
> +
> +    char *kernel;
> +    char *initrd;
> +    char *cmdline;
> +} S390IPLState;
> +
> +
> +static void s390_ipl_cpu(uint64_t pswaddr)
> +{
> +    CPUS390XState *env = qemu_get_cpu(0);
> +    env->psw.addr = pswaddr;
> +    env->psw.mask = IPL_PSW_MASK;
> +    s390_add_running_cpu(env);
> +}
> +
> +static int s390_ipl_init(SysBusDevice *dev)
> +{
> +    S390IPLState *ipl = S390_IPL(dev);
> +    ram_addr_t kernel_size = 0;
> +
> +    if (!ipl->kernel) {
> +        ram_addr_t bios_size = 0;
> +        char *bios_filename;
> +
> +        /* Load zipl bootloader */
> +        if (bios_name == NULL) {
> +            bios_name = ZIPL_FILENAME;
> +        }
> +
> +        bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> +        bios_size = load_image_targphys(bios_filename, ZIPL_IMAGE_START, 4096);
> +        g_free(bios_filename);
> +
> +        if ((long)bios_size < 0) {
> +            hw_error("could not load bootloader '%s'\n", bios_name);
> +        }
> +
> +        if (bios_size > 4096) {
> +            hw_error("stage1 bootloader is > 4k\n");
> +        }
> +        return 0;
> +    } else {
> +        kernel_size = load_elf(ipl->kernel, NULL, NULL, NULL, NULL,
> +                               NULL, 1, ELF_MACHINE, 0);
> +        if (kernel_size == -1UL) {
> +            kernel_size = load_image_targphys(ipl->kernel, 0, ram_size);
> +        }
> +        if (kernel_size == -1UL) {
> +            fprintf(stderr, "could not load kernel '%s'\n", ipl->kernel);
> +            return -1;
> +        }
> +        /* we have to overwrite values in the kernel image, which are "rom" */
> +        strcpy(rom_ptr(KERN_PARM_AREA), ipl->cmdline);
> +    }
> +    if (ipl->initrd) {
> +        ram_addr_t initrd_offset, initrd_size;
> +
> +        initrd_offset = INITRD_START;
> +        while (kernel_size + 0x100000 > initrd_offset) {
> +            initrd_offset += 0x100000;
> +        }
> +        initrd_size = load_image_targphys(ipl->initrd, initrd_offset,
> +                                          ram_size - initrd_offset);
> +        if (initrd_size == -1UL) {
> +            fprintf(stderr, "qemu: could not load initrd '%s'\n", ipl->initrd);
> +            exit(1);
> +        }
> +
> +        /* we have to overwrite values in the kernel image, which are "rom" */
> +        stq_p(rom_ptr(INITRD_PARM_START), initrd_offset);
> +        stq_p(rom_ptr(INITRD_PARM_SIZE), initrd_size);

In the long run, these should be overwritten in RAM manually on reset, so that we can change load_image_targphys to reload the kernel from a file on reset.


Thanks, applied to s390-next.

Alex

[Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler

[Jens Freimann]

Add a CPU reset handler to have all CPUs in a PoP compliant
state.

Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>

---
v2 -> v3:
* remove FIXME
* separate parent reset from local reset by adding a while line
* use defines for register reset values

v1 -> v2:
* move setting of control registers and psa to s390_cpu_reset
  and call it from the new s390_machine_cpu_reset_cb()
  This makes it more similar to how it is done on x86
* in s390_cpu_reset() set env->halted state of cpu after
  the memset. This is needed to keep our s390_cpu_running
  counter in sync when s390_cpu_reset is called via the
  qemu_devices_reset path
* set env->halted state in s390_cpu_initfn to 1 to avoid
  decrementing the cpu counter during first reset
---
 target-s390x/cpu.c | 35 +++++++++++++++++++++++++++++++++--
 target-s390x/kvm.c |  9 ++++++++-
 2 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 619b202..58e412a 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -4,6 +4,7 @@
  * Copyright (c) 2009 Ulrich Hecht
  * Copyright (c) 2011 Alexander Graf
  * Copyright (c) 2012 SUSE LINUX Products GmbH
+ * Copyright (c) 2012 IBM Corp.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -18,12 +19,19 @@
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see
  * <http://www.gnu.org/licenses/lgpl-2.1.html>
+ * Contributions after 2012-12-11 are licensed under the terms of the
+ * GNU GPL, version 2 or (at your option) any later version.
+ *
  */
 
 #include "cpu.h"
+#include "hw/hw.h"
 #include "qemu-common.h"
 #include "qemu-timer.h"
 
+#define IPL_PSW_MASK    0x0000000180000000ULL
+#define CR0_RESET       0xE0UL
+#define CR14_RESET      0xC2000000UL;
 
 /* CPUClass::reset() */
 static void s390_cpu_reset(CPUState *s)
@@ -37,12 +45,30 @@ static void s390_cpu_reset(CPUState *s)
         log_cpu_state(env, 0);
     }
 
+    s390_del_running_cpu(env);
+
     scc->parent_reset(s);
 
     memset(env, 0, offsetof(CPUS390XState, breakpoints));
-    /* FIXME: reset vector? */
+
+    /* architectured initial values for CR 0 and 14 */
+    env->cregs[0] = CR0_RESET;
+    env->cregs[14] = CR14_RESET;
+    /* set to z/Architecture mode */
+    env->psw.mask = IPL_PSW_MASK;
+    env->psa = 0;
+    /* set halted to 1 to make sure we can add the cpu in
+     * s390_ipl_cpu code, where env->halted is set back to 0
+     * after incrementing the cpu counter */
+    env->halted = 1;
     tlb_flush(env, 1);
-    s390_add_running_cpu(env);
+}
+
+static void s390_cpu_machine_reset_cb(void *opaque)
+{
+    S390CPU *cpu = opaque;
+
+    cpu_reset(CPU(cpu));
 }
 
 static void s390_cpu_initfn(Object *obj)
@@ -66,7 +92,12 @@ static void s390_cpu_initfn(Object *obj)
     env->cpu_num = cpu_num++;
     env->ext_index = -1;
 
+    /* set env->halted state to 1 to avoid decrementing the running
+     * cpu counter in s390_cpu_reset to a negative number at 
+     * initial ipl */
+    env->halted = 1;
     cpu_reset(CPU(cpu));
+    qemu_register_reset(s390_cpu_machine_reset_cb, cpu);
 }
 
 static void s390_cpu_class_init(ObjectClass *oc, void *data)
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 94de764..fda9f1f 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -85,7 +85,14 @@ int kvm_arch_init_vcpu(CPUS390XState *env)
 
 void kvm_arch_reset_vcpu(CPUS390XState *env)
 {
-    /* FIXME: add code to reset vcpu. */
+   /* The initial reset call is needed here to reset in-kernel
+    * vcpu data that we can't access directly from QEMU
+    * (i.e. with older kernels which don't support sync_regs/ONE_REG).
+    * Before this ioctl cpu_synchronize_state() is called in common kvm
+    * code (kvm-all) */
+    if (kvm_vcpu_ioctl(env, KVM_S390_INITIAL_RESET, NULL)) {
+        perror("Can't reset vcpu\n");
+    }
 }
 
 int kvm_arch_put_registers(CPUS390XState *env, int level)
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler

[Alexander Graf]

On 18.12.2012, at 18:50, Jens Freimann wrote:

> Add a CPU reset handler to have all CPUs in a PoP compliant
> state.
> 
> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
> 
> ---
> v2 -> v3:
> * remove FIXME
> * separate parent reset from local reset by adding a while line
> * use defines for register reset values
> 
> v1 -> v2:
> * move setting of control registers and psa to s390_cpu_reset
>  and call it from the new s390_machine_cpu_reset_cb()
>  This makes it more similar to how it is done on x86
> * in s390_cpu_reset() set env->halted state of cpu after
>  the memset. This is needed to keep our s390_cpu_running
>  counter in sync when s390_cpu_reset is called via the
>  qemu_devices_reset path
> * set env->halted state in s390_cpu_initfn to 1 to avoid
>  decrementing the cpu counter during first reset
> ---
> target-s390x/cpu.c | 35 +++++++++++++++++++++++++++++++++--
> target-s390x/kvm.c |  9 ++++++++-
> 2 files changed, 41 insertions(+), 3 deletions(-)
> 
> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> index 619b202..58e412a 100644
> --- a/target-s390x/cpu.c
> +++ b/target-s390x/cpu.c
> @@ -4,6 +4,7 @@
>  * Copyright (c) 2009 Ulrich Hecht
>  * Copyright (c) 2011 Alexander Graf
>  * Copyright (c) 2012 SUSE LINUX Products GmbH
> + * Copyright (c) 2012 IBM Corp.
>  *
>  * This library is free software; you can redistribute it and/or
>  * modify it under the terms of the GNU Lesser General Public
> @@ -18,12 +19,19 @@
>  * You should have received a copy of the GNU Lesser General Public
>  * License along with this library; if not, see
>  * <http://www.gnu.org/licenses/lgpl-2.1.html>
> + * Contributions after 2012-12-11 are licensed under the terms of the
> + * GNU GPL, version 2 or (at your option) any later version.
> + *
>  */
> 
> #include "cpu.h"
> +#include "hw/hw.h"
> #include "qemu-common.h"
> #include "qemu-timer.h"
> 
> +#define IPL_PSW_MASK    0x0000000180000000ULL
> +#define CR0_RESET       0xE0UL
> +#define CR14_RESET      0xC2000000UL;
> 
> /* CPUClass::reset() */
> static void s390_cpu_reset(CPUState *s)
> @@ -37,12 +45,30 @@ static void s390_cpu_reset(CPUState *s)
>         log_cpu_state(env, 0);
>     }
> 
> +    s390_del_running_cpu(env);
> +
>     scc->parent_reset(s);
> 
>     memset(env, 0, offsetof(CPUS390XState, breakpoints));
> -    /* FIXME: reset vector? */
> +
> +    /* architectured initial values for CR 0 and 14 */
> +    env->cregs[0] = CR0_RESET;
> +    env->cregs[14] = CR14_RESET;
> +    /* set to z/Architecture mode */
> +    env->psw.mask = IPL_PSW_MASK;

Why would we set psw.mask, but not psw.addr? In fact, why are we setting psw at all here? Shouldn't we just leave it at 0 from the memset and simply override it in the ipl device?

Alex

Re: [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler

[Jens Freimann]

On Thu, Jan 03, 2013 at 01:50:22PM +0100, Alexander Graf wrote:
> 
> On 18.12.2012, at 18:50, Jens Freimann wrote:
> 
> > Add a CPU reset handler to have all CPUs in a PoP compliant
> > state.
> > 
> > Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
> > 
> > ---
> > v2 -> v3:
> > * remove FIXME
> > * separate parent reset from local reset by adding a while line
> > * use defines for register reset values
> > 
> > v1 -> v2:
> > * move setting of control registers and psa to s390_cpu_reset
> >  and call it from the new s390_machine_cpu_reset_cb()
> >  This makes it more similar to how it is done on x86
> > * in s390_cpu_reset() set env->halted state of cpu after
> >  the memset. This is needed to keep our s390_cpu_running
> >  counter in sync when s390_cpu_reset is called via the
> >  qemu_devices_reset path
> > * set env->halted state in s390_cpu_initfn to 1 to avoid
> >  decrementing the cpu counter during first reset
> > ---
> > target-s390x/cpu.c | 35 +++++++++++++++++++++++++++++++++--
> > target-s390x/kvm.c |  9 ++++++++-
> > 2 files changed, 41 insertions(+), 3 deletions(-)
> > 
> > diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> > index 619b202..58e412a 100644
> > --- a/target-s390x/cpu.c
> > +++ b/target-s390x/cpu.c
> > @@ -4,6 +4,7 @@
> >  * Copyright (c) 2009 Ulrich Hecht
> >  * Copyright (c) 2011 Alexander Graf
> >  * Copyright (c) 2012 SUSE LINUX Products GmbH
> > + * Copyright (c) 2012 IBM Corp.
> >  *
> >  * This library is free software; you can redistribute it and/or
> >  * modify it under the terms of the GNU Lesser General Public
> > @@ -18,12 +19,19 @@
> >  * You should have received a copy of the GNU Lesser General Public
> >  * License along with this library; if not, see
> >  * <http://www.gnu.org/licenses/lgpl-2.1.html>
> > + * Contributions after 2012-12-11 are licensed under the terms of the
> > + * GNU GPL, version 2 or (at your option) any later version.
> > + *
> >  */
> > 
> > #include "cpu.h"
> > +#include "hw/hw.h"
> > #include "qemu-common.h"
> > #include "qemu-timer.h"
> > 
> > +#define IPL_PSW_MASK    0x0000000180000000ULL
> > +#define CR0_RESET       0xE0UL
> > +#define CR14_RESET      0xC2000000UL;
> > 
> > /* CPUClass::reset() */
> > static void s390_cpu_reset(CPUState *s)
> > @@ -37,12 +45,30 @@ static void s390_cpu_reset(CPUState *s)
> >         log_cpu_state(env, 0);
> >     }
> > 
> > +    s390_del_running_cpu(env);
> > +
> >     scc->parent_reset(s);
> > 
> >     memset(env, 0, offsetof(CPUS390XState, breakpoints));
> > -    /* FIXME: reset vector? */
> > +
> > +    /* architectured initial values for CR 0 and 14 */
> > +    env->cregs[0] = CR0_RESET;
> > +    env->cregs[14] = CR14_RESET;
> > +    /* set to z/Architecture mode */
> > +    env->psw.mask = IPL_PSW_MASK;
> 
> Why would we set psw.mask, but not psw.addr? In fact, why are we setting psw at all here? Shouldn't we just leave it at 0 from the memset and simply override it in the ipl device?

I agree that it's redundand as we set it in the ipl device code and that it would work without setting
the mask here. I put it in here because I consider it part of the CPU reset code since we always want to start
in z/Architecture mode regardless of what code runs after the reset.

Jens

> 
> Alex
> 

Re: [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler

[Alexander Graf]

On 18.12.2012, at 18:50, Jens Freimann wrote:

> Add a CPU reset handler to have all CPUs in a PoP compliant
> state.
> 
> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
> 
> ---
> v2 -> v3:
> * remove FIXME
> * separate parent reset from local reset by adding a while line
> * use defines for register reset values
> 
> v1 -> v2:
> * move setting of control registers and psa to s390_cpu_reset
>  and call it from the new s390_machine_cpu_reset_cb()
>  This makes it more similar to how it is done on x86
> * in s390_cpu_reset() set env->halted state of cpu after
>  the memset. This is needed to keep our s390_cpu_running
>  counter in sync when s390_cpu_reset is called via the
>  qemu_devices_reset path
> * set env->halted state in s390_cpu_initfn to 1 to avoid
>  decrementing the cpu counter during first reset
> ---
> target-s390x/cpu.c | 35 +++++++++++++++++++++++++++++++++--
> target-s390x/kvm.c |  9 ++++++++-
> 2 files changed, 41 insertions(+), 3 deletions(-)
> 
> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> index 619b202..58e412a 100644
> --- a/target-s390x/cpu.c
> +++ b/target-s390x/cpu.c
> @@ -4,6 +4,7 @@
>  * Copyright (c) 2009 Ulrich Hecht
>  * Copyright (c) 2011 Alexander Graf
>  * Copyright (c) 2012 SUSE LINUX Products GmbH
> + * Copyright (c) 2012 IBM Corp.
>  *
>  * This library is free software; you can redistribute it and/or
>  * modify it under the terms of the GNU Lesser General Public
> @@ -18,12 +19,19 @@
>  * You should have received a copy of the GNU Lesser General Public
>  * License along with this library; if not, see
>  * <http://www.gnu.org/licenses/lgpl-2.1.html>
> + * Contributions after 2012-12-11 are licensed under the terms of the
> + * GNU GPL, version 2 or (at your option) any later version.
> + *
>  */
> 
> #include "cpu.h"
> +#include "hw/hw.h"

Also, have you verified that this doesn't break s390x-linux-user?

> #include "qemu-common.h"
> #include "qemu-timer.h"
> 
> +#define IPL_PSW_MASK    0x0000000180000000ULL
> +#define CR0_RESET       0xE0UL
> +#define CR14_RESET      0xC2000000UL;
> 
> /* CPUClass::reset() */
> static void s390_cpu_reset(CPUState *s)
> @@ -37,12 +45,30 @@ static void s390_cpu_reset(CPUState *s)
>         log_cpu_state(env, 0);
>     }
> 
> +    s390_del_running_cpu(env);
> +
>     scc->parent_reset(s);
> 
>     memset(env, 0, offsetof(CPUS390XState, breakpoints));
> -    /* FIXME: reset vector? */
> +
> +    /* architectured initial values for CR 0 and 14 */
> +    env->cregs[0] = CR0_RESET;
> +    env->cregs[14] = CR14_RESET;
> +    /* set to z/Architecture mode */
> +    env->psw.mask = IPL_PSW_MASK;

In fact this one is correct for CONFIG_USER_ONLY.

> +    env->psa = 0;
> +    /* set halted to 1 to make sure we can add the cpu in
> +     * s390_ipl_cpu code, where env->halted is set back to 0
> +     * after incrementing the cpu counter */
> +    env->halted = 1;

While this again probably breaks s390x-linux-user, no?


Alex

>     tlb_flush(env, 1);
> -    s390_add_running_cpu(env);
> +}
> +
> +static void s390_cpu_machine_reset_cb(void *opaque)
> +{
> +    S390CPU *cpu = opaque;
> +
> +    cpu_reset(CPU(cpu));
> }
> 
> static void s390_cpu_initfn(Object *obj)
> @@ -66,7 +92,12 @@ static void s390_cpu_initfn(Object *obj)
>     env->cpu_num = cpu_num++;
>     env->ext_index = -1;
> 
> +    /* set env->halted state to 1 to avoid decrementing the running
> +     * cpu counter in s390_cpu_reset to a negative number at 
> +     * initial ipl */
> +    env->halted = 1;
>     cpu_reset(CPU(cpu));
> +    qemu_register_reset(s390_cpu_machine_reset_cb, cpu);
> }
> 
> static void s390_cpu_class_init(ObjectClass *oc, void *data)
> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
> index 94de764..fda9f1f 100644
> --- a/target-s390x/kvm.c
> +++ b/target-s390x/kvm.c
> @@ -85,7 +85,14 @@ int kvm_arch_init_vcpu(CPUS390XState *env)
> 
> void kvm_arch_reset_vcpu(CPUS390XState *env)
> {
> -    /* FIXME: add code to reset vcpu. */
> +   /* The initial reset call is needed here to reset in-kernel
> +    * vcpu data that we can't access directly from QEMU
> +    * (i.e. with older kernels which don't support sync_regs/ONE_REG).
> +    * Before this ioctl cpu_synchronize_state() is called in common kvm
> +    * code (kvm-all) */
> +    if (kvm_vcpu_ioctl(env, KVM_S390_INITIAL_RESET, NULL)) {
> +        perror("Can't reset vcpu\n");
> +    }
> }
> 
> int kvm_arch_put_registers(CPUS390XState *env, int level)
> -- 
> 1.7.12.4
> 

Re: [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler

[Jens Freimann]

On Thu, Jan 03, 2013 at 01:55:19PM +0100, Alexander Graf wrote:
> 
> On 18.12.2012, at 18:50, Jens Freimann wrote:
> 
> > Add a CPU reset handler to have all CPUs in a PoP compliant
> > state.
> > 
> > Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
> > 
> > ---
> > v2 -> v3:
> > * remove FIXME
> > * separate parent reset from local reset by adding a while line
> > * use defines for register reset values
> > 
> > v1 -> v2:
> > * move setting of control registers and psa to s390_cpu_reset
> >  and call it from the new s390_machine_cpu_reset_cb()
> >  This makes it more similar to how it is done on x86
> > * in s390_cpu_reset() set env->halted state of cpu after
> >  the memset. This is needed to keep our s390_cpu_running
> >  counter in sync when s390_cpu_reset is called via the
> >  qemu_devices_reset path
> > * set env->halted state in s390_cpu_initfn to 1 to avoid
> >  decrementing the cpu counter during first reset
> > ---
> > target-s390x/cpu.c | 35 +++++++++++++++++++++++++++++++++--
> > target-s390x/kvm.c |  9 ++++++++-
> > 2 files changed, 41 insertions(+), 3 deletions(-)
> > 
> > diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> > index 619b202..58e412a 100644
> > --- a/target-s390x/cpu.c
> > +++ b/target-s390x/cpu.c
> > @@ -4,6 +4,7 @@
> >  * Copyright (c) 2009 Ulrich Hecht
> >  * Copyright (c) 2011 Alexander Graf
> >  * Copyright (c) 2012 SUSE LINUX Products GmbH
> > + * Copyright (c) 2012 IBM Corp.
> >  *
> >  * This library is free software; you can redistribute it and/or
> >  * modify it under the terms of the GNU Lesser General Public
> > @@ -18,12 +19,19 @@
> >  * You should have received a copy of the GNU Lesser General Public
> >  * License along with this library; if not, see
> >  * <http://www.gnu.org/licenses/lgpl-2.1.html>
> > + * Contributions after 2012-12-11 are licensed under the terms of the
> > + * GNU GPL, version 2 or (at your option) any later version.
> > + *
> >  */
> > 
> > #include "cpu.h"
> > +#include "hw/hw.h"
> 
> Also, have you verified that this doesn't break s390x-linux-user?

I verified s390x-linux-user still builds. 
 
> > #include "qemu-common.h"
> > #include "qemu-timer.h"
> > 
> > +#define IPL_PSW_MASK    0x0000000180000000ULL
> > +#define CR0_RESET       0xE0UL
> > +#define CR14_RESET      0xC2000000UL;
> > 
> > /* CPUClass::reset() */
> > static void s390_cpu_reset(CPUState *s)
> > @@ -37,12 +45,30 @@ static void s390_cpu_reset(CPUState *s)
> >         log_cpu_state(env, 0);
> >     }
> > 
> > +    s390_del_running_cpu(env);
> > +
> >     scc->parent_reset(s);
> > 
> >     memset(env, 0, offsetof(CPUS390XState, breakpoints));
> > -    /* FIXME: reset vector? */
> > +
> > +    /* architectured initial values for CR 0 and 14 */
> > +    env->cregs[0] = CR0_RESET;
> > +    env->cregs[14] = CR14_RESET;
> > +    /* set to z/Architecture mode */
> > +    env->psw.mask = IPL_PSW_MASK;
> 
> In fact this one is correct for CONFIG_USER_ONLY.
> 
> > +    env->psa = 0;
> > +    /* set halted to 1 to make sure we can add the cpu in
> > +     * s390_ipl_cpu code, where env->halted is set back to 0
> > +     * after incrementing the cpu counter */
> > +    env->halted = 1;
> 
> While this again probably breaks s390x-linux-user, no?

It still builds fine, if that's what you mean? env->halted is
not within an #ifdef !CONFIG_USER_ONLY clause.

Jens
 
> 
> Alex
> 
> >     tlb_flush(env, 1);
> > -    s390_add_running_cpu(env);
> > +}
> > +
> > +static void s390_cpu_machine_reset_cb(void *opaque)
> > +{
> > +    S390CPU *cpu = opaque;
> > +
> > +    cpu_reset(CPU(cpu));
> > }
> > 
> > static void s390_cpu_initfn(Object *obj)
> > @@ -66,7 +92,12 @@ static void s390_cpu_initfn(Object *obj)
> >     env->cpu_num = cpu_num++;
> >     env->ext_index = -1;
> > 
> > +    /* set env->halted state to 1 to avoid decrementing the running
> > +     * cpu counter in s390_cpu_reset to a negative number at 
> > +     * initial ipl */
> > +    env->halted = 1;
> >     cpu_reset(CPU(cpu));
> > +    qemu_register_reset(s390_cpu_machine_reset_cb, cpu);
> > }
> > 
> > static void s390_cpu_class_init(ObjectClass *oc, void *data)
> > diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
> > index 94de764..fda9f1f 100644
> > --- a/target-s390x/kvm.c
> > +++ b/target-s390x/kvm.c
> > @@ -85,7 +85,14 @@ int kvm_arch_init_vcpu(CPUS390XState *env)
> > 
> > void kvm_arch_reset_vcpu(CPUS390XState *env)
> > {
> > -    /* FIXME: add code to reset vcpu. */
> > +   /* The initial reset call is needed here to reset in-kernel
> > +    * vcpu data that we can't access directly from QEMU
> > +    * (i.e. with older kernels which don't support sync_regs/ONE_REG).
> > +    * Before this ioctl cpu_synchronize_state() is called in common kvm
> > +    * code (kvm-all) */
> > +    if (kvm_vcpu_ioctl(env, KVM_S390_INITIAL_RESET, NULL)) {
> > +        perror("Can't reset vcpu\n");
> > +    }
> > }
> > 
> > int kvm_arch_put_registers(CPUS390XState *env, int level)
> > -- 
> > 1.7.12.4
> > 
> 

Re: [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler

[Alexander Graf]

On 04.01.2013, at 15:09, Jens Freimann wrote:

> On Thu, Jan 03, 2013 at 01:55:19PM +0100, Alexander Graf wrote:
>> 
>> On 18.12.2012, at 18:50, Jens Freimann wrote:
>> 
>>> Add a CPU reset handler to have all CPUs in a PoP compliant
>>> state.
>>> 
>>> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
>>> 
>>> ---
>>> v2 -> v3:
>>> * remove FIXME
>>> * separate parent reset from local reset by adding a while line
>>> * use defines for register reset values
>>> 
>>> v1 -> v2:
>>> * move setting of control registers and psa to s390_cpu_reset
>>> and call it from the new s390_machine_cpu_reset_cb()
>>> This makes it more similar to how it is done on x86
>>> * in s390_cpu_reset() set env->halted state of cpu after
>>> the memset. This is needed to keep our s390_cpu_running
>>> counter in sync when s390_cpu_reset is called via the
>>> qemu_devices_reset path
>>> * set env->halted state in s390_cpu_initfn to 1 to avoid
>>> decrementing the cpu counter during first reset
>>> ---
>>> target-s390x/cpu.c | 35 +++++++++++++++++++++++++++++++++--
>>> target-s390x/kvm.c |  9 ++++++++-
>>> 2 files changed, 41 insertions(+), 3 deletions(-)
>>> 
>>> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
>>> index 619b202..58e412a 100644
>>> --- a/target-s390x/cpu.c
>>> +++ b/target-s390x/cpu.c
>>> @@ -4,6 +4,7 @@
>>> * Copyright (c) 2009 Ulrich Hecht
>>> * Copyright (c) 2011 Alexander Graf
>>> * Copyright (c) 2012 SUSE LINUX Products GmbH
>>> + * Copyright (c) 2012 IBM Corp.
>>> *
>>> * This library is free software; you can redistribute it and/or
>>> * modify it under the terms of the GNU Lesser General Public
>>> @@ -18,12 +19,19 @@
>>> * You should have received a copy of the GNU Lesser General Public
>>> * License along with this library; if not, see
>>> * <http://www.gnu.org/licenses/lgpl-2.1.html>
>>> + * Contributions after 2012-12-11 are licensed under the terms of the
>>> + * GNU GPL, version 2 or (at your option) any later version.
>>> + *
>>> */
>>> 
>>> #include "cpu.h"
>>> +#include "hw/hw.h"
>> 
>> Also, have you verified that this doesn't break s390x-linux-user?
> 
> I verified s390x-linux-user still builds. 
> 
>>> #include "qemu-common.h"
>>> #include "qemu-timer.h"
>>> 
>>> +#define IPL_PSW_MASK    0x0000000180000000ULL
>>> +#define CR0_RESET       0xE0UL
>>> +#define CR14_RESET      0xC2000000UL;
>>> 
>>> /* CPUClass::reset() */
>>> static void s390_cpu_reset(CPUState *s)
>>> @@ -37,12 +45,30 @@ static void s390_cpu_reset(CPUState *s)
>>>        log_cpu_state(env, 0);
>>>    }
>>> 
>>> +    s390_del_running_cpu(env);
>>> +
>>>    scc->parent_reset(s);
>>> 
>>>    memset(env, 0, offsetof(CPUS390XState, breakpoints));
>>> -    /* FIXME: reset vector? */
>>> +
>>> +    /* architectured initial values for CR 0 and 14 */
>>> +    env->cregs[0] = CR0_RESET;
>>> +    env->cregs[14] = CR14_RESET;
>>> +    /* set to z/Architecture mode */
>>> +    env->psw.mask = IPL_PSW_MASK;
>> 
>> In fact this one is correct for CONFIG_USER_ONLY.
>> 
>>> +    env->psa = 0;
>>> +    /* set halted to 1 to make sure we can add the cpu in
>>> +     * s390_ipl_cpu code, where env->halted is set back to 0
>>> +     * after incrementing the cpu counter */
>>> +    env->halted = 1;
>> 
>> While this again probably breaks s390x-linux-user, no?
> 
> It still builds fine, if that's what you mean? env->halted is
> not within an #ifdef !CONFIG_USER_ONLY clause.

Does it run? Or does the vcpu come up halted and never gets around to execute code?


Alex

[Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions

[Jens Freimann]

From: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>

This enables qemu -cpu help to return a list of supported CPU models
on s390 and also to query for cpu definitions in the monitor.
Initially only cpu model = host is returned. This needs to be reworked
into a full-fledged CPU model handling later on.
This change is needed to allow libvirt exploiters (like OpenStack)
to specify a CPU model.

Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 hw/s390-virtio.c   |  6 +++++-
 target-s390x/cpu.c | 23 +++++++++++++++++++++++
 target-s390x/cpu.h |  3 +++
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
index a350430..60fde26 100644
--- a/hw/s390-virtio.c
+++ b/hw/s390-virtio.c
@@ -2,6 +2,7 @@
  * QEMU S390 virtio target
  *
  * Copyright (c) 2009 Alexander Graf <agraf@suse.de>
+ * Copyright IBM Corp 2012
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -13,7 +14,10 @@
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
  * Lesser General Public License for more details.
  *
- * You should have received a copy of the GNU Lesser General Public
+ * Contributions after 2012-10-29 are licensed under the terms of the
+ * GNU GPL, version 2 or (at your option) any later version.
+ *
+ * You should have received a copy of the GNU (Lesser) General Public
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
 
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 58e412a..1cddf41 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -28,11 +28,34 @@
 #include "hw/hw.h"
 #include "qemu-common.h"
 #include "qemu-timer.h"
+#include "arch_init.h"
 
 #define IPL_PSW_MASK    0x0000000180000000ULL
 #define CR0_RESET       0xE0UL
 #define CR14_RESET      0xC2000000UL;
 
+/* generate CPU information for cpu -? */
+void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf)
+{
+#ifdef CONFIG_KVM
+    (*cpu_fprintf)(f, "s390 %16s\n", "host");
+#endif
+}
+
+CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
+{
+    CpuDefinitionInfoList *entry;
+    CpuDefinitionInfo *info;
+
+    info = g_malloc0(sizeof(*info));
+    info->name = g_strdup("host");
+
+    entry = g_malloc0(sizeof(*entry));
+    entry->value = info;
+
+    return entry;
+}
+
 /* CPUClass::reset() */
 static void s390_cpu_reset(CPUState *s)
 {
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 0f9a1f7..3513976 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -350,6 +350,9 @@ static inline void cpu_set_tls(CPUS390XState *env, target_ulong newtls)
 #define cpu_gen_code cpu_s390x_gen_code
 #define cpu_signal_handler cpu_s390x_signal_handler
 
+void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf);
+#define cpu_list s390_cpu_list
+
 #include "exec-all.h"
 
 #ifdef CONFIG_USER_ONLY
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions

[Alexander Graf]

On 18.12.2012, at 18:50, Jens Freimann wrote:

> From: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
> 
> This enables qemu -cpu help to return a list of supported CPU models
> on s390 and also to query for cpu definitions in the monitor.
> Initially only cpu model = host is returned. This needs to be reworked
> into a full-fledged CPU model handling later on.
> This change is needed to allow libvirt exploiters (like OpenStack)
> to specify a CPU model.
> 
> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>

Thanks, applied to s390-next.


Alex

[Qemu-devel] [PATCH 0/3] s390: ipl device, cpu reset handler and cpu model support

[Jens Freimann]

Hi Alex,

here's a reworked version of the ipl-device patch, the cpu reset handler
and the patch to query cpu definitions via QMP.

Viktors cpu query patch now has an #ifdef CONFIG_KVM  and the square brackets around
"host" in the text output were removed.  It provides the basic funtionality we need and will 
be followed by further patches in the future. 

Patch 1: Creates a new ipl device and moves ipl code from s390_virtio.c
Patch 2: Adds a cpu reset handler to 
Patch 3  Allow to query cpu types via commandline -? argument as well
         as via qmp

Christian Borntraeger (1):
  s390: Move IPL code into a separate device

Jens Freimann (1):
  s390: Add CPU reset handler

Viktor Mihajlovski (1):
  S390: Enable -cpu help and QMP query-cpu-definitions

 hw/s390-virtio.c       | 104 +++++----------------------------
 hw/s390x/Makefile.objs |   1 +
 hw/s390x/ipl.c         | 153 +++++++++++++++++++++++++++++++++++++++++++++++++
 target-s390x/cpu.c     |  55 +++++++++++++++++-
 target-s390x/cpu.h     |   3 +
 target-s390x/kvm.c     |   9 ++-
 6 files changed, 233 insertions(+), 92 deletions(-)
 create mode 100644 hw/s390x/ipl.c

-- 
1.7.12.4

[Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device

[Jens Freimann]

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

Lets move the code to setup IPL for external kernel
or via the zipl rom into a separate file. This allows to

- define a reboot handler, setting up the PSW appropriately
- enhance the boot code to IPL disks that contain a bootmap that
  was created with zipl under LPAR or z/VM (future patch)
- reuse that code for several machines (e.g. virtio-ccw and virtio-s390)
- allow different machines to provide different defaults

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
v1 -> v2:
 * get rid of ipl.h
 * move defines to ipl.c and make s390_ipl_cpu static

---
 hw/s390-virtio.c       |  98 ++++---------------------------
 hw/s390x/Makefile.objs |   1 +
 hw/s390x/ipl.c         | 153 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 164 insertions(+), 88 deletions(-)
 create mode 100644 hw/s390x/ipl.c

diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
index ca1bb09..a350430 100644
--- a/hw/s390-virtio.c
+++ b/hw/s390-virtio.c
@@ -25,7 +25,6 @@
 #include "boards.h"
 #include "monitor.h"
 #include "loader.h"
-#include "elf.h"
 #include "hw/virtio.h"
 #include "hw/sysbus.h"
 #include "kvm.h"
@@ -48,17 +47,6 @@
 #define KVM_S390_VIRTIO_RESET           1
 #define KVM_S390_VIRTIO_SET_STATUS      2
 
-#define KERN_IMAGE_START                0x010000UL
-#define KERN_PARM_AREA                  0x010480UL
-#define INITRD_START                    0x800000UL
-#define INITRD_PARM_START               0x010408UL
-#define INITRD_PARM_SIZE                0x010410UL
-#define PARMFILE_START                  0x001000UL
-
-#define ZIPL_START			0x009000UL
-#define ZIPL_LOAD_ADDR			0x009000UL
-#define ZIPL_FILENAME			"s390-zipl.rom"
-
 #define MAX_BLK_DEVS                    10
 
 static VirtIOS390Bus *s390_bus;
@@ -156,15 +144,10 @@ static void s390_init(QEMUMachineInitArgs *args)
 {
     ram_addr_t my_ram_size = args->ram_size;
     const char *cpu_model = args->cpu_model;
-    const char *kernel_filename = args->kernel_filename;
-    const char *kernel_cmdline = args->kernel_cmdline;
-    const char *initrd_filename = args->initrd_filename;
     CPUS390XState *env = NULL;
+    DeviceState *dev;
     MemoryRegion *sysmem = get_system_memory();
     MemoryRegion *ram = g_new(MemoryRegion, 1);
-    ram_addr_t kernel_size = 0;
-    ram_addr_t initrd_offset;
-    ram_addr_t initrd_size = 0;
     int shift = 0;
     uint8_t *storage_keys;
     void *virtio_region;
@@ -185,6 +168,15 @@ static void s390_init(QEMUMachineInitArgs *args)
     /* get a BUS */
     s390_bus = s390_virtio_bus_init(&my_ram_size);
     s390_sclp_init();
+    dev  = qdev_create(NULL, "s390-ipl");
+    if (args->kernel_filename) {
+        qdev_prop_set_string(dev, "kernel", args->kernel_filename);
+    }
+    if (args->initrd_filename) {
+        qdev_prop_set_string(dev, "initrd", args->initrd_filename);
+    }
+    qdev_prop_set_string(dev, "cmdline", args->kernel_cmdline);
+    qdev_init_nofail(dev);
 
     /* allocate RAM */
     memory_region_init_ram(ram, "s390.ram", my_ram_size);
@@ -225,76 +217,6 @@ static void s390_init(QEMUMachineInitArgs *args)
         tmp_env->storage_keys = storage_keys;
     }
 
-    /* One CPU has to run */
-    s390_add_running_cpu(env);
-
-    if (kernel_filename) {
-
-        kernel_size = load_elf(kernel_filename, NULL, NULL, NULL, NULL,
-                               NULL, 1, ELF_MACHINE, 0);
-        if (kernel_size == -1UL) {
-            kernel_size = load_image_targphys(kernel_filename, 0, ram_size);
-        }
-        if (kernel_size == -1UL) {
-            fprintf(stderr, "qemu: could not load kernel '%s'\n",
-                    kernel_filename);
-            exit(1);
-        }
-        /*
-         * we can not rely on the ELF entry point, since up to 3.2 this
-         * value was 0x800 (the SALIPL loader) and it wont work. For
-         * all (Linux) cases 0x10000 (KERN_IMAGE_START) should be fine.
-         */
-        env->psw.addr = KERN_IMAGE_START;
-        env->psw.mask = 0x0000000180000000ULL;
-    } else {
-        ram_addr_t bios_size = 0;
-        char *bios_filename;
-
-        /* Load zipl bootloader */
-        if (bios_name == NULL) {
-            bios_name = ZIPL_FILENAME;
-        }
-
-        bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
-        bios_size = load_image_targphys(bios_filename, ZIPL_LOAD_ADDR, 4096);
-        g_free(bios_filename);
-
-        if ((long)bios_size < 0) {
-            hw_error("could not load bootloader '%s'\n", bios_name);
-        }
-
-        if (bios_size > 4096) {
-            hw_error("stage1 bootloader is > 4k\n");
-        }
-
-        env->psw.addr = ZIPL_START;
-        env->psw.mask = 0x0000000180000000ULL;
-    }
-
-    if (initrd_filename) {
-        initrd_offset = INITRD_START;
-        while (kernel_size + 0x100000 > initrd_offset) {
-            initrd_offset += 0x100000;
-        }
-        initrd_size = load_image_targphys(initrd_filename, initrd_offset,
-                                          ram_size - initrd_offset);
-        if (initrd_size == -1UL) {
-            fprintf(stderr, "qemu: could not load initrd '%s'\n",
-                    initrd_filename);
-            exit(1);
-        }
-
-        /* we have to overwrite values in the kernel image, which are "rom" */
-        stq_p(rom_ptr(INITRD_PARM_START), initrd_offset);
-        stq_p(rom_ptr(INITRD_PARM_SIZE), initrd_size);
-    }
-
-    if (rom_ptr(KERN_PARM_AREA)) {
-        /* we have to overwrite values in the kernel image, which are "rom" */
-        memcpy(rom_ptr(KERN_PARM_AREA), kernel_cmdline,
-               strlen(kernel_cmdline) + 1);
-    }
 
     /* Create VirtIO network adapters */
     for(i = 0; i < nb_nics; i++) {
diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
index 096dfcd..4a5a5d8 100644
--- a/hw/s390x/Makefile.objs
+++ b/hw/s390x/Makefile.objs
@@ -4,3 +4,4 @@ obj-y := $(addprefix ../,$(obj-y))
 obj-y += sclp.o
 obj-y += event-facility.o
 obj-y += sclpquiesce.o sclpconsole.o
+obj-y += ipl.o
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
new file mode 100644
index 0000000..945a9ba
--- /dev/null
+++ b/hw/s390x/ipl.c
@@ -0,0 +1,153 @@
+/*
+ * bootloader support
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ *  Christian Borntraeger <borntraeger@de.ibm.com>
+ *
+ * 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.
+ *
+ */
+
+#include <sysemu.h>
+#include "cpu.h"
+#include "elf.h"
+#include "hw/loader.h"
+#include "hw/sysbus.h"
+
+#define KERN_IMAGE_START                0x010000UL
+#define KERN_PARM_AREA                  0x010480UL
+#define INITRD_START                    0x800000UL
+#define INITRD_PARM_START               0x010408UL
+#define INITRD_PARM_SIZE                0x010410UL
+#define PARMFILE_START                  0x001000UL
+#define ZIPL_FILENAME                   "s390-zipl.rom"
+#define ZIPL_IMAGE_START                0x009000UL
+#define IPL_PSW_MASK                    0x0000000180000000ULL
+
+typedef struct {
+    SysBusDevice dev;
+    char *kernel;
+    char *initrd;
+    char *cmdline;
+} S390IPLState;
+
+static void s390_ipl_cpu(uint64_t pswaddr)
+{
+    CPUS390XState *env = qemu_get_cpu(0);
+    env->psw.addr = pswaddr;
+    env->psw.mask = IPL_PSW_MASK;
+    s390_add_running_cpu(env);
+}
+
+static int s390_ipl_init(SysBusDevice *dev)
+{
+    S390IPLState *ipl = DO_UPCAST(S390IPLState, dev, dev);
+    ram_addr_t kernel_size = 0;
+
+    if (!ipl->kernel) {
+        ram_addr_t bios_size = 0;
+        char *bios_filename;
+
+        /* Load zipl bootloader */
+        if (bios_name == NULL) {
+            bios_name = ZIPL_FILENAME;
+        }
+
+        bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
+        bios_size = load_image_targphys(bios_filename, ZIPL_IMAGE_START, 4096);
+        g_free(bios_filename);
+
+        if ((long)bios_size < 0) {
+            hw_error("could not load bootloader '%s'\n", bios_name);
+        }
+
+        if (bios_size > 4096) {
+            hw_error("stage1 bootloader is > 4k\n");
+        }
+        return 0;
+    } else {
+        kernel_size = load_elf(ipl->kernel, NULL, NULL, NULL, NULL,
+                               NULL, 1, ELF_MACHINE, 0);
+        if (kernel_size == -1UL) {
+            kernel_size = load_image_targphys(ipl->kernel, 0, ram_size);
+        }
+        if (kernel_size == -1UL) {
+            fprintf(stderr, "could not load kernel '%s'\n", ipl->kernel);
+            return -1;
+        }
+        /* we have to overwrite values in the kernel image, which are "rom" */
+        strcpy(rom_ptr(KERN_PARM_AREA), ipl->cmdline);
+    }
+    if (ipl->initrd) {
+        ram_addr_t initrd_offset, initrd_size;
+
+        initrd_offset = INITRD_START;
+        while (kernel_size + 0x100000 > initrd_offset) {
+            initrd_offset += 0x100000;
+        }
+        initrd_size = load_image_targphys(ipl->initrd, initrd_offset,
+                                          ram_size - initrd_offset);
+        if (initrd_size == -1UL) {
+            fprintf(stderr, "qemu: could not load initrd '%s'\n", ipl->initrd);
+            exit(1);
+        }
+
+        /* we have to overwrite values in the kernel image, which are "rom" */
+        stq_p(rom_ptr(INITRD_PARM_START), initrd_offset);
+        stq_p(rom_ptr(INITRD_PARM_SIZE), initrd_size);
+    }
+
+    return 0;
+}
+
+static Property s390_ipl_properties[] = {
+    DEFINE_PROP_STRING("kernel", S390IPLState, kernel),
+    DEFINE_PROP_STRING("initrd", S390IPLState, initrd),
+    DEFINE_PROP_STRING("cmdline", S390IPLState, cmdline),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void s390_ipl_reset(DeviceState *dev)
+{
+    S390IPLState *ipl = DO_UPCAST(S390IPLState, dev.qdev, dev);
+
+    if (ipl->kernel) {
+        /*
+         * we can not rely on the ELF entry point, since up to 3.2 this
+         * value was 0x800 (the SALIPL loader) and it wont work. For
+         * all (Linux) cases 0x10000 (KERN_IMAGE_START) should be fine.
+         */
+        return s390_ipl_cpu(KERN_IMAGE_START);
+    } else {
+        return s390_ipl_cpu(ZIPL_IMAGE_START);
+    }
+}
+
+static void s390_ipl_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+
+    k->init = s390_ipl_init;
+    dc->props = s390_ipl_properties;
+    dc->reset = s390_ipl_reset;
+    dc->no_user = 1;
+}
+
+static TypeInfo s390_ipl_info = {
+    .class_init = s390_ipl_class_init,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .name  = "s390-ipl",
+    .instance_size  = sizeof(S390IPLState),
+};
+
+static void s390_register_ipl(void)
+{
+    type_register_static(&s390_ipl_info);
+}
+
+type_init(s390_register_ipl)
+
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device

[Andreas Färber]

Am 14.12.2012 17:46, schrieb Jens Freimann:
> From: Christian Borntraeger <borntraeger@de.ibm.com>
> 
> Lets move the code to setup IPL for external kernel
> or via the zipl rom into a separate file. This allows to
> 
> - define a reboot handler, setting up the PSW appropriately

Careful with the ordering then: Since patch 2/3 adds another reset
handler in the CPU instance_init, the ipl device must be created after
the CPU - I'm guessing this is the case here but will also need to be
assured in the ccw machine.

> - enhance the boot code to IPL disks that contain a bootmap that
>   was created with zipl under LPAR or z/VM (future patch)
> - reuse that code for several machines (e.g. virtio-ccw and virtio-s390)
> - allow different machines to provide different defaults
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
> ---
> v1 -> v2:
>  * get rid of ipl.h
>  * move defines to ipl.c and make s390_ipl_cpu static
> 
> ---
>  hw/s390-virtio.c       |  98 ++++---------------------------
>  hw/s390x/Makefile.objs |   1 +
>  hw/s390x/ipl.c         | 153 +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 164 insertions(+), 88 deletions(-)
>  create mode 100644 hw/s390x/ipl.c
> 
> diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
> index ca1bb09..a350430 100644
> --- a/hw/s390-virtio.c
> +++ b/hw/s390-virtio.c
[...]
> @@ -185,6 +168,15 @@ static void s390_init(QEMUMachineInitArgs *args)
>      /* get a BUS */
>      s390_bus = s390_virtio_bus_init(&my_ram_size);
>      s390_sclp_init();
> +    dev  = qdev_create(NULL, "s390-ipl");
> +    if (args->kernel_filename) {
> +        qdev_prop_set_string(dev, "kernel", args->kernel_filename);
> +    }
> +    if (args->initrd_filename) {
> +        qdev_prop_set_string(dev, "initrd", args->initrd_filename);
> +    }
> +    qdev_prop_set_string(dev, "cmdline", args->kernel_cmdline);

Why NULL checks for 2 out of 3 string properties?

> +    qdev_init_nofail(dev);
>  
>      /* allocate RAM */
>      memory_region_init_ram(ram, "s390.ram", my_ram_size);
[...]
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> new file mode 100644
> index 0000000..945a9ba
> --- /dev/null
> +++ b/hw/s390x/ipl.c

Nice location. :)

> @@ -0,0 +1,153 @@
> +/*
> + * bootloader support
> + *
> + * Copyright IBM, Corp. 2012
> + *
> + * Authors:
> + *  Christian Borntraeger <borntraeger@de.ibm.com>
> + *
> + * 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.
> + *
> + */
> +
> +#include <sysemu.h>

"sysemu.h"?

> +#include "cpu.h"
> +#include "elf.h"
> +#include "hw/loader.h"
> +#include "hw/sysbus.h"
> +
> +#define KERN_IMAGE_START                0x010000UL
> +#define KERN_PARM_AREA                  0x010480UL
> +#define INITRD_START                    0x800000UL
> +#define INITRD_PARM_START               0x010408UL
> +#define INITRD_PARM_SIZE                0x010410UL
> +#define PARMFILE_START                  0x001000UL
> +#define ZIPL_FILENAME                   "s390-zipl.rom"
> +#define ZIPL_IMAGE_START                0x009000UL
> +#define IPL_PSW_MASK                    0x0000000180000000ULL
> +
> +typedef struct {

Anonymous structs are discouraged (not sure where that makes a
difference, maybe gdb?), i.e. typedef struct S390IPLState {

> +    SysBusDevice dev;

Please adopt the following QOM convention:

SysBusDevice parent_obj; // this field is then referenced nowhere
// white line; in header files /*< private/public >*/ gtk-doc annotation
...
> +    char *kernel;
> +    char *initrd;
> +    char *cmdline;
> +} S390IPLState;

I read that you got rid of an ipl.h; since you are using this device
from a machine that seems okay - if used from another object, header
files are encouraged. Or if memory address constants are to be shared
with a qtest test case (don't think that makes sense for a bootloader).

> +
> +static void s390_ipl_cpu(uint64_t pswaddr)
> +{
> +    CPUS390XState *env = qemu_get_cpu(0);
> +    env->psw.addr = pswaddr;
> +    env->psw.mask = IPL_PSW_MASK;
> +    s390_add_running_cpu(env);
> +}
> +
> +static int s390_ipl_init(SysBusDevice *dev)
> +{
> +    S390IPLState *ipl = DO_UPCAST(S390IPLState, dev, dev);

Please use a QOM cast macro S390_IPL(dev) instead of DO_UPCAST().

You'll find many examples in
https://lists.gnu.org/archive/html/qemu-devel/2012-11/msg02746.html

> +    ram_addr_t kernel_size = 0;
> +
> +    if (!ipl->kernel) {
> +        ram_addr_t bios_size = 0;
> +        char *bios_filename;
> +
> +        /* Load zipl bootloader */
> +        if (bios_name == NULL) {
> +            bios_name = ZIPL_FILENAME;
> +        }
> +
> +        bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> +        bios_size = load_image_targphys(bios_filename, ZIPL_IMAGE_START, 4096);
> +        g_free(bios_filename);
> +
> +        if ((long)bios_size < 0) {
> +            hw_error("could not load bootloader '%s'\n", bios_name);
> +        }
> +
> +        if (bios_size > 4096) {
> +            hw_error("stage1 bootloader is > 4k\n");
> +        }
> +        return 0;
> +    } else {
> +        kernel_size = load_elf(ipl->kernel, NULL, NULL, NULL, NULL,
> +                               NULL, 1, ELF_MACHINE, 0);
> +        if (kernel_size == -1UL) {
> +            kernel_size = load_image_targphys(ipl->kernel, 0, ram_size);
> +        }
> +        if (kernel_size == -1UL) {
> +            fprintf(stderr, "could not load kernel '%s'\n", ipl->kernel);
> +            return -1;
> +        }
> +        /* we have to overwrite values in the kernel image, which are "rom" */
> +        strcpy(rom_ptr(KERN_PARM_AREA), ipl->cmdline);
> +    }
> +    if (ipl->initrd) {
> +        ram_addr_t initrd_offset, initrd_size;
> +
> +        initrd_offset = INITRD_START;
> +        while (kernel_size + 0x100000 > initrd_offset) {
> +            initrd_offset += 0x100000;
> +        }
> +        initrd_size = load_image_targphys(ipl->initrd, initrd_offset,
> +                                          ram_size - initrd_offset);
> +        if (initrd_size == -1UL) {
> +            fprintf(stderr, "qemu: could not load initrd '%s'\n", ipl->initrd);
> +            exit(1);
> +        }
> +
> +        /* we have to overwrite values in the kernel image, which are "rom" */
> +        stq_p(rom_ptr(INITRD_PARM_START), initrd_offset);
> +        stq_p(rom_ptr(INITRD_PARM_SIZE), initrd_size);
> +    }
> +
> +    return 0;
> +}
> +
> +static Property s390_ipl_properties[] = {
> +    DEFINE_PROP_STRING("kernel", S390IPLState, kernel),
> +    DEFINE_PROP_STRING("initrd", S390IPLState, initrd),
> +    DEFINE_PROP_STRING("cmdline", S390IPLState, cmdline),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void s390_ipl_reset(DeviceState *dev)
> +{
> +    S390IPLState *ipl = DO_UPCAST(S390IPLState, dev.qdev, dev);

Ditto.

> +
> +    if (ipl->kernel) {
> +        /*
> +         * we can not rely on the ELF entry point, since up to 3.2 this
> +         * value was 0x800 (the SALIPL loader) and it wont work. For
> +         * all (Linux) cases 0x10000 (KERN_IMAGE_START) should be fine.
> +         */
> +        return s390_ipl_cpu(KERN_IMAGE_START);
> +    } else {
> +        return s390_ipl_cpu(ZIPL_IMAGE_START);
> +    }
> +}
> +
> +static void s390_ipl_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
> +
> +    k->init = s390_ipl_init;
> +    dc->props = s390_ipl_properties;
> +    dc->reset = s390_ipl_reset;
> +    dc->no_user = 1;
> +}
> +
> +static TypeInfo s390_ipl_info = {

static const

> +    .class_init = s390_ipl_class_init,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .name  = "s390-ipl",
> +    .instance_size  = sizeof(S390IPLState),
> +};
> +
> +static void s390_register_ipl(void)

s390_ipl_register_types?

> +{
> +    type_register_static(&s390_ipl_info);
> +}
> +
> +type_init(s390_register_ipl)
> +

Trailing white line.

Can't fully judge the IPL logic but the code movement looks sensible.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device

[Christian Borntraeger]

On 16/12/12 17:26, Andreas Färber wrote:
> Am 14.12.2012 17:46, schrieb Jens Freimann:
>> From: Christian Borntraeger <borntraeger@de.ibm.com>
>>
>> Lets move the code to setup IPL for external kernel
>> or via the zipl rom into a separate file. This allows to
>>
>> - define a reboot handler, setting up the PSW appropriately
> 
> Careful with the ordering then: Since patch 2/3 adds another reset
> handler in the CPU instance_init, the ipl device must be created after
> the CPU - I'm guessing this is the case here but will also need to be
> assured in the ccw machine.
> 
>> - enhance the boot code to IPL disks that contain a bootmap that
>>   was created with zipl under LPAR or z/VM (future patch)
>> - reuse that code for several machines (e.g. virtio-ccw and virtio-s390)
>> - allow different machines to provide different defaults
>>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
>> ---
>> v1 -> v2:
>>  * get rid of ipl.h
>>  * move defines to ipl.c and make s390_ipl_cpu static
>>
>> ---
>>  hw/s390-virtio.c       |  98 ++++---------------------------
>>  hw/s390x/Makefile.objs |   1 +
>>  hw/s390x/ipl.c         | 153 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 164 insertions(+), 88 deletions(-)
>>  create mode 100644 hw/s390x/ipl.c
>>
>> diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
>> index ca1bb09..a350430 100644
>> --- a/hw/s390-virtio.c
>> +++ b/hw/s390-virtio.c
> [...]
>> @@ -185,6 +168,15 @@ static void s390_init(QEMUMachineInitArgs *args)
>>      /* get a BUS */
>>      s390_bus = s390_virtio_bus_init(&my_ram_size);
>>      s390_sclp_init();
>> +    dev  = qdev_create(NULL, "s390-ipl");
>> +    if (args->kernel_filename) {
>> +        qdev_prop_set_string(dev, "kernel", args->kernel_filename);
>> +    }
>> +    if (args->initrd_filename) {
>> +        qdev_prop_set_string(dev, "initrd", args->initrd_filename);
>> +    }
>> +    qdev_prop_set_string(dev, "cmdline", args->kernel_cmdline);
> 
> Why NULL checks for 2 out of 3 string properties?

cmdline is always a valid string, (never NULL), but kernel and initrd can
be NULL, which kills qdev_prop_set_string.


>> + * Authors:
>> + *  Christian Borntraeger <borntraeger@de.ibm.com>
>> + *
>> + * 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.
>> + *
>> + */
>> +
>> +#include <sysemu.h>
> 
> "sysemu.h"?

bios_name. 
I could use another property which is modified/set by the machine init.

> 
>> +#include "cpu.h"
>> +#include "elf.h"
>> +#include "hw/loader.h"
>> +#include "hw/sysbus.h"
>> +
>> +#define KERN_IMAGE_START                0x010000UL
>> +#define KERN_PARM_AREA                  0x010480UL
>> +#define INITRD_START                    0x800000UL
>> +#define INITRD_PARM_START               0x010408UL
>> +#define INITRD_PARM_SIZE                0x010410UL
>> +#define PARMFILE_START                  0x001000UL
>> +#define ZIPL_FILENAME                   "s390-zipl.rom"
>> +#define ZIPL_IMAGE_START                0x009000UL
>> +#define IPL_PSW_MASK                    0x0000000180000000ULL
>> +
>> +typedef struct {
> 
> Anonymous structs are discouraged (not sure where that makes a
> difference, maybe gdb?), i.e. typedef struct S390IPLState {
> 
>> +    SysBusDevice dev;
> 
> Please adopt the following QOM convention:
> 
> SysBusDevice parent_obj; // this field is then referenced nowhere


ok

>> +
>> +static void s390_ipl_cpu(uint64_t pswaddr)
>> +{
>> +    CPUS390XState *env = qemu_get_cpu(0);
>> +    env->psw.addr = pswaddr;
>> +    env->psw.mask = IPL_PSW_MASK;
>> +    s390_add_running_cpu(env);
>> +}
>> +
>> +static int s390_ipl_init(SysBusDevice *dev)
>> +{
>> +    S390IPLState *ipl = DO_UPCAST(S390IPLState, dev, dev);
> 
> Please use a QOM cast macro S390_IPL(dev) instead of DO_UPCAST().
> 
> You'll find many examples in
> https://lists.gnu.org/archive/html/qemu-devel/2012-11/msg02746.html

OK.

[..]
> 
>> +static TypeInfo s390_ipl_info = {
> 
> static const

ok
> 
>> +    .class_init = s390_ipl_class_init,
>> +    .parent = TYPE_SYS_BUS_DEVICE,
>> +    .name  = "s390-ipl",
>> +    .instance_size  = sizeof(S390IPLState),
>> +};
>> +
>> +static void s390_register_ipl(void)
> 
> s390_ipl_register_types?

makes sense.
> 
>> +{
>> +    type_register_static(&s390_ipl_info);
>> +}
>> +
>> +type_init(s390_register_ipl)
>> +
> 
> Trailing white line.
ok

[Qemu-devel] [PATCH 0/3] s390: ipl device, cpu reset handler and cpu model support

[Jens Freimann]

Hi Alex,

here's a reworked version of the ipl-device patch, the cpu reset handler
and a new patch to query cpu definitions via QMP.

Patch 1: Creates a new ipl device and moves ipl code from s390_virtio.c
Patch 2: Adds a cpu reset handler to 
Patch 3  Allow to query cpu types via commandline -? argument as well
         as via qmp


Christian Borntraeger (1):
  s390: Move IPL code into a separate device

Jens Freimann (1):
  s390: Add CPU reset handler

Viktor Mihajlovski (1):
  S390: Enable -cpu help and QMP query-cpu-definitions

 hw/s390-virtio.c       | 106 ++++++------------------------------
 hw/s390x/Makefile.objs |   1 +
 hw/s390x/ipl.c         | 144 +++++++++++++++++++++++++++++++++++++++++++++++++
 hw/s390x/ipl.h         |  31 +++++++++++
 target-s390x/cpu.c     |  52 +++++++++++++++++-
 target-s390x/cpu.h     |   3 ++
 target-s390x/kvm.c     |   9 +++-
 7 files changed, 254 insertions(+), 92 deletions(-)
 create mode 100644 hw/s390x/ipl.c
 create mode 100644 hw/s390x/ipl.h

-- 
1.7.12.4

[Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device

[Jens Freimann]

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

Lets move the code to setup IPL for external kernel
or via the zipl rom into a separate file. This allows to

- define a reboot handler, setting up the PSW appropriately
- reuse that code for several machines (e.g. virtio-ccw and virtio-s390)
- allow different machines to provide different defaults
- enhance the boot code to IPL disks that contain a bootmap that
  was created with zipl under LPAR or z/VM (in a future patch)

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 hw/s390-virtio.c       | 100 +++++-----------------------------
 hw/s390x/Makefile.objs |   1 +
 hw/s390x/ipl.c         | 144 +++++++++++++++++++++++++++++++++++++++++++++++++
 hw/s390x/ipl.h         |  31 +++++++++++
 4 files changed, 188 insertions(+), 88 deletions(-)
 create mode 100644 hw/s390x/ipl.c
 create mode 100644 hw/s390x/ipl.h

diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
index ca1bb09..18050b1 100644
--- a/hw/s390-virtio.c
+++ b/hw/s390-virtio.c
@@ -25,7 +25,6 @@
 #include "boards.h"
 #include "monitor.h"
 #include "loader.h"
-#include "elf.h"
 #include "hw/virtio.h"
 #include "hw/sysbus.h"
 #include "kvm.h"
@@ -33,6 +32,7 @@
 
 #include "hw/s390-virtio-bus.h"
 #include "hw/s390x/sclp.h"
+#include "hw/s390x/ipl.h"
 
 //#define DEBUG_S390
 
@@ -48,17 +48,6 @@
 #define KVM_S390_VIRTIO_RESET           1
 #define KVM_S390_VIRTIO_SET_STATUS      2
 
-#define KERN_IMAGE_START                0x010000UL
-#define KERN_PARM_AREA                  0x010480UL
-#define INITRD_START                    0x800000UL
-#define INITRD_PARM_START               0x010408UL
-#define INITRD_PARM_SIZE                0x010410UL
-#define PARMFILE_START                  0x001000UL
-
-#define ZIPL_START			0x009000UL
-#define ZIPL_LOAD_ADDR			0x009000UL
-#define ZIPL_FILENAME			"s390-zipl.rom"
-
 #define MAX_BLK_DEVS                    10
 
 static VirtIOS390Bus *s390_bus;
@@ -156,15 +145,10 @@ static void s390_init(QEMUMachineInitArgs *args)
 {
     ram_addr_t my_ram_size = args->ram_size;
     const char *cpu_model = args->cpu_model;
-    const char *kernel_filename = args->kernel_filename;
-    const char *kernel_cmdline = args->kernel_cmdline;
-    const char *initrd_filename = args->initrd_filename;
     CPUS390XState *env = NULL;
+    DeviceState *dev;
     MemoryRegion *sysmem = get_system_memory();
     MemoryRegion *ram = g_new(MemoryRegion, 1);
-    ram_addr_t kernel_size = 0;
-    ram_addr_t initrd_offset;
-    ram_addr_t initrd_size = 0;
     int shift = 0;
     uint8_t *storage_keys;
     void *virtio_region;
@@ -185,6 +169,15 @@ static void s390_init(QEMUMachineInitArgs *args)
     /* get a BUS */
     s390_bus = s390_virtio_bus_init(&my_ram_size);
     s390_sclp_init();
+    dev  = qdev_create(NULL, "s390-ipl");
+    if (args->kernel_filename) {
+        qdev_prop_set_string(dev, "kernel", args->kernel_filename);
+    }
+    if (args->initrd_filename) {
+        qdev_prop_set_string(dev, "initrd", args->initrd_filename);
+    }
+    qdev_prop_set_string(dev, "cmdline", args->kernel_cmdline);
+    qdev_init_nofail(dev);
 
     /* allocate RAM */
     memory_region_init_ram(ram, "s390.ram", my_ram_size);
@@ -225,76 +218,6 @@ static void s390_init(QEMUMachineInitArgs *args)
         tmp_env->storage_keys = storage_keys;
     }
 
-    /* One CPU has to run */
-    s390_add_running_cpu(env);
-
-    if (kernel_filename) {
-
-        kernel_size = load_elf(kernel_filename, NULL, NULL, NULL, NULL,
-                               NULL, 1, ELF_MACHINE, 0);
-        if (kernel_size == -1UL) {
-            kernel_size = load_image_targphys(kernel_filename, 0, ram_size);
-        }
-        if (kernel_size == -1UL) {
-            fprintf(stderr, "qemu: could not load kernel '%s'\n",
-                    kernel_filename);
-            exit(1);
-        }
-        /*
-         * we can not rely on the ELF entry point, since up to 3.2 this
-         * value was 0x800 (the SALIPL loader) and it wont work. For
-         * all (Linux) cases 0x10000 (KERN_IMAGE_START) should be fine.
-         */
-        env->psw.addr = KERN_IMAGE_START;
-        env->psw.mask = 0x0000000180000000ULL;
-    } else {
-        ram_addr_t bios_size = 0;
-        char *bios_filename;
-
-        /* Load zipl bootloader */
-        if (bios_name == NULL) {
-            bios_name = ZIPL_FILENAME;
-        }
-
-        bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
-        bios_size = load_image_targphys(bios_filename, ZIPL_LOAD_ADDR, 4096);
-        g_free(bios_filename);
-
-        if ((long)bios_size < 0) {
-            hw_error("could not load bootloader '%s'\n", bios_name);
-        }
-
-        if (bios_size > 4096) {
-            hw_error("stage1 bootloader is > 4k\n");
-        }
-
-        env->psw.addr = ZIPL_START;
-        env->psw.mask = 0x0000000180000000ULL;
-    }
-
-    if (initrd_filename) {
-        initrd_offset = INITRD_START;
-        while (kernel_size + 0x100000 > initrd_offset) {
-            initrd_offset += 0x100000;
-        }
-        initrd_size = load_image_targphys(initrd_filename, initrd_offset,
-                                          ram_size - initrd_offset);
-        if (initrd_size == -1UL) {
-            fprintf(stderr, "qemu: could not load initrd '%s'\n",
-                    initrd_filename);
-            exit(1);
-        }
-
-        /* we have to overwrite values in the kernel image, which are "rom" */
-        stq_p(rom_ptr(INITRD_PARM_START), initrd_offset);
-        stq_p(rom_ptr(INITRD_PARM_SIZE), initrd_size);
-    }
-
-    if (rom_ptr(KERN_PARM_AREA)) {
-        /* we have to overwrite values in the kernel image, which are "rom" */
-        memcpy(rom_ptr(KERN_PARM_AREA), kernel_cmdline,
-               strlen(kernel_cmdline) + 1);
-    }
 
     /* Create VirtIO network adapters */
     for(i = 0; i < nb_nics; i++) {
@@ -352,3 +275,4 @@ static void s390_machine_init(void)
 }
 
 machine_init(s390_machine_init);
+
diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
index 096dfcd..4a5a5d8 100644
--- a/hw/s390x/Makefile.objs
+++ b/hw/s390x/Makefile.objs
@@ -4,3 +4,4 @@ obj-y := $(addprefix ../,$(obj-y))
 obj-y += sclp.o
 obj-y += event-facility.o
 obj-y += sclpquiesce.o sclpconsole.o
+obj-y += ipl.o
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
new file mode 100644
index 0000000..1f768e3
--- /dev/null
+++ b/hw/s390x/ipl.c
@@ -0,0 +1,144 @@
+/*
+ * bootloader support
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ *  Christian Borntraeger <borntraeger@de.ibm.com>
+ *
+ * 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.
+ *
+ */
+
+#include <sysemu.h>
+#include "cpu.h"
+#include "elf.h"
+#include "hw/loader.h"
+#include "hw/sysbus.h"
+#include "hw/s390x/ipl.h"
+
+void s390_ipl_cpu(uint64_t pswaddr)
+{
+    CPUS390XState *env = qemu_get_cpu(0);
+    env->psw.addr = pswaddr;
+    env->psw.mask = IPL_PSW_MASK;
+    s390_add_running_cpu(env);
+}
+
+typedef struct {
+    SysBusDevice dev;
+    char *kernel;
+    char *initrd;
+    char *cmdline;
+} S390IPLState;
+
+static int s390_ipl_init(SysBusDevice *dev)
+{
+    S390IPLState *ipl = DO_UPCAST(S390IPLState, dev, dev);
+    ram_addr_t kernel_size = 0;
+
+    if (!ipl->kernel) {
+        ram_addr_t bios_size = 0;
+        char *bios_filename;
+
+        /* Load zipl bootloader */
+        if (bios_name == NULL) {
+            bios_name = ZIPL_FILENAME;
+        }
+
+        bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
+        bios_size = load_image_targphys(bios_filename, ZIPL_IMAGE_START, 4096);
+        g_free(bios_filename);
+
+        if ((long)bios_size < 0) {
+            hw_error("could not load bootloader '%s'\n", bios_name);
+        }
+
+        if (bios_size > 4096) {
+            hw_error("stage1 bootloader is > 4k\n");
+        }
+        return 0;
+    } else {
+        kernel_size = load_elf(ipl->kernel, NULL, NULL, NULL, NULL,
+                               NULL, 1, ELF_MACHINE, 0);
+        if (kernel_size == -1UL) {
+            kernel_size = load_image_targphys(ipl->kernel, 0, ram_size);
+        }
+        if (kernel_size == -1UL) {
+            fprintf(stderr, "could not load kernel '%s'\n", ipl->kernel);
+            return -1;
+        }
+        /* we have to overwrite values in the kernel image, which are "rom" */
+        strcpy(rom_ptr(KERN_PARM_AREA), ipl->cmdline);
+    }
+    if (ipl->initrd) {
+        ram_addr_t initrd_offset, initrd_size;
+
+        initrd_offset = INITRD_START;
+        while (kernel_size + 0x100000 > initrd_offset) {
+            initrd_offset += 0x100000;
+        }
+        initrd_size = load_image_targphys(ipl->initrd, initrd_offset,
+                                          ram_size - initrd_offset);
+        if (initrd_size == -1UL) {
+            fprintf(stderr, "qemu: could not load initrd '%s'\n", ipl->initrd);
+            exit(1);
+        }
+
+        /* we have to overwrite values in the kernel image, which are "rom" */
+        stq_p(rom_ptr(INITRD_PARM_START), initrd_offset);
+        stq_p(rom_ptr(INITRD_PARM_SIZE), initrd_size);
+    }
+
+    return 0;
+}
+
+static Property s390_ipl_properties[] = {
+    DEFINE_PROP_STRING("kernel", S390IPLState, kernel),
+    DEFINE_PROP_STRING("initrd", S390IPLState, initrd),
+    DEFINE_PROP_STRING("cmdline", S390IPLState, cmdline),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void s390_ipl_reset(DeviceState *dev)
+{
+    S390IPLState *ipl = DO_UPCAST(S390IPLState, dev.qdev, dev);
+
+    if (ipl->kernel) {
+        /*
+         * we can not rely on the ELF entry point, since up to 3.2 this
+         * value was 0x800 (the SALIPL loader) and it wont work. For
+         * all (Linux) cases 0x10000 (KERN_IMAGE_START) should be fine.
+         */
+        return s390_ipl_cpu(KERN_IMAGE_START);
+    } else {
+        return s390_ipl_cpu(ZIPL_IMAGE_START);
+    }
+}
+
+static void s390_ipl_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+
+    k->init = s390_ipl_init;
+    dc->props = s390_ipl_properties;
+    dc->reset = s390_ipl_reset;
+    dc->no_user = 1;
+}
+
+static TypeInfo s390_ipl_info = {
+    .class_init = s390_ipl_class_init,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .name  = "s390-ipl",
+    .instance_size  = sizeof(S390IPLState),
+};
+
+static void s390_register_ipl(void)
+{
+    type_register_static(&s390_ipl_info);
+}
+
+type_init(s390_register_ipl)
+
diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
new file mode 100644
index 0000000..d6318e0
--- /dev/null
+++ b/hw/s390x/ipl.h
@@ -0,0 +1,31 @@
+/*
+ * ipl support
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ *  Christian Borntraeger <borntraeger@de.ibm.com>
+ *
+ * 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.
+ *
+ */
+
+
+#ifndef S390_IPL_H
+#define S390_IPL_H
+
+#define KERN_IMAGE_START                0x010000UL
+#define KERN_PARM_AREA                  0x010480UL
+#define INITRD_START                    0x800000UL
+#define INITRD_PARM_START               0x010408UL
+#define INITRD_PARM_SIZE                0x010410UL
+#define PARMFILE_START                  0x001000UL
+#define ZIPL_FILENAME                   "s390-zipl.rom"
+#define ZIPL_IMAGE_START                0x009000UL
+#define IPL_PSW_MASK                    0x0000000180000000ULL
+
+/* starts the first cpu with the given address and a default psw mask */
+void s390_ipl_cpu(uint64_t pswaddr);
+
+#endif //S390_IPL_H
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device

[Alexander Graf]

On 12.12.2012, at 14:08, Jens Freimann wrote:

> From: Christian Borntraeger <borntraeger@de.ibm.com>
> 
> Lets move the code to setup IPL for external kernel
> or via the zipl rom into a separate file. This allows to
> 
> - define a reboot handler, setting up the PSW appropriately
> - reuse that code for several machines (e.g. virtio-ccw and virtio-s390)
> - allow different machines to provide different defaults
> - enhance the boot code to IPL disks that contain a bootmap that
>  was created with zipl under LPAR or z/VM (in a future patch)
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
> ---
> hw/s390-virtio.c       | 100 +++++-----------------------------
> hw/s390x/Makefile.objs |   1 +
> hw/s390x/ipl.c         | 144 +++++++++++++++++++++++++++++++++++++++++++++++++
> hw/s390x/ipl.h         |  31 +++++++++++
> 4 files changed, 188 insertions(+), 88 deletions(-)
> create mode 100644 hw/s390x/ipl.c
> create mode 100644 hw/s390x/ipl.h
> 
> diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
> index ca1bb09..18050b1 100644
> --- a/hw/s390-virtio.c
> +++ b/hw/s390-virtio.c
> @@ -25,7 +25,6 @@
> #include "boards.h"
> #include "monitor.h"
> #include "loader.h"
> -#include "elf.h"
> #include "hw/virtio.h"
> #include "hw/sysbus.h"
> #include "kvm.h"
> @@ -33,6 +32,7 @@
> 
> #include "hw/s390-virtio-bus.h"
> #include "hw/s390x/sclp.h"
> +#include "hw/s390x/ipl.h"
> 
> //#define DEBUG_S390
> 
> @@ -48,17 +48,6 @@
> #define KVM_S390_VIRTIO_RESET           1
> #define KVM_S390_VIRTIO_SET_STATUS      2
> 
> -#define KERN_IMAGE_START                0x010000UL
> -#define KERN_PARM_AREA                  0x010480UL
> -#define INITRD_START                    0x800000UL
> -#define INITRD_PARM_START               0x010408UL
> -#define INITRD_PARM_SIZE                0x010410UL
> -#define PARMFILE_START                  0x001000UL
> -
> -#define ZIPL_START			0x009000UL
> -#define ZIPL_LOAD_ADDR			0x009000UL
> -#define ZIPL_FILENAME			"s390-zipl.rom"
> -
> #define MAX_BLK_DEVS                    10
> 
> static VirtIOS390Bus *s390_bus;
> @@ -156,15 +145,10 @@ static void s390_init(QEMUMachineInitArgs *args)
> {
>     ram_addr_t my_ram_size = args->ram_size;
>     const char *cpu_model = args->cpu_model;
> -    const char *kernel_filename = args->kernel_filename;
> -    const char *kernel_cmdline = args->kernel_cmdline;
> -    const char *initrd_filename = args->initrd_filename;
>     CPUS390XState *env = NULL;
> +    DeviceState *dev;
>     MemoryRegion *sysmem = get_system_memory();
>     MemoryRegion *ram = g_new(MemoryRegion, 1);
> -    ram_addr_t kernel_size = 0;
> -    ram_addr_t initrd_offset;
> -    ram_addr_t initrd_size = 0;
>     int shift = 0;
>     uint8_t *storage_keys;
>     void *virtio_region;
> @@ -185,6 +169,15 @@ static void s390_init(QEMUMachineInitArgs *args)
>     /* get a BUS */
>     s390_bus = s390_virtio_bus_init(&my_ram_size);
>     s390_sclp_init();
> +    dev  = qdev_create(NULL, "s390-ipl");
> +    if (args->kernel_filename) {
> +        qdev_prop_set_string(dev, "kernel", args->kernel_filename);
> +    }
> +    if (args->initrd_filename) {
> +        qdev_prop_set_string(dev, "initrd", args->initrd_filename);
> +    }
> +    qdev_prop_set_string(dev, "cmdline", args->kernel_cmdline);
> +    qdev_init_nofail(dev);
> 
>     /* allocate RAM */
>     memory_region_init_ram(ram, "s390.ram", my_ram_size);
> @@ -225,76 +218,6 @@ static void s390_init(QEMUMachineInitArgs *args)
>         tmp_env->storage_keys = storage_keys;
>     }
> 
> -    /* One CPU has to run */
> -    s390_add_running_cpu(env);
> -
> -    if (kernel_filename) {
> -
> -        kernel_size = load_elf(kernel_filename, NULL, NULL, NULL, NULL,
> -                               NULL, 1, ELF_MACHINE, 0);
> -        if (kernel_size == -1UL) {
> -            kernel_size = load_image_targphys(kernel_filename, 0, ram_size);
> -        }
> -        if (kernel_size == -1UL) {
> -            fprintf(stderr, "qemu: could not load kernel '%s'\n",
> -                    kernel_filename);
> -            exit(1);
> -        }
> -        /*
> -         * we can not rely on the ELF entry point, since up to 3.2 this
> -         * value was 0x800 (the SALIPL loader) and it wont work. For
> -         * all (Linux) cases 0x10000 (KERN_IMAGE_START) should be fine.
> -         */
> -        env->psw.addr = KERN_IMAGE_START;
> -        env->psw.mask = 0x0000000180000000ULL;
> -    } else {
> -        ram_addr_t bios_size = 0;
> -        char *bios_filename;
> -
> -        /* Load zipl bootloader */
> -        if (bios_name == NULL) {
> -            bios_name = ZIPL_FILENAME;
> -        }
> -
> -        bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> -        bios_size = load_image_targphys(bios_filename, ZIPL_LOAD_ADDR, 4096);
> -        g_free(bios_filename);
> -
> -        if ((long)bios_size < 0) {
> -            hw_error("could not load bootloader '%s'\n", bios_name);
> -        }
> -
> -        if (bios_size > 4096) {
> -            hw_error("stage1 bootloader is > 4k\n");
> -        }
> -
> -        env->psw.addr = ZIPL_START;
> -        env->psw.mask = 0x0000000180000000ULL;
> -    }
> -
> -    if (initrd_filename) {
> -        initrd_offset = INITRD_START;
> -        while (kernel_size + 0x100000 > initrd_offset) {
> -            initrd_offset += 0x100000;
> -        }
> -        initrd_size = load_image_targphys(initrd_filename, initrd_offset,
> -                                          ram_size - initrd_offset);
> -        if (initrd_size == -1UL) {
> -            fprintf(stderr, "qemu: could not load initrd '%s'\n",
> -                    initrd_filename);
> -            exit(1);
> -        }
> -
> -        /* we have to overwrite values in the kernel image, which are "rom" */
> -        stq_p(rom_ptr(INITRD_PARM_START), initrd_offset);
> -        stq_p(rom_ptr(INITRD_PARM_SIZE), initrd_size);
> -    }
> -
> -    if (rom_ptr(KERN_PARM_AREA)) {
> -        /* we have to overwrite values in the kernel image, which are "rom" */
> -        memcpy(rom_ptr(KERN_PARM_AREA), kernel_cmdline,
> -               strlen(kernel_cmdline) + 1);
> -    }
> 
>     /* Create VirtIO network adapters */
>     for(i = 0; i < nb_nics; i++) {
> @@ -352,3 +275,4 @@ static void s390_machine_init(void)
> }
> 
> machine_init(s390_machine_init);
> +
> diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
> index 096dfcd..4a5a5d8 100644
> --- a/hw/s390x/Makefile.objs
> +++ b/hw/s390x/Makefile.objs
> @@ -4,3 +4,4 @@ obj-y := $(addprefix ../,$(obj-y))
> obj-y += sclp.o
> obj-y += event-facility.o
> obj-y += sclpquiesce.o sclpconsole.o
> +obj-y += ipl.o
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> new file mode 100644
> index 0000000..1f768e3
> --- /dev/null
> +++ b/hw/s390x/ipl.c
> @@ -0,0 +1,144 @@
> +/*
> + * bootloader support
> + *
> + * Copyright IBM, Corp. 2012
> + *
> + * Authors:
> + *  Christian Borntraeger <borntraeger@de.ibm.com>
> + *
> + * 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.
> + *
> + */
> +
> +#include <sysemu.h>
> +#include "cpu.h"
> +#include "elf.h"
> +#include "hw/loader.h"
> +#include "hw/sysbus.h"
> +#include "hw/s390x/ipl.h"
> +
> +void s390_ipl_cpu(uint64_t pswaddr)

Any reason this isn't inlined inside the reset handler? And why is this public?

> +{
> +    CPUS390XState *env = qemu_get_cpu(0);
> +    env->psw.addr = pswaddr;
> +    env->psw.mask = IPL_PSW_MASK;
> +    s390_add_running_cpu(env);
> +}
> +
> +typedef struct {
> +    SysBusDevice dev;
> +    char *kernel;
> +    char *initrd;
> +    char *cmdline;
> +} S390IPLState;
> +
> +static int s390_ipl_init(SysBusDevice *dev)
> +{
> +    S390IPLState *ipl = DO_UPCAST(S390IPLState, dev, dev);
> +    ram_addr_t kernel_size = 0;
> +
> +    if (!ipl->kernel) {
> +        ram_addr_t bios_size = 0;
> +        char *bios_filename;
> +
> +        /* Load zipl bootloader */
> +        if (bios_name == NULL) {
> +            bios_name = ZIPL_FILENAME;
> +        }
> +
> +        bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> +        bios_size = load_image_targphys(bios_filename, ZIPL_IMAGE_START, 4096);
> +        g_free(bios_filename);
> +
> +        if ((long)bios_size < 0) {
> +            hw_error("could not load bootloader '%s'\n", bios_name);
> +        }
> +
> +        if (bios_size > 4096) {
> +            hw_error("stage1 bootloader is > 4k\n");
> +        }
> +        return 0;
> +    } else {
> +        kernel_size = load_elf(ipl->kernel, NULL, NULL, NULL, NULL,
> +                               NULL, 1, ELF_MACHINE, 0);
> +        if (kernel_size == -1UL) {
> +            kernel_size = load_image_targphys(ipl->kernel, 0, ram_size);
> +        }
> +        if (kernel_size == -1UL) {
> +            fprintf(stderr, "could not load kernel '%s'\n", ipl->kernel);
> +            return -1;
> +        }
> +        /* we have to overwrite values in the kernel image, which are "rom" */
> +        strcpy(rom_ptr(KERN_PARM_AREA), ipl->cmdline);
> +    }
> +    if (ipl->initrd) {
> +        ram_addr_t initrd_offset, initrd_size;
> +
> +        initrd_offset = INITRD_START;
> +        while (kernel_size + 0x100000 > initrd_offset) {
> +            initrd_offset += 0x100000;
> +        }
> +        initrd_size = load_image_targphys(ipl->initrd, initrd_offset,
> +                                          ram_size - initrd_offset);
> +        if (initrd_size == -1UL) {
> +            fprintf(stderr, "qemu: could not load initrd '%s'\n", ipl->initrd);
> +            exit(1);
> +        }
> +
> +        /* we have to overwrite values in the kernel image, which are "rom" */
> +        stq_p(rom_ptr(INITRD_PARM_START), initrd_offset);
> +        stq_p(rom_ptr(INITRD_PARM_SIZE), initrd_size);
> +    }
> +
> +    return 0;
> +}
> +
> +static Property s390_ipl_properties[] = {
> +    DEFINE_PROP_STRING("kernel", S390IPLState, kernel),
> +    DEFINE_PROP_STRING("initrd", S390IPLState, initrd),
> +    DEFINE_PROP_STRING("cmdline", S390IPLState, cmdline),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void s390_ipl_reset(DeviceState *dev)
> +{
> +    S390IPLState *ipl = DO_UPCAST(S390IPLState, dev.qdev, dev);
> +
> +    if (ipl->kernel) {
> +        /*
> +         * we can not rely on the ELF entry point, since up to 3.2 this
> +         * value was 0x800 (the SALIPL loader) and it wont work. For
> +         * all (Linux) cases 0x10000 (KERN_IMAGE_START) should be fine.
> +         */
> +        return s390_ipl_cpu(KERN_IMAGE_START);
> +    } else {
> +        return s390_ipl_cpu(ZIPL_IMAGE_START);
> +    }
> +}
> +
> +static void s390_ipl_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
> +
> +    k->init = s390_ipl_init;
> +    dc->props = s390_ipl_properties;
> +    dc->reset = s390_ipl_reset;
> +    dc->no_user = 1;
> +}
> +
> +static TypeInfo s390_ipl_info = {
> +    .class_init = s390_ipl_class_init,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .name  = "s390-ipl",
> +    .instance_size  = sizeof(S390IPLState),
> +};
> +
> +static void s390_register_ipl(void)
> +{
> +    type_register_static(&s390_ipl_info);
> +}
> +
> +type_init(s390_register_ipl)
> +
> diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
> new file mode 100644
> index 0000000..d6318e0
> --- /dev/null
> +++ b/hw/s390x/ipl.h
> @@ -0,0 +1,31 @@
> +/*
> + * ipl support
> + *
> + * Copyright IBM, Corp. 2012
> + *
> + * Authors:
> + *  Christian Borntraeger <borntraeger@de.ibm.com>
> + *
> + * 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.
> + *
> + */
> +
> +
> +#ifndef S390_IPL_H
> +#define S390_IPL_H
> +
> +#define KERN_IMAGE_START                0x010000UL
> +#define KERN_PARM_AREA                  0x010480UL
> +#define INITRD_START                    0x800000UL
> +#define INITRD_PARM_START               0x010408UL
> +#define INITRD_PARM_SIZE                0x010410UL
> +#define PARMFILE_START                  0x001000UL
> +#define ZIPL_FILENAME                   "s390-zipl.rom"
> +#define ZIPL_IMAGE_START                0x009000UL
> +#define IPL_PSW_MASK                    0x0000000180000000ULL

I don't think we need the above values outside of ipl.c, no? :)


Alex

> +
> +/* starts the first cpu with the given address and a default psw mask */
> +void s390_ipl_cpu(uint64_t pswaddr);
> +
> +#endif //S390_IPL_H
> -- 
> 1.7.12.4
> 

Re: [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device

[Christian Borntraeger]

On 12/12/12 14:31, Alexander Graf wrote:

>> +void s390_ipl_cpu(uint64_t pswaddr)
> 
> Any reason this isn't inlined inside the reset handler? And why is this public?

Well, the former patch version had the disk bootmap parsing in a separate
file, but we can certainly unexport that and make it inline in this patch.
[...]

>> +#define KERN_IMAGE_START                0x010000UL
>> +#define KERN_PARM_AREA                  0x010480UL
>> +#define INITRD_START                    0x800000UL
>> +#define INITRD_PARM_START               0x010408UL
>> +#define INITRD_PARM_SIZE                0x010410UL
>> +#define PARMFILE_START                  0x001000UL
>> +#define ZIPL_FILENAME                   "s390-zipl.rom"
>> +#define ZIPL_IMAGE_START                0x009000UL
>> +#define IPL_PSW_MASK                    0x0000000180000000ULL
> 
> I don't think we need the above values outside of ipl.c, no? :)

See above. 
Will move that into ipl.c