| Message ID | 1342176724-43776-3-git-send-email-borntraeger@de.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Jul 13, 2012 at 10:52 AM, Christian Borntraeger <borntraeger@de.ibm.com> wrote: > From: Heinz Graalfs <graalfs@linux.vnet.ibm.com> > > This adds a more generic infrastructure for handling Service-Call > requests on s390. Currently we only support a small subset of Read > SCP Info directly in target-s390x. This patch provides the base > infrastructure for supporting more commands and moves Read SCP > Info. > In the future we could add additional commands for hotplug, call > home and event handling. > > Signed-off-by: Heinz Graalfs <graalfs@linux.vnet.ibm.com> > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > --- > hw/s390-sclp.c | 148 ++++++++++++++++++++++++++++++++++++++++++++++ > hw/s390-sclp.h | 80 +++++++++++++++++++++++++ > hw/s390-virtio.c | 3 + > hw/s390x/Makefile.objs | 1 + > target-s390x/cpu.c | 17 ++++++ > target-s390x/cpu.h | 18 ++---- > target-s390x/kvm.c | 5 +- > target-s390x/op_helper.c | 45 ++------------ > 8 files changed, 261 insertions(+), 56 deletions(-) > create mode 100644 hw/s390-sclp.c > create mode 100644 hw/s390-sclp.h > > diff --git a/hw/s390-sclp.c b/hw/s390-sclp.c > new file mode 100644 > index 0000000..74a3e66 > --- /dev/null > +++ b/hw/s390-sclp.c > @@ -0,0 +1,148 @@ > +/* > + * SCLP Support > + * > + * Copyright IBM, Corp. 2007, 2012 2007, really? > + * > + * Authors: > + * Christian Borntraeger <borntraeger@de.ibm.com> > + * Heinz Graalfs <graalfs@linux.vnet.ibm.com> > + * > + * This work is licensed under the terms of the GNU GPL, version 2. See Why GPLv2only, can't this be licensed under later versions? > + * the COPYING file in the top-level directory. > + * > + */ > + > +#include "cpu.h" > +#include "kvm.h" > +#include "sysbus.h" > + > +#include "s390-sclp.h" > + > +/* Provide information about the configuration, CPUs and storage */ > +static int read_SCP_info(SCCB *sccb) > +{ > + ReadInfo *read_info = (ReadInfo *) sccb; > + int shift = 0; > + > + while ((ram_size >> (20 + shift)) > 65535) { > + shift++; > + } > + read_info->rnmax = cpu_to_be16(ram_size >> (20 + shift)); > + read_info->rnsize = 1 << shift; > + sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_READ_COMPLETION); > + > + return 0; > +} > + > +static int sclp_execute(SCCB *sccb, uint64_t code) > +{ > + int r = 0; > + > + switch (code) { > + case SCLP_CMDW_READ_SCP_INFO: > + case SCLP_CMDW_READ_SCP_INFO_FORCED: > + r = read_SCP_info(sccb); > + break; > + default: > +#ifdef DEBUG_HELPER > + printf("KVM: invalid sclp call 0x%x / 0x%" PRIx64 "x\n", sccb, code); > +#endif > + sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND); > + break; > + } > + return r; > +} > + > +int do_sclp_service_call(uint32_t sccb, uint64_t code) sccb could be target_phys_addr_t. > +{ > + int r = 0; > + SCCB work_sccb; > + > + target_phys_addr_t sccb_len = sizeof(SCCB); > + > + /* > + * we want to work on a private copy of the sccb, to prevent guests > + * from playing dirty tricks by modifying the memory content after > + * the host has checked the values > + */ > + cpu_physical_memory_read(sccb, &work_sccb, sccb_len); > + > + /* Valid sccb sizes */ > + if (be16_to_cpu(work_sccb.h.length) < 8 || > + be16_to_cpu(work_sccb.h.length) > 4096) { > + r = -PGM_SPECIFICATION; > + goto out; > + } > + > + r = sclp_execute((SCCB *)&work_sccb, code); > + > + cpu_physical_memory_write(sccb, &work_sccb, > + be16_to_cpu(work_sccb.h.length)); Perhaps the DMA helpers should be used instead. > + if (!r) { > + sclp_service_interrupt(sccb); > + } > + > +out: > + return r; > +} > + > +void sclp_service_interrupt(uint32_t sccb) > +{ > + if (!sccb) { > + return; > + } > + s390_sclp_extint(sccb & ~3); > +} > + > +/* qemu object creation and initialization functions */ > + > +#define S390_SCLP_BUS(obj) OBJECT_CHECK(SCLPS390Bus, (obj), TYPE_S390_SCLP_BUS) > +static const TypeInfo s390_sclp_bus_info = { > + .name = TYPE_S390_SCLP_BUS, > + .parent = TYPE_BUS, > + .instance_size = sizeof(SCLPS390Bus), > + }; > + > +SCLPS390Bus *s390_sclp_bus_init(void) > +{ > + SCLPS390Bus *bus; > + BusState *bus_state; > + DeviceState *dev; > + > + dev = qdev_create(NULL, "s390-sclp-bridge"); > + qdev_init_nofail(dev); > + > + bus_state = qbus_create(TYPE_S390_SCLP_BUS, dev, "s390-sclp-bus"); > + bus_state->allow_hotplug = 0; > + > + bus = DO_UPCAST(SCLPS390Bus, bus, bus_state); > + return bus; > +} > + > +static int s390_sclp_bridge_init(SysBusDevice *dev) > +{ > + return 0; > +} > + > +static void s390_sclp_bridge_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); > + > + k->init = s390_sclp_bridge_init; > + dc->no_user = 1; > +} > + > +static TypeInfo s390_sclp_bridge_info = { > + .name = "s390-sclp-bridge", > + .parent = TYPE_SYS_BUS_DEVICE, > + .instance_size = sizeof(SysBusDevice), > + .class_init = s390_sclp_bridge_class_init, > +}; > + > +static void s390_sclp_register_types(void) > +{ > + type_register_static(&s390_sclp_bridge_info); > + type_register_static(&s390_sclp_bus_info); > +} > +type_init(s390_sclp_register_types) > diff --git a/hw/s390-sclp.h b/hw/s390-sclp.h > new file mode 100644 > index 0000000..f7bf140 > --- /dev/null > +++ b/hw/s390-sclp.h > @@ -0,0 +1,80 @@ > +/* > + * SCLP Support > + * > + * Copyright IBM, Corp. 2007, 2012 > + * > + * Authors: > + * Christian Borntraeger <borntraeger@de.ibm.com> > + * > + * This work is licensed under the terms of the GNU GPL, version 2. See > + * the COPYING file in the top-level directory. > + * > + */ > + > +#ifndef _QEMU_S390_SCLP_H > +#define _QEMU_S390_SCLP_H HW_S390_SCLP_H > + > +#include "qdev.h" > + > +/* SCLP command codes */ > +#define SCLP_CMDW_READ_SCP_INFO 0x00020001 > +#define SCLP_CMDW_READ_SCP_INFO_FORCED 0x00120001 > + > +/* SCLP response codes */ > +#define SCLP_RC_NORMAL_READ_COMPLETION 0x0010 > +#define SCLP_RC_INVALID_SCLP_COMMAND 0x01f0 > + > +/* Service Call Control Block (SCCB) and its elements */ > + > +#define SCCB_SIZE 4096 > + > +/* > + * Normally packed structures are not the right thing to do, since all code > + * must take care of endianess. We cant use ldl_phys and friends for two > + * reasons, though: > + * - some of the embedded structures below the SCCB can appear multiple times > + * at different locations, so there is no fixed offset > + * - we work on a private copy of the SCCB, since there are several length > + * fields, that would cause a security nightmare if we allow the guest to > + * alter the structure while we parse it. We cannot use ldl_p and friends > + * either without doing pointer arithmetics > + * So we have to double check that all users of sclp data structures use the > + * right endianess wrappers. > + */ > +typedef struct SCCBHeader { > + uint16_t length; > + uint8_t function_code; > + uint8_t control_mask[3]; > + uint16_t response_code; > +} __attribute__((packed)) SCCBHeader; QEMU_PACKED > + > +#define SCCB_DATA_LEN (SCCB_SIZE - sizeof(SCCBHeader)) > + > +typedef struct ReadInfo { > + SCCBHeader h; > + uint16_t rnmax; > + uint8_t rnsize; > +} __attribute__((packed)) ReadInfo; > + > +typedef struct SCCB { > + SCCBHeader h; > + char data[SCCB_DATA_LEN]; > + } __attribute__((packed)) SCCB; > + > +#define TYPE_S390_SCLP_BUS "s390-sclp-bus" > + > +typedef struct S390SCLPDevice { > + DeviceState qdev; > +} S390SCLPDevice; > + > +typedef struct SCLPS390Bus { > + BusState bus; > +} SCLPS390Bus; > + > +extern SCLPS390Bus *sclp_bus; Global state is suspicious, usually it can be avoided with parameter passing. > + > +SCLPS390Bus *s390_sclp_bus_init(void); > + > +void sclp_service_interrupt(uint32_t sccb); > + > +#endif > diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c > index 47eed35..577fcee 100644 > --- a/hw/s390-virtio.c > +++ b/hw/s390-virtio.c > @@ -32,6 +32,7 @@ > #include "exec-memory.h" > > #include "hw/s390-virtio-bus.h" > +#include "hw/s390-sclp.h" > > //#define DEBUG_S390 > > @@ -61,6 +62,7 @@ > #define MAX_BLK_DEVS 10 > > static VirtIOS390Bus *s390_bus; > +SCLPS390Bus *sclp_bus; > static S390CPU **ipi_states; > > S390CPU *s390_cpu_addr2state(uint16_t cpu_addr) > @@ -183,6 +185,7 @@ static void s390_init(ram_addr_t my_ram_size, > > /* get a BUS */ > s390_bus = s390_virtio_bus_init(&my_ram_size); > + sclp_bus = s390_sclp_bus_init(); > > /* allocate RAM */ > memory_region_init_ram(ram, "s390.ram", my_ram_size); > diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs > index dcdcac8..b2d577b 100644 > --- a/hw/s390x/Makefile.objs > +++ b/hw/s390x/Makefile.objs > @@ -1,3 +1,4 @@ > obj-y = s390-virtio-bus.o s390-virtio.o > +obj-y += s390-sclp.o > > obj-y := $(addprefix ../,$(obj-y)) > diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c > index 619b202..8900872 100644 > --- a/target-s390x/cpu.c > +++ b/target-s390x/cpu.c > @@ -21,9 +21,26 @@ > */ > > #include "cpu.h" > +#include "kvm.h" > #include "qemu-common.h" > #include "qemu-timer.h" > > +/* service interrupts are floating therefore we must not pass an cpustate */ > +void s390_sclp_extint(uint32_t parm) > +{ > + S390CPU *dummy_cpu = s390_cpu_addr2state(0); > + CPUS390XState *env = &dummy_cpu->env; > + > + if (kvm_enabled()) { > +#ifdef CONFIG_KVM > + kvm_s390_interrupt_internal(env, KVM_S390_INT_SERVICE, parm, 0, 1); > +#endif > + } else { > + env->psw.addr += 4; > + cpu_inject_ext(env, EXT_SERVICE, parm, 0); > + } > +} > + > > /* CPUClass::reset() */ > static void s390_cpu_reset(CPUState *s) > diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h > index c30ac3a..abdd838 100644 > --- a/target-s390x/cpu.h > +++ b/target-s390x/cpu.h > @@ -294,6 +294,7 @@ void s390x_tod_timer(void *opaque); > void s390x_cpu_timer(void *opaque); > > int s390_virtio_hypercall(CPUS390XState *env, uint64_t mem, uint64_t hypercall); > +int do_sclp_service_call(uint32_t sccb, uint64_t code); > > #ifdef CONFIG_KVM > void kvm_s390_interrupt(CPUS390XState *env, int type, uint32_t code); > @@ -315,11 +316,15 @@ static inline void kvm_s390_interrupt_internal(CPUS390XState *env, int type, > int vm) > { > } > + > #endif > S390CPU *s390_cpu_addr2state(uint16_t cpu_addr); > void s390_add_running_cpu(CPUS390XState *env); > unsigned s390_del_running_cpu(CPUS390XState *env); > > +/* service interrupts are floating therefore we must not pass an cpustate */ > +void s390_sclp_extint(uint32_t parm); > + > /* from s390-virtio-bus */ > extern const target_phys_addr_t virtio_size; > > @@ -593,17 +598,6 @@ static inline const char *cc_name(int cc_op) > return cc_names[cc_op]; > } > > -/* SCLP PV interface defines */ > -#define SCLP_CMDW_READ_SCP_INFO 0x00020001 > -#define SCLP_CMDW_READ_SCP_INFO_FORCED 0x00120001 > - > -#define SCP_LENGTH 0x00 > -#define SCP_FUNCTION_CODE 0x02 > -#define SCP_CONTROL_MASK 0x03 > -#define SCP_RESPONSE_CODE 0x06 > -#define SCP_MEM_CODE 0x08 > -#define SCP_INCREMENT 0x0a > - > typedef struct LowCore > { > /* prefix area: defined by architecture */ > @@ -952,7 +946,7 @@ static inline void ebcdic_put(uint8_t *p, const char *ascii, int len) > void load_psw(CPUS390XState *env, uint64_t mask, uint64_t addr); > int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc, > target_ulong *raddr, int *flags); > -int sclp_service_call(CPUS390XState *env, uint32_t sccb, uint64_t code); > +int sclp_service_call(uint32_t sccb, uint64_t code); > uint32_t calc_cc(CPUS390XState *env, uint32_t cc_op, uint64_t src, uint64_t dst, > uint64_t vr); > > diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c > index 654f87d..68c4f3e 100644 > --- a/target-s390x/kvm.c > +++ b/target-s390x/kvm.c > @@ -60,9 +60,6 @@ > #define SIGP_STORE_STATUS_ADDR 0x0e > #define SIGP_SET_ARCH 0x12 > > -#define SCLP_CMDW_READ_SCP_INFO 0x00020001 > -#define SCLP_CMDW_READ_SCP_INFO_FORCED 0x00120001 > - > const KVMCapabilityInfo kvm_arch_required_capabilities[] = { > KVM_CAP_LAST_INFO > }; > @@ -237,7 +234,7 @@ static int kvm_sclp_service_call(CPUS390XState *env, struct kvm_run *run, > sccb = env->regs[ipbh0 & 0xf]; > code = env->regs[(ipbh0 & 0xf0) >> 4]; > > - r = sclp_service_call(env, sccb, code); > + r = sclp_service_call(sccb, code); > if (r < 0) { > enter_pgmcheck(env, -r); > } > diff --git a/target-s390x/op_helper.c b/target-s390x/op_helper.c > index 91dd8dc..e7bb980 100644 > --- a/target-s390x/op_helper.c > +++ b/target-s390x/op_helper.c > @@ -2362,20 +2362,12 @@ static void program_interrupt(CPUS390XState *env, uint32_t code, int ilc) > } > } > > -static void ext_interrupt(CPUS390XState *env, int type, uint32_t param, > - uint64_t param64) > -{ > - cpu_inject_ext(env, type, param, param64); > -} > - > /* > + * we handle here the part that belongs to the cpu, e.g. program checks > * ret < 0 indicates program check, ret = 0,1,2,3 -> cc > */ > -int sclp_service_call(CPUS390XState *env, uint32_t sccb, uint64_t code) > +int sclp_service_call(uint32_t sccb, uint64_t code) > { > - int r = 0; > - int shift = 0; > - > #ifdef DEBUG_HELPER > printf("sclp(0x%x, 0x%" PRIx64 ")\n", sccb, code); > #endif > @@ -2388,35 +2380,8 @@ int sclp_service_call(CPUS390XState *env, uint32_t sccb, uint64_t code) > return -PGM_SPECIFICATION; > } > > - switch(code) { > - case SCLP_CMDW_READ_SCP_INFO: > - case SCLP_CMDW_READ_SCP_INFO_FORCED: > - while ((ram_size >> (20 + shift)) > 65535) { > - shift++; > - } > - stw_phys(sccb + SCP_MEM_CODE, ram_size >> (20 + shift)); > - stb_phys(sccb + SCP_INCREMENT, 1 << shift); > - stw_phys(sccb + SCP_RESPONSE_CODE, 0x10); > - > - if (kvm_enabled()) { > -#ifdef CONFIG_KVM > - kvm_s390_interrupt_internal(env, KVM_S390_INT_SERVICE, > - sccb & ~3, 0, 1); > -#endif > - } else { > - env->psw.addr += 4; > - ext_interrupt(env, EXT_SERVICE, sccb & ~3, 0); > - } > - break; > - default: > -#ifdef DEBUG_HELPER > - printf("KVM: invalid sclp call 0x%x / 0x%" PRIx64 "x\n", sccb, code); > -#endif > - r = 3; > - break; > - } > - > - return r; > + /* the complex part is handled by external components */ > + return do_sclp_service_call(sccb, code); > } > > /* SCLP service call */ > @@ -2424,7 +2389,7 @@ uint32_t HELPER(servc)(uint32_t r1, uint64_t r2) > { > int r; > > - r = sclp_service_call(env, r1, r2); > + r = sclp_service_call(r1, r2); > if (r < 0) { > program_interrupt(env, -r, 4); > return 0; > -- > 1.7.10.5 > >
Thanks fpr the review, On 13/07/12 17:08, Blue Swirl wrote: >> + * Copyright IBM, Corp. 2007, 2012 > > 2007, really? Well, yes and no. The first userspace for kvm on s390 was kuli and some of that code was used for bringup. But it looks pretty different now, so we can change that to be 2012 as the first release. [...] >> + * This work is licensed under the terms of the GNU GPL, version 2. See > > Why GPLv2only, can't this be licensed under later versions? Was simply copied from other qemu files. We can certainly change that to 2 and later. [...] >> +int do_sclp_service_call(uint32_t sccb, uint64_t code) > > sccb could be target_phys_addr_t. the architecture explicitely requires to have the 32 msbs to be 0. target_phys_addr_t would certainly work as well, but [...] >> + cpu_physical_memory_write(sccb, &work_sccb, >> + be16_to_cpu(work_sccb.h.length)); > > Perhaps the DMA helpers should be used instead. > Is there any rule what to use under which circumstances. [...] >> +#ifndef _QEMU_S390_SCLP_H >> +#define _QEMU_S390_SCLP_H > > HW_S390_SCLP_H Ok [...] >> +} __attribute__((packed)) SCCBHeader; > > QEMU_PACKED Ok [...] >> +extern SCLPS390Bus *sclp_bus; > > Global state is suspicious, usually it can be avoided with parameter passing. [...] Will check if we can make that go away.
On Fri, Jul 13, 2012 at 4:44 PM, Christian Borntraeger <borntraeger@de.ibm.com> wrote: > Thanks fpr the review, > > > On 13/07/12 17:08, Blue Swirl wrote: > >>> + * Copyright IBM, Corp. 2007, 2012 >> >> 2007, really? > > Well, yes and no. The first userspace for kvm on s390 was kuli and some of that > code was used for bringup. But it looks pretty different now, so we can > change that to be 2012 as the first release. > > [...] >>> + * This work is licensed under the terms of the GNU GPL, version 2. See >> >> Why GPLv2only, can't this be licensed under later versions? > > Was simply copied from other qemu files. We can certainly change that to 2 and later. Great! > > > [...] > >>> +int do_sclp_service_call(uint32_t sccb, uint64_t code) >> >> sccb could be target_phys_addr_t. > > the architecture explicitely requires to have the 32 msbs to be 0. target_phys_addr_t > would certainly work as well, but > > [...] >>> + cpu_physical_memory_write(sccb, &work_sccb, >>> + be16_to_cpu(work_sccb.h.length)); >> >> Perhaps the DMA helpers should be used instead. >> > > Is there any rule what to use under which circumstances. No, it's a recent addition. Devices (at least those which may be used in IOMMU environment) should use DMA helpers for their memory accesses in general but virtio may be an exception. I'm not sure which category this falls in. > > > [...] >>> +#ifndef _QEMU_S390_SCLP_H >>> +#define _QEMU_S390_SCLP_H >> >> HW_S390_SCLP_H > > Ok > > [...] >>> +} __attribute__((packed)) SCCBHeader; >> >> QEMU_PACKED > > Ok > > [...] >>> +extern SCLPS390Bus *sclp_bus; >> >> Global state is suspicious, usually it can be avoided with parameter passing. > [...] > > Will check if we can make that go away. >
On 14/07/12 10:48, Blue Swirl wrote: >>>> + cpu_physical_memory_write(sccb, &work_sccb, >>>> + be16_to_cpu(work_sccb.h.length)); >>> >>> Perhaps the DMA helpers should be used instead. >>> >> >> Is there any rule what to use under which circumstances. > > No, it's a recent addition. Devices (at least those which may be used > in IOMMU environment) should use DMA helpers for their memory accesses > in general but virtio may be an exception. I'm not sure which category > this falls in. Ok, I read this as, if we need to handle DMA into virtual addresses at some point in time dma helpers are the way to go. With SCLP we access real storage (no translation) so I think cpu_physical_memory_write is fine here. Christian
Am 13.07.2012 12:52, schrieb Christian Borntraeger: > From: Heinz Graalfs <graalfs@linux.vnet.ibm.com> > > This adds a more generic infrastructure for handling Service-Call > requests on s390. Currently we only support a small subset of Read > SCP Info directly in target-s390x. This patch provides the base > infrastructure for supporting more commands and moves Read SCP > Info. > In the future we could add additional commands for hotplug, call > home and event handling. > > Signed-off-by: Heinz Graalfs <graalfs@linux.vnet.ibm.com> > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > --- > hw/s390-sclp.c | 148 ++++++++++++++++++++++++++++++++++++++++++++++ > hw/s390-sclp.h | 80 +++++++++++++++++++++++++ > hw/s390-virtio.c | 3 + > hw/s390x/Makefile.objs | 1 + > target-s390x/cpu.c | 17 ++++++ > target-s390x/cpu.h | 18 ++---- > target-s390x/kvm.c | 5 +- > target-s390x/op_helper.c | 45 ++------------ > 8 files changed, 261 insertions(+), 56 deletions(-) > create mode 100644 hw/s390-sclp.c > create mode 100644 hw/s390-sclp.h > > diff --git a/hw/s390-sclp.c b/hw/s390-sclp.c > new file mode 100644 > index 0000000..74a3e66 > --- /dev/null > +++ b/hw/s390-sclp.c > @@ -0,0 +1,148 @@ > +/* > + * SCLP Support > + * > + * Copyright IBM, Corp. 2007, 2012 > + * > + * Authors: > + * Christian Borntraeger <borntraeger@de.ibm.com> > + * Heinz Graalfs <graalfs@linux.vnet.ibm.com> > + * > + * This work is licensed under the terms of the GNU GPL, version 2. See > + * the COPYING file in the top-level directory. > + * > + */ > + > +#include "cpu.h" > +#include "kvm.h" > +#include "sysbus.h" > + > +#include "s390-sclp.h" > + > +/* Provide information about the configuration, CPUs and storage */ > +static int read_SCP_info(SCCB *sccb) > +{ > + ReadInfo *read_info = (ReadInfo *) sccb; > + int shift = 0; > + > + while ((ram_size >> (20 + shift)) > 65535) { > + shift++; > + } > + read_info->rnmax = cpu_to_be16(ram_size >> (20 + shift)); > + read_info->rnsize = 1 << shift; > + sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_READ_COMPLETION); > + > + return 0; > +} > + > +static int sclp_execute(SCCB *sccb, uint64_t code) > +{ > + int r = 0; > + > + switch (code) { > + case SCLP_CMDW_READ_SCP_INFO: > + case SCLP_CMDW_READ_SCP_INFO_FORCED: > + r = read_SCP_info(sccb); > + break; > + default: > +#ifdef DEBUG_HELPER > + printf("KVM: invalid sclp call 0x%x / 0x%" PRIx64 "x\n", sccb, code); sccb is a pointer so %x seems wrong for 64-bit host. %p? > +#endif > + sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND); > + break; > + } > + return r; > +} > + > +int do_sclp_service_call(uint32_t sccb, uint64_t code) > +{ > + int r = 0; > + SCCB work_sccb; > + > + target_phys_addr_t sccb_len = sizeof(SCCB); > + > + /* > + * we want to work on a private copy of the sccb, to prevent guests > + * from playing dirty tricks by modifying the memory content after > + * the host has checked the values > + */ > + cpu_physical_memory_read(sccb, &work_sccb, sccb_len); > + > + /* Valid sccb sizes */ > + if (be16_to_cpu(work_sccb.h.length) < 8 || > + be16_to_cpu(work_sccb.h.length) > 4096) { > + r = -PGM_SPECIFICATION; > + goto out; > + } > + > + r = sclp_execute((SCCB *)&work_sccb, code); > + > + cpu_physical_memory_write(sccb, &work_sccb, > + be16_to_cpu(work_sccb.h.length)); > + if (!r) { > + sclp_service_interrupt(sccb); > + } > + > +out: > + return r; > +} > + > +void sclp_service_interrupt(uint32_t sccb) > +{ > + if (!sccb) { > + return; > + } > + s390_sclp_extint(sccb & ~3); > +} > + > +/* qemu object creation and initialization functions */ > + > +#define S390_SCLP_BUS(obj) OBJECT_CHECK(SCLPS390Bus, (obj), TYPE_S390_SCLP_BUS) This should probably be in the header alongside TYPE_S390_SCLP_BUS? Otherwise a separating white line would be nice. > +static const TypeInfo s390_sclp_bus_info = { > + .name = TYPE_S390_SCLP_BUS, > + .parent = TYPE_BUS, > + .instance_size = sizeof(SCLPS390Bus), > + }; > + > +SCLPS390Bus *s390_sclp_bus_init(void) > +{ > + SCLPS390Bus *bus; > + BusState *bus_state; > + DeviceState *dev; > + > + dev = qdev_create(NULL, "s390-sclp-bridge"); > + qdev_init_nofail(dev); > + > + bus_state = qbus_create(TYPE_S390_SCLP_BUS, dev, "s390-sclp-bus"); Use TYPE_S390_SCLP_BUS constant? > + bus_state->allow_hotplug = 0; > + > + bus = DO_UPCAST(SCLPS390Bus, bus, bus_state); > + return bus; > +} > + > +static int s390_sclp_bridge_init(SysBusDevice *dev) > +{ > + return 0; > +} > + > +static void s390_sclp_bridge_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); > + > + k->init = s390_sclp_bridge_init; > + dc->no_user = 1; > +} > + > +static TypeInfo s390_sclp_bridge_info = { > + .name = "s390-sclp-bridge", > + .parent = TYPE_SYS_BUS_DEVICE, > + .instance_size = sizeof(SysBusDevice), > + .class_init = s390_sclp_bridge_class_init, > +}; > + > +static void s390_sclp_register_types(void) > +{ > + type_register_static(&s390_sclp_bridge_info); > + type_register_static(&s390_sclp_bus_info); > +} > +type_init(s390_sclp_register_types) > diff --git a/hw/s390-sclp.h b/hw/s390-sclp.h > new file mode 100644 > index 0000000..f7bf140 > --- /dev/null > +++ b/hw/s390-sclp.h > @@ -0,0 +1,80 @@ > +/* > + * SCLP Support > + * > + * Copyright IBM, Corp. 2007, 2012 > + * > + * Authors: > + * Christian Borntraeger <borntraeger@de.ibm.com> > + * > + * This work is licensed under the terms of the GNU GPL, version 2. See > + * the COPYING file in the top-level directory. > + * > + */ > + > +#ifndef _QEMU_S390_SCLP_H > +#define _QEMU_S390_SCLP_H > + > +#include "qdev.h" > + > +/* SCLP command codes */ > +#define SCLP_CMDW_READ_SCP_INFO 0x00020001 > +#define SCLP_CMDW_READ_SCP_INFO_FORCED 0x00120001 > + > +/* SCLP response codes */ > +#define SCLP_RC_NORMAL_READ_COMPLETION 0x0010 > +#define SCLP_RC_INVALID_SCLP_COMMAND 0x01f0 > + > +/* Service Call Control Block (SCCB) and its elements */ > + > +#define SCCB_SIZE 4096 > + > +/* > + * Normally packed structures are not the right thing to do, since all code > + * must take care of endianess. We cant use ldl_phys and friends for two > + * reasons, though: > + * - some of the embedded structures below the SCCB can appear multiple times > + * at different locations, so there is no fixed offset > + * - we work on a private copy of the SCCB, since there are several length > + * fields, that would cause a security nightmare if we allow the guest to > + * alter the structure while we parse it. We cannot use ldl_p and friends > + * either without doing pointer arithmetics > + * So we have to double check that all users of sclp data structures use the > + * right endianess wrappers. > + */ > +typedef struct SCCBHeader { > + uint16_t length; > + uint8_t function_code; > + uint8_t control_mask[3]; > + uint16_t response_code; > +} __attribute__((packed)) SCCBHeader; > + > +#define SCCB_DATA_LEN (SCCB_SIZE - sizeof(SCCBHeader)) > + > +typedef struct ReadInfo { > + SCCBHeader h; > + uint16_t rnmax; > + uint8_t rnsize; > +} __attribute__((packed)) ReadInfo; > + > +typedef struct SCCB { > + SCCBHeader h; > + char data[SCCB_DATA_LEN]; > + } __attribute__((packed)) SCCB; > + > +#define TYPE_S390_SCLP_BUS "s390-sclp-bus" > + > +typedef struct S390SCLPDevice { > + DeviceState qdev; I note that this is probably the first device directly derived from DeviceState. This should be possible now after having merged the QOM QBus series. Test case to check is "info qdm" from the monitor. > +} S390SCLPDevice; > + > +typedef struct SCLPS390Bus { > + BusState bus; > +} SCLPS390Bus; > + > +extern SCLPS390Bus *sclp_bus; > + > +SCLPS390Bus *s390_sclp_bus_init(void); > + > +void sclp_service_interrupt(uint32_t sccb); > + > +#endif > diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c > index 47eed35..577fcee 100644 > --- a/hw/s390-virtio.c > +++ b/hw/s390-virtio.c > @@ -32,6 +32,7 @@ > #include "exec-memory.h" > > #include "hw/s390-virtio-bus.h" > +#include "hw/s390-sclp.h" > > //#define DEBUG_S390 > > @@ -61,6 +62,7 @@ > #define MAX_BLK_DEVS 10 > > static VirtIOS390Bus *s390_bus; > +SCLPS390Bus *sclp_bus; I don't like this so much. We shouldn't be following that example and adding global state for each bus there. Can't we add a child property to the machine for the hosting device so that we can obtain that via QOM path and access the bus from there? > static S390CPU **ipi_states; > > S390CPU *s390_cpu_addr2state(uint16_t cpu_addr) > @@ -183,6 +185,7 @@ static void s390_init(ram_addr_t my_ram_size, > > /* get a BUS */ > s390_bus = s390_virtio_bus_init(&my_ram_size); > + sclp_bus = s390_sclp_bus_init(); > > /* allocate RAM */ > memory_region_init_ram(ram, "s390.ram", my_ram_size); > diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs > index dcdcac8..b2d577b 100644 > --- a/hw/s390x/Makefile.objs > +++ b/hw/s390x/Makefile.objs > @@ -1,3 +1,4 @@ > obj-y = s390-virtio-bus.o s390-virtio.o > +obj-y += s390-sclp.o > > obj-y := $(addprefix ../,$(obj-y)) IIUC we have a dependency on the CPU inside the device code. If so, please move the s390-sclp.c file into hw/s390x/ and move the obj-y += line below the addprefix line. > diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c > index 619b202..8900872 100644 > --- a/target-s390x/cpu.c > +++ b/target-s390x/cpu.c > @@ -21,9 +21,26 @@ > */ > > #include "cpu.h" > +#include "kvm.h" > #include "qemu-common.h" > #include "qemu-timer.h" > > +/* service interrupts are floating therefore we must not pass an cpustate */ > +void s390_sclp_extint(uint32_t parm) > +{ > + S390CPU *dummy_cpu = s390_cpu_addr2state(0); > + CPUS390XState *env = &dummy_cpu->env; > + > + if (kvm_enabled()) { > +#ifdef CONFIG_KVM > + kvm_s390_interrupt_internal(env, KVM_S390_INT_SERVICE, parm, 0, 1); > +#endif > + } else { > + env->psw.addr += 4; > + cpu_inject_ext(env, EXT_SERVICE, parm, 0); > + } > +} Not so happy about this placement, it is not called s390_cpu_, the prototype is in cpu.h not cpu-qom.h and it operates only indirectly on the CPU. Is there another place for it? > + > > /* CPUClass::reset() */ > static void s390_cpu_reset(CPUState *s) > diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h > index c30ac3a..abdd838 100644 > --- a/target-s390x/cpu.h > +++ b/target-s390x/cpu.h > @@ -294,6 +294,7 @@ void s390x_tod_timer(void *opaque); > void s390x_cpu_timer(void *opaque); > > int s390_virtio_hypercall(CPUS390XState *env, uint64_t mem, uint64_t hypercall); > +int do_sclp_service_call(uint32_t sccb, uint64_t code); > > #ifdef CONFIG_KVM > void kvm_s390_interrupt(CPUS390XState *env, int type, uint32_t code); > @@ -315,11 +316,15 @@ static inline void kvm_s390_interrupt_internal(CPUS390XState *env, int type, > int vm) > { > } > + > #endif Stray whitespace change? If we want to do it, we might want to add one after #endif as well and after #ifdef in the preceding hunk. Regards, Andreas > S390CPU *s390_cpu_addr2state(uint16_t cpu_addr); > void s390_add_running_cpu(CPUS390XState *env); > unsigned s390_del_running_cpu(CPUS390XState *env); > > +/* service interrupts are floating therefore we must not pass an cpustate */ > +void s390_sclp_extint(uint32_t parm); > + > /* from s390-virtio-bus */ > extern const target_phys_addr_t virtio_size; > > @@ -593,17 +598,6 @@ static inline const char *cc_name(int cc_op) > return cc_names[cc_op]; > } > > -/* SCLP PV interface defines */ > -#define SCLP_CMDW_READ_SCP_INFO 0x00020001 > -#define SCLP_CMDW_READ_SCP_INFO_FORCED 0x00120001 > - > -#define SCP_LENGTH 0x00 > -#define SCP_FUNCTION_CODE 0x02 > -#define SCP_CONTROL_MASK 0x03 > -#define SCP_RESPONSE_CODE 0x06 > -#define SCP_MEM_CODE 0x08 > -#define SCP_INCREMENT 0x0a > - > typedef struct LowCore > { > /* prefix area: defined by architecture */ > @@ -952,7 +946,7 @@ static inline void ebcdic_put(uint8_t *p, const char *ascii, int len) > void load_psw(CPUS390XState *env, uint64_t mask, uint64_t addr); > int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc, > target_ulong *raddr, int *flags); > -int sclp_service_call(CPUS390XState *env, uint32_t sccb, uint64_t code); > +int sclp_service_call(uint32_t sccb, uint64_t code); > uint32_t calc_cc(CPUS390XState *env, uint32_t cc_op, uint64_t src, uint64_t dst, > uint64_t vr); > > diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c > index 654f87d..68c4f3e 100644 > --- a/target-s390x/kvm.c > +++ b/target-s390x/kvm.c > @@ -60,9 +60,6 @@ > #define SIGP_STORE_STATUS_ADDR 0x0e > #define SIGP_SET_ARCH 0x12 > > -#define SCLP_CMDW_READ_SCP_INFO 0x00020001 > -#define SCLP_CMDW_READ_SCP_INFO_FORCED 0x00120001 > - > const KVMCapabilityInfo kvm_arch_required_capabilities[] = { > KVM_CAP_LAST_INFO > }; > @@ -237,7 +234,7 @@ static int kvm_sclp_service_call(CPUS390XState *env, struct kvm_run *run, > sccb = env->regs[ipbh0 & 0xf]; > code = env->regs[(ipbh0 & 0xf0) >> 4]; > > - r = sclp_service_call(env, sccb, code); > + r = sclp_service_call(sccb, code); > if (r < 0) { > enter_pgmcheck(env, -r); > } > diff --git a/target-s390x/op_helper.c b/target-s390x/op_helper.c > index 91dd8dc..e7bb980 100644 > --- a/target-s390x/op_helper.c > +++ b/target-s390x/op_helper.c > @@ -2362,20 +2362,12 @@ static void program_interrupt(CPUS390XState *env, uint32_t code, int ilc) > } > } > > -static void ext_interrupt(CPUS390XState *env, int type, uint32_t param, > - uint64_t param64) > -{ > - cpu_inject_ext(env, type, param, param64); > -} > - > /* > + * we handle here the part that belongs to the cpu, e.g. program checks > * ret < 0 indicates program check, ret = 0,1,2,3 -> cc > */ > -int sclp_service_call(CPUS390XState *env, uint32_t sccb, uint64_t code) > +int sclp_service_call(uint32_t sccb, uint64_t code) > { > - int r = 0; > - int shift = 0; > - > #ifdef DEBUG_HELPER > printf("sclp(0x%x, 0x%" PRIx64 ")\n", sccb, code); > #endif > @@ -2388,35 +2380,8 @@ int sclp_service_call(CPUS390XState *env, uint32_t sccb, uint64_t code) > return -PGM_SPECIFICATION; > } > > - switch(code) { > - case SCLP_CMDW_READ_SCP_INFO: > - case SCLP_CMDW_READ_SCP_INFO_FORCED: > - while ((ram_size >> (20 + shift)) > 65535) { > - shift++; > - } > - stw_phys(sccb + SCP_MEM_CODE, ram_size >> (20 + shift)); > - stb_phys(sccb + SCP_INCREMENT, 1 << shift); > - stw_phys(sccb + SCP_RESPONSE_CODE, 0x10); > - > - if (kvm_enabled()) { > -#ifdef CONFIG_KVM > - kvm_s390_interrupt_internal(env, KVM_S390_INT_SERVICE, > - sccb & ~3, 0, 1); > -#endif > - } else { > - env->psw.addr += 4; > - ext_interrupt(env, EXT_SERVICE, sccb & ~3, 0); > - } > - break; > - default: > -#ifdef DEBUG_HELPER > - printf("KVM: invalid sclp call 0x%x / 0x%" PRIx64 "x\n", sccb, code); > -#endif > - r = 3; > - break; > - } > - > - return r; > + /* the complex part is handled by external components */ > + return do_sclp_service_call(sccb, code); > } > > /* SCLP service call */ > @@ -2424,7 +2389,7 @@ uint32_t HELPER(servc)(uint32_t r1, uint64_t r2) > { > int r; > > - r = sclp_service_call(env, r1, r2); > + r = sclp_service_call(r1, r2); > if (r < 0) { > program_interrupt(env, -r, 4); > return 0; >
Andreas, thanks for having a look. I no other comments arrive today I will resubmit the patch set with all comments addressed. On 20/07/12 16:06, Andreas Färber wrote: >> +#ifdef DEBUG_HELPER >> + printf("KVM: invalid sclp call 0x%x / 0x%" PRIx64 "x\n", sccb, code); > > sccb is a pointer so %x seems wrong for 64-bit host. %p? yes its broken. We will probably just remove that, since it will be changed by followon patch. >> +typedef struct S390SCLPDevice { >> + DeviceState qdev; > > I note that this is probably the first device directly derived from > DeviceState. This should be possible now after having merged the QOM > QBus series. Test case to check is "info qdm" from the monitor. info qdm seems to work. what test did you have in mind? [...] >> //#define DEBUG_S390 >> >> @@ -61,6 +62,7 @@ >> #define MAX_BLK_DEVS 10 >> >> static VirtIOS390Bus *s390_bus; >> +SCLPS390Bus *sclp_bus; > > I don't like this so much. We shouldn't be following that example and > adding global state for each bus there. Can't we add a child property to > the machine for the hosting device so that we can obtain that via QOM > path and access the bus from there? Since Blue already compained Heinz already moved that bus into s390-sclp.c as a static variable without any export since we only touch that there. We also added an comment that by architecture definition there is only one sclp per machine, so this should be fine. >> --- a/hw/s390x/Makefile.objs >> +++ b/hw/s390x/Makefile.objs >> @@ -1,3 +1,4 @@ >> obj-y = s390-virtio-bus.o s390-virtio.o >> +obj-y += s390-sclp.o >> >> obj-y := $(addprefix ../,$(obj-y)) > > IIUC we have a dependency on the CPU inside the device code. If so, > please move the s390-sclp.c file into hw/s390x/ and move the obj-y += > line below the addprefix line. Ok. >> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c >> index 619b202..8900872 100644 >> --- a/target-s390x/cpu.c >> +++ b/target-s390x/cpu.c >> @@ -21,9 +21,26 @@ >> */ >> >> #include "cpu.h" >> +#include "kvm.h" >> #include "qemu-common.h" >> #include "qemu-timer.h" >> >> +/* service interrupts are floating therefore we must not pass an cpustate */ >> +void s390_sclp_extint(uint32_t parm) >> +{ >> + S390CPU *dummy_cpu = s390_cpu_addr2state(0); >> + CPUS390XState *env = &dummy_cpu->env; >> + >> + if (kvm_enabled()) { >> +#ifdef CONFIG_KVM >> + kvm_s390_interrupt_internal(env, KVM_S390_INT_SERVICE, parm, 0, 1); >> +#endif >> + } else { >> + env->psw.addr += 4; >> + cpu_inject_ext(env, EXT_SERVICE, parm, 0); >> + } >> +} > > Not so happy about this placement, it is not called s390_cpu_, the > prototype is in cpu.h not cpu-qom.h and it operates only indirectly on > the CPU. Is there another place for it? Not yet. I will try to introduce a new one, e.g. target-s390x/interrupt.c We can then move other code there as well.
diff --git a/hw/s390-sclp.c b/hw/s390-sclp.c new file mode 100644 index 0000000..74a3e66 --- /dev/null +++ b/hw/s390-sclp.c @@ -0,0 +1,148 @@ +/* + * SCLP Support + * + * Copyright IBM, Corp. 2007, 2012 + * + * Authors: + * Christian Borntraeger <borntraeger@de.ibm.com> + * Heinz Graalfs <graalfs@linux.vnet.ibm.com> + * + * This work is licensed under the terms of the GNU GPL, version 2. See + * the COPYING file in the top-level directory. + * + */ + +#include "cpu.h" +#include "kvm.h" +#include "sysbus.h" + +#include "s390-sclp.h" + +/* Provide information about the configuration, CPUs and storage */ +static int read_SCP_info(SCCB *sccb) +{ + ReadInfo *read_info = (ReadInfo *) sccb; + int shift = 0; + + while ((ram_size >> (20 + shift)) > 65535) { + shift++; + } + read_info->rnmax = cpu_to_be16(ram_size >> (20 + shift)); + read_info->rnsize = 1 << shift; + sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_READ_COMPLETION); + + return 0; +} + +static int sclp_execute(SCCB *sccb, uint64_t code) +{ + int r = 0; + + switch (code) { + case SCLP_CMDW_READ_SCP_INFO: + case SCLP_CMDW_READ_SCP_INFO_FORCED: + r = read_SCP_info(sccb); + break; + default: +#ifdef DEBUG_HELPER + printf("KVM: invalid sclp call 0x%x / 0x%" PRIx64 "x\n", sccb, code); +#endif + sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND); + break; + } + return r; +} + +int do_sclp_service_call(uint32_t sccb, uint64_t code) +{ + int r = 0; + SCCB work_sccb; + + target_phys_addr_t sccb_len = sizeof(SCCB); + + /* + * we want to work on a private copy of the sccb, to prevent guests + * from playing dirty tricks by modifying the memory content after + * the host has checked the values + */ + cpu_physical_memory_read(sccb, &work_sccb, sccb_len); + + /* Valid sccb sizes */ + if (be16_to_cpu(work_sccb.h.length) < 8 || + be16_to_cpu(work_sccb.h.length) > 4096) { + r = -PGM_SPECIFICATION; + goto out; + } + + r = sclp_execute((SCCB *)&work_sccb, code); + + cpu_physical_memory_write(sccb, &work_sccb, + be16_to_cpu(work_sccb.h.length)); + if (!r) { + sclp_service_interrupt(sccb); + } + +out: + return r; +} + +void sclp_service_interrupt(uint32_t sccb) +{ + if (!sccb) { + return; + } + s390_sclp_extint(sccb & ~3); +} + +/* qemu object creation and initialization functions */ + +#define S390_SCLP_BUS(obj) OBJECT_CHECK(SCLPS390Bus, (obj), TYPE_S390_SCLP_BUS) +static const TypeInfo s390_sclp_bus_info = { + .name = TYPE_S390_SCLP_BUS, + .parent = TYPE_BUS, + .instance_size = sizeof(SCLPS390Bus), + }; + +SCLPS390Bus *s390_sclp_bus_init(void) +{ + SCLPS390Bus *bus; + BusState *bus_state; + DeviceState *dev; + + dev = qdev_create(NULL, "s390-sclp-bridge"); + qdev_init_nofail(dev); + + bus_state = qbus_create(TYPE_S390_SCLP_BUS, dev, "s390-sclp-bus"); + bus_state->allow_hotplug = 0; + + bus = DO_UPCAST(SCLPS390Bus, bus, bus_state); + return bus; +} + +static int s390_sclp_bridge_init(SysBusDevice *dev) +{ + return 0; +} + +static void s390_sclp_bridge_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); + + k->init = s390_sclp_bridge_init; + dc->no_user = 1; +} + +static TypeInfo s390_sclp_bridge_info = { + .name = "s390-sclp-bridge", + .parent = TYPE_SYS_BUS_DEVICE, + .instance_size = sizeof(SysBusDevice), + .class_init = s390_sclp_bridge_class_init, +}; + +static void s390_sclp_register_types(void) +{ + type_register_static(&s390_sclp_bridge_info); + type_register_static(&s390_sclp_bus_info); +} +type_init(s390_sclp_register_types) diff --git a/hw/s390-sclp.h b/hw/s390-sclp.h new file mode 100644 index 0000000..f7bf140 --- /dev/null +++ b/hw/s390-sclp.h @@ -0,0 +1,80 @@ +/* + * SCLP Support + * + * Copyright IBM, Corp. 2007, 2012 + * + * Authors: + * Christian Borntraeger <borntraeger@de.ibm.com> + * + * This work is licensed under the terms of the GNU GPL, version 2. See + * the COPYING file in the top-level directory. + * + */ + +#ifndef _QEMU_S390_SCLP_H +#define _QEMU_S390_SCLP_H + +#include "qdev.h" + +/* SCLP command codes */ +#define SCLP_CMDW_READ_SCP_INFO 0x00020001 +#define SCLP_CMDW_READ_SCP_INFO_FORCED 0x00120001 + +/* SCLP response codes */ +#define SCLP_RC_NORMAL_READ_COMPLETION 0x0010 +#define SCLP_RC_INVALID_SCLP_COMMAND 0x01f0 + +/* Service Call Control Block (SCCB) and its elements */ + +#define SCCB_SIZE 4096 + +/* + * Normally packed structures are not the right thing to do, since all code + * must take care of endianess. We cant use ldl_phys and friends for two + * reasons, though: + * - some of the embedded structures below the SCCB can appear multiple times + * at different locations, so there is no fixed offset + * - we work on a private copy of the SCCB, since there are several length + * fields, that would cause a security nightmare if we allow the guest to + * alter the structure while we parse it. We cannot use ldl_p and friends + * either without doing pointer arithmetics + * So we have to double check that all users of sclp data structures use the + * right endianess wrappers. + */ +typedef struct SCCBHeader { + uint16_t length; + uint8_t function_code; + uint8_t control_mask[3]; + uint16_t response_code; +} __attribute__((packed)) SCCBHeader; + +#define SCCB_DATA_LEN (SCCB_SIZE - sizeof(SCCBHeader)) + +typedef struct ReadInfo { + SCCBHeader h; + uint16_t rnmax; + uint8_t rnsize; +} __attribute__((packed)) ReadInfo; + +typedef struct SCCB { + SCCBHeader h; + char data[SCCB_DATA_LEN]; + } __attribute__((packed)) SCCB; + +#define TYPE_S390_SCLP_BUS "s390-sclp-bus" + +typedef struct S390SCLPDevice { + DeviceState qdev; +} S390SCLPDevice; + +typedef struct SCLPS390Bus { + BusState bus; +} SCLPS390Bus; + +extern SCLPS390Bus *sclp_bus; + +SCLPS390Bus *s390_sclp_bus_init(void); + +void sclp_service_interrupt(uint32_t sccb); + +#endif diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c index 47eed35..577fcee 100644 --- a/hw/s390-virtio.c +++ b/hw/s390-virtio.c @@ -32,6 +32,7 @@ #include "exec-memory.h" #include "hw/s390-virtio-bus.h" +#include "hw/s390-sclp.h" //#define DEBUG_S390 @@ -61,6 +62,7 @@ #define MAX_BLK_DEVS 10 static VirtIOS390Bus *s390_bus; +SCLPS390Bus *sclp_bus; static S390CPU **ipi_states; S390CPU *s390_cpu_addr2state(uint16_t cpu_addr) @@ -183,6 +185,7 @@ static void s390_init(ram_addr_t my_ram_size, /* get a BUS */ s390_bus = s390_virtio_bus_init(&my_ram_size); + sclp_bus = s390_sclp_bus_init(); /* allocate RAM */ memory_region_init_ram(ram, "s390.ram", my_ram_size); diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs index dcdcac8..b2d577b 100644 --- a/hw/s390x/Makefile.objs +++ b/hw/s390x/Makefile.objs @@ -1,3 +1,4 @@ obj-y = s390-virtio-bus.o s390-virtio.o +obj-y += s390-sclp.o obj-y := $(addprefix ../,$(obj-y)) diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c index 619b202..8900872 100644 --- a/target-s390x/cpu.c +++ b/target-s390x/cpu.c @@ -21,9 +21,26 @@ */ #include "cpu.h" +#include "kvm.h" #include "qemu-common.h" #include "qemu-timer.h" +/* service interrupts are floating therefore we must not pass an cpustate */ +void s390_sclp_extint(uint32_t parm) +{ + S390CPU *dummy_cpu = s390_cpu_addr2state(0); + CPUS390XState *env = &dummy_cpu->env; + + if (kvm_enabled()) { +#ifdef CONFIG_KVM + kvm_s390_interrupt_internal(env, KVM_S390_INT_SERVICE, parm, 0, 1); +#endif + } else { + env->psw.addr += 4; + cpu_inject_ext(env, EXT_SERVICE, parm, 0); + } +} + /* CPUClass::reset() */ static void s390_cpu_reset(CPUState *s) diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h index c30ac3a..abdd838 100644 --- a/target-s390x/cpu.h +++ b/target-s390x/cpu.h @@ -294,6 +294,7 @@ void s390x_tod_timer(void *opaque); void s390x_cpu_timer(void *opaque); int s390_virtio_hypercall(CPUS390XState *env, uint64_t mem, uint64_t hypercall); +int do_sclp_service_call(uint32_t sccb, uint64_t code); #ifdef CONFIG_KVM void kvm_s390_interrupt(CPUS390XState *env, int type, uint32_t code); @@ -315,11 +316,15 @@ static inline void kvm_s390_interrupt_internal(CPUS390XState *env, int type, int vm) { } + #endif S390CPU *s390_cpu_addr2state(uint16_t cpu_addr); void s390_add_running_cpu(CPUS390XState *env); unsigned s390_del_running_cpu(CPUS390XState *env); +/* service interrupts are floating therefore we must not pass an cpustate */ +void s390_sclp_extint(uint32_t parm); + /* from s390-virtio-bus */ extern const target_phys_addr_t virtio_size; @@ -593,17 +598,6 @@ static inline const char *cc_name(int cc_op) return cc_names[cc_op]; } -/* SCLP PV interface defines */ -#define SCLP_CMDW_READ_SCP_INFO 0x00020001 -#define SCLP_CMDW_READ_SCP_INFO_FORCED 0x00120001 - -#define SCP_LENGTH 0x00 -#define SCP_FUNCTION_CODE 0x02 -#define SCP_CONTROL_MASK 0x03 -#define SCP_RESPONSE_CODE 0x06 -#define SCP_MEM_CODE 0x08 -#define SCP_INCREMENT 0x0a - typedef struct LowCore { /* prefix area: defined by architecture */ @@ -952,7 +946,7 @@ static inline void ebcdic_put(uint8_t *p, const char *ascii, int len) void load_psw(CPUS390XState *env, uint64_t mask, uint64_t addr); int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc, target_ulong *raddr, int *flags); -int sclp_service_call(CPUS390XState *env, uint32_t sccb, uint64_t code); +int sclp_service_call(uint32_t sccb, uint64_t code); uint32_t calc_cc(CPUS390XState *env, uint32_t cc_op, uint64_t src, uint64_t dst, uint64_t vr); diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c index 654f87d..68c4f3e 100644 --- a/target-s390x/kvm.c +++ b/target-s390x/kvm.c @@ -60,9 +60,6 @@ #define SIGP_STORE_STATUS_ADDR 0x0e #define SIGP_SET_ARCH 0x12 -#define SCLP_CMDW_READ_SCP_INFO 0x00020001 -#define SCLP_CMDW_READ_SCP_INFO_FORCED 0x00120001 - const KVMCapabilityInfo kvm_arch_required_capabilities[] = { KVM_CAP_LAST_INFO }; @@ -237,7 +234,7 @@ static int kvm_sclp_service_call(CPUS390XState *env, struct kvm_run *run, sccb = env->regs[ipbh0 & 0xf]; code = env->regs[(ipbh0 & 0xf0) >> 4]; - r = sclp_service_call(env, sccb, code); + r = sclp_service_call(sccb, code); if (r < 0) { enter_pgmcheck(env, -r); } diff --git a/target-s390x/op_helper.c b/target-s390x/op_helper.c index 91dd8dc..e7bb980 100644 --- a/target-s390x/op_helper.c +++ b/target-s390x/op_helper.c @@ -2362,20 +2362,12 @@ static void program_interrupt(CPUS390XState *env, uint32_t code, int ilc) } } -static void ext_interrupt(CPUS390XState *env, int type, uint32_t param, - uint64_t param64) -{ - cpu_inject_ext(env, type, param, param64); -} - /* + * we handle here the part that belongs to the cpu, e.g. program checks * ret < 0 indicates program check, ret = 0,1,2,3 -> cc */ -int sclp_service_call(CPUS390XState *env, uint32_t sccb, uint64_t code) +int sclp_service_call(uint32_t sccb, uint64_t code) { - int r = 0; - int shift = 0; - #ifdef DEBUG_HELPER printf("sclp(0x%x, 0x%" PRIx64 ")\n", sccb, code); #endif @@ -2388,35 +2380,8 @@ int sclp_service_call(CPUS390XState *env, uint32_t sccb, uint64_t code) return -PGM_SPECIFICATION; } - switch(code) { - case SCLP_CMDW_READ_SCP_INFO: - case SCLP_CMDW_READ_SCP_INFO_FORCED: - while ((ram_size >> (20 + shift)) > 65535) { - shift++; - } - stw_phys(sccb + SCP_MEM_CODE, ram_size >> (20 + shift)); - stb_phys(sccb + SCP_INCREMENT, 1 << shift); - stw_phys(sccb + SCP_RESPONSE_CODE, 0x10); - - if (kvm_enabled()) { -#ifdef CONFIG_KVM - kvm_s390_interrupt_internal(env, KVM_S390_INT_SERVICE, - sccb & ~3, 0, 1); -#endif - } else { - env->psw.addr += 4; - ext_interrupt(env, EXT_SERVICE, sccb & ~3, 0); - } - break; - default: -#ifdef DEBUG_HELPER - printf("KVM: invalid sclp call 0x%x / 0x%" PRIx64 "x\n", sccb, code); -#endif - r = 3; - break; - } - - return r; + /* the complex part is handled by external components */ + return do_sclp_service_call(sccb, code); } /* SCLP service call */ @@ -2424,7 +2389,7 @@ uint32_t HELPER(servc)(uint32_t r1, uint64_t r2) { int r; - r = sclp_service_call(env, r1, r2); + r = sclp_service_call(r1, r2); if (r < 0) { program_interrupt(env, -r, 4); return 0;