On 10/09/2020 11.36, 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 <wall...@linux.ibm.com>
> Acked-by: Janosch Frank <fran...@linux.ibm.com>
> ---
> hw/s390x/sclp.c | 5 +++++
> include/hw/s390x/sclp.h | 3 +++
> target/s390x/cpu.h | 2 ++
> target/s390x/cpu_features.h | 1 +
> target/s390x/cpu_features_def.h.inc | 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(+)
>
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index 87d468087b..ad5d70e14d 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -139,6 +139,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);
> + }
Wasn't this feature also possible if there are less than 240 CPUs? Or do
I mix that up with something else? ... well, maybe it's best anyway if
we only allow this when ELS is enabled.
> 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 141e57f765..516f949109 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[];
Could you please add a comment after entries[] like,
/* only here with "ELS", it's at offset 128 otherwise */
?
> } QEMU_PACKED ReadInfo;
>
[...]
> 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;
> + }
> +}
What's supposed to happen if a guest calls diag 318 but the
S390_FEAT_DIAG_318 has not been enabled? Shouldn't there be a program
interrupt in that case?
Thomas
