Re: [PATCH v5 5/9] s390x/pci: enable for load/store intepretation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 4/20/22 17:12, Matthew Rosato wrote:
On 4/19/22 3:47 PM, Pierre Morel wrote:


On 4/4/22 20:17, Matthew Rosato wrote:
If the appropriate CPU facilty is available as well as the necessary
ZPCI_OP ioctl, then the underlying KVM host will enable load/store
intepretation for any guest device without a SHM bit in the guest
function handle.  For a device that will be using interpretation
support, ensure the guest function handle matches the host function
handle; this value is re-checked every time the guest issues a SET PCI FN
to enable the guest device as it is the only opportunity to reflect
function handle changes.

By default, unless interpret=off is specified, interpretation support will
always be assumed and exploited if the necessary ioctl and features are
available on the host kernel.  When these are unavailable, we will silently revert to the interception model; this allows existing guest configurations to work unmodified on hosts with and without zPCI interpretation support,
allowing QEMU to choose the best support model available.

Signed-off-by: Matthew Rosato <mjrosato@xxxxxxxxxxxxx>
---
  hw/s390x/meson.build            |  1 +
  hw/s390x/s390-pci-bus.c         | 66 ++++++++++++++++++++++++++++++++-
  hw/s390x/s390-pci-inst.c        | 12 ++++++
  hw/s390x/s390-pci-kvm.c         | 21 +++++++++++
  include/hw/s390x/s390-pci-bus.h |  1 +
  include/hw/s390x/s390-pci-kvm.h | 24 ++++++++++++
  target/s390x/kvm/kvm.c          |  7 ++++
  target/s390x/kvm/kvm_s390x.h    |  1 +
  8 files changed, 132 insertions(+), 1 deletion(-)
  create mode 100644 hw/s390x/s390-pci-kvm.c
  create mode 100644 include/hw/s390x/s390-pci-kvm.h


...snip...

          if (s390_pci_msix_init(pbdev)) {
@@ -1360,6 +1423,7 @@ static Property s390_pci_device_properties[] = {
      DEFINE_PROP_UINT16("uid", S390PCIBusDevice, uid, UID_UNDEFINED),
      DEFINE_PROP_S390_PCI_FID("fid", S390PCIBusDevice, fid),
      DEFINE_PROP_STRING("target", S390PCIBusDevice, target),
+    DEFINE_PROP_BOOL("interpret", S390PCIBusDevice, interp, true),
      DEFINE_PROP_END_OF_LIST(),
  };
diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index 6d400d4147..c898c8abe9 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -18,6 +18,8 @@
  #include "sysemu/hw_accel.h"
  #include "hw/s390x/s390-pci-inst.h"
  #include "hw/s390x/s390-pci-bus.h"
+#include "hw/s390x/s390-pci-kvm.h"
+#include "hw/s390x/s390-pci-vfio.h"
  #include "hw/s390x/tod.h"
  #ifndef DEBUG_S390PCI_INST
@@ -246,6 +248,16 @@ int clp_service_call(S390CPU *cpu, uint8_t r2, uintptr_t ra)
                  goto out;
              }
+            /*
+             * Take this opportunity to make sure we still have an accurate +             * host fh.  It's possible part of the handle changed while the +             * device was disabled to the guest (e.g. vfio hot reset for
+             * ISM during plug)
+             */
+            if (pbdev->interp) {
+                /* Take this opportunity to make sure we are sync'd with host */
+                s390_pci_get_host_fh(pbdev, &pbdev->fh);

Here we should check the return value and, AFAIU, assume that the device disappear if it did return false.

+            }
              pbdev->fh |= FH_MASK_ENABLE;

Are we sure here that the PCI device is always enabled?
Shouldn't we check?

I guess you mean the host device?  Interesting thought.

So, to be clear, the idea on setting FH_MASK_ENABLE here is that we are handling a guest CLP SET PCI FN enable so the guest fh should always have FH_MASK_ENABLE set if we return CLP_RC_OK to the guest.

But for interpretation, if we find the host function is disabled, I suppose we could return an error on the guest CLP (not sure which error yet); otherwise, if we return the force-enabled handle and CLP_RC_OK as we do here then the guest will just get errors attempting to use it.

hum, in this case can't we have a loop on
clp enable->error->clp disable->clp enable->error...

I think we should return an error if what the guest asked for could not be done.






--
Pierre Morel
IBM Lab Boeblingen



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux