Re: [PATCH v3 5/5] s390x/pci: Honor DMA limits set by vfio

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

 



On 9/16/20 7:05 AM, Cornelia Huck wrote:
On Tue, 15 Sep 2020 15:14:43 -0400
Matthew Rosato <mjrosato@xxxxxxxxxxxxx> wrote:

When an s390 guest is using lazy unmapping, it can result in a very
large number of oustanding DMA requests, far beyond the default
limit configured for vfio.  Let's track DMA usage similar to vfio
in the host, and trigger the guest to flush their DMA mappings
before vfio runs out.

Signed-off-by: Matthew Rosato <mjrosato@xxxxxxxxxxxxx>
---
  hw/s390x/s390-pci-bus.c  | 56 +++++++++++++++++++++++++++++++++++++++++++-----
  hw/s390x/s390-pci-bus.h  |  9 ++++++++
  hw/s390x/s390-pci-inst.c | 34 +++++++++++++++++++++++------
  hw/s390x/s390-pci-inst.h |  3 +++
  4 files changed, 91 insertions(+), 11 deletions(-)

(...)

@@ -737,6 +740,41 @@ static void s390_pci_iommu_free(S390pciState *s, PCIBus *bus, int32_t devfn)
      object_unref(OBJECT(iommu));
  }
+static S390PCIDMACount *s390_start_dma_count(S390pciState *s, VFIODevice *vdev)

Should these go into the new vfio-related file?

+{
+    int id = vdev->group->container->fd;
+    S390PCIDMACount *cnt;
+    uint32_t avail;
+
+    if (!s390_pci_update_dma_avail(id, &avail)) {
+        return NULL;
+    }
+
+    QTAILQ_FOREACH(cnt, &s->zpci_dma_limit, link) {
+        if (cnt->id  == id) {
+            cnt->users++;
+            return cnt;
+        }
+    }
+
+    cnt = g_new0(S390PCIDMACount, 1);
+    cnt->id = id;
+    cnt->users = 1;
+    cnt->avail = avail;
+    QTAILQ_INSERT_TAIL(&s->zpci_dma_limit, cnt, link);
+    return cnt;
+}
+
+static void s390_end_dma_count(S390pciState *s, S390PCIDMACount *cnt)
+{
+    assert(cnt);
+
+    cnt->users--;
+    if (cnt->users == 0) {
+        QTAILQ_REMOVE(&s->zpci_dma_limit, cnt, link);
+    }
+}
+
  static void s390_pcihost_realize(DeviceState *dev, Error **errp)
  {
      PCIBus *b;
@@ -764,6 +802,7 @@ static void s390_pcihost_realize(DeviceState *dev, Error **errp)
      s->bus_no = 0;
      QTAILQ_INIT(&s->pending_sei);
      QTAILQ_INIT(&s->zpci_devs);
+    QTAILQ_INIT(&s->zpci_dma_limit);
css_register_io_adapters(CSS_IO_ADAPTER_PCI, true, false,
                               S390_ADAPTER_SUPPRESSIBLE, errp);
@@ -902,6 +941,7 @@ static void s390_pcihost_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
  {
      S390pciState *s = S390_PCI_HOST_BRIDGE(hotplug_dev);
      PCIDevice *pdev = NULL;
+    VFIOPCIDevice *vpdev = NULL;
      S390PCIBusDevice *pbdev = NULL;
if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_BRIDGE)) {
@@ -941,17 +981,20 @@ static void s390_pcihost_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
              }
          }
+ pbdev->pdev = pdev;
+        pbdev->iommu = s390_pci_get_iommu(s, pci_get_bus(pdev), pdev->devfn);
+        pbdev->iommu->pbdev = pbdev;
+        pbdev->state = ZPCI_FS_DISABLED;
+
          if (object_dynamic_cast(OBJECT(dev), "vfio-pci")) {
              pbdev->fh |= FH_SHM_VFIO;
+            vpdev = container_of(pbdev->pdev, VFIOPCIDevice, pdev);
+            pbdev->iommu->dma_limit = s390_start_dma_count(s,
+                                                           &vpdev->vbasedev);

I think you can just pass s and pbdev to that function... that would
move dealing with vfio specifics from this file.

I had considered this as well, should have went with my gut -- I'll move them.


          } else {
              pbdev->fh |= FH_SHM_EMUL;
          }
- pbdev->pdev = pdev;
-        pbdev->iommu = s390_pci_get_iommu(s, pci_get_bus(pdev), pdev->devfn);
-        pbdev->iommu->pbdev = pbdev;
-        pbdev->state = ZPCI_FS_DISABLED;
-
          if (s390_pci_msix_init(pbdev)) {
              error_setg(errp, "MSI-X support is mandatory "
                         "in the S390 architecture");

(...)

diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index 2f7a7d7..cc34b17 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -32,6 +32,9 @@
          }                                                          \
      } while (0)
+#define inc_dma_avail(iommu) if (iommu->dma_limit) iommu->dma_limit->avail++;

I was thinking more of something like

static inline void inc_dma_avail(S390PCIIOMMU *iommu)
{
     if (iommu->dma_limit) {
         iommu->dma_limit->avail++;
     }
}


Ah, I read the 'lowercase' and missed the 'inline function' part of your previous comment, sorry. Will change.

+#define dec_dma_avail(iommu) if (iommu->dma_limit) iommu->dma_limit->avail--;
+
  static void s390_set_status_code(CPUS390XState *env,
                                   uint8_t r, uint64_t status_code)
  {

(...)





[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