[v1] s390: guest support for diagnose 318 and limit max VCPUs to 247

Commit Message

Collin Walling Dec. 4, 2018, 10:18 p.m. UTC

Add migration and reset support for diagnose 318. This is a new z14 GA2 
hardware feature, but we can provide guest support starting with the 
zEC12-full CPU model.

Because new hardware introduces a new facility-availability byte in 
the Read SCP Info block, we lose one byte in the CPU entries list 
and must limit the maximum VCPUs to 247 (down from 248).

Signed-off-by: Collin Walling <[e-mail]>
---

Changelog

    RFC -> v1
        - introduced kvm stubs for set/get cpc
        - s/fac134/byte_134
        - moved diag318 vmstate description to diag.c
        - reduced S390_VCPU_MAX to 247

 hw/s390x/ipl.c                  |  3 +++
 hw/s390x/s390-virtio-ccw.c      |  3 +++
 hw/s390x/sclp.c                 |  2 ++
 include/hw/s390x/sclp.h         |  2 ++
 linux-headers/asm-s390/kvm.h    |  5 ++++
 target/s390x/cpu.h              |  2 +-
 target/s390x/cpu_features.c     |  3 +++
 target/s390x/cpu_features.h     |  1 +
 target/s390x/cpu_features_def.h |  3 +++
 target/s390x/diag.c             | 53 +++++++++++++++++++++++++++++++++++++++++
 target/s390x/gen-features.c     |  1 +
 target/s390x/internal.h         |  4 +++-
 target/s390x/kvm-stub.c         | 10 ++++++++
 target/s390x/kvm.c              | 23 ++++++++++++++++++
 target/s390x/kvm_s390x.h        |  3 +++
 15 files changed, 116 insertions(+), 2 deletions(-)

Comments

Collin Walling Dec. 4, 2018, 10:26 p.m. UTC | #1

I screen-scraped the @ibm address again (Conny was the victim this time)

Reply to this thread to avoid any delivery failures.

On 12/4/18 5:18 PM, Collin Walling wrote:
> Add migration and reset support for diagnose 318. This is a new z14 GA2 
> hardware feature, but we can provide guest support starting with the 
> zEC12-full CPU model.
> 
> Because new hardware introduces a new facility-availability byte in 
> the Read SCP Info block, we lose one byte in the CPU entries list 
> and must limit the maximum VCPUs to 247 (down from 248).
> 
> Signed-off-by: Collin Walling <[e-mail]>
> ---
> 
> Changelog
> 
>     RFC -> v1
>         - introduced kvm stubs for set/get cpc
>         - s/fac134/byte_134
>         - moved diag318 vmstate description to diag.c
>         - reduced S390_VCPU_MAX to 247
> 
>  hw/s390x/ipl.c                  |  3 +++
>  hw/s390x/s390-virtio-ccw.c      |  3 +++
>  hw/s390x/sclp.c                 |  2 ++
>  include/hw/s390x/sclp.h         |  2 ++
>  linux-headers/asm-s390/kvm.h    |  5 ++++
>  target/s390x/cpu.h              |  2 +-
>  target/s390x/cpu_features.c     |  3 +++
>  target/s390x/cpu_features.h     |  1 +
>  target/s390x/cpu_features_def.h |  3 +++
>  target/s390x/diag.c             | 53 +++++++++++++++++++++++++++++++++++++++++
>  target/s390x/gen-features.c     |  1 +
>  target/s390x/internal.h         |  4 +++-
>  target/s390x/kvm-stub.c         | 10 ++++++++
>  target/s390x/kvm.c              | 23 ++++++++++++++++++
>  target/s390x/kvm_s390x.h        |  3 +++
>  15 files changed, 116 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index 21f64ad..6e3af65 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -27,6 +27,7 @@
>  #include "qemu/cutils.h"
>  #include "qemu/option.h"
>  #include "exec/exec-all.h"
> +#include "internal.h"
>  
>  #define KERN_IMAGE_START                0x010000UL
>  #define LINUX_MAGIC_ADDR                0x010008UL
> @@ -230,6 +231,8 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
>      ipl->compat_start_addr = ipl->start_addr;
>      ipl->compat_bios_start_addr = ipl->bios_start_addr;
>      qemu_register_reset(qdev_reset_all_fn, dev);
> +
> +    diag318_register();
>  error:
>      error_propagate(errp, err);
>  }
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index a0615a8..2c670fc 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -37,6 +37,7 @@
>  #include "cpu_models.h"
>  #include "hw/nmi.h"
>  #include "hw/s390x/tod.h"
> +#include "internal.h"
>  
>  S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
>  {
> @@ -357,6 +358,7 @@ static void s390_machine_reset(void)
>          }
>          subsystem_reset();
>          s390_crypto_reset();
> +        diag318_reset();
>          run_on_cpu(cs, s390_do_cpu_load_normal, RUN_ON_CPU_NULL);
>          break;
>      case S390_RESET_LOAD_NORMAL:
> @@ -364,6 +366,7 @@ static void s390_machine_reset(void)
>              run_on_cpu(t, s390_do_cpu_reset, RUN_ON_CPU_NULL);
>          }
>          subsystem_reset();
> +        diag318_reset();
>          run_on_cpu(cs, s390_do_cpu_initial_reset, RUN_ON_CPU_NULL);
>          run_on_cpu(cs, s390_do_cpu_load_normal, RUN_ON_CPU_NULL);
>          break;
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index 4510a80..183c627 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -73,6 +73,8 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>                           read_info->conf_char);
>      s390_get_feat_block(S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT,
>                           read_info->conf_char_ext);
> +    /* Read Info byte 134 */
> +    s390_get_feat_block(S390_FEAT_TYPE_SCLP_BYTE_134, read_info->byte_134);
>  
>      read_info->facilities = cpu_to_be64(SCLP_HAS_CPU_INFO |
>                                          SCLP_HAS_IOA_RECONFIG);
> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
> index f9db243..eb12ba2 100644
> --- a/include/hw/s390x/sclp.h
> +++ b/include/hw/s390x/sclp.h
> @@ -133,6 +133,8 @@ typedef struct ReadInfo {
>      uint16_t highest_cpu;
>      uint8_t  _reserved5[124 - 122];     /* 122-123 */
>      uint32_t hmfai;
> +    uint8_t  _reserved7[134 - 128];     /* 128-133 */
> +    uint8_t  byte_134[1];
>      struct CPUEntry entries[0];
>  } QEMU_PACKED ReadInfo;
>  
> diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
> index 0265482..8c206d2 100644
> --- a/linux-headers/asm-s390/kvm.h
> +++ b/linux-headers/asm-s390/kvm.h
> @@ -74,6 +74,7 @@ struct kvm_s390_io_adapter_req {
>  #define KVM_S390_VM_CRYPTO		2
>  #define KVM_S390_VM_CPU_MODEL		3
>  #define KVM_S390_VM_MIGRATION		4
> +#define KVM_S390_VM_MISC		5
>  
>  /* kvm attributes for mem_ctrl */
>  #define KVM_S390_VM_MEM_ENABLE_CMMA	0
> @@ -130,6 +131,7 @@ struct kvm_s390_vm_cpu_machine {
>  #define KVM_S390_VM_CPU_FEAT_PFMFI	11
>  #define KVM_S390_VM_CPU_FEAT_SIGPIF	12
>  #define KVM_S390_VM_CPU_FEAT_KSS	13
> +#define KVM_S390_VM_CPU_FEAT_DIAG318	14
>  struct kvm_s390_vm_cpu_feat {
>  	__u64 feat[16];
>  };
> @@ -168,6 +170,9 @@ struct kvm_s390_vm_cpu_subfunc {
>  #define KVM_S390_VM_MIGRATION_START	1
>  #define KVM_S390_VM_MIGRATION_STATUS	2
>  
> +/* kvm attributes for KVM_S390_VM_MISC */
> +#define KVM_S390_VM_MISC_CPC		0
> +
>  /* for KVM_GET_REGS and KVM_SET_REGS */
>  struct kvm_regs {
>  	/* general purpose regs for s390 */
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index 8c2320e..594b4a4 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -52,7 +52,7 @@
>  
>  #define MMU_USER_IDX 0
>  
> -#define S390_MAX_CPUS 248
> +#define S390_MAX_CPUS 247
>  
>  typedef struct PSW {
>      uint64_t mask;
> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
> index 60cfeba..d05afa5 100644
> --- a/target/s390x/cpu_features.c
> +++ b/target/s390x/cpu_features.c
> @@ -121,6 +121,9 @@ static const S390FeatDef s390_features[] = {
>      FEAT_INIT("pfmfi", S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT, 9, "SIE: PFMF interpretation facility"),
>      FEAT_INIT("ibs", S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT, 10, "SIE: Interlock-and-broadcast-suppression facility"),
>  
> +    /* SCLP SCCB Byte 134 */
> +    FEAT_INIT("diag318", S390_FEAT_TYPE_SCLP_BYTE_134, 0, "SIE: Diagnose 318"),
> +
>      FEAT_INIT("sief2", S390_FEAT_TYPE_SCLP_CPU, 4, "SIE: interception format 2 (Virtual SIE)"),
>      FEAT_INIT("skey", S390_FEAT_TYPE_SCLP_CPU, 5, "SIE: Storage-key facility"),
>      FEAT_INIT("gpereh", S390_FEAT_TYPE_SCLP_CPU, 10, "SIE: Guest-PER enhancement facility"),
> diff --git a/target/s390x/cpu_features.h b/target/s390x/cpu_features.h
> index effe790..e7248df 100644
> --- a/target/s390x/cpu_features.h
> +++ b/target/s390x/cpu_features.h
> @@ -23,6 +23,7 @@ typedef enum {
>      S390_FEAT_TYPE_STFL,
>      S390_FEAT_TYPE_SCLP_CONF_CHAR,
>      S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT,
> +    S390_FEAT_TYPE_SCLP_BYTE_134,
>      S390_FEAT_TYPE_SCLP_CPU,
>      S390_FEAT_TYPE_MISC,
>      S390_FEAT_TYPE_PLO,
> diff --git a/target/s390x/cpu_features_def.h b/target/s390x/cpu_features_def.h
> index 5fc7e7b..d99da1d 100644
> --- a/target/s390x/cpu_features_def.h
> +++ b/target/s390x/cpu_features_def.h
> @@ -109,6 +109,9 @@ typedef enum {
>      S390_FEAT_SIE_PFMFI,
>      S390_FEAT_SIE_IBS,
>  
> +    /* Read Info Byte 134 */
> +    S390_FEAT_DIAG318,
> +
>      /* Sclp Cpu */
>      S390_FEAT_SIE_F2,
>      S390_FEAT_SIE_SKEY,
> diff --git a/target/s390x/diag.c b/target/s390x/diag.c
> index acb0f3d..2207c08 100644
> --- a/target/s390x/diag.c
> +++ b/target/s390x/diag.c
> @@ -20,6 +20,8 @@
>  #include "sysemu/cpus.h"
>  #include "hw/s390x/ipl.h"
>  #include "hw/s390x/s390-virtio-ccw.h"
> +#include "kvm_s390x.h"
> +#include "sysemu/kvm.h"
>  
>  int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3)
>  {
> @@ -134,3 +136,54 @@ out:
>          break;
>      }
>  }
> +
> +typedef struct Diag318Data {
> +    uint64_t cpc;
> +} Diag318Data;
> +static Diag318Data diag318data;
> +
> +void diag318_reset(void)
> +{
> +    if (s390_has_feat(S390_FEAT_DIAG318)) {
> +        kvm_s390_set_cpc(0);
> +    }
> +}
> +
> +static int diag318_post_load(void *opaque, int version_id)
> +{
> +    Diag318Data *d = opaque;
> +
> +    kvm_s390_set_cpc(d->cpc);
> +    return 0;
> +}
> +
> +static int diag318_pre_save(void *opaque)
> +{
> +    Diag318Data *d = opaque;
> +
> +    kvm_s390_get_cpc(&d->cpc);
> +    return 0;
> +}
> +
> +static bool diag318_needed(void *opaque)
> +{
> +    return s390_has_feat(S390_FEAT_DIAG318);
> +}
> +
> +const VMStateDescription vmstate_diag318 = {
> +    .name = "diag318",
> +    .post_load = diag318_post_load,
> +    .pre_save = diag318_pre_save,
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = diag318_needed,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT64(cpc, Diag318Data),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +void diag318_register(void)
> +{
> +    vmstate_register(NULL, 0, &vmstate_diag318, &diag318data);
> +}
> \ No newline at end of file
> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
> index 70015ea..a3d1457 100644
> --- a/target/s390x/gen-features.c
> +++ b/target/s390x/gen-features.c
> @@ -450,6 +450,7 @@ static uint16_t full_GEN12_GA1[] = {
>      S390_FEAT_AP_QUERY_CONFIG_INFO,
>      S390_FEAT_AP_FACILITIES_TEST,
>      S390_FEAT_AP,
> +    S390_FEAT_DIAG318,
>  };
>  
>  static uint16_t full_GEN12_GA2[] = {
> diff --git a/target/s390x/internal.h b/target/s390x/internal.h
> index f2a771e..d53b1d0 100644
> --- a/target/s390x/internal.h
> +++ b/target/s390x/internal.h
> @@ -383,10 +383,12 @@ int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw,
>                         target_ulong *addr, int *flags);
>  
>  
> -/* misc_helper.c */
> +/* diag.c */
>  int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3);
>  void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3,
>                       uintptr_t ra);
> +void diag318_register(void);
> +void diag318_reset(void);
>  
>  
>  /* translate.c */
> diff --git a/target/s390x/kvm-stub.c b/target/s390x/kvm-stub.c
> index bf7795e..1f0bee3 100644
> --- a/target/s390x/kvm-stub.c
> +++ b/target/s390x/kvm-stub.c
> @@ -104,3 +104,13 @@ void kvm_s390_stop_interrupt(S390CPU *cpu)
>  void kvm_s390_restart_interrupt(S390CPU *cpu)
>  {
>  }
> +
> +int kvm_s390_get_cpc(uint64_t *cpc)
> +{
> +    return 0;
> +}
> +
> +int kvm_s390_set_cpc(uint64_t cpc)
> +{
> +    return 0;
> +}
> \ No newline at end of file
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 2ebf26a..9ed4cea 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -749,6 +749,28 @@ int kvm_s390_set_clock_ext(uint8_t tod_high, uint64_t tod_low)
>      return kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attr);
>  }
>  
> +int kvm_s390_get_cpc(uint64_t *cpc)
> +{
> +    struct kvm_device_attr attr = {
> +        .group = KVM_S390_VM_MISC,
> +        .attr = KVM_S390_VM_MISC_CPC,
> +        .addr = (uint64_t)cpc,
> +    };
> +
> +    return kvm_vm_ioctl(kvm_state, KVM_GET_DEVICE_ATTR, &attr);
> +}
> +
> +int kvm_s390_set_cpc(uint64_t cpc)
> +{
> +    struct kvm_device_attr attr = {
> +        .group = KVM_S390_VM_MISC,
> +        .attr = KVM_S390_VM_MISC_CPC,
> +        .addr = (uint64_t)&cpc,
> +    };
> +
> +    return kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attr);
> +}
> +
>  /**
>   * kvm_s390_mem_op:
>   * @addr:      the logical start address in guest memory
> @@ -2142,6 +2164,7 @@ static int kvm_to_feat[][2] = {
>      { KVM_S390_VM_CPU_FEAT_PFMFI, S390_FEAT_SIE_PFMFI},
>      { KVM_S390_VM_CPU_FEAT_SIGPIF, S390_FEAT_SIE_SIGPIF},
>      { KVM_S390_VM_CPU_FEAT_KSS, S390_FEAT_SIE_KSS},
> +    { KVM_S390_VM_CPU_FEAT_DIAG318, S390_FEAT_DIAG318},
>  };
>  
>  static int query_cpu_feat(S390FeatBitmap features)
> diff --git a/target/s390x/kvm_s390x.h b/target/s390x/kvm_s390x.h
> index 6e52287..53f165f 100644
> --- a/target/s390x/kvm_s390x.h
> +++ b/target/s390x/kvm_s390x.h
> @@ -29,6 +29,9 @@ int kvm_s390_get_clock(uint8_t *tod_high, uint64_t *tod_clock);
>  int kvm_s390_get_clock_ext(uint8_t *tod_high, uint64_t *tod_clock);
>  int kvm_s390_set_clock(uint8_t tod_high, uint64_t tod_clock);
>  int kvm_s390_set_clock_ext(uint8_t tod_high, uint64_t tod_clock);
> +int kvm_s390_get_cpc(uint64_t *cpc);
> +int kvm_s390_set_cpc(uint64_t cpc);
> +bool kvm_s390_has_diag318(void);
>  void kvm_s390_enable_css_support(S390CPU *cpu);
>  int kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch,
>                                      int vq, bool assign);
>

[e-mail] Dec. 4, 2018, 11:50 p.m. UTC | #2

Patchew URL: https://patchew.org/QEMU/[e-mail]/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Subject: [Qemu-devel] [PATCH v1] s390: guest support for diagnose 318 and limit max VCPUs to 247
Message-id: [e-mail]

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
b5218a7 s390: guest support for diagnose 318 and limit max VCPUs to 247

=== OUTPUT BEGIN ===
Checking PATCH 1/1: s390: guest support for diagnose 318 and limit max VCPUs to 247...
ERROR: adding a line without newline at end of file
#243: FILE: target/s390x/diag.c:189:
+}

ERROR: adding a line without newline at end of file
#292: FILE: target/s390x/kvm-stub.c:116:
+}

total: 2 errors, 0 warnings, 247 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/[e-mail]/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to [e-mail]

David Hildenbrand Dec. 5, 2018, 8:26 a.m. UTC | #3

On 04.12.18 23:18, Collin Walling wrote:
> Add migration and reset support for diagnose 318. This is a new z14 GA2 
> hardware feature, but we can provide guest support starting with the 
> zEC12-full CPU model.
> 
> Because new hardware introduces a new facility-availability byte in 
> the Read SCP Info block, we lose one byte in the CPU entries list 
> and must limit the maximum VCPUs to 247 (down from 248).

This could break setups that upgrade/migrate. At least forward migration
can be broken. Do we care about that?

Can you split off

1. linux-header changes
2. CPU model changes? (introduction and definition of new feature, but
not when it is used?)

> 
> Signed-off-by: Collin Walling <[e-mail]>
> ---
> 
> Changelog
> 
>     RFC -> v1
>         - introduced kvm stubs for set/get cpc
>         - s/fac134/byte_134
>         - moved diag318 vmstate description to diag.c
>         - reduced S390_VCPU_MAX to 247
> 
>  hw/s390x/ipl.c                  |  3 +++
>  hw/s390x/s390-virtio-ccw.c      |  3 +++
>  hw/s390x/sclp.c                 |  2 ++
>  include/hw/s390x/sclp.h         |  2 ++
>  linux-headers/asm-s390/kvm.h    |  5 ++++
>  target/s390x/cpu.h              |  2 +-
>  target/s390x/cpu_features.c     |  3 +++
>  target/s390x/cpu_features.h     |  1 +
>  target/s390x/cpu_features_def.h |  3 +++
>  target/s390x/diag.c             | 53 +++++++++++++++++++++++++++++++++++++++++
>  target/s390x/gen-features.c     |  1 +
>  target/s390x/internal.h         |  4 +++-
>  target/s390x/kvm-stub.c         | 10 ++++++++
>  target/s390x/kvm.c              | 23 ++++++++++++++++++
>  target/s390x/kvm_s390x.h        |  3 +++
>  15 files changed, 116 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index 21f64ad..6e3af65 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -27,6 +27,7 @@
>  #include "qemu/cutils.h"
>  #include "qemu/option.h"
>  #include "exec/exec-all.h"
> +#include "internal.h"
>  
>  #define KERN_IMAGE_START                0x010000UL
>  #define LINUX_MAGIC_ADDR                0x010008UL
> @@ -230,6 +231,8 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
>      ipl->compat_start_addr = ipl->start_addr;
>      ipl->compat_bios_start_addr = ipl->bios_start_addr;
>      qemu_register_reset(qdev_reset_all_fn, dev);
> +
> +    diag318_register();
>  error:
>      error_propagate(errp, err);
>  }
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index a0615a8..2c670fc 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -37,6 +37,7 @@
>  #include "cpu_models.h"
>  #include "hw/nmi.h"
>  #include "hw/s390x/tod.h"
> +#include "internal.h"
>  
>  S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
>  {
> @@ -357,6 +358,7 @@ static void s390_machine_reset(void)
>          }
>          subsystem_reset();
>          s390_crypto_reset();
> +        diag318_reset();
>          run_on_cpu(cs, s390_do_cpu_load_normal, RUN_ON_CPU_NULL);
>          break;
>      case S390_RESET_LOAD_NORMAL:
> @@ -364,6 +366,7 @@ static void s390_machine_reset(void)
>              run_on_cpu(t, s390_do_cpu_reset, RUN_ON_CPU_NULL);
>          }
>          subsystem_reset();
> +        diag318_reset();
>          run_on_cpu(cs, s390_do_cpu_initial_reset, RUN_ON_CPU_NULL);
>          run_on_cpu(cs, s390_do_cpu_load_normal, RUN_ON_CPU_NULL);
>          break;

Why don't we have to reset this during reipl or external reset
("system_reset")?


> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index 4510a80..183c627 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -73,6 +73,8 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>                           read_info->conf_char);
>      s390_get_feat_block(S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT,
>                           read_info->conf_char_ext);
> +    /* Read Info byte 134 */
> +    s390_get_feat_block(S390_FEAT_TYPE_SCLP_BYTE_134, read_info->byte_134);
>  
>      read_info->facilities = cpu_to_be64(SCLP_HAS_CPU_INFO |
>                                          SCLP_HAS_IOA_RECONFIG);
> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
> index f9db243..eb12ba2 100644
> --- a/include/hw/s390x/sclp.h
> +++ b/include/hw/s390x/sclp.h
> @@ -133,6 +133,8 @@ typedef struct ReadInfo {
>      uint16_t highest_cpu;
>      uint8_t  _reserved5[124 - 122];     /* 122-123 */
>      uint32_t hmfai;
> +    uint8_t  _reserved7[134 - 128];     /* 128-133 */
> +    uint8_t  byte_134[1];
>      struct CPUEntry entries[0];
>  } QEMU_PACKED ReadInfo;
>  
> diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
> index 0265482..8c206d2 100644
> --- a/linux-headers/asm-s390/kvm.h
> +++ b/linux-headers/asm-s390/kvm.h
> @@ -74,6 +74,7 @@ struct kvm_s390_io_adapter_req {
>  #define KVM_S390_VM_CRYPTO		2
>  #define KVM_S390_VM_CPU_MODEL		3
>  #define KVM_S390_VM_MIGRATION		4
> +#define KVM_S390_VM_MISC		5
>  
>  /* kvm attributes for mem_ctrl */
>  #define KVM_S390_VM_MEM_ENABLE_CMMA	0
> @@ -130,6 +131,7 @@ struct kvm_s390_vm_cpu_machine {
>  #define KVM_S390_VM_CPU_FEAT_PFMFI	11
>  #define KVM_S390_VM_CPU_FEAT_SIGPIF	12
>  #define KVM_S390_VM_CPU_FEAT_KSS	13
> +#define KVM_S390_VM_CPU_FEAT_DIAG318	14
>  struct kvm_s390_vm_cpu_feat {
>  	__u64 feat[16];
>  };
> @@ -168,6 +170,9 @@ struct kvm_s390_vm_cpu_subfunc {
>  #define KVM_S390_VM_MIGRATION_START	1
>  #define KVM_S390_VM_MIGRATION_STATUS	2
>  
> +/* kvm attributes for KVM_S390_VM_MISC */
> +#define KVM_S390_VM_MISC_CPC		0
> +
>  /* for KVM_GET_REGS and KVM_SET_REGS */
>  struct kvm_regs {
>  	/* general purpose regs for s390 */
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index 8c2320e..594b4a4 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -52,7 +52,7 @@
>  
>  #define MMU_USER_IDX 0
>  
> -#define S390_MAX_CPUS 248
> +#define S390_MAX_CPUS 247
>  
>  typedef struct PSW {
>      uint64_t mask;
> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
> index 60cfeba..d05afa5 100644
> --- a/target/s390x/cpu_features.c
> +++ b/target/s390x/cpu_features.c
> @@ -121,6 +121,9 @@ static const S390FeatDef s390_features[] = {
>      FEAT_INIT("pfmfi", S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT, 9, "SIE: PFMF interpretation facility"),
>      FEAT_INIT("ibs", S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT, 10, "SIE: Interlock-and-broadcast-suppression facility"),
>  
> +    /* SCLP SCCB Byte 134 */
> +    FEAT_INIT("diag318", S390_FEAT_TYPE_SCLP_BYTE_134, 0, "SIE: Diagnose 318"),

Is this bit "SIE: Diagnose 318" or "SIE: SCLP Byte 14" ?

Or both?

> +
>      FEAT_INIT("sief2", S390_FEAT_TYPE_SCLP_CPU, 4, "SIE: interception format 2 (Virtual SIE)"),
>      FEAT_INIT("skey", S390_FEAT_TYPE_SCLP_CPU, 5, "SIE: Storage-key facility"),
>      FEAT_INIT("gpereh", S390_FEAT_TYPE_SCLP_CPU, 10, "SIE: Guest-PER enhancement facility"),
> diff --git a/target/s390x/cpu_features.h b/target/s390x/cpu_features.h
> index effe790..e7248df 100644
> --- a/target/s390x/cpu_features.h
> +++ b/target/s390x/cpu_features.h
> @@ -23,6 +23,7 @@ typedef enum {
>      S390_FEAT_TYPE_STFL,
>      S390_FEAT_TYPE_SCLP_CONF_CHAR,
>      S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT,
> +    S390_FEAT_TYPE_SCLP_BYTE_134,
>      S390_FEAT_TYPE_SCLP_CPU,
>      S390_FEAT_TYPE_MISC,
>      S390_FEAT_TYPE_PLO,
> diff --git a/target/s390x/cpu_features_def.h b/target/s390x/cpu_features_def.h
> index 5fc7e7b..d99da1d 100644
> --- a/target/s390x/cpu_features_def.h
> +++ b/target/s390x/cpu_features_def.h
> @@ -109,6 +109,9 @@ typedef enum {
>      S390_FEAT_SIE_PFMFI,
>      S390_FEAT_SIE_IBS,
>  
> +    /* Read Info Byte 134 */
> +    S390_FEAT_DIAG318,
> +
>      /* Sclp Cpu */
>      S390_FEAT_SIE_F2,
>      S390_FEAT_SIE_SKEY,
> diff --git a/target/s390x/diag.c b/target/s390x/diag.c
> index acb0f3d..2207c08 100644
> --- a/target/s390x/diag.c
> +++ b/target/s390x/diag.c
> @@ -20,6 +20,8 @@
>  #include "sysemu/cpus.h"
>  #include "hw/s390x/ipl.h"
>  #include "hw/s390x/s390-virtio-ccw.h"
> +#include "kvm_s390x.h"
> +#include "sysemu/kvm.h"
>  
>  int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3)
>  {
> @@ -134,3 +136,54 @@ out:
>          break;
>      }
>  }
> +
> +typedef struct Diag318Data {
> +    uint64_t cpc;
> +} Diag318Data;
> +static Diag318Data diag318data;
> +
> +void diag318_reset(void)

please name this function

s390_diag318_reset()

or even (my preference, as we actually reset the CPC value)

s390_cpc_reset()

> +{
> +    if (s390_has_feat(S390_FEAT_DIAG318)) {
> +        kvm_s390_set_cpc(0);
> +    }
> +}
> +
> +static int diag318_post_load(void *opaque, int version_id)
> +{
> +    Diag318Data *d = opaque;
> +
> +    kvm_s390_set_cpc(d->cpc);
> +    return 0;
> +}
> +
> +static int diag318_pre_save(void *opaque)
> +{
> +    Diag318Data *d = opaque;
> +
> +    kvm_s390_get_cpc(&d->cpc);
> +    return 0;
> +}
> +
> +static bool diag318_needed(void *opaque)
> +{
> +    return s390_has_feat(S390_FEAT_DIAG318);
> +}
> +
> +const VMStateDescription vmstate_diag318 = {
> +    .name = "diag318",
> +    .post_load = diag318_post_load,
> +    .pre_save = diag318_pre_save,
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = diag318_needed,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT64(cpc, Diag318Data),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +void diag318_register(void)

That somehow does not feel right nor look nice.

This code smells like it should go into hw/s390x/ ... as it is a machine
specific property. Registering the vmstate can than be done in a nicer
way than via this function.

> +{
> +    vmstate_register(NULL, 0, &vmstate_diag318, &diag318data);
> +}
> \ No newline at end of file
> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
> index 70015ea..a3d1457 100644
> --- a/target/s390x/gen-features.c
> +++ b/target/s390x/gen-features.c
> @@ -450,6 +450,7 @@ static uint16_t full_GEN12_GA1[] = {
>      S390_FEAT_AP_QUERY_CONFIG_INFO,
>      S390_FEAT_AP_FACILITIES_TEST,
>      S390_FEAT_AP,
> +    S390_FEAT_DIAG318,
>  };
>  
>  static uint16_t full_GEN12_GA2[] = {
> diff --git a/target/s390x/internal.h b/target/s390x/internal.h
> index f2a771e..d53b1d0 100644
> --- a/target/s390x/internal.h
> +++ b/target/s390x/internal.h
> @@ -383,10 +383,12 @@ int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw,
>                         target_ulong *addr, int *flags);
>  
>  
> -/* misc_helper.c */
> +/* diag.c */
>  int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3);
>  void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3,
>                       uintptr_t ra);
> +void diag318_register(void);
> +void diag318_reset(void);
>  
>  
>  /* translate.c */
> diff --git a/target/s390x/kvm-stub.c b/target/s390x/kvm-stub.c
> index bf7795e..1f0bee3 100644
> --- a/target/s390x/kvm-stub.c
> +++ b/target/s390x/kvm-stub.c
> @@ -104,3 +104,13 @@ void kvm_s390_stop_interrupt(S390CPU *cpu)
>  void kvm_s390_restart_interrupt(S390CPU *cpu)
>  {
>  }
> +
> +int kvm_s390_get_cpc(uint64_t *cpc)
> +{
> +    return 0;
> +}
> +
> +int kvm_s390_set_cpc(uint64_t cpc)
> +{
> +    return 0;
> +}
> \ No newline at end of file
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 2ebf26a..9ed4cea 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -749,6 +749,28 @@ int kvm_s390_set_clock_ext(uint8_t tod_high, uint64_t tod_low)
>      return kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attr);
>  }
>  
> +int kvm_s390_get_cpc(uint64_t *cpc)
> +{
> +    struct kvm_device_attr attr = {
> +        .group = KVM_S390_VM_MISC,
> +        .attr = KVM_S390_VM_MISC_CPC,
> +        .addr = (uint64_t)cpc,
> +    };
> +
> +    return kvm_vm_ioctl(kvm_state, KVM_GET_DEVICE_ATTR, &attr);
> +}
> +
> +int kvm_s390_set_cpc(uint64_t cpc)
> +{
> +    struct kvm_device_attr attr = {
> +        .group = KVM_S390_VM_MISC,
> +        .attr = KVM_S390_VM_MISC_CPC,
> +        .addr = (uint64_t)&cpc,
> +    };
> +
> +    return kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attr);
> +}
> +
>  /**
>   * kvm_s390_mem_op:
>   * @addr:      the logical start address in guest memory
> @@ -2142,6 +2164,7 @@ static int kvm_to_feat[][2] = {
>      { KVM_S390_VM_CPU_FEAT_PFMFI, S390_FEAT_SIE_PFMFI},
>      { KVM_S390_VM_CPU_FEAT_SIGPIF, S390_FEAT_SIE_SIGPIF},
>      { KVM_S390_VM_CPU_FEAT_KSS, S390_FEAT_SIE_KSS},
> +    { KVM_S390_VM_CPU_FEAT_DIAG318, S390_FEAT_DIAG318},
>  };
>  
>  static int query_cpu_feat(S390FeatBitmap features)
> diff --git a/target/s390x/kvm_s390x.h b/target/s390x/kvm_s390x.h
> index 6e52287..53f165f 100644
> --- a/target/s390x/kvm_s390x.h
> +++ b/target/s390x/kvm_s390x.h
> @@ -29,6 +29,9 @@ int kvm_s390_get_clock(uint8_t *tod_high, uint64_t *tod_clock);
>  int kvm_s390_get_clock_ext(uint8_t *tod_high, uint64_t *tod_clock);
>  int kvm_s390_set_clock(uint8_t tod_high, uint64_t tod_clock);
>  int kvm_s390_set_clock_ext(uint8_t tod_high, uint64_t tod_clock);
> +int kvm_s390_get_cpc(uint64_t *cpc);
> +int kvm_s390_set_cpc(uint64_t cpc);
> +bool kvm_s390_has_diag318(void);
>  void kvm_s390_enable_css_support(S390CPU *cpu);
>  int kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch,
>                                      int vq, bool assign);
>

Christian Borntraeger Dec. 5, 2018, 8:27 a.m. UTC | #4

On 05.12.2018 09:26, David Hildenbrand wrote:
> On 04.12.18 23:18, Collin Walling wrote:
>> Add migration and reset support for diagnose 318. This is a new z14 GA2 
>> hardware feature, but we can provide guest support starting with the 
>> zEC12-full CPU model.
>>
>> Because new hardware introduces a new facility-availability byte in 
>> the Read SCP Info block, we lose one byte in the CPU entries list 
>> and must limit the maximum VCPUs to 247 (down from 248).
> 
> This could break setups that upgrade/migrate. At least forward migration
> can be broken. Do we care about that?

Can we maybe bind this feature and the cpu limit to the 4.0 machine?
> 
> Can you split off
> 
> 1. linux-header changes
> 2. CPU model changes? (introduction and definition of new feature, but
> not when it is used?)
> 
>>
>> Signed-off-by: Collin Walling <[e-mail]>
>> ---
>>
>> Changelog
>>
>>     RFC -> v1
>>         - introduced kvm stubs for set/get cpc
>>         - s/fac134/byte_134
>>         - moved diag318 vmstate description to diag.c
>>         - reduced S390_VCPU_MAX to 247
>>
>>  hw/s390x/ipl.c                  |  3 +++
>>  hw/s390x/s390-virtio-ccw.c      |  3 +++
>>  hw/s390x/sclp.c                 |  2 ++
>>  include/hw/s390x/sclp.h         |  2 ++
>>  linux-headers/asm-s390/kvm.h    |  5 ++++
>>  target/s390x/cpu.h              |  2 +-
>>  target/s390x/cpu_features.c     |  3 +++
>>  target/s390x/cpu_features.h     |  1 +
>>  target/s390x/cpu_features_def.h |  3 +++
>>  target/s390x/diag.c             | 53 +++++++++++++++++++++++++++++++++++++++++
>>  target/s390x/gen-features.c     |  1 +
>>  target/s390x/internal.h         |  4 +++-
>>  target/s390x/kvm-stub.c         | 10 ++++++++
>>  target/s390x/kvm.c              | 23 ++++++++++++++++++
>>  target/s390x/kvm_s390x.h        |  3 +++
>>  15 files changed, 116 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
>> index 21f64ad..6e3af65 100644
>> --- a/hw/s390x/ipl.c
>> +++ b/hw/s390x/ipl.c
>> @@ -27,6 +27,7 @@
>>  #include "qemu/cutils.h"
>>  #include "qemu/option.h"
>>  #include "exec/exec-all.h"
>> +#include "internal.h"
>>  
>>  #define KERN_IMAGE_START                0x010000UL
>>  #define LINUX_MAGIC_ADDR                0x010008UL
>> @@ -230,6 +231,8 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
>>      ipl->compat_start_addr = ipl->start_addr;
>>      ipl->compat_bios_start_addr = ipl->bios_start_addr;
>>      qemu_register_reset(qdev_reset_all_fn, dev);
>> +
>> +    diag318_register();
>>  error:
>>      error_propagate(errp, err);
>>  }
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index a0615a8..2c670fc 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -37,6 +37,7 @@
>>  #include "cpu_models.h"
>>  #include "hw/nmi.h"
>>  #include "hw/s390x/tod.h"
>> +#include "internal.h"
>>  
>>  S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
>>  {
>> @@ -357,6 +358,7 @@ static void s390_machine_reset(void)
>>          }
>>          subsystem_reset();
>>          s390_crypto_reset();
>> +        diag318_reset();
>>          run_on_cpu(cs, s390_do_cpu_load_normal, RUN_ON_CPU_NULL);
>>          break;
>>      case S390_RESET_LOAD_NORMAL:
>> @@ -364,6 +366,7 @@ static void s390_machine_reset(void)
>>              run_on_cpu(t, s390_do_cpu_reset, RUN_ON_CPU_NULL);
>>          }
>>          subsystem_reset();
>> +        diag318_reset();
>>          run_on_cpu(cs, s390_do_cpu_initial_reset, RUN_ON_CPU_NULL);
>>          run_on_cpu(cs, s390_do_cpu_load_normal, RUN_ON_CPU_NULL);
>>          break;
> 
> Why don't we have to reset this during reipl or external reset
> ("system_reset")?
> 
> 
>> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
>> index 4510a80..183c627 100644
>> --- a/hw/s390x/sclp.c
>> +++ b/hw/s390x/sclp.c
>> @@ -73,6 +73,8 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>>                           read_info->conf_char);
>>      s390_get_feat_block(S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT,
>>                           read_info->conf_char_ext);
>> +    /* Read Info byte 134 */
>> +    s390_get_feat_block(S390_FEAT_TYPE_SCLP_BYTE_134, read_info->byte_134);
>>  
>>      read_info->facilities = cpu_to_be64(SCLP_HAS_CPU_INFO |
>>                                          SCLP_HAS_IOA_RECONFIG);
>> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
>> index f9db243..eb12ba2 100644
>> --- a/include/hw/s390x/sclp.h
>> +++ b/include/hw/s390x/sclp.h
>> @@ -133,6 +133,8 @@ typedef struct ReadInfo {
>>      uint16_t highest_cpu;
>>      uint8_t  _reserved5[124 - 122];     /* 122-123 */
>>      uint32_t hmfai;
>> +    uint8_t  _reserved7[134 - 128];     /* 128-133 */
>> +    uint8_t  byte_134[1];
>>      struct CPUEntry entries[0];
>>  } QEMU_PACKED ReadInfo;
>>  
>> diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
>> index 0265482..8c206d2 100644
>> --- a/linux-headers/asm-s390/kvm.h
>> +++ b/linux-headers/asm-s390/kvm.h
>> @@ -74,6 +74,7 @@ struct kvm_s390_io_adapter_req {
>>  #define KVM_S390_VM_CRYPTO		2
>>  #define KVM_S390_VM_CPU_MODEL		3
>>  #define KVM_S390_VM_MIGRATION		4
>> +#define KVM_S390_VM_MISC		5
>>  
>>  /* kvm attributes for mem_ctrl */
>>  #define KVM_S390_VM_MEM_ENABLE_CMMA	0
>> @@ -130,6 +131,7 @@ struct kvm_s390_vm_cpu_machine {
>>  #define KVM_S390_VM_CPU_FEAT_PFMFI	11
>>  #define KVM_S390_VM_CPU_FEAT_SIGPIF	12
>>  #define KVM_S390_VM_CPU_FEAT_KSS	13
>> +#define KVM_S390_VM_CPU_FEAT_DIAG318	14
>>  struct kvm_s390_vm_cpu_feat {
>>  	__u64 feat[16];
>>  };
>> @@ -168,6 +170,9 @@ struct kvm_s390_vm_cpu_subfunc {
>>  #define KVM_S390_VM_MIGRATION_START	1
>>  #define KVM_S390_VM_MIGRATION_STATUS	2
>>  
>> +/* kvm attributes for KVM_S390_VM_MISC */
>> +#define KVM_S390_VM_MISC_CPC		0
>> +
>>  /* for KVM_GET_REGS and KVM_SET_REGS */
>>  struct kvm_regs {
>>  	/* general purpose regs for s390 */
>> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
>> index 8c2320e..594b4a4 100644
>> --- a/target/s390x/cpu.h
>> +++ b/target/s390x/cpu.h
>> @@ -52,7 +52,7 @@
>>  
>>  #define MMU_USER_IDX 0
>>  
>> -#define S390_MAX_CPUS 248
>> +#define S390_MAX_CPUS 247
>>  
>>  typedef struct PSW {
>>      uint64_t mask;
>> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
>> index 60cfeba..d05afa5 100644
>> --- a/target/s390x/cpu_features.c
>> +++ b/target/s390x/cpu_features.c
>> @@ -121,6 +121,9 @@ static const S390FeatDef s390_features[] = {
>>      FEAT_INIT("pfmfi", S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT, 9, "SIE: PFMF interpretation facility"),
>>      FEAT_INIT("ibs", S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT, 10, "SIE: Interlock-and-broadcast-suppression facility"),
>>  
>> +    /* SCLP SCCB Byte 134 */
>> +    FEAT_INIT("diag318", S390_FEAT_TYPE_SCLP_BYTE_134, 0, "SIE: Diagnose 318"),
> 
> Is this bit "SIE: Diagnose 318" or "SIE: SCLP Byte 14" ?
> 
> Or both?
> 
>> +
>>      FEAT_INIT("sief2", S390_FEAT_TYPE_SCLP_CPU, 4, "SIE: interception format 2 (Virtual SIE)"),
>>      FEAT_INIT("skey", S390_FEAT_TYPE_SCLP_CPU, 5, "SIE: Storage-key facility"),
>>      FEAT_INIT("gpereh", S390_FEAT_TYPE_SCLP_CPU, 10, "SIE: Guest-PER enhancement facility"),
>> diff --git a/target/s390x/cpu_features.h b/target/s390x/cpu_features.h
>> index effe790..e7248df 100644
>> --- a/target/s390x/cpu_features.h
>> +++ b/target/s390x/cpu_features.h
>> @@ -23,6 +23,7 @@ typedef enum {
>>      S390_FEAT_TYPE_STFL,
>>      S390_FEAT_TYPE_SCLP_CONF_CHAR,
>>      S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT,
>> +    S390_FEAT_TYPE_SCLP_BYTE_134,
>>      S390_FEAT_TYPE_SCLP_CPU,
>>      S390_FEAT_TYPE_MISC,
>>      S390_FEAT_TYPE_PLO,
>> diff --git a/target/s390x/cpu_features_def.h b/target/s390x/cpu_features_def.h
>> index 5fc7e7b..d99da1d 100644
>> --- a/target/s390x/cpu_features_def.h
>> +++ b/target/s390x/cpu_features_def.h
>> @@ -109,6 +109,9 @@ typedef enum {
>>      S390_FEAT_SIE_PFMFI,
>>      S390_FEAT_SIE_IBS,
>>  
>> +    /* Read Info Byte 134 */
>> +    S390_FEAT_DIAG318,
>> +
>>      /* Sclp Cpu */
>>      S390_FEAT_SIE_F2,
>>      S390_FEAT_SIE_SKEY,
>> diff --git a/target/s390x/diag.c b/target/s390x/diag.c
>> index acb0f3d..2207c08 100644
>> --- a/target/s390x/diag.c
>> +++ b/target/s390x/diag.c
>> @@ -20,6 +20,8 @@
>>  #include "sysemu/cpus.h"
>>  #include "hw/s390x/ipl.h"
>>  #include "hw/s390x/s390-virtio-ccw.h"
>> +#include "kvm_s390x.h"
>> +#include "sysemu/kvm.h"
>>  
>>  int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3)
>>  {
>> @@ -134,3 +136,54 @@ out:
>>          break;
>>      }
>>  }
>> +
>> +typedef struct Diag318Data {
>> +    uint64_t cpc;
>> +} Diag318Data;
>> +static Diag318Data diag318data;
>> +
>> +void diag318_reset(void)
> 
> please name this function
> 
> s390_diag318_reset()
> 
> or even (my preference, as we actually reset the CPC value)
> 
> s390_cpc_reset()
> 
>> +{
>> +    if (s390_has_feat(S390_FEAT_DIAG318)) {
>> +        kvm_s390_set_cpc(0);
>> +    }
>> +}
>> +
>> +static int diag318_post_load(void *opaque, int version_id)
>> +{
>> +    Diag318Data *d = opaque;
>> +
>> +    kvm_s390_set_cpc(d->cpc);
>> +    return 0;
>> +}
>> +
>> +static int diag318_pre_save(void *opaque)
>> +{
>> +    Diag318Data *d = opaque;
>> +
>> +    kvm_s390_get_cpc(&d->cpc);
>> +    return 0;
>> +}
>> +
>> +static bool diag318_needed(void *opaque)
>> +{
>> +    return s390_has_feat(S390_FEAT_DIAG318);
>> +}
>> +
>> +const VMStateDescription vmstate_diag318 = {
>> +    .name = "diag318",
>> +    .post_load = diag318_post_load,
>> +    .pre_save = diag318_pre_save,
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .needed = diag318_needed,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_UINT64(cpc, Diag318Data),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>> +void diag318_register(void)
> 
> That somehow does not feel right nor look nice.
> 
> This code smells like it should go into hw/s390x/ ... as it is a machine
> specific property. Registering the vmstate can than be done in a nicer
> way than via this function.
> 
>> +{
>> +    vmstate_register(NULL, 0, &vmstate_diag318, &diag318data);
>> +}
>> \ No newline at end of file
>> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
>> index 70015ea..a3d1457 100644
>> --- a/target/s390x/gen-features.c
>> +++ b/target/s390x/gen-features.c
>> @@ -450,6 +450,7 @@ static uint16_t full_GEN12_GA1[] = {
>>      S390_FEAT_AP_QUERY_CONFIG_INFO,
>>      S390_FEAT_AP_FACILITIES_TEST,
>>      S390_FEAT_AP,
>> +    S390_FEAT_DIAG318,
>>  };
>>  
>>  static uint16_t full_GEN12_GA2[] = {
>> diff --git a/target/s390x/internal.h b/target/s390x/internal.h
>> index f2a771e..d53b1d0 100644
>> --- a/target/s390x/internal.h
>> +++ b/target/s390x/internal.h
>> @@ -383,10 +383,12 @@ int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw,
>>                         target_ulong *addr, int *flags);
>>  
>>  
>> -/* misc_helper.c */
>> +/* diag.c */
>>  int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3);
>>  void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3,
>>                       uintptr_t ra);
>> +void diag318_register(void);
>> +void diag318_reset(void);
>>  
>>  
>>  /* translate.c */
>> diff --git a/target/s390x/kvm-stub.c b/target/s390x/kvm-stub.c
>> index bf7795e..1f0bee3 100644
>> --- a/target/s390x/kvm-stub.c
>> +++ b/target/s390x/kvm-stub.c
>> @@ -104,3 +104,13 @@ void kvm_s390_stop_interrupt(S390CPU *cpu)
>>  void kvm_s390_restart_interrupt(S390CPU *cpu)
>>  {
>>  }
>> +
>> +int kvm_s390_get_cpc(uint64_t *cpc)
>> +{
>> +    return 0;
>> +}
>> +
>> +int kvm_s390_set_cpc(uint64_t cpc)
>> +{
>> +    return 0;
>> +}
>> \ No newline at end of file
>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>> index 2ebf26a..9ed4cea 100644
>> --- a/target/s390x/kvm.c
>> +++ b/target/s390x/kvm.c
>> @@ -749,6 +749,28 @@ int kvm_s390_set_clock_ext(uint8_t tod_high, uint64_t tod_low)
>>      return kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attr);
>>  }
>>  
>> +int kvm_s390_get_cpc(uint64_t *cpc)
>> +{
>> +    struct kvm_device_attr attr = {
>> +        .group = KVM_S390_VM_MISC,
>> +        .attr = KVM_S390_VM_MISC_CPC,
>> +        .addr = (uint64_t)cpc,
>> +    };
>> +
>> +    return kvm_vm_ioctl(kvm_state, KVM_GET_DEVICE_ATTR, &attr);
>> +}
>> +
>> +int kvm_s390_set_cpc(uint64_t cpc)
>> +{
>> +    struct kvm_device_attr attr = {
>> +        .group = KVM_S390_VM_MISC,
>> +        .attr = KVM_S390_VM_MISC_CPC,
>> +        .addr = (uint64_t)&cpc,
>> +    };
>> +
>> +    return kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attr);
>> +}
>> +
>>  /**
>>   * kvm_s390_mem_op:
>>   * @addr:      the logical start address in guest memory
>> @@ -2142,6 +2164,7 @@ static int kvm_to_feat[][2] = {
>>      { KVM_S390_VM_CPU_FEAT_PFMFI, S390_FEAT_SIE_PFMFI},
>>      { KVM_S390_VM_CPU_FEAT_SIGPIF, S390_FEAT_SIE_SIGPIF},
>>      { KVM_S390_VM_CPU_FEAT_KSS, S390_FEAT_SIE_KSS},
>> +    { KVM_S390_VM_CPU_FEAT_DIAG318, S390_FEAT_DIAG318},
>>  };
>>  
>>  static int query_cpu_feat(S390FeatBitmap features)
>> diff --git a/target/s390x/kvm_s390x.h b/target/s390x/kvm_s390x.h
>> index 6e52287..53f165f 100644
>> --- a/target/s390x/kvm_s390x.h
>> +++ b/target/s390x/kvm_s390x.h
>> @@ -29,6 +29,9 @@ int kvm_s390_get_clock(uint8_t *tod_high, uint64_t *tod_clock);
>>  int kvm_s390_get_clock_ext(uint8_t *tod_high, uint64_t *tod_clock);
>>  int kvm_s390_set_clock(uint8_t tod_high, uint64_t tod_clock);
>>  int kvm_s390_set_clock_ext(uint8_t tod_high, uint64_t tod_clock);
>> +int kvm_s390_get_cpc(uint64_t *cpc);
>> +int kvm_s390_set_cpc(uint64_t cpc);
>> +bool kvm_s390_has_diag318(void);
>>  void kvm_s390_enable_css_support(S390CPU *cpu);
>>  int kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch,
>>                                      int vq, bool assign);
>>
> 
>

Christian Borntraeger Dec. 5, 2018, 8:29 a.m. UTC | #5

You should clearly review your email list.....

Adding the "new" Conny, removing Carsten.


On 04.12.2018 23:18, Collin Walling wrote:
> Add migration and reset support for diagnose 318. This is a new z14 GA2 
> hardware feature, but we can provide guest support starting with the 
> zEC12-full CPU model.
> 
> Because new hardware introduces a new facility-availability byte in 
> the Read SCP Info block, we lose one byte in the CPU entries list 
> and must limit the maximum VCPUs to 247 (down from 248).
> 
> Signed-off-by: Collin Walling <[e-mail]>
> ---
> 
> Changelog
> 
>     RFC -> v1
>         - introduced kvm stubs for set/get cpc
>         - s/fac134/byte_134
>         - moved diag318 vmstate description to diag.c
>         - reduced S390_VCPU_MAX to 247
> 
>  hw/s390x/ipl.c                  |  3 +++
>  hw/s390x/s390-virtio-ccw.c      |  3 +++
>  hw/s390x/sclp.c                 |  2 ++
>  include/hw/s390x/sclp.h         |  2 ++
>  linux-headers/asm-s390/kvm.h    |  5 ++++
>  target/s390x/cpu.h              |  2 +-
>  target/s390x/cpu_features.c     |  3 +++
>  target/s390x/cpu_features.h     |  1 +
>  target/s390x/cpu_features_def.h |  3 +++
>  target/s390x/diag.c             | 53 +++++++++++++++++++++++++++++++++++++++++
>  target/s390x/gen-features.c     |  1 +
>  target/s390x/internal.h         |  4 +++-
>  target/s390x/kvm-stub.c         | 10 ++++++++
>  target/s390x/kvm.c              | 23 ++++++++++++++++++
>  target/s390x/kvm_s390x.h        |  3 +++
>  15 files changed, 116 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index 21f64ad..6e3af65 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -27,6 +27,7 @@
>  #include "qemu/cutils.h"
>  #include "qemu/option.h"
>  #include "exec/exec-all.h"
> +#include "internal.h"
>  
>  #define KERN_IMAGE_START                0x010000UL
>  #define LINUX_MAGIC_ADDR                0x010008UL
> @@ -230,6 +231,8 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
>      ipl->compat_start_addr = ipl->start_addr;
>      ipl->compat_bios_start_addr = ipl->bios_start_addr;
>      qemu_register_reset(qdev_reset_all_fn, dev);
> +
> +    diag318_register();
>  error:
>      error_propagate(errp, err);
>  }
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index a0615a8..2c670fc 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -37,6 +37,7 @@
>  #include "cpu_models.h"
>  #include "hw/nmi.h"
>  #include "hw/s390x/tod.h"
> +#include "internal.h"
>  
>  S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
>  {
> @@ -357,6 +358,7 @@ static void s390_machine_reset(void)
>          }
>          subsystem_reset();
>          s390_crypto_reset();
> +        diag318_reset();
>          run_on_cpu(cs, s390_do_cpu_load_normal, RUN_ON_CPU_NULL);
>          break;
>      case S390_RESET_LOAD_NORMAL:
> @@ -364,6 +366,7 @@ static void s390_machine_reset(void)
>              run_on_cpu(t, s390_do_cpu_reset, RUN_ON_CPU_NULL);
>          }
>          subsystem_reset();
> +        diag318_reset();
>          run_on_cpu(cs, s390_do_cpu_initial_reset, RUN_ON_CPU_NULL);
>          run_on_cpu(cs, s390_do_cpu_load_normal, RUN_ON_CPU_NULL);
>          break;
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index 4510a80..183c627 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -73,6 +73,8 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>                           read_info->conf_char);
>      s390_get_feat_block(S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT,
>                           read_info->conf_char_ext);
> +    /* Read Info byte 134 */
> +    s390_get_feat_block(S390_FEAT_TYPE_SCLP_BYTE_134, read_info->byte_134);
>  
>      read_info->facilities = cpu_to_be64(SCLP_HAS_CPU_INFO |
>                                          SCLP_HAS_IOA_RECONFIG);
> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
> index f9db243..eb12ba2 100644
> --- a/include/hw/s390x/sclp.h
> +++ b/include/hw/s390x/sclp.h
> @@ -133,6 +133,8 @@ typedef struct ReadInfo {
>      uint16_t highest_cpu;
>      uint8_t  _reserved5[124 - 122];     /* 122-123 */
>      uint32_t hmfai;
> +    uint8_t  _reserved7[134 - 128];     /* 128-133 */
> +    uint8_t  byte_134[1];
>      struct CPUEntry entries[0];
>  } QEMU_PACKED ReadInfo;
>  
> diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
> index 0265482..8c206d2 100644
> --- a/linux-headers/asm-s390/kvm.h
> +++ b/linux-headers/asm-s390/kvm.h
> @@ -74,6 +74,7 @@ struct kvm_s390_io_adapter_req {
>  #define KVM_S390_VM_CRYPTO		2
>  #define KVM_S390_VM_CPU_MODEL		3
>  #define KVM_S390_VM_MIGRATION		4
> +#define KVM_S390_VM_MISC		5
>  
>  /* kvm attributes for mem_ctrl */
>  #define KVM_S390_VM_MEM_ENABLE_CMMA	0
> @@ -130,6 +131,7 @@ struct kvm_s390_vm_cpu_machine {
>  #define KVM_S390_VM_CPU_FEAT_PFMFI	11
>  #define KVM_S390_VM_CPU_FEAT_SIGPIF	12
>  #define KVM_S390_VM_CPU_FEAT_KSS	13
> +#define KVM_S390_VM_CPU_FEAT_DIAG318	14
>  struct kvm_s390_vm_cpu_feat {
>  	__u64 feat[16];
>  };
> @@ -168,6 +170,9 @@ struct kvm_s390_vm_cpu_subfunc {
>  #define KVM_S390_VM_MIGRATION_START	1
>  #define KVM_S390_VM_MIGRATION_STATUS	2
>  
> +/* kvm attributes for KVM_S390_VM_MISC */
> +#define KVM_S390_VM_MISC_CPC		0
> +
>  /* for KVM_GET_REGS and KVM_SET_REGS */
>  struct kvm_regs {
>  	/* general purpose regs for s390 */
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index 8c2320e..594b4a4 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -52,7 +52,7 @@
>  
>  #define MMU_USER_IDX 0
>  
> -#define S390_MAX_CPUS 248
> +#define S390_MAX_CPUS 247
>  
>  typedef struct PSW {
>      uint64_t mask;
> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
> index 60cfeba..d05afa5 100644
> --- a/target/s390x/cpu_features.c
> +++ b/target/s390x/cpu_features.c
> @@ -121,6 +121,9 @@ static const S390FeatDef s390_features[] = {
>      FEAT_INIT("pfmfi", S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT, 9, "SIE: PFMF interpretation facility"),
>      FEAT_INIT("ibs", S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT, 10, "SIE: Interlock-and-broadcast-suppression facility"),
>  
> +    /* SCLP SCCB Byte 134 */
> +    FEAT_INIT("diag318", S390_FEAT_TYPE_SCLP_BYTE_134, 0, "SIE: Diagnose 318"),
> +
>      FEAT_INIT("sief2", S390_FEAT_TYPE_SCLP_CPU, 4, "SIE: interception format 2 (Virtual SIE)"),
>      FEAT_INIT("skey", S390_FEAT_TYPE_SCLP_CPU, 5, "SIE: Storage-key facility"),
>      FEAT_INIT("gpereh", S390_FEAT_TYPE_SCLP_CPU, 10, "SIE: Guest-PER enhancement facility"),
> diff --git a/target/s390x/cpu_features.h b/target/s390x/cpu_features.h
> index effe790..e7248df 100644
> --- a/target/s390x/cpu_features.h
> +++ b/target/s390x/cpu_features.h
> @@ -23,6 +23,7 @@ typedef enum {
>      S390_FEAT_TYPE_STFL,
>      S390_FEAT_TYPE_SCLP_CONF_CHAR,
>      S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT,
> +    S390_FEAT_TYPE_SCLP_BYTE_134,
>      S390_FEAT_TYPE_SCLP_CPU,
>      S390_FEAT_TYPE_MISC,
>      S390_FEAT_TYPE_PLO,
> diff --git a/target/s390x/cpu_features_def.h b/target/s390x/cpu_features_def.h
> index 5fc7e7b..d99da1d 100644
> --- a/target/s390x/cpu_features_def.h
> +++ b/target/s390x/cpu_features_def.h
> @@ -109,6 +109,9 @@ typedef enum {
>      S390_FEAT_SIE_PFMFI,
>      S390_FEAT_SIE_IBS,
>  
> +    /* Read Info Byte 134 */
> +    S390_FEAT_DIAG318,
> +
>      /* Sclp Cpu */
>      S390_FEAT_SIE_F2,
>      S390_FEAT_SIE_SKEY,
> diff --git a/target/s390x/diag.c b/target/s390x/diag.c
> index acb0f3d..2207c08 100644
> --- a/target/s390x/diag.c
> +++ b/target/s390x/diag.c
> @@ -20,6 +20,8 @@
>  #include "sysemu/cpus.h"
>  #include "hw/s390x/ipl.h"
>  #include "hw/s390x/s390-virtio-ccw.h"
> +#include "kvm_s390x.h"
> +#include "sysemu/kvm.h"
>  
>  int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3)
>  {
> @@ -134,3 +136,54 @@ out:
>          break;
>      }
>  }
> +
> +typedef struct Diag318Data {
> +    uint64_t cpc;
> +} Diag318Data;
> +static Diag318Data diag318data;
> +
> +void diag318_reset(void)
> +{
> +    if (s390_has_feat(S390_FEAT_DIAG318)) {
> +        kvm_s390_set_cpc(0);
> +    }
> +}
> +
> +static int diag318_post_load(void *opaque, int version_id)
> +{
> +    Diag318Data *d = opaque;
> +
> +    kvm_s390_set_cpc(d->cpc);
> +    return 0;
> +}
> +
> +static int diag318_pre_save(void *opaque)
> +{
> +    Diag318Data *d = opaque;
> +
> +    kvm_s390_get_cpc(&d->cpc);
> +    return 0;
> +}
> +
> +static bool diag318_needed(void *opaque)
> +{
> +    return s390_has_feat(S390_FEAT_DIAG318);
> +}
> +
> +const VMStateDescription vmstate_diag318 = {
> +    .name = "diag318",
> +    .post_load = diag318_post_load,
> +    .pre_save = diag318_pre_save,
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = diag318_needed,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT64(cpc, Diag318Data),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +void diag318_register(void)
> +{
> +    vmstate_register(NULL, 0, &vmstate_diag318, &diag318data);
> +}
> \ No newline at end of file
> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
> index 70015ea..a3d1457 100644
> --- a/target/s390x/gen-features.c
> +++ b/target/s390x/gen-features.c
> @@ -450,6 +450,7 @@ static uint16_t full_GEN12_GA1[] = {
>      S390_FEAT_AP_QUERY_CONFIG_INFO,
>      S390_FEAT_AP_FACILITIES_TEST,
>      S390_FEAT_AP,
> +    S390_FEAT_DIAG318,
>  };
>  
>  static uint16_t full_GEN12_GA2[] = {
> diff --git a/target/s390x/internal.h b/target/s390x/internal.h
> index f2a771e..d53b1d0 100644
> --- a/target/s390x/internal.h
> +++ b/target/s390x/internal.h
> @@ -383,10 +383,12 @@ int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw,
>                         target_ulong *addr, int *flags);
>  
>  
> -/* misc_helper.c */
> +/* diag.c */
>  int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3);
>  void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3,
>                       uintptr_t ra);
> +void diag318_register(void);
> +void diag318_reset(void);
>  
>  
>  /* translate.c */
> diff --git a/target/s390x/kvm-stub.c b/target/s390x/kvm-stub.c
> index bf7795e..1f0bee3 100644
> --- a/target/s390x/kvm-stub.c
> +++ b/target/s390x/kvm-stub.c
> @@ -104,3 +104,13 @@ void kvm_s390_stop_interrupt(S390CPU *cpu)
>  void kvm_s390_restart_interrupt(S390CPU *cpu)
>  {
>  }
> +
> +int kvm_s390_get_cpc(uint64_t *cpc)
> +{
> +    return 0;
> +}
> +
> +int kvm_s390_set_cpc(uint64_t cpc)
> +{
> +    return 0;
> +}
> \ No newline at end of file
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 2ebf26a..9ed4cea 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -749,6 +749,28 @@ int kvm_s390_set_clock_ext(uint8_t tod_high, uint64_t tod_low)
>      return kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attr);
>  }
>  
> +int kvm_s390_get_cpc(uint64_t *cpc)
> +{
> +    struct kvm_device_attr attr = {
> +        .group = KVM_S390_VM_MISC,
> +        .attr = KVM_S390_VM_MISC_CPC,
> +        .addr = (uint64_t)cpc,
> +    };
> +
> +    return kvm_vm_ioctl(kvm_state, KVM_GET_DEVICE_ATTR, &attr);
> +}
> +
> +int kvm_s390_set_cpc(uint64_t cpc)
> +{
> +    struct kvm_device_attr attr = {
> +        .group = KVM_S390_VM_MISC,
> +        .attr = KVM_S390_VM_MISC_CPC,
> +        .addr = (uint64_t)&cpc,
> +    };
> +
> +    return kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attr);
> +}
> +
>  /**
>   * kvm_s390_mem_op:
>   * @addr:      the logical start address in guest memory
> @@ -2142,6 +2164,7 @@ static int kvm_to_feat[][2] = {
>      { KVM_S390_VM_CPU_FEAT_PFMFI, S390_FEAT_SIE_PFMFI},
>      { KVM_S390_VM_CPU_FEAT_SIGPIF, S390_FEAT_SIE_SIGPIF},
>      { KVM_S390_VM_CPU_FEAT_KSS, S390_FEAT_SIE_KSS},
> +    { KVM_S390_VM_CPU_FEAT_DIAG318, S390_FEAT_DIAG318},
>  };
>  
>  static int query_cpu_feat(S390FeatBitmap features)
> diff --git a/target/s390x/kvm_s390x.h b/target/s390x/kvm_s390x.h
> index 6e52287..53f165f 100644
> --- a/target/s390x/kvm_s390x.h
> +++ b/target/s390x/kvm_s390x.h
> @@ -29,6 +29,9 @@ int kvm_s390_get_clock(uint8_t *tod_high, uint64_t *tod_clock);
>  int kvm_s390_get_clock_ext(uint8_t *tod_high, uint64_t *tod_clock);
>  int kvm_s390_set_clock(uint8_t tod_high, uint64_t tod_clock);
>  int kvm_s390_set_clock_ext(uint8_t tod_high, uint64_t tod_clock);
> +int kvm_s390_get_cpc(uint64_t *cpc);
> +int kvm_s390_set_cpc(uint64_t cpc);
> +bool kvm_s390_has_diag318(void);
>  void kvm_s390_enable_css_support(S390CPU *cpu);
>  int kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch,
>                                      int vq, bool assign);
>

Cornelia Huck Dec. 5, 2018, 11:07 a.m. UTC | #6

On Tue, 4 Dec 2018 17:26:36 -0500
Collin Walling <[e-mail]> wrote:

> I screen-scraped the @ibm address again (Conny was the victim this time)
> 
> Reply to this thread to avoid any delivery failures.
> 
> On 12/4/18 5:18 PM, Collin Walling wrote:
> > Add migration and reset support for diagnose 318. This is a new z14 GA2 
> > hardware feature, but we can provide guest support starting with the 
> > zEC12-full CPU model.
> > 
> > Because new hardware introduces a new facility-availability byte in 
> > the Read SCP Info block, we lose one byte in the CPU entries list 
> > and must limit the maximum VCPUs to 247 (down from 248).
> > 
> > Signed-off-by: Collin Walling <[e-mail]>
> > ---
> > 
> > Changelog
> > 
> >     RFC -> v1
> >         - introduced kvm stubs for set/get cpc
> >         - s/fac134/byte_134
> >         - moved diag318 vmstate description to diag.c
> >         - reduced S390_VCPU_MAX to 247
> > 
> >  hw/s390x/ipl.c                  |  3 +++
> >  hw/s390x/s390-virtio-ccw.c      |  3 +++
> >  hw/s390x/sclp.c                 |  2 ++
> >  include/hw/s390x/sclp.h         |  2 ++
> >  linux-headers/asm-s390/kvm.h    |  5 ++++
> >  target/s390x/cpu.h              |  2 +-
> >  target/s390x/cpu_features.c     |  3 +++
> >  target/s390x/cpu_features.h     |  1 +
> >  target/s390x/cpu_features_def.h |  3 +++
> >  target/s390x/diag.c             | 53 +++++++++++++++++++++++++++++++++++++++++
> >  target/s390x/gen-features.c     |  1 +
> >  target/s390x/internal.h         |  4 +++-
> >  target/s390x/kvm-stub.c         | 10 ++++++++
> >  target/s390x/kvm.c              | 23 ++++++++++++++++++
> >  target/s390x/kvm_s390x.h        |  3 +++
> >  15 files changed, 116 insertions(+), 2 deletions(-)

> > diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
> > index 0265482..8c206d2 100644
> > --- a/linux-headers/asm-s390/kvm.h
> > +++ b/linux-headers/asm-s390/kvm.h

Please split this out into a dummy headers update patch, which will be
replaced with a real headers update when applied.

Cornelia Huck Dec. 5, 2018, 11:54 a.m. UTC | #7

On Wed, 5 Dec 2018 09:27:44 +0100
Christian Borntraeger <[e-mail]> wrote:

> On 05.12.2018 09:26, David Hildenbrand wrote:
> > On 04.12.18 23:18, Collin Walling wrote:  
> >> Add migration and reset support for diagnose 318. This is a new z14 GA2 
> >> hardware feature, but we can provide guest support starting with the 
> >> zEC12-full CPU model.
> >>
> >> Because new hardware introduces a new facility-availability byte in 
> >> the Read SCP Info block, we lose one byte in the CPU entries list 
> >> and must limit the maximum VCPUs to 247 (down from 248).  
> > 
> > This could break setups that upgrade/migrate. At least forward migration
> > can be broken. Do we care about that?  
> 
> Can we maybe bind this feature and the cpu limit to the 4.0 machine?

I think that would make sense.

> > 
> > Can you split off
> > 
> > 1. linux-header changes
> > 2. CPU model changes? (introduction and definition of new feature, but
> > not when it is used?)
> >   
> >>
> >> Signed-off-by: Collin Walling <[e-mail]>
> >> ---

David Hildenbrand Dec. 5, 2018, 11:57 a.m. UTC | #8

On 05.12.18 12:54, Cornelia Huck wrote:
> On Wed, 5 Dec 2018 09:27:44 +0100
> Christian Borntraeger <[e-mail]> wrote:
> 
>> On 05.12.2018 09:26, David Hildenbrand wrote:
>>> On 04.12.18 23:18, Collin Walling wrote:  
>>>> Add migration and reset support for diagnose 318. This is a new z14 GA2 
>>>> hardware feature, but we can provide guest support starting with the 
>>>> zEC12-full CPU model.
>>>>
>>>> Because new hardware introduces a new facility-availability byte in 
>>>> the Read SCP Info block, we lose one byte in the CPU entries list 
>>>> and must limit the maximum VCPUs to 247 (down from 248).  
>>>
>>> This could break setups that upgrade/migrate. At least forward migration
>>> can be broken. Do we care about that?  
>>
>> Can we maybe bind this feature and the cpu limit to the 4.0 machine?
> 
> I think that would make sense.

Won't be that easy, as we'll have different sizes of the struct size.
Something that might uglify the code quite a bit.

Also we'll get a dependence between the cpu model features (e.g. probed
via the NULL machine) and the compat machine. Have to think about this.

> 
>>>
>>> Can you split off
>>>
>>> 1. linux-header changes
>>> 2. CPU model changes? (introduction and definition of new feature, but
>>> not when it is used?)
>>>   
>>>>
>>>> Signed-off-by: Collin Walling <[e-mail]>
>>>> ---

Collin Walling Dec. 5, 2018, 2:52 p.m. UTC | #9

On 12/5/18 6:07 AM, Cornelia Huck wrote:
> On Tue, 4 Dec 2018 17:26:36 -0500
> Collin Walling <[e-mail]> wrote:
> 
>> I screen-scraped the @ibm address again (Conny was the victim this time)
>>
>> Reply to this thread to avoid any delivery failures.
>>
>> On 12/4/18 5:18 PM, Collin Walling wrote:
>>> Add migration and reset support for diagnose 318. This is a new z14 GA2 
>>> hardware feature, but we can provide guest support starting with the 
>>> zEC12-full CPU model.
>>>
>>> Because new hardware introduces a new facility-availability byte in 
>>> the Read SCP Info block, we lose one byte in the CPU entries list 
>>> and must limit the maximum VCPUs to 247 (down from 248).
>>>
>>> Signed-off-by: Collin Walling <[e-mail]>
>>> ---
>>>
>>> Changelog
>>>
>>>     RFC -> v1
>>>         - introduced kvm stubs for set/get cpc
>>>         - s/fac134/byte_134
>>>         - moved diag318 vmstate description to diag.c
>>>         - reduced S390_VCPU_MAX to 247
>>>
>>>  hw/s390x/ipl.c                  |  3 +++
>>>  hw/s390x/s390-virtio-ccw.c      |  3 +++
>>>  hw/s390x/sclp.c                 |  2 ++
>>>  include/hw/s390x/sclp.h         |  2 ++
>>>  linux-headers/asm-s390/kvm.h    |  5 ++++
>>>  target/s390x/cpu.h              |  2 +-
>>>  target/s390x/cpu_features.c     |  3 +++
>>>  target/s390x/cpu_features.h     |  1 +
>>>  target/s390x/cpu_features_def.h |  3 +++
>>>  target/s390x/diag.c             | 53 +++++++++++++++++++++++++++++++++++++++++
>>>  target/s390x/gen-features.c     |  1 +
>>>  target/s390x/internal.h         |  4 +++-
>>>  target/s390x/kvm-stub.c         | 10 ++++++++
>>>  target/s390x/kvm.c              | 23 ++++++++++++++++++
>>>  target/s390x/kvm_s390x.h        |  3 +++
>>>  15 files changed, 116 insertions(+), 2 deletions(-)
> 
>>> diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
>>> index 0265482..8c206d2 100644
>>> --- a/linux-headers/asm-s390/kvm.h
>>> +++ b/linux-headers/asm-s390/kvm.h
> 
> Please split this out into a dummy headers update patch, which will be
> replaced with a real headers update when applied.
> 
> 

Will do.

Collin Walling Dec. 5, 2018, 2:58 p.m. UTC | #10

On 12/5/18 6:57 AM, David Hildenbrand wrote:
> On 05.12.18 12:54, Cornelia Huck wrote:
>> On Wed, 5 Dec 2018 09:27:44 +0100
>> Christian Borntraeger <[e-mail]> wrote:
>>
>>> On 05.12.2018 09:26, David Hildenbrand wrote:
>>>> On 04.12.18 23:18, Collin Walling wrote:  
>>>>> Add migration and reset support for diagnose 318. This is a new z14 GA2 
>>>>> hardware feature, but we can provide guest support starting with the 
>>>>> zEC12-full CPU model.
>>>>>
>>>>> Because new hardware introduces a new facility-availability byte in 
>>>>> the Read SCP Info block, we lose one byte in the CPU entries list 
>>>>> and must limit the maximum VCPUs to 247 (down from 248).  
>>>>
>>>> This could break setups that upgrade/migrate. At least forward migration
>>>> can be broken. Do we care about that?  
>>>
>>> Can we maybe bind this feature and the cpu limit to the 4.0 machine?
>>
>> I think that would make sense.
> 
> Won't be that easy, as we'll have different sizes of the struct size.
> Something that might uglify the code quite a bit.
> 
> Also we'll get a dependence between the cpu model features (e.g. probed
> via the NULL machine) and the compat machine. Have to think about this.
> 

Are you referring to the struct CPUS390XState? Could we get away by adding
a one-byte "unused" field after the bitmap?

>>
>>>>
>>>> Can you split off
>>>>
>>>> 1. linux-header changes
>>>> 2. CPU model changes? (introduction and definition of new feature, but
>>>> not when it is used?)
>>>>   
>>>>>
>>>>> Signed-off-by: Collin Walling <[e-mail]>
>>>>> ---
> 
> 

I'll split the patches into:

1) linux-header changes
2) CPU model changes
3) diag318 usages / migration

Collin Walling Dec. 5, 2018, 2:59 p.m. UTC | #11

On 12/5/18 3:29 AM, Christian Borntraeger wrote:
> You should clearly review your email list.....
> 
> Adding the "new" Conny, removing Carsten.
> 

Thanks for taking care of that. I'll be more careful when using the
addresses at the top of the files. 

> 
> On 04.12.2018 23:18, Collin Walling wrote:
>> Add migration and reset support for diagnose 318. This is a new z14 GA2 
>> hardware feature, but we can provide guest support starting with the 
>> zEC12-full CPU model.
>>
>> Because new hardware introduces a new facility-availability byte in 
>> the Read SCP Info block, we lose one byte in the CPU entries list 
>> and must limit the maximum VCPUs to 247 (down from 248).
>>
>> Signed-off-by: Collin Walling <[e-mail]>
>> ---
>>
>> Changelog
>>
>>     RFC -> v1
>>         - introduced kvm stubs for set/get cpc
>>         - s/fac134/byte_134
>>         - moved diag318 vmstate description to diag.c
>>         - reduced S390_VCPU_MAX to 247
>>
>>  hw/s390x/ipl.c                  |  3 +++
>>  hw/s390x/s390-virtio-ccw.c      |  3 +++
>>  hw/s390x/sclp.c                 |  2 ++
>>  include/hw/s390x/sclp.h         |  2 ++
>>  linux-headers/asm-s390/kvm.h    |  5 ++++
>>  target/s390x/cpu.h              |  2 +-
>>  target/s390x/cpu_features.c     |  3 +++
>>  target/s390x/cpu_features.h     |  1 +
>>  target/s390x/cpu_features_def.h |  3 +++
>>  target/s390x/diag.c             | 53 +++++++++++++++++++++++++++++++++++++++++
>>  target/s390x/gen-features.c     |  1 +
>>  target/s390x/internal.h         |  4 +++-
>>  target/s390x/kvm-stub.c         | 10 ++++++++
>>  target/s390x/kvm.c              | 23 ++++++++++++++++++
>>  target/s390x/kvm_s390x.h        |  3 +++
>>  15 files changed, 116 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
>> index 21f64ad..6e3af65 100644
>> --- a/hw/s390x/ipl.c
>> +++ b/hw/s390x/ipl.c
>> @@ -27,6 +27,7 @@
>>  #include "qemu/cutils.h"
>>  #include "qemu/option.h"
>>  #include "exec/exec-all.h"
>> +#include "internal.h"
>>  
>>  #define KERN_IMAGE_START                0x010000UL
>>  #define LINUX_MAGIC_ADDR                0x010008UL
>> @@ -230,6 +231,8 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
>>      ipl->compat_start_addr = ipl->start_addr;
>>      ipl->compat_bios_start_addr = ipl->bios_start_addr;
>>      qemu_register_reset(qdev_reset_all_fn, dev);
>> +
>> +    diag318_register();
>>  error:
>>      error_propagate(errp, err);
>>  }
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index a0615a8..2c670fc 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -37,6 +37,7 @@
>>  #include "cpu_models.h"
>>  #include "hw/nmi.h"
>>  #include "hw/s390x/tod.h"
>> +#include "internal.h"
>>  
>>  S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
>>  {
>> @@ -357,6 +358,7 @@ static void s390_machine_reset(void)
>>          }
>>          subsystem_reset();
>>          s390_crypto_reset();
>> +        diag318_reset();
>>          run_on_cpu(cs, s390_do_cpu_load_normal, RUN_ON_CPU_NULL);
>>          break;
>>      case S390_RESET_LOAD_NORMAL:
>> @@ -364,6 +366,7 @@ static void s390_machine_reset(void)
>>              run_on_cpu(t, s390_do_cpu_reset, RUN_ON_CPU_NULL);
>>          }
>>          subsystem_reset();
>> +        diag318_reset();
>>          run_on_cpu(cs, s390_do_cpu_initial_reset, RUN_ON_CPU_NULL);
>>          run_on_cpu(cs, s390_do_cpu_load_normal, RUN_ON_CPU_NULL);
>>          break;
>> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
>> index 4510a80..183c627 100644
>> --- a/hw/s390x/sclp.c
>> +++ b/hw/s390x/sclp.c
>> @@ -73,6 +73,8 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>>                           read_info->conf_char);
>>      s390_get_feat_block(S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT,
>>                           read_info->conf_char_ext);
>> +    /* Read Info byte 134 */
>> +    s390_get_feat_block(S390_FEAT_TYPE_SCLP_BYTE_134, read_info->byte_134);
>>  
>>      read_info->facilities = cpu_to_be64(SCLP_HAS_CPU_INFO |
>>                                          SCLP_HAS_IOA_RECONFIG);
>> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
>> index f9db243..eb12ba2 100644
>> --- a/include/hw/s390x/sclp.h
>> +++ b/include/hw/s390x/sclp.h
>> @@ -133,6 +133,8 @@ typedef struct ReadInfo {
>>      uint16_t highest_cpu;
>>      uint8_t  _reserved5[124 - 122];     /* 122-123 */
>>      uint32_t hmfai;
>> +    uint8_t  _reserved7[134 - 128];     /* 128-133 */
>> +    uint8_t  byte_134[1];
>>      struct CPUEntry entries[0];
>>  } QEMU_PACKED ReadInfo;
>>  
>> diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
>> index 0265482..8c206d2 100644
>> --- a/linux-headers/asm-s390/kvm.h
>> +++ b/linux-headers/asm-s390/kvm.h
>> @@ -74,6 +74,7 @@ struct kvm_s390_io_adapter_req {
>>  #define KVM_S390_VM_CRYPTO		2
>>  #define KVM_S390_VM_CPU_MODEL		3
>>  #define KVM_S390_VM_MIGRATION		4
>> +#define KVM_S390_VM_MISC		5
>>  
>>  /* kvm attributes for mem_ctrl */
>>  #define KVM_S390_VM_MEM_ENABLE_CMMA	0
>> @@ -130,6 +131,7 @@ struct kvm_s390_vm_cpu_machine {
>>  #define KVM_S390_VM_CPU_FEAT_PFMFI	11
>>  #define KVM_S390_VM_CPU_FEAT_SIGPIF	12
>>  #define KVM_S390_VM_CPU_FEAT_KSS	13
>> +#define KVM_S390_VM_CPU_FEAT_DIAG318	14
>>  struct kvm_s390_vm_cpu_feat {
>>  	__u64 feat[16];
>>  };
>> @@ -168,6 +170,9 @@ struct kvm_s390_vm_cpu_subfunc {
>>  #define KVM_S390_VM_MIGRATION_START	1
>>  #define KVM_S390_VM_MIGRATION_STATUS	2
>>  
>> +/* kvm attributes for KVM_S390_VM_MISC */
>> +#define KVM_S390_VM_MISC_CPC		0
>> +
>>  /* for KVM_GET_REGS and KVM_SET_REGS */
>>  struct kvm_regs {
>>  	/* general purpose regs for s390 */
>> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
>> index 8c2320e..594b4a4 100644
>> --- a/target/s390x/cpu.h
>> +++ b/target/s390x/cpu.h
>> @@ -52,7 +52,7 @@
>>  
>>  #define MMU_USER_IDX 0
>>  
>> -#define S390_MAX_CPUS 248
>> +#define S390_MAX_CPUS 247
>>  
>>  typedef struct PSW {
>>      uint64_t mask;
>> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
>> index 60cfeba..d05afa5 100644
>> --- a/target/s390x/cpu_features.c
>> +++ b/target/s390x/cpu_features.c
>> @@ -121,6 +121,9 @@ static const S390FeatDef s390_features[] = {
>>      FEAT_INIT("pfmfi", S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT, 9, "SIE: PFMF interpretation facility"),
>>      FEAT_INIT("ibs", S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT, 10, "SIE: Interlock-and-broadcast-suppression facility"),
>>  
>> +    /* SCLP SCCB Byte 134 */
>> +    FEAT_INIT("diag318", S390_FEAT_TYPE_SCLP_BYTE_134, 0, "SIE: Diagnose 318"),
>> +
>>      FEAT_INIT("sief2", S390_FEAT_TYPE_SCLP_CPU, 4, "SIE: interception format 2 (Virtual SIE)"),
>>      FEAT_INIT("skey", S390_FEAT_TYPE_SCLP_CPU, 5, "SIE: Storage-key facility"),
>>      FEAT_INIT("gpereh", S390_FEAT_TYPE_SCLP_CPU, 10, "SIE: Guest-PER enhancement facility"),
>> diff --git a/target/s390x/cpu_features.h b/target/s390x/cpu_features.h
>> index effe790..e7248df 100644
>> --- a/target/s390x/cpu_features.h
>> +++ b/target/s390x/cpu_features.h
>> @@ -23,6 +23,7 @@ typedef enum {
>>      S390_FEAT_TYPE_STFL,
>>      S390_FEAT_TYPE_SCLP_CONF_CHAR,
>>      S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT,
>> +    S390_FEAT_TYPE_SCLP_BYTE_134,
>>      S390_FEAT_TYPE_SCLP_CPU,
>>      S390_FEAT_TYPE_MISC,
>>      S390_FEAT_TYPE_PLO,
>> diff --git a/target/s390x/cpu_features_def.h b/target/s390x/cpu_features_def.h
>> index 5fc7e7b..d99da1d 100644
>> --- a/target/s390x/cpu_features_def.h
>> +++ b/target/s390x/cpu_features_def.h
>> @@ -109,6 +109,9 @@ typedef enum {
>>      S390_FEAT_SIE_PFMFI,
>>      S390_FEAT_SIE_IBS,
>>  
>> +    /* Read Info Byte 134 */
>> +    S390_FEAT_DIAG318,
>> +
>>      /* Sclp Cpu */
>>      S390_FEAT_SIE_F2,
>>      S390_FEAT_SIE_SKEY,
>> diff --git a/target/s390x/diag.c b/target/s390x/diag.c
>> index acb0f3d..2207c08 100644
>> --- a/target/s390x/diag.c
>> +++ b/target/s390x/diag.c
>> @@ -20,6 +20,8 @@
>>  #include "sysemu/cpus.h"
>>  #include "hw/s390x/ipl.h"
>>  #include "hw/s390x/s390-virtio-ccw.h"
>> +#include "kvm_s390x.h"
>> +#include "sysemu/kvm.h"
>>  
>>  int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3)
>>  {
>> @@ -134,3 +136,54 @@ out:
>>          break;
>>      }
>>  }
>> +
>> +typedef struct Diag318Data {
>> +    uint64_t cpc;
>> +} Diag318Data;
>> +static Diag318Data diag318data;
>> +
>> +void diag318_reset(void)
>> +{
>> +    if (s390_has_feat(S390_FEAT_DIAG318)) {
>> +        kvm_s390_set_cpc(0);
>> +    }
>> +}
>> +
>> +static int diag318_post_load(void *opaque, int version_id)
>> +{
>> +    Diag318Data *d = opaque;
>> +
>> +    kvm_s390_set_cpc(d->cpc);
>> +    return 0;
>> +}
>> +
>> +static int diag318_pre_save(void *opaque)
>> +{
>> +    Diag318Data *d = opaque;
>> +
>> +    kvm_s390_get_cpc(&d->cpc);
>> +    return 0;
>> +}
>> +
>> +static bool diag318_needed(void *opaque)
>> +{
>> +    return s390_has_feat(S390_FEAT_DIAG318);
>> +}
>> +
>> +const VMStateDescription vmstate_diag318 = {
>> +    .name = "diag318",
>> +    .post_load = diag318_post_load,
>> +    .pre_save = diag318_pre_save,
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .needed = diag318_needed,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_UINT64(cpc, Diag318Data),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>> +void diag318_register(void)
>> +{
>> +    vmstate_register(NULL, 0, &vmstate_diag318, &diag318data);
>> +}
>> \ No newline at end of file
>> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
>> index 70015ea..a3d1457 100644
>> --- a/target/s390x/gen-features.c
>> +++ b/target/s390x/gen-features.c
>> @@ -450,6 +450,7 @@ static uint16_t full_GEN12_GA1[] = {
>>      S390_FEAT_AP_QUERY_CONFIG_INFO,
>>      S390_FEAT_AP_FACILITIES_TEST,
>>      S390_FEAT_AP,
>> +    S390_FEAT_DIAG318,
>>  };
>>  
>>  static uint16_t full_GEN12_GA2[] = {
>> diff --git a/target/s390x/internal.h b/target/s390x/internal.h
>> index f2a771e..d53b1d0 100644
>> --- a/target/s390x/internal.h
>> +++ b/target/s390x/internal.h
>> @@ -383,10 +383,12 @@ int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw,
>>                         target_ulong *addr, int *flags);
>>  
>>  
>> -/* misc_helper.c */
>> +/* diag.c */
>>  int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3);
>>  void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3,
>>                       uintptr_t ra);
>> +void diag318_register(void);
>> +void diag318_reset(void);
>>  
>>  
>>  /* translate.c */
>> diff --git a/target/s390x/kvm-stub.c b/target/s390x/kvm-stub.c
>> index bf7795e..1f0bee3 100644
>> --- a/target/s390x/kvm-stub.c
>> +++ b/target/s390x/kvm-stub.c
>> @@ -104,3 +104,13 @@ void kvm_s390_stop_interrupt(S390CPU *cpu)
>>  void kvm_s390_restart_interrupt(S390CPU *cpu)
>>  {
>>  }
>> +
>> +int kvm_s390_get_cpc(uint64_t *cpc)
>> +{
>> +    return 0;
>> +}
>> +
>> +int kvm_s390_set_cpc(uint64_t cpc)
>> +{
>> +    return 0;
>> +}
>> \ No newline at end of file
>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>> index 2ebf26a..9ed4cea 100644
>> --- a/target/s390x/kvm.c
>> +++ b/target/s390x/kvm.c
>> @@ -749,6 +749,28 @@ int kvm_s390_set_clock_ext(uint8_t tod_high, uint64_t tod_low)
>>      return kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attr);
>>  }
>>  
>> +int kvm_s390_get_cpc(uint64_t *cpc)
>> +{
>> +    struct kvm_device_attr attr = {
>> +        .group = KVM_S390_VM_MISC,
>> +        .attr = KVM_S390_VM_MISC_CPC,
>> +        .addr = (uint64_t)cpc,
>> +    };
>> +
>> +    return kvm_vm_ioctl(kvm_state, KVM_GET_DEVICE_ATTR, &attr);
>> +}
>> +
>> +int kvm_s390_set_cpc(uint64_t cpc)
>> +{
>> +    struct kvm_device_attr attr = {
>> +        .group = KVM_S390_VM_MISC,
>> +        .attr = KVM_S390_VM_MISC_CPC,
>> +        .addr = (uint64_t)&cpc,
>> +    };
>> +
>> +    return kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attr);
>> +}
>> +
>>  /**
>>   * kvm_s390_mem_op:
>>   * @addr:      the logical start address in guest memory
>> @@ -2142,6 +2164,7 @@ static int kvm_to_feat[][2] = {
>>      { KVM_S390_VM_CPU_FEAT_PFMFI, S390_FEAT_SIE_PFMFI},
>>      { KVM_S390_VM_CPU_FEAT_SIGPIF, S390_FEAT_SIE_SIGPIF},
>>      { KVM_S390_VM_CPU_FEAT_KSS, S390_FEAT_SIE_KSS},
>> +    { KVM_S390_VM_CPU_FEAT_DIAG318, S390_FEAT_DIAG318},
>>  };
>>  
>>  static int query_cpu_feat(S390FeatBitmap features)
>> diff --git a/target/s390x/kvm_s390x.h b/target/s390x/kvm_s390x.h
>> index 6e52287..53f165f 100644
>> --- a/target/s390x/kvm_s390x.h
>> +++ b/target/s390x/kvm_s390x.h
>> @@ -29,6 +29,9 @@ int kvm_s390_get_clock(uint8_t *tod_high, uint64_t *tod_clock);
>>  int kvm_s390_get_clock_ext(uint8_t *tod_high, uint64_t *tod_clock);
>>  int kvm_s390_set_clock(uint8_t tod_high, uint64_t tod_clock);
>>  int kvm_s390_set_clock_ext(uint8_t tod_high, uint64_t tod_clock);
>> +int kvm_s390_get_cpc(uint64_t *cpc);
>> +int kvm_s390_set_cpc(uint64_t cpc);
>> +bool kvm_s390_has_diag318(void);
>>  void kvm_s390_enable_css_support(S390CPU *cpu);
>>  int kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch,
>>                                      int vq, bool assign);
>>
> 
>

Cornelia Huck Dec. 5, 2018, 3:22 p.m. UTC | #12

On Wed, 5 Dec 2018 09:59:54 -0500
Collin Walling <[e-mail]> wrote:

> On 12/5/18 3:29 AM, Christian Borntraeger wrote:
> > You should clearly review your email list.....
> > 
> > Adding the "new" Conny, removing Carsten.
> >   
> 
> Thanks for taking care of that. I'll be more careful when using the
> addresses at the top of the files. 

scripts/get_maintainer.pl -f <file> is probably more useful to get
current addresses :)

Collin Walling Dec. 5, 2018, 3:27 p.m. UTC | #13

On 12/5/18 10:22 AM, Cornelia Huck wrote:
> On Wed, 5 Dec 2018 09:59:54 -0500
> Collin Walling <[e-mail]> wrote:
> 
>> On 12/5/18 3:29 AM, Christian Borntraeger wrote:
>>> You should clearly review your email list.....
>>>
>>> Adding the "new" Conny, removing Carsten.
>>>   
>>
>> Thanks for taking care of that. I'll be more careful when using the
>> addresses at the top of the files. 
> 
> scripts/get_maintainer.pl -f <file> is probably more useful to get
> current addresses :)
> 

I'll give it a whirl next time. I appreciate the advice! :)

Patch

diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 21f64ad..6e3af65 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -27,6 +27,7 @@ 
 #include "qemu/cutils.h"
 #include "qemu/option.h"
 #include "exec/exec-all.h"
+#include "internal.h"
 
 #define KERN_IMAGE_START                0x010000UL
 #define LINUX_MAGIC_ADDR                0x010008UL
@@ -230,6 +231,8 @@  static void s390_ipl_realize(DeviceState *dev, Error **errp)
     ipl->compat_start_addr = ipl->start_addr;
     ipl->compat_bios_start_addr = ipl->bios_start_addr;
     qemu_register_reset(qdev_reset_all_fn, dev);
+
+    diag318_register();
 error:
     error_propagate(errp, err);
 }
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index a0615a8..2c670fc 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -37,6 +37,7 @@ 
 #include "cpu_models.h"
 #include "hw/nmi.h"
 #include "hw/s390x/tod.h"
+#include "internal.h"
 
 S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
 {
@@ -357,6 +358,7 @@  static void s390_machine_reset(void)
         }
         subsystem_reset();
         s390_crypto_reset();
+        diag318_reset();
         run_on_cpu(cs, s390_do_cpu_load_normal, RUN_ON_CPU_NULL);
         break;
     case S390_RESET_LOAD_NORMAL:
@@ -364,6 +366,7 @@  static void s390_machine_reset(void)
             run_on_cpu(t, s390_do_cpu_reset, RUN_ON_CPU_NULL);
         }
         subsystem_reset();
+        diag318_reset();
         run_on_cpu(cs, s390_do_cpu_initial_reset, RUN_ON_CPU_NULL);
         run_on_cpu(cs, s390_do_cpu_load_normal, RUN_ON_CPU_NULL);
         break;
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 4510a80..183c627 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -73,6 +73,8 @@  static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
                          read_info->conf_char);
     s390_get_feat_block(S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT,
                          read_info->conf_char_ext);
+    /* Read Info byte 134 */
+    s390_get_feat_block(S390_FEAT_TYPE_SCLP_BYTE_134, read_info->byte_134);
 
     read_info->facilities = cpu_to_be64(SCLP_HAS_CPU_INFO |
                                         SCLP_HAS_IOA_RECONFIG);
diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
index f9db243..eb12ba2 100644
--- a/include/hw/s390x/sclp.h
+++ b/include/hw/s390x/sclp.h
@@ -133,6 +133,8 @@  typedef struct ReadInfo {
     uint16_t highest_cpu;
     uint8_t  _reserved5[124 - 122];     /* 122-123 */
     uint32_t hmfai;
+    uint8_t  _reserved7[134 - 128];     /* 128-133 */
+    uint8_t  byte_134[1];
     struct CPUEntry entries[0];
 } QEMU_PACKED ReadInfo;
 
diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
index 0265482..8c206d2 100644
--- a/linux-headers/asm-s390/kvm.h
+++ b/linux-headers/asm-s390/kvm.h
@@ -74,6 +74,7 @@  struct kvm_s390_io_adapter_req {
 #define KVM_S390_VM_CRYPTO		2
 #define KVM_S390_VM_CPU_MODEL		3
 #define KVM_S390_VM_MIGRATION		4
+#define KVM_S390_VM_MISC		5
 
 /* kvm attributes for mem_ctrl */
 #define KVM_S390_VM_MEM_ENABLE_CMMA	0
@@ -130,6 +131,7 @@  struct kvm_s390_vm_cpu_machine {
 #define KVM_S390_VM_CPU_FEAT_PFMFI	11
 #define KVM_S390_VM_CPU_FEAT_SIGPIF	12
 #define KVM_S390_VM_CPU_FEAT_KSS	13
+#define KVM_S390_VM_CPU_FEAT_DIAG318	14
 struct kvm_s390_vm_cpu_feat {
 	__u64 feat[16];
 };
@@ -168,6 +170,9 @@  struct kvm_s390_vm_cpu_subfunc {
 #define KVM_S390_VM_MIGRATION_START	1
 #define KVM_S390_VM_MIGRATION_STATUS	2
 
+/* kvm attributes for KVM_S390_VM_MISC */
+#define KVM_S390_VM_MISC_CPC		0
+
 /* for KVM_GET_REGS and KVM_SET_REGS */
 struct kvm_regs {
 	/* general purpose regs for s390 */
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 8c2320e..594b4a4 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -52,7 +52,7 @@ 
 
 #define MMU_USER_IDX 0
 
-#define S390_MAX_CPUS 248
+#define S390_MAX_CPUS 247
 
 typedef struct PSW {
     uint64_t mask;
diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
index 60cfeba..d05afa5 100644
--- a/target/s390x/cpu_features.c
+++ b/target/s390x/cpu_features.c
@@ -121,6 +121,9 @@  static const S390FeatDef s390_features[] = {
     FEAT_INIT("pfmfi", S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT, 9, "SIE: PFMF interpretation facility"),
     FEAT_INIT("ibs", S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT, 10, "SIE: Interlock-and-broadcast-suppression facility"),
 
+    /* SCLP SCCB Byte 134 */
+    FEAT_INIT("diag318", S390_FEAT_TYPE_SCLP_BYTE_134, 0, "SIE: Diagnose 318"),
+
     FEAT_INIT("sief2", S390_FEAT_TYPE_SCLP_CPU, 4, "SIE: interception format 2 (Virtual SIE)"),
     FEAT_INIT("skey", S390_FEAT_TYPE_SCLP_CPU, 5, "SIE: Storage-key facility"),
     FEAT_INIT("gpereh", S390_FEAT_TYPE_SCLP_CPU, 10, "SIE: Guest-PER enhancement facility"),
diff --git a/target/s390x/cpu_features.h b/target/s390x/cpu_features.h
index effe790..e7248df 100644
--- a/target/s390x/cpu_features.h
+++ b/target/s390x/cpu_features.h
@@ -23,6 +23,7 @@  typedef enum {
     S390_FEAT_TYPE_STFL,
     S390_FEAT_TYPE_SCLP_CONF_CHAR,
     S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT,
+    S390_FEAT_TYPE_SCLP_BYTE_134,
     S390_FEAT_TYPE_SCLP_CPU,
     S390_FEAT_TYPE_MISC,
     S390_FEAT_TYPE_PLO,
diff --git a/target/s390x/cpu_features_def.h b/target/s390x/cpu_features_def.h
index 5fc7e7b..d99da1d 100644
--- a/target/s390x/cpu_features_def.h
+++ b/target/s390x/cpu_features_def.h
@@ -109,6 +109,9 @@  typedef enum {
     S390_FEAT_SIE_PFMFI,
     S390_FEAT_SIE_IBS,
 
+    /* Read Info Byte 134 */
+    S390_FEAT_DIAG318,
+
     /* Sclp Cpu */
     S390_FEAT_SIE_F2,
     S390_FEAT_SIE_SKEY,
diff --git a/target/s390x/diag.c b/target/s390x/diag.c
index acb0f3d..2207c08 100644
--- a/target/s390x/diag.c
+++ b/target/s390x/diag.c
@@ -20,6 +20,8 @@ 
 #include "sysemu/cpus.h"
 #include "hw/s390x/ipl.h"
 #include "hw/s390x/s390-virtio-ccw.h"
+#include "kvm_s390x.h"
+#include "sysemu/kvm.h"
 
 int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3)
 {
@@ -134,3 +136,54 @@  out:
         break;
     }
 }
+
+typedef struct Diag318Data {
+    uint64_t cpc;
+} Diag318Data;
+static Diag318Data diag318data;
+
+void diag318_reset(void)
+{
+    if (s390_has_feat(S390_FEAT_DIAG318)) {
+        kvm_s390_set_cpc(0);
+    }
+}
+
+static int diag318_post_load(void *opaque, int version_id)
+{
+    Diag318Data *d = opaque;
+
+    kvm_s390_set_cpc(d->cpc);
+    return 0;
+}
+
+static int diag318_pre_save(void *opaque)
+{
+    Diag318Data *d = opaque;
+
+    kvm_s390_get_cpc(&d->cpc);
+    return 0;
+}
+
+static bool diag318_needed(void *opaque)
+{
+    return s390_has_feat(S390_FEAT_DIAG318);
+}
+
+const VMStateDescription vmstate_diag318 = {
+    .name = "diag318",
+    .post_load = diag318_post_load,
+    .pre_save = diag318_pre_save,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = diag318_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT64(cpc, Diag318Data),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+void diag318_register(void)
+{
+    vmstate_register(NULL, 0, &vmstate_diag318, &diag318data);
+}
\ No newline at end of file
diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index 70015ea..a3d1457 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -450,6 +450,7 @@  static uint16_t full_GEN12_GA1[] = {
     S390_FEAT_AP_QUERY_CONFIG_INFO,
     S390_FEAT_AP_FACILITIES_TEST,
     S390_FEAT_AP,
+    S390_FEAT_DIAG318,
 };
 
 static uint16_t full_GEN12_GA2[] = {
diff --git a/target/s390x/internal.h b/target/s390x/internal.h
index f2a771e..d53b1d0 100644
--- a/target/s390x/internal.h
+++ b/target/s390x/internal.h
@@ -383,10 +383,12 @@  int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw,
                        target_ulong *addr, int *flags);
 
 
-/* misc_helper.c */
+/* diag.c */
 int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3);
 void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3,
                      uintptr_t ra);
+void diag318_register(void);
+void diag318_reset(void);
 
 
 /* translate.c */
diff --git a/target/s390x/kvm-stub.c b/target/s390x/kvm-stub.c
index bf7795e..1f0bee3 100644
--- a/target/s390x/kvm-stub.c
+++ b/target/s390x/kvm-stub.c
@@ -104,3 +104,13 @@  void kvm_s390_stop_interrupt(S390CPU *cpu)
 void kvm_s390_restart_interrupt(S390CPU *cpu)
 {
 }
+
+int kvm_s390_get_cpc(uint64_t *cpc)
+{
+    return 0;
+}
+
+int kvm_s390_set_cpc(uint64_t cpc)
+{
+    return 0;
+}
\ No newline at end of file
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 2ebf26a..9ed4cea 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -749,6 +749,28 @@  int kvm_s390_set_clock_ext(uint8_t tod_high, uint64_t tod_low)
     return kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attr);
 }
 
+int kvm_s390_get_cpc(uint64_t *cpc)
+{
+    struct kvm_device_attr attr = {
+        .group = KVM_S390_VM_MISC,
+        .attr = KVM_S390_VM_MISC_CPC,
+        .addr = (uint64_t)cpc,
+    };
+
+    return kvm_vm_ioctl(kvm_state, KVM_GET_DEVICE_ATTR, &attr);
+}
+
+int kvm_s390_set_cpc(uint64_t cpc)
+{
+    struct kvm_device_attr attr = {
+        .group = KVM_S390_VM_MISC,
+        .attr = KVM_S390_VM_MISC_CPC,
+        .addr = (uint64_t)&cpc,
+    };
+
+    return kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attr);
+}
+
 /**
  * kvm_s390_mem_op:
  * @addr:      the logical start address in guest memory
@@ -2142,6 +2164,7 @@  static int kvm_to_feat[][2] = {
     { KVM_S390_VM_CPU_FEAT_PFMFI, S390_FEAT_SIE_PFMFI},
     { KVM_S390_VM_CPU_FEAT_SIGPIF, S390_FEAT_SIE_SIGPIF},
     { KVM_S390_VM_CPU_FEAT_KSS, S390_FEAT_SIE_KSS},
+    { KVM_S390_VM_CPU_FEAT_DIAG318, S390_FEAT_DIAG318},
 };
 
 static int query_cpu_feat(S390FeatBitmap features)
diff --git a/target/s390x/kvm_s390x.h b/target/s390x/kvm_s390x.h
index 6e52287..53f165f 100644
--- a/target/s390x/kvm_s390x.h
+++ b/target/s390x/kvm_s390x.h
@@ -29,6 +29,9 @@  int kvm_s390_get_clock(uint8_t *tod_high, uint64_t *tod_clock);
 int kvm_s390_get_clock_ext(uint8_t *tod_high, uint64_t *tod_clock);
 int kvm_s390_set_clock(uint8_t tod_high, uint64_t tod_clock);
 int kvm_s390_set_clock_ext(uint8_t tod_high, uint64_t tod_clock);
+int kvm_s390_get_cpc(uint64_t *cpc);
+int kvm_s390_set_cpc(uint64_t cpc);
+bool kvm_s390_has_diag318(void);
 void kvm_s390_enable_css_support(S390CPU *cpu);
 int kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch,
                                     int vq, bool assign);