From: Cornelia Huck <cohuck@redhat.com>
To: Collin Walling <walling@linux.ibm.com>
Cc: thuth@redhat.com, frankja@linux.ibm.com, mst@redhat.com,
david@redhat.com, qemu-devel@nongnu.org, pasic@linux.ibm.com,
borntraeger@de.ibm.com, qemu-s390x@nongnu.org,
svens@linux.ibm.com, pbonzini@redhat.com, mihajlov@linux.ibm.com,
rth@twiddle.net
Subject: Re: [PATCH v2 8/8] s390: guest support for diagnose 0x318
Date: Wed, 20 May 2020 13:30:12 +0200 [thread overview]
Message-ID: <20200520133012.1ad60b71.cohuck@redhat.com> (raw)
In-Reply-To: <20200515222032.18838-9-walling@linux.ibm.com>
On Fri, 15 May 2020 18:20:32 -0400
Collin Walling <walling@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 <walling@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);
(...)
next prev parent reply other threads:[~2020-05-20 11:32 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-15 22:20 [PATCH v2 0/8] s390: Extended-Length SCCB & DIAGNOSE 0x318 Collin Walling
2020-05-15 22:20 ` [PATCH v2 1/8] s390/sclp: get machine once during read scp/cpu info Collin Walling
2020-05-18 8:38 ` David Hildenbrand
2020-05-18 17:30 ` Collin Walling
2020-06-11 11:33 ` Thomas Huth
2020-05-15 22:20 ` [PATCH v2 2/8] s390/sclp: check sccb len before filling in data Collin Walling
2020-05-18 8:37 ` Janosch Frank
2020-05-18 11:46 ` David Hildenbrand
2020-05-18 14:32 ` Collin Walling
2020-05-18 15:43 ` David Hildenbrand
2020-05-18 17:31 ` Collin Walling
2020-06-11 12:01 ` Thomas Huth
2020-06-15 15:47 ` Collin Walling
2020-05-15 22:20 ` [PATCH v2 3/8] s390/sclp: rework sclp boundary and length checks Collin Walling
2020-05-18 8:50 ` Janosch Frank
2020-05-18 15:15 ` Collin Walling
2020-05-19 13:19 ` Cornelia Huck
2020-05-25 10:53 ` Janosch Frank
2020-06-11 12:56 ` Thomas Huth
2020-06-15 15:47 ` Collin Walling
2020-05-15 22:20 ` [PATCH v2 4/8] s390/sclp: read sccb from mem based on sccb length Collin Walling
2020-06-11 13:05 ` Thomas Huth
2020-05-15 22:20 ` [PATCH v2 5/8] s390/sclp: use cpu offset to locate cpu entries Collin Walling
2020-06-11 14:33 ` Thomas Huth
2020-05-15 22:20 ` [PATCH v2 6/8] s390/sclp: add extended-length sccb support for kvm guest Collin Walling
2020-05-18 8:55 ` Janosch Frank
2020-05-18 14:31 ` Collin Walling
2020-05-25 10:50 ` Janosch Frank
2020-05-26 14:38 ` Collin Walling
2020-05-19 13:47 ` Cornelia Huck
2020-05-15 22:20 ` [PATCH v2 7/8] s390/kvm: header sync for diag318 Collin Walling
2020-05-15 22:20 ` [PATCH v2 8/8] s390: guest support for diagnose 0x318 Collin Walling
2020-05-20 11:30 ` Cornelia Huck [this message]
2020-05-21 6:18 ` Collin Walling
2020-05-16 6:41 ` [PATCH v2 0/8] s390: Extended-Length SCCB & DIAGNOSE 0x318 no-reply
2020-05-18 17:34 ` Collin Walling
2020-05-18 17:51 ` David Hildenbrand
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200520133012.1ad60b71.cohuck@redhat.com \
--to=cohuck@redhat.com \
--cc=borntraeger@de.ibm.com \
--cc=david@redhat.com \
--cc=frankja@linux.ibm.com \
--cc=mihajlov@linux.ibm.com \
--cc=mst@redhat.com \
--cc=pasic@linux.ibm.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-s390x@nongnu.org \
--cc=rth@twiddle.net \
--cc=svens@linux.ibm.com \
--cc=thuth@redhat.com \
--cc=walling@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).