Re: [PATCH 07/12] s390x/pci: enable for load/store intepretation

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

 



On 12/15/21 2:44 AM, Pierre Morel wrote:


On 12/7/21 22:04, Matthew Rosato wrote:
Use the associated vfio feature ioctl to enable interpretation for devices
when requested.  As part of this process, we must use the host function
handle rather than a QEMU-generated one -- this is provided as part of the
ioctl payload.

Signed-off-by: Matthew Rosato <mjrosato@xxxxxxxxxxxxx>
---
  hw/s390x/s390-pci-bus.c          | 69 +++++++++++++++++++++++++++++++-
  hw/s390x/s390-pci-inst.c         | 63 ++++++++++++++++++++++++++++-
  hw/s390x/s390-pci-vfio.c         | 55 +++++++++++++++++++++++++
  include/hw/s390x/s390-pci-bus.h  |  1 +
  include/hw/s390x/s390-pci-vfio.h | 15 +++++++
  5 files changed, 201 insertions(+), 2 deletions(-)

diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 01b58ebc70..451bd32d92 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -971,12 +971,57 @@ static void s390_pci_update_subordinate(PCIDevice *dev, uint32_t nr)
      }
  }
+static int s390_pci_interp_plug(S390pciState *s, S390PCIBusDevice *pbdev)
+{
+    uint32_t idx;
+    int rc;
+
+    rc = s390_pci_probe_interp(pbdev);
+    if (rc) {
+        return rc;
+    }
+
+    rc = s390_pci_update_passthrough_fh(pbdev);
+    if (rc) {
+        return rc;
+    }
+
+    /*
+     * The host device is in an enabled state, but the device must
+     * begin as disabled for the guest so mask off the enable bit
+     * from the passthrough handle.
+     */

I think you should explain why the device must be seen disabled for the guest. AFAIU it is because we need the guest to issue a CLP_ENABLE for us to activate interpretation. This is due to the choice of activate/deactivate interpretation during device enable/disable.


Not completely. While it is true that we need the guest to issue the CLP_ENABLE to activate interpretation with the way this is currently implemented, existing code also starts all plugged devices with a QEMU internal state of

pbdev->state = ZPCI_FS_DISABLED;

and a disabled pbdev->fh, thus expecting a subsequent CLP ENABLE from the guest when/if it decides to use the device.

If we were to set the handle to an enabled state here, we must also udpate the pbdev state, introducing a difference in how we present intercept/emulated devices vs interpreted devices during plug. I don't think we want to do that -- but I can improve this comment here to try and better explain that all devices begin plugged as disabled to the guest


Just curious: why not activate/deactivate interpretation on plug/unplug?


I think it would be possible to do so (while still showing a disabled handle initially), but my thinking is that tying these actions to guest CLP disable/enable will also be useful later when looking at trying to better-handle error events from the guest e.g. hot reset.





[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