qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device
  2012-12-12 13:08 [Qemu-devel] [PATCH 0/3] s390: ipl device, cpu reset handler and cpu model support Jens Freimann
@ 2012-12-12 13:08 ` Jens Freimann
  2012-12-12 13:31   ` Alexander Graf
  0 siblings, 1 reply; 17+ messages in thread
From: Jens Freimann @ 2012-12-12 13:08 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Heinz Graalfs, qemu-devel, Andreas Faerber, Christian Borntraeger,
	Jens Freimann, Cornelia Huck, Einar Lueck

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device
  2012-12-12 13:08 ` [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device Jens Freimann
@ 2012-12-12 13:31   ` Alexander Graf
  2012-12-12 19:56     ` Christian Borntraeger
  0 siblings, 1 reply; 17+ messages in thread
From: Alexander Graf @ 2012-12-12 13:31 UTC (permalink / raw)
  To: Jens Freimann
  Cc: Heinz Graalfs, qemu-devel, Andreas Faerber, Christian Borntraeger,
	Cornelia Huck, Einar Lueck


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
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device
  2012-12-12 13:31   ` Alexander Graf
@ 2012-12-12 19:56     ` Christian Borntraeger
  0 siblings, 0 replies; 17+ messages in thread
From: Christian Borntraeger @ 2012-12-12 19:56 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Heinz Graalfs, qemu-devel, Andreas Faerber, Jens Freimann,
	Cornelia Huck, Einar Lueck

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 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device
  2012-12-14 16:46 [Qemu-devel] [PATCH 0/3] s390: ipl device, cpu reset handler and cpu model support Jens Freimann
@ 2012-12-14 16:46 ` Jens Freimann
  2012-12-16 16:26   ` Andreas Färber
  0 siblings, 1 reply; 17+ messages in thread
From: Jens Freimann @ 2012-12-14 16:46 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Heinz Graalfs, qemu-devel, Andreas Faerber, Christian Borntraeger,
	Jens Freimann, Cornelia Huck, Einar Lueck

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device
  2012-12-14 16:46 ` [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device Jens Freimann
@ 2012-12-16 16:26   ` Andreas Färber
  2012-12-16 21:15     ` Christian Borntraeger
  0 siblings, 1 reply; 17+ messages in thread
From: Andreas Färber @ 2012-12-16 16:26 UTC (permalink / raw)
  To: Jens Freimann, Christian Borntraeger
  Cc: Cornelia Huck, Heinz Graalfs, Einar Lueck, Alexander Graf,
	qemu-devel

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device
  2012-12-16 16:26   ` Andreas Färber
@ 2012-12-16 21:15     ` Christian Borntraeger
  0 siblings, 0 replies; 17+ messages in thread
From: Christian Borntraeger @ 2012-12-16 21:15 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Heinz Graalfs, Alexander Graf, qemu-devel, Jens Freimann,
	Cornelia Huck, Einar Lueck

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH 0/3] s390: ipl device, cpu reset handler and cpu model support
@ 2012-12-18 17:50 Jens Freimann
  2012-12-18 17:50 ` [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device Jens Freimann
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Jens Freimann @ 2012-12-18 17:50 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Heinz Graalfs, qemu-devel, Andreas Faerber, Christian Borntraeger,
	Jens Freimann, Cornelia Huck, Einar Lueck

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device
  2012-12-18 17:50 [Qemu-devel] [PATCH 0/3] s390: ipl device, cpu reset handler and cpu model support Jens Freimann
@ 2012-12-18 17:50 ` Jens Freimann
  2013-01-03 12:47   ` Alexander Graf
  2012-12-18 17:50 ` [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler Jens Freimann
  2012-12-18 17:50 ` [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions Jens Freimann
  2 siblings, 1 reply; 17+ messages in thread
From: Jens Freimann @ 2012-12-18 17:50 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Heinz Graalfs, qemu-devel, Andreas Faerber, Christian Borntraeger,
	Jens Freimann, Cornelia Huck, Einar Lueck

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler
  2012-12-18 17:50 [Qemu-devel] [PATCH 0/3] s390: ipl device, cpu reset handler and cpu model support Jens Freimann
  2012-12-18 17:50 ` [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device Jens Freimann
@ 2012-12-18 17:50 ` Jens Freimann
  2013-01-03 12:50   ` Alexander Graf
  2013-01-03 12:55   ` Alexander Graf
  2012-12-18 17:50 ` [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions Jens Freimann
  2 siblings, 2 replies; 17+ messages in thread
From: Jens Freimann @ 2012-12-18 17:50 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Heinz Graalfs, qemu-devel, Andreas Faerber, Christian Borntraeger,
	Jens Freimann, Cornelia Huck, Einar Lueck

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions
  2012-12-18 17:50 [Qemu-devel] [PATCH 0/3] s390: ipl device, cpu reset handler and cpu model support Jens Freimann
  2012-12-18 17:50 ` [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device Jens Freimann
  2012-12-18 17:50 ` [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler Jens Freimann
@ 2012-12-18 17:50 ` Jens Freimann
  2013-01-03 13:01   ` Alexander Graf
  2 siblings, 1 reply; 17+ messages in thread
From: Jens Freimann @ 2012-12-18 17:50 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Heinz Graalfs, qemu-devel, Andreas Faerber, Christian Borntraeger,
	Jens Freimann, Cornelia Huck, Viktor Mihajlovski, Einar Lueck

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device
  2012-12-18 17:50 ` [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device Jens Freimann
@ 2013-01-03 12:47   ` Alexander Graf
  0 siblings, 0 replies; 17+ messages in thread
From: Alexander Graf @ 2013-01-03 12:47 UTC (permalink / raw)
  To: Jens Freimann
  Cc: Heinz Graalfs, qemu-devel, Andreas Faerber, Christian Borntraeger,
	Cornelia Huck, Einar Lueck


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler
  2012-12-18 17:50 ` [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler Jens Freimann
@ 2013-01-03 12:50   ` Alexander Graf
  2013-01-04 13:55     ` Jens Freimann
  2013-01-03 12:55   ` Alexander Graf
  1 sibling, 1 reply; 17+ messages in thread
From: Alexander Graf @ 2013-01-03 12:50 UTC (permalink / raw)
  To: Jens Freimann
  Cc: Heinz Graalfs, qemu-devel, Andreas Faerber, Christian Borntraeger,
	Cornelia Huck, Einar Lueck


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler
  2012-12-18 17:50 ` [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler Jens Freimann
  2013-01-03 12:50   ` Alexander Graf
@ 2013-01-03 12:55   ` Alexander Graf
  2013-01-04 14:09     ` Jens Freimann
  1 sibling, 1 reply; 17+ messages in thread
From: Alexander Graf @ 2013-01-03 12:55 UTC (permalink / raw)
  To: Jens Freimann
  Cc: Heinz Graalfs, qemu-devel, Andreas Faerber, Christian Borntraeger,
	Cornelia Huck, Einar Lueck


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
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions
  2012-12-18 17:50 ` [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions Jens Freimann
@ 2013-01-03 13:01   ` Alexander Graf
  0 siblings, 0 replies; 17+ messages in thread
From: Alexander Graf @ 2013-01-03 13:01 UTC (permalink / raw)
  To: Jens Freimann
  Cc: Heinz Graalfs, qemu-devel, Andreas Faerber, Christian Borntraeger,
	Cornelia Huck, Viktor Mihajlovski, Einar Lueck


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler
  2013-01-03 12:50   ` Alexander Graf
@ 2013-01-04 13:55     ` Jens Freimann
  0 siblings, 0 replies; 17+ messages in thread
From: Jens Freimann @ 2013-01-04 13:55 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Heinz Graalfs, qemu-devel, Andreas Faerber, Christian Borntraeger,
	Cornelia Huck, Einar Lueck

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
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler
  2013-01-03 12:55   ` Alexander Graf
@ 2013-01-04 14:09     ` Jens Freimann
  2013-01-04 14:11       ` Alexander Graf
  0 siblings, 1 reply; 17+ messages in thread
From: Jens Freimann @ 2013-01-04 14:09 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Heinz Graalfs, qemu-devel, Andreas Faerber, Christian Borntraeger,
	Cornelia Huck, Einar Lueck

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
> > 
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler
  2013-01-04 14:09     ` Jens Freimann
@ 2013-01-04 14:11       ` Alexander Graf
  0 siblings, 0 replies; 17+ messages in thread
From: Alexander Graf @ 2013-01-04 14:11 UTC (permalink / raw)
  To: Jens Freimann
  Cc: Heinz Graalfs, qemu-devel, Andreas Faerber, Christian Borntraeger,
	Cornelia Huck, Einar Lueck


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2013-01-04 14:11 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-18 17:50 [Qemu-devel] [PATCH 0/3] s390: ipl device, cpu reset handler and cpu model support Jens Freimann
2012-12-18 17:50 ` [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device Jens Freimann
2013-01-03 12:47   ` Alexander Graf
2012-12-18 17:50 ` [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler Jens Freimann
2013-01-03 12:50   ` Alexander Graf
2013-01-04 13:55     ` Jens Freimann
2013-01-03 12:55   ` Alexander Graf
2013-01-04 14:09     ` Jens Freimann
2013-01-04 14:11       ` Alexander Graf
2012-12-18 17:50 ` [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions Jens Freimann
2013-01-03 13:01   ` Alexander Graf
  -- strict thread matches above, loose matches on Subject: below --
2012-12-14 16:46 [Qemu-devel] [PATCH 0/3] s390: ipl device, cpu reset handler and cpu model support Jens Freimann
2012-12-14 16:46 ` [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device Jens Freimann
2012-12-16 16:26   ` Andreas Färber
2012-12-16 21:15     ` Christian Borntraeger
2012-12-12 13:08 [Qemu-devel] [PATCH 0/3] s390: ipl device, cpu reset handler and cpu model support Jens Freimann
2012-12-12 13:08 ` [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device Jens Freimann
2012-12-12 13:31   ` Alexander Graf
2012-12-12 19:56     ` Christian Borntraeger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).