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

[Alexander Graf]

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

> From: Christian Borntraeger <address@hidden>
> 
> 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 <address@hidden>
> Signed-off-by: Jens Freimann <address@hidden>
> ---
> 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 <address@hidden>
> + *
> + * 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 <address@hidden>
> + *
> + * 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
>