Re: [PATCH v3 8/8] s390: guest support for diagnose 0x318

[Janosch Frank]

On 6/19/20 12:22 AM, Collin Walling wrote:
> DIAGNOSE 0x318 (diag318) is an s390 instruction that allows the storage
> of diagnostic information that is collected by the firmware in the case
> of hardware/firmware service events.
> 
> QEMU handles the instruction by storing the info in the CPU state. A
> subsequent register sync will communicate the data to the hypervisor.
> 
> QEMU handles the migration via a VM State Description.
> 
> This feature depends on the Extended-Length SCCB (els) feature. If
> els is not present, then a warning will be printed and the SCLP bit
> that allows the Linux kernel to execute the instruction will not be
> set.
> 
> Availability of this instruction is determined by byte 134 (aka fac134)
> bit 0 of the SCLP Read Info block. This coincidentally expands into the
> space used for CPU entries, which means VMs running with the diag318
> capability may not be able to read information regarding all CPUs
> unless the guest kernel supports an extended-length SCCB.
> 
> This feature is not supported in protected virtualization mode.
> 
> Signed-off-by: Collin Walling <walling@linux.ibm.com>

Acked-by: Janosch Frank <frankja@linux.ibm.com>

> ---
>  hw/s390x/sclp.c                     |  5 +++++
>  include/hw/s390x/sclp.h             |  3 +++
>  target/s390x/cpu.h                  |  3 ++-
>  target/s390x/cpu_features.h         |  1 +
>  target/s390x/cpu_features_def.inc.h |  3 +++
>  target/s390x/cpu_models.c           |  1 +
>  target/s390x/gen-features.c         |  1 +
>  target/s390x/kvm.c                  | 31 +++++++++++++++++++++++++++++
>  target/s390x/machine.c              | 17 ++++++++++++++++
>  9 files changed, 64 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index f7c49e339e..78dbfbe427 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -152,6 +152,11 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>      s390_get_feat_block(S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT,
>                           read_info->conf_char_ext);
>  
> +    if (s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB)) {
> +        s390_get_feat_block(S390_FEAT_TYPE_SCLP_FAC134,
> +                            &read_info->fac134);
> +    }
> +
>      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 ef2d63eae9..ccb9f0a676 100644
> --- a/include/hw/s390x/sclp.h
> +++ b/include/hw/s390x/sclp.h
> @@ -133,6 +133,9 @@ 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  fac134;
> +    uint8_t  _reserved8[144 - 135];     /* 135-143 */
>      struct CPUEntry entries[];
>  } QEMU_PACKED ReadInfo;
>  
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index 035427521c..52765961cf 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -112,6 +112,8 @@ struct CPUS390XState {
>      uint16_t external_call_addr;
>      DECLARE_BITMAP(emergency_signals, S390_MAX_CPUS);
>  
> +    uint64_t diag318_info;
> +
>      /* Fields up to this point are cleared by a CPU reset */
>      struct {} end_reset_fields;
>  
> @@ -136,7 +138,6 @@ struct CPUS390XState {
>  
>      /* currently processed sigp order */
>      uint8_t sigp_order;
> -

Whitespace damage

>  };
>  
>  static inline uint64_t *get_freg(CPUS390XState *cs, int nr)
> diff --git a/target/s390x/cpu_features.h b/target/s390x/cpu_features.h
> index da695a8346..f74f7fc3a1 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_FAC134,
>      S390_FEAT_TYPE_SCLP_CPU,
>      S390_FEAT_TYPE_MISC,
>      S390_FEAT_TYPE_PLO,
> diff --git a/target/s390x/cpu_features_def.inc.h 
> b/target/s390x/cpu_features_def.inc.h
> index 1c04cc18f4..f82b4b5ec1 100644
> --- a/target/s390x/cpu_features_def.inc.h
> +++ b/target/s390x/cpu_features_def.inc.h
> @@ -122,6 +122,9 @@ DEF_FEAT(SIE_CMMA, "cmma", SCLP_CONF_CHAR_EXT, 1, "SIE: 
> Collaborative-memory-man
>  DEF_FEAT(SIE_PFMFI, "pfmfi", SCLP_CONF_CHAR_EXT, 9, "SIE: PFMF 
> interpretation facility")
>  DEF_FEAT(SIE_IBS, "ibs", SCLP_CONF_CHAR_EXT, 10, "SIE: 
> Interlock-and-broadcast-suppression facility")
>  
> +/* Features exposed via SCLP SCCB Facilities byte 134 (bit numbers relative 
> to byte-134) */
> +DEF_FEAT(DIAG_318, "diag318", SCLP_FAC134, 0, "Control program name and 
> version codes")
> +
>  /* Features exposed via SCLP CPU info. */
>  DEF_FEAT(SIE_F2, "sief2", SCLP_CPU, 4, "SIE: interception format 2 (Virtual 
> SIE)")
>  DEF_FEAT(SIE_SKEY, "skey", SCLP_CPU, 5, "SIE: Storage-key facility")
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index 2fa609bffe..034673be54 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -827,6 +827,7 @@ static void check_consistency(const S390CPUModel *model)
>          { S390_FEAT_PTFF_STOE, S390_FEAT_MULTIPLE_EPOCH },
>          { S390_FEAT_PTFF_STOUE, S390_FEAT_MULTIPLE_EPOCH },
>          { S390_FEAT_AP_QUEUE_INTERRUPT_CONTROL, S390_FEAT_AP },
> +        { S390_FEAT_DIAG_318, S390_FEAT_EXTENDED_LENGTH_SCCB },
>      };
>      int i;
>  
> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
> index 6857f657fb..a1f0a6f3c6 100644
> --- a/target/s390x/gen-features.c
> +++ b/target/s390x/gen-features.c
> @@ -523,6 +523,7 @@ static uint16_t full_GEN12_GA1[] = {
>      S390_FEAT_AP_FACILITIES_TEST,
>      S390_FEAT_AP,
>      S390_FEAT_EXTENDED_LENGTH_SCCB,
> +    S390_FEAT_DIAG_318,
>  };
>  
>  static uint16_t full_GEN12_GA2[] = {
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index a2d5ad78f6..b79feeba9f 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -105,6 +105,7 @@
>  
>  #define DIAG_TIMEREVENT                 0x288
>  #define DIAG_IPL                        0x308
> +#define DIAG_SET_CONTROL_PROGRAM_CODES  0x318
>  #define DIAG_KVM_HYPERCALL              0x500
>  #define DIAG_KVM_BREAKPOINT             0x501
>  
> @@ -602,6 +603,11 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>          cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_ETOKEN;
>      }
>  
> +    if (can_sync_regs(cs, KVM_SYNC_DIAG318)) {
> +        cs->kvm_run->s.regs.diag318 = env->diag318_info;
> +        cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_DIAG318;
> +    }
> +
>      /* Finally the prefix */
>      if (can_sync_regs(cs, KVM_SYNC_PREFIX)) {
>          cs->kvm_run->s.regs.prefix = env->psa;
> @@ -741,6 +747,10 @@ int kvm_arch_get_registers(CPUState *cs)
>          }
>      }
>  
> +    if (can_sync_regs(cs, KVM_SYNC_DIAG318)) {
> +        env->diag318_info = cs->kvm_run->s.regs.diag318;
> +    }
> +
>      return 0;
>  }
>  
> @@ -1601,6 +1611,19 @@ static int handle_sw_breakpoint(S390CPU *cpu, struct 
> kvm_run *run)
>      return -ENOENT;
>  }
>  
> +static void handle_diag_318(S390CPU *cpu, struct kvm_run *run)
> +{
> +    uint64_t reg = (run->s390_sieic.ipa & 0x00f0) >> 4;
> +    uint64_t diag318_info = run->s.regs.gprs[reg];
> +
> +    cpu->env.diag318_info = diag318_info;
> +
> +    if (can_sync_regs(CPU(cpu), KVM_SYNC_DIAG318)) {
> +        run->s.regs.diag318 = diag318_info;
> +        run->kvm_dirty_regs |= KVM_SYNC_DIAG318;
> +    }
> +}
> +
>  #define DIAG_KVM_CODE_MASK 0x000000000000ffff
>  
>  static int handle_diag(S390CPU *cpu, struct kvm_run *run, uint32_t ipb)
> @@ -1620,6 +1643,9 @@ static int handle_diag(S390CPU *cpu, struct kvm_run 
> *run, uint32_t ipb)
>      case DIAG_IPL:
>          kvm_handle_diag_308(cpu, run);
>          break;
> +    case DIAG_SET_CONTROL_PROGRAM_CODES:
> +        handle_diag_318(cpu, run);
> +        break;
>      case DIAG_KVM_HYPERCALL:
>          r = handle_hypercall(cpu, run);
>          break;
> @@ -2464,6 +2490,11 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, 
> Error **errp)
>       */
>      set_bit(S390_FEAT_EXTENDED_LENGTH_SCCB, model->features);
>  
> +    /* DIAGNOSE 0x318 is not supported under protected virtualization */
> +    if (!s390_is_pv() && kvm_check_extension(kvm_state, 
> KVM_CAP_S390_DIAG318)) {
> +        set_bit(S390_FEAT_DIAG_318, model->features);
> +    }
> +
>      /* strip of features that are not part of the maximum model */
>      bitmap_and(model->features, model->features, model->def->full_feat,
>                 S390_FEAT_MAX);
> diff --git a/target/s390x/machine.c b/target/s390x/machine.c
> index 549bb6c280..5b4e82f1ab 100644
> --- a/target/s390x/machine.c
> +++ b/target/s390x/machine.c
> @@ -234,6 +234,22 @@ const VMStateDescription vmstate_etoken = {
>      }
>  };
>  
> +static bool diag318_needed(void *opaque)
> +{
> +    return s390_has_feat(S390_FEAT_DIAG_318);
> +}
> +
> +const VMStateDescription vmstate_diag318 = {
> +    .name = "cpu/diag318",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = diag318_needed,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT64(env.diag318_info, S390CPU),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  const VMStateDescription vmstate_s390_cpu = {
>      .name = "cpu",
>      .post_load = cpu_post_load,
> @@ -270,6 +286,7 @@ const VMStateDescription vmstate_s390_cpu = {
>          &vmstate_gscb,
>          &vmstate_bpbc,
>          &vmstate_etoken,
> +        &vmstate_diag318,
>          NULL
>      },
>  };
>

signature.asc
Description: OpenPGP digital signature