Message ID | 1355503606-54131-2-git-send-email-jfrei@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
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
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
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) +