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.
