Re: [PATCH 09/12] s390x/pci: enable adapter event notification for interpreted devices
On 12/8/21 6:29 AM, Thomas Huth wrote:
On 07/12/2021 22.04, Matthew Rosato wrote:
Use the associated vfio feature ioctl to enable adapter event
notification
and forwarding for devices when requested. This feature will be set up
with or without firmware assist based upon the 'intassist' setting.
Signed-off-by: Matthew Rosato
---
hw/s390x/s390-pci-bus.c | 24 +++--
hw/s390x/s390-pci-inst.c | 54 +++-
hw/s390x/s390-pci-vfio.c | 88
include/hw/s390x/s390-pci-bus.h | 1 +
include/hw/s390x/s390-pci-vfio.h | 20
5 files changed, 182 insertions(+), 5 deletions(-)
[...]
diff --git a/hw/s390x/s390-pci-vfio.c b/hw/s390x/s390-pci-vfio.c
index 78093aaac7..6f9271df87 100644
--- a/hw/s390x/s390-pci-vfio.c
+++ b/hw/s390x/s390-pci-vfio.c
@@ -152,6 +152,94 @@ int
s390_pci_update_passthrough_fh(S390PCIBusDevice *pbdev)
return 0;
}
+int s390_pci_probe_aif(S390PCIBusDevice *pbdev)
+{
+ VFIOPCIDevice *vdev = container_of(pbdev->pdev, VFIOPCIDevice,
pdev);
Should this use VFIO_PCI() instead of container_of ?
Yes, VFIO_PCI(pbdev->pdev) should work.
+ struct vfio_device_feature feat = {
+ .argsz = sizeof(struct vfio_device_feature),
+ .flags = VFIO_DEVICE_FEATURE_PROBE +
VFIO_DEVICE_FEATURE_ZPCI_AIF
+ };
+
+ assert(vdev);
... then you could likely also drop the assert(), I think?
If I've understood qom.h correctly then yes you're right, if we use the
instance checker VFIO_PCI() we should trigger an assert through
object_dynamic_cast_assert() already if the pdev isn't a vfio-pci device
-- I just verified that by trying to call VFIO_PCI() with something else
and we indeed get an assert e.g. 'VFIO_PCI: Object 0x... is not an
instance of type vfio-pci'
So I'll change these and get rid of the extra asserts, thanks.
Re: [PATCH 09/12] s390x/pci: enable adapter event notification for interpreted devices
On 07/12/2021 22.04, Matthew Rosato wrote:
Use the associated vfio feature ioctl to enable adapter event notification
and forwarding for devices when requested. This feature will be set up
with or without firmware assist based upon the 'intassist' setting.
Signed-off-by: Matthew Rosato
---
hw/s390x/s390-pci-bus.c | 24 +++--
hw/s390x/s390-pci-inst.c | 54 +++-
hw/s390x/s390-pci-vfio.c | 88
include/hw/s390x/s390-pci-bus.h | 1 +
include/hw/s390x/s390-pci-vfio.h | 20
5 files changed, 182 insertions(+), 5 deletions(-)
[...]
diff --git a/hw/s390x/s390-pci-vfio.c b/hw/s390x/s390-pci-vfio.c
index 78093aaac7..6f9271df87 100644
--- a/hw/s390x/s390-pci-vfio.c
+++ b/hw/s390x/s390-pci-vfio.c
@@ -152,6 +152,94 @@ int s390_pci_update_passthrough_fh(S390PCIBusDevice *pbdev)
return 0;
}
+int s390_pci_probe_aif(S390PCIBusDevice *pbdev)
+{
+VFIOPCIDevice *vdev = container_of(pbdev->pdev, VFIOPCIDevice, pdev);
Should this use VFIO_PCI() instead of container_of ?
+struct vfio_device_feature feat = {
+.argsz = sizeof(struct vfio_device_feature),
+.flags = VFIO_DEVICE_FEATURE_PROBE + VFIO_DEVICE_FEATURE_ZPCI_AIF
+};
+
+assert(vdev);
... then you could likely also drop the assert(), I think?
+return ioctl(vdev->vbasedev.fd, VFIO_DEVICE_FEATURE, );
+}
+
+int s390_pci_set_aif(S390PCIBusDevice *pbdev, ZpciFib *fib, bool enable,
+ bool assist)
+{
+VFIOPCIDevice *vdev = container_of(pbdev->pdev, VFIOPCIDevice, pdev);
+g_autofree struct vfio_device_feature *feat;
+struct vfio_device_zpci_aif *data;
+int size;
+
+assert(vdev);
dito
+size = sizeof(*feat) + sizeof(*data);
+feat = g_malloc0(size);
+feat->argsz = size;
+feat->flags = VFIO_DEVICE_FEATURE_SET + VFIO_DEVICE_FEATURE_ZPCI_AIF;
+
+data = (struct vfio_device_zpci_aif *)>data;
+if (enable) {
+data->flags = VFIO_DEVICE_ZPCI_FLAG_AIF_FLOAT;
+if (!pbdev->intassist) {
+data->flags |= VFIO_DEVICE_ZPCI_FLAG_AIF_HOST;
+}
+/* Fill in the guest fib info */
+data->ibv = fib->aibv;
+data->sb = fib->aisb;
+data->noi = FIB_DATA_NOI(fib->data);
+data->isc = FIB_DATA_ISC(fib->data);
+data->sbo = FIB_DATA_AISBO(fib->data);
+} else {
+data->flags = 0;
+}
+
+return ioctl(vdev->vbasedev.fd, VFIO_DEVICE_FEATURE, feat);
+}
+
+int s390_pci_get_aif(S390PCIBusDevice *pbdev, bool enable, bool assist)
+{
+VFIOPCIDevice *vdev = container_of(pbdev->pdev, VFIOPCIDevice, pdev);
+g_autofree struct vfio_device_feature *feat;
+struct vfio_device_zpci_aif *data;
+int size, rc;
+
+assert(vdev);
dito
+size = sizeof(*feat) + sizeof(*data);
+feat = g_malloc0(size);
+feat->argsz = size;
+feat->flags = VFIO_DEVICE_FEATURE_GET + VFIO_DEVICE_FEATURE_ZPCI_AIF;
+
+rc = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_FEATURE, feat);
+if (rc) {
+return rc;
+}
+
+/* Determine if current interrupt settings match the host */
+data = (struct vfio_device_zpci_aif *)>data;
+if (enable && (!(data->flags & VFIO_DEVICE_ZPCI_FLAG_AIF_FLOAT))) {
+rc = -EINVAL;
+} else if (!enable && (data->flags & VFIO_DEVICE_ZPCI_FLAG_AIF_FLOAT)) {
+rc = -EINVAL;
+}
+
+/*
+ * When enabled for interrupts, the assist and forced host-delivery are
+ * mututally exclusive
+ */
+if (enable && assist && (data->flags & VFIO_DEVICE_ZPCI_FLAG_AIF_HOST)) {
+rc = -EINVAL;
+} else if (enable && (!assist) && (!(data->flags &
+ VFIO_DEVICE_ZPCI_FLAG_AIF_HOST))) {
+rc = -EINVAL;
+}
+
+return rc;
+}
Thomas
[PATCH 09/12] s390x/pci: enable adapter event notification for interpreted devices
Use the associated vfio feature ioctl to enable adapter event notification
and forwarding for devices when requested. This feature will be set up
with or without firmware assist based upon the 'intassist' setting.
Signed-off-by: Matthew Rosato
---
hw/s390x/s390-pci-bus.c | 24 +++--
hw/s390x/s390-pci-inst.c | 54 +++-
hw/s390x/s390-pci-vfio.c | 88
include/hw/s390x/s390-pci-bus.h | 1 +
include/hw/s390x/s390-pci-vfio.h | 20
5 files changed, 182 insertions(+), 5 deletions(-)
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 503326210a..1ae8792a26 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -189,7 +189,10 @@ void s390_pci_sclp_deconfigure(SCCB *sccb)
rc = SCLP_RC_NO_ACTION_REQUIRED;
break;
default:
-if (pbdev->summary_ind) {
+if (pbdev->interp) {
+/* Interpreted devices were using interrupt forwarding */
+s390_pci_set_aif(pbdev, NULL, false, pbdev->intassist);
+} else if (pbdev->summary_ind) {
pci_dereg_irqs(pbdev);
}
if (pbdev->iommu->enabled) {
@@ -981,6 +984,11 @@ static int s390_pci_interp_plug(S390pciState *s,
S390PCIBusDevice *pbdev)
return rc;
}
+rc = s390_pci_probe_aif(pbdev);
+if (rc) {
+return rc;
+}
+
rc = s390_pci_update_passthrough_fh(pbdev);
if (rc) {
return rc;
@@ -1075,6 +1083,7 @@ static void s390_pcihost_plug(HotplugHandler
*hotplug_dev, DeviceState *dev,
if (pbdev->interp && !s390_has_feat(S390_FEAT_ZPCI_INTERP)) {
DPRINTF("zPCI interpretation facilities missing.\n");
pbdev->interp = false;
+pbdev->intassist = false;
}
if (pbdev->interp) {
rc = s390_pci_interp_plug(s, pbdev);
@@ -1089,11 +1098,13 @@ static void s390_pcihost_plug(HotplugHandler
*hotplug_dev, DeviceState *dev,
if (!pbdev->interp) {
/* Do vfio passthrough but intercept for I/O */
pbdev->fh |= FH_SHM_VFIO;
+pbdev->intassist = false;
}
} else {
pbdev->fh |= FH_SHM_EMUL;
/* Always intercept emulated devices */
pbdev->interp = false;
+pbdev->intassist = false;
}
if (s390_pci_msix_init(pbdev) && !pbdev->interp) {
@@ -1243,7 +1254,10 @@ static void s390_pcihost_reset(DeviceState *dev)
/* Process all pending unplug requests */
QTAILQ_FOREACH_SAFE(pbdev, >zpci_devs, link, next) {
if (pbdev->unplug_requested) {
-if (pbdev->summary_ind) {
+if (pbdev->interp) {
+/* Interpreted devices were using interrupt forwarding */
+s390_pci_set_aif(pbdev, NULL, false, pbdev->intassist);
+} else if (pbdev->summary_ind) {
pci_dereg_irqs(pbdev);
}
if (pbdev->iommu->enabled) {
@@ -1381,7 +1395,10 @@ static void s390_pci_device_reset(DeviceState *dev)
break;
}
-if (pbdev->summary_ind) {
+if (pbdev->interp) {
+/* Interpreted devices were using interrupt forwarding */
+s390_pci_set_aif(pbdev, NULL, false, pbdev->intassist);
+} else if (pbdev->summary_ind) {
pci_dereg_irqs(pbdev);
}
if (pbdev->iommu->enabled) {
@@ -1427,6 +1444,7 @@ static Property s390_pci_device_properties[] = {
DEFINE_PROP_S390_PCI_FID("fid", S390PCIBusDevice, fid),
DEFINE_PROP_STRING("target", S390PCIBusDevice, target),
DEFINE_PROP_BOOL("interp", S390PCIBusDevice, interp, true),
+DEFINE_PROP_BOOL("intassist", S390PCIBusDevice, intassist, true),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index ba4017474e..02093e82f9 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -,6 +,46 @@ static void fmb_update(void *opaque)
timer_mod(pbdev->fmb_timer, t + DEFAULT_MUI);
}
+static int mpcifc_reg_int_interp(S390PCIBusDevice *pbdev, ZpciFib *fib)
+{
+int rc;
+
+/* Interpreted devices must also use interrupt forwarding */
+rc = s390_pci_get_aif(pbdev, false, pbdev->intassist);
+if (rc) {
+DPRINTF("Bad interrupt forwarding state\n");
+return rc;
+}
+
+rc = s390_pci_set_aif(pbdev, fib, true, pbdev->intassist);
+if (rc) {
+DPRINTF("Failed to enable interrupt forwarding\n");
+return rc;
+}
+
+return 0;
+}
+
+static int mpcifc_dereg_int_interp(S390PCIBusDevice *pbdev, ZpciFib *fib)
+{
+int rc;
+
+/* Interpreted devices were using interrupt forwarding */
+rc = s390_pci_get_aif(pbdev, true, pbdev->intassist);
+if (rc) {
+DPRINTF("Bad interrupt forwarding state\n");
+return rc;
+}
+
+rc = s390_pci_set_aif(pbdev, fib, false,
