On Fri, 15 May 2020 18:20:32 -0400
Collin Walling <wall...@linux.ibm.com> wrote:
> DIAGNOSE 0x318 (diag 318) allows the storage of diagnostic data that
> is collected by the firmware in the case of hardware/firmware service
> events.
>
> The instruction is invoked in the Linux kernel and is handled,
> migrated, and reset (modified clear and load normal) by QEMU.
> KVM assists with the get/set of the relevent data via IOCTLs.
>
> 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 diag 318
> 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.
I think it should be easy enough to wire it up for !kvm as well
(although I'm not sure how useful it would be there -- mostly for
seeing what a guest does with it, I guess.)
>
> Signed-off-by: Collin Walling <wall...@linux.ibm.com>
> ---
> hw/s390x/s390-virtio-ccw.c | 45 +++++++++++++++++++++++++++++
> hw/s390x/sclp.c | 5 ++++
> include/hw/s390x/s390-virtio-ccw.h | 1 +
> include/hw/s390x/sclp.h | 3 ++
> target/s390x/cpu.c | 23 +++++++++++++++
> target/s390x/cpu.h | 4 +++
> 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-stub.c | 10 +++++++
> target/s390x/kvm.c | 40 +++++++++++++++++++++++++
> target/s390x/kvm_s390x.h | 2 ++
> 13 files changed, 139 insertions(+)
>
(...)
> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
> index f2ccf0a06a..367a54c173 100644
> --- a/target/s390x/cpu.c
> +++ b/target/s390x/cpu.c
> @@ -446,6 +446,29 @@ void s390_enable_css_support(S390CPU *cpu)
> kvm_s390_enable_css_support(cpu);
> }
> }
> +
> +void s390_get_diag_318_info(uint64_t *info)
> +{
> + if (kvm_enabled()) {
> + kvm_s390_get_diag_318_info(info);
> + }
> +}
> +
> +void s390_set_diag_318_info(uint64_t info)
> +{
> + if (kvm_enabled()) {
> + kvm_s390_set_diag_318_info(info);
> + }
> +}
> +
> +bool s390_diag_318_is_allowed(void)
> +{
> + if (kvm_enabled()) {
I would recommend not tying this explicitly to kvm -- assuming that the
feature checks should be enough.
> + return s390_has_feat(S390_FEAT_DIAG_318) &&
> + s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB);
> + }
> + return false;
> +}
> #endif
>
> static gchar *s390_gdb_arch_name(CPUState *cs)
(...)
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 380fb81822..5d7dc60c85 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
>
> @@ -814,6 +815,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_diag_318_info(uint64_t *info)
> +{
> + struct kvm_device_attr attr = {
> + .group = KVM_S390_VM_MISC,
> + .attr = KVM_S390_VM_MISC_DIAG_318,
> + .addr = (uint64_t)info,
> + };
> +
> + return kvm_vm_ioctl(kvm_state, KVM_GET_DEVICE_ATTR, &attr);
> +}
> +
> +int kvm_s390_set_diag_318_info(uint64_t info)
> +{
> + struct kvm_device_attr attr = {
> + .group = KVM_S390_VM_MISC,
> + .attr = KVM_S390_VM_MISC_DIAG_318,
> + .addr = (uint64_t)&info,
> + };
> +
> + return kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attr);
> +}
> +
> /**
> * kvm_s390_mem_op:
> * @addr: the logical start address in guest memory
> @@ -1601,6 +1624,14 @@ static int handle_sw_breakpoint(S390CPU *cpu, struct
> kvm_run *run)
> return -ENOENT;
> }
>
> +static void kvm_handle_diag_318(struct kvm_run *run)
> +{
> + uint64_t reg = (run->s390_sieic.ipa & 0x00f0) >> 4;
> + uint64_t info = run->s.regs.gprs[reg];
> +
> + kvm_s390_set_diag_318_info(info);
Follow the other diag handlers and rather call a common
handle_diag_318() function, which will in turn call
s390_set_diag_318_info()? While that feels like a bit of extra churn at
a glance, it allows non-kvm access to this diag more easily.
> +}
> +
> #define DIAG_KVM_CODE_MASK 0x000000000000ffff
>
> static int handle_diag(S390CPU *cpu, struct kvm_run *run, uint32_t ipb)
> @@ -1620,6 +1651,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:
> + kvm_handle_diag_318(run);
> + break;
> case DIAG_KVM_HYPERCALL:
> r = handle_hypercall(cpu, run);
> break;
> @@ -2460,6 +2494,12 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model,
> Error **errp)
> /* Extended-Length SCCB is handled entirely within QEMU */
> set_bit(S390_FEAT_EXTENDED_LENGTH_SCCB, model->features);
>
> + /* Allow diag 0x318 iff KVM supported and not in PV mode */
"iff supported by KVM" ?
> + if (!s390_is_pv() && kvm_vm_check_attr(kvm_state,
> + KVM_S390_VM_MISC, KVM_S390_VM_MISC_DIAG_318)) {
> + 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);
(...)
