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.
> } 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++;
}
}
> +#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)
> {
(...)
