From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38853) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YYWnn-00069q-E8 for qemu-devel@nongnu.org; Thu, 19 Mar 2015 05:31:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YYWnj-0006iu-B7 for qemu-devel@nongnu.org; Thu, 19 Mar 2015 05:31:43 -0400 Received: from mail-wg0-x233.google.com ([2a00:1450:400c:c00::233]:36171) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YYWnj-0006im-1U for qemu-devel@nongnu.org; Thu, 19 Mar 2015 05:31:39 -0400 Received: by wgra20 with SMTP id a20so57046559wgr.3 for ; Thu, 19 Mar 2015 02:31:38 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <550A9776.5050509@redhat.com> Date: Thu, 19 Mar 2015 10:31:34 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1424290943-22480-1-git-send-email-borntraeger@de.ibm.com> <1424290943-22480-3-git-send-email-borntraeger@de.ibm.com> In-Reply-To: <1424290943-22480-3-git-send-email-borntraeger@de.ibm.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PULL 02/29] s390x/ipl: support diagnose 308 subcodes 5 and 6 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Christian Borntraeger , Peter Maydell Cc: qemu-devel , Fan Zhang , Alexander Graf , Jens Freimann , Cornelia Huck , Richard Henderson On 18/02/2015 21:21, Christian Borntraeger wrote: > From: Fan Zhang > > To support dynamically updating the IPL device from inside the KVM > guest on the s390 platform, DIAG 308 instruction is intercepted > in QEMU to handle the request. > > Subcode 5 allows to specify a new boot device, which is saved for > later in the s390_ipl device. This also allows to switch from an > external kernel to a boot device. > > Subcode 6 retrieves boot device configuration that has been previously > set. > > Reviewed-by: Cornelia Huck > Reviewed-by: David Hildenbrand > Acked-by: Christian Borntraeger > Signed-off-by: Jens Freimann > Signed-off-by: Fan Zhang > Signed-off-by: Christian Borntraeger > --- > hw/s390x/ipl.c | 87 ++++++++++++++++++++++++++++++++++++++-------- > hw/s390x/ipl.h | 24 +++++++++++++ > hw/s390x/s390-virtio.c | 2 ++ > target-s390x/misc_helper.c | 33 ++++++++++++++++-- > 4 files changed, 129 insertions(+), 17 deletions(-) > create mode 100644 hw/s390x/ipl.h > > diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c > index 4014a6a..231713d 100644 > --- a/hw/s390x/ipl.c > +++ b/hw/s390x/ipl.c > @@ -18,6 +18,7 @@ > #include "hw/sysbus.h" > #include "hw/s390x/virtio-ccw.h" > #include "hw/s390x/css.h" > +#include "ipl.h" > > #define KERN_IMAGE_START 0x010000UL > #define KERN_PARM_AREA 0x010480UL > @@ -52,12 +53,17 @@ typedef struct S390IPLState { > uint64_t start_addr; > uint64_t bios_start_addr; > bool enforce_bios; > + IplParameterBlock iplb; > + bool iplb_valid; > > /*< public >*/ > char *kernel; > char *initrd; > char *cmdline; > char *firmware; > + uint8_t cssid; > + uint8_t ssid; > + uint16_t devno; > } S390IPLState; > > > @@ -164,6 +170,69 @@ static Property s390_ipl_properties[] = { > DEFINE_PROP_END_OF_LIST(), > }; > > +/* > + * In addition to updating the iplstate, this function returns: > + * - 0 if system was ipled with external kernel > + * - -1 if no valid boot device was found > + * - ccw id of the boot device otherwise > + */ > +static uint64_t s390_update_iplstate(CPUS390XState *env, S390IPLState *ipl) This should probably return uint32_t, because otherwise... > + return ipl->cssid << 24 | ipl->ssid << 16 | ipl->devno; ... a cssid above 127 results in the bits of the high word all set to 1. The reason is that, even though ipl->cssid is a uint8_t, before the shift it is extended to int. Then the return statement does a sign extension from int to uint64_t. Paolo > +} > + > +int s390_ipl_update_diag308(IplParameterBlock *iplb) > +{ > + S390IPLState *ipl; > + > + ipl = S390_IPL(object_resolve_path(TYPE_S390_IPL, NULL)); > + if (ipl) { > + ipl->iplb = *iplb; > + ipl->iplb_valid = true; > + return 0; > + } > + return -1; > +} > + > +IplParameterBlock *s390_ipl_get_iplb(void) > +{ > + S390IPLState *ipl; > + > + ipl = S390_IPL(object_resolve_path(TYPE_S390_IPL, NULL)); > + if (!ipl || !ipl->iplb_valid) { > + return NULL; > + } > + return &ipl->iplb; > +} > + > static void s390_ipl_reset(DeviceState *dev) > { > S390IPLState *ipl = S390_IPL(dev); > @@ -173,21 +242,9 @@ static void s390_ipl_reset(DeviceState *dev) > env->psw.addr = ipl->start_addr; > env->psw.mask = IPL_PSW_MASK; > > - if (!ipl->kernel) { > - /* Tell firmware, if there is a preferred boot device */ > - env->regs[7] = -1; > - DeviceState *dev_st = get_boot_device(0); > - if (dev_st) { > - VirtioCcwDevice *ccw_dev = (VirtioCcwDevice *) object_dynamic_cast( > - OBJECT(qdev_get_parent_bus(dev_st)->parent), > - TYPE_VIRTIO_CCW_DEVICE); > - > - if (ccw_dev) { > - env->regs[7] = ccw_dev->sch->cssid << 24 | > - ccw_dev->sch->ssid << 16 | > - ccw_dev->sch->devno; > - } > - } > + if (!ipl->kernel || ipl->iplb_valid) { > + env->psw.addr = ipl->bios_start_addr; > + env->regs[7] = s390_update_iplstate(env, ipl); > } > > s390_cpu_set_state(CPU_STATE_OPERATING, cpu); > diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h > new file mode 100644 > index 0000000..f1d082f > --- /dev/null > +++ b/hw/s390x/ipl.h > @@ -0,0 +1,24 @@ > +/* > + * s390 IPL device > + * > + * Copyright 2015 IBM Corp. > + * Author(s): Zhang Fan > + * > + * 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 HW_S390_IPL_H > +#define HW_S390_IPL_H > + > +typedef struct IplParameterBlock { > + uint8_t reserved1[110]; > + uint16_t devno; > + uint8_t reserved2[88]; > +} IplParameterBlock; > + > +int s390_ipl_update_diag308(IplParameterBlock *iplb); > +IplParameterBlock *s390_ipl_get_iplb(void); > + > +#endif > diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c > index 13f9e49..412e49b 100644 > --- a/hw/s390x/s390-virtio.c > +++ b/hw/s390x/s390-virtio.c > @@ -143,6 +143,8 @@ void s390_init_ipl_dev(const char *kernel_filename, > qdev_prop_set_string(dev, "cmdline", kernel_cmdline); > qdev_prop_set_string(dev, "firmware", firmware); > qdev_prop_set_bit(dev, "enforce_bios", enforce_bios); > + object_property_add_child(qdev_get_machine(), "s390-ipl", > + OBJECT(dev), NULL); > qdev_init_nofail(dev); > } > > diff --git a/target-s390x/misc_helper.c b/target-s390x/misc_helper.c > index ef9758a..1c3df8e 100644 > --- a/target-s390x/misc_helper.c > +++ b/target-s390x/misc_helper.c > @@ -25,6 +25,7 @@ > #include > #include "sysemu/kvm.h" > #include "qemu/timer.h" > +#include "exec/address-spaces.h" > #ifdef CONFIG_KVM > #include > #endif > @@ -34,6 +35,7 @@ > #include "sysemu/cpus.h" > #include "sysemu/sysemu.h" > #include "hw/s390x/ebcdic.h" > +#include "hw/s390x/ipl.h" > #endif > > /* #define DEBUG_HELPER */ > @@ -151,12 +153,15 @@ static int load_normal_reset(S390CPU *cpu) > return 0; > } > > +#define DIAG_308_RC_OK 0x0001 > #define DIAG_308_RC_NO_CONF 0x0102 > #define DIAG_308_RC_INVALID 0x0402 > + > void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3) > { > uint64_t addr = env->regs[r1]; > uint64_t subcode = env->regs[r3]; > + IplParameterBlock *iplb; > > if (env->psw.mask & PSW_MASK_PSTATE) { > program_interrupt(env, PGM_PRIVILEGED, ILEN_LATER_INC); > @@ -180,14 +185,38 @@ void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3) > program_interrupt(env, PGM_SPECIFICATION, ILEN_LATER_INC); > return; > } > - env->regs[r1+1] = DIAG_308_RC_INVALID; > + if (!address_space_access_valid(&address_space_memory, addr, > + sizeof(IplParameterBlock), false)) { > + program_interrupt(env, PGM_ADDRESSING, ILEN_LATER_INC); > + return; > + } > + iplb = g_malloc0(sizeof(struct IplParameterBlock)); > + cpu_physical_memory_read(addr, iplb, sizeof(struct IplParameterBlock)); > + if (!s390_ipl_update_diag308(iplb)) { > + env->regs[r1 + 1] = DIAG_308_RC_OK; > + } else { > + env->regs[r1 + 1] = DIAG_308_RC_INVALID; > + } > + g_free(iplb); > return; > case 6: > if ((r1 & 1) || (addr & 0x0fffULL)) { > program_interrupt(env, PGM_SPECIFICATION, ILEN_LATER_INC); > return; > } > - env->regs[r1+1] = DIAG_308_RC_NO_CONF; > + if (!address_space_access_valid(&address_space_memory, addr, > + sizeof(IplParameterBlock), true)) { > + program_interrupt(env, PGM_ADDRESSING, ILEN_LATER_INC); > + return; > + } > + iplb = s390_ipl_get_iplb(); > + if (iplb) { > + cpu_physical_memory_write(addr, iplb, > + sizeof(struct IplParameterBlock)); > + env->regs[r1 + 1] = DIAG_308_RC_OK; > + } else { > + env->regs[r1 + 1] = DIAG_308_RC_NO_CONF; > + } > return; > default: > hw_error("Unhandled diag308 subcode %" PRIx64, subcode); >