On 30.09.24 13:11, Christian Borntraeger wrote:
Am 27.09.24 um 20:05 schrieb Halil Pasic:
On Thu, 12 Sep 2024 10:19:00 +0200
Thomas Huth <th...@redhat.com> wrote:
diff --git a/hw/s390x/s390-hypercall.h b/hw/s390x/s390-hypercall.h
index b7ac29f444..f0ca62bcbb 100644
--- a/hw/s390x/s390-hypercall.h
+++ b/hw/s390x/s390-hypercall.h
@@ -18,6 +18,7 @@
#define DIAG500_VIRTIO_RESET 1 /* legacy */
#define DIAG500_VIRTIO_SET_STATUS 2 /* legacy */
#define DIAG500_VIRTIO_CCW_NOTIFY 3 /* KVM_S390_VIRTIO_CCW_NOTIFY */
+#define DIAG500_STORAGE_LIMIT 4
int handle_diag_500(CPUS390XState *env);
Reviewed-by: Thomas Huth <th...@redhat.com>
Sounds very reasonable to me - but it would be good to get an
Ack/Reviewed-by from IBM folks here (in case they prefer a different
interface)... hope they'll join the discussion!
Thomas
According to Documentation/virt/kvm/s390/s390-diag.rst in the
linux source tree DIAG 500 is for kvm virtio funcions.
Based on the commit message this storagelimit DIAG is rather loosely
tied to virtio if at all, so from that perspective DIAG may not be a
perfect fit. OTOH I don't see a better fit either. I would prefer to
have Christian's opinion on this. I have no strong opinion myself.
Some remarks:
500 with a new subcode would work, it is marked as KVM hypervisor call
in our docs. 501 was used in the past for software breakpoints.
Right, we use it in the absence of KVM_CAP_S390_USER_INSTR0.
So we could use 502 as the next free one (This is reserved for KVM).
We do have kvm_stat counters for 500, not sure if people debugging virtio
will care.
It would be one additional trigger during system boot, so likely not
really an issue. We could always add new stats for selected subcodes
(i.e, KVM_S390_VIRTIO_CCW_NOTIFY).
The only important question for me is, what code is generated by gcc for
the switch statement and will any variant slow down the virtio doorbell.
diag.c has
if (!vcpu->kvm->arch.css_support ||
(vcpu->run->s.regs.gprs[1] != KVM_S390_VIRTIO_CCW_NOTIFY))
return -EOPNOTSUPP;
So 500+4 should probably not cause any harm apart from branch prediction
going wrong the first 2 or 3 notifies.
Right, it's very unlikely to be noticeable at all.
Thanks for the feedback!
--
Cheers,
David / dhildenb
