Re: [Qemu-devel] [PATCH 2/6] s390: sclp base support

[Blue Swirl]

On Fri, Jul 13, 2012 at 10:52 AM, Christian Borntraeger
<address@hidden> wrote:
> From: Heinz Graalfs <address@hidden>
>
> 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 <address@hidden>
> Signed-off-by: Christian Borntraeger <address@hidden>
> ---
>  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 <address@hidden>
> + *  Heinz Graalfs <address@hidden>
> + *
> + * 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 <address@hidden>
> + *
> + * 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
>
>