[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v7 12/17] vfio-user: IOMMU support for remote device
From: |
Stefan Hajnoczi |
Subject: |
Re: [PATCH v7 12/17] vfio-user: IOMMU support for remote device |
Date: |
Thu, 31 Mar 2022 10:47:33 +0100 |
On Wed, Mar 30, 2022 at 01:13:03PM -0400, Peter Xu wrote:
> On Wed, Mar 30, 2022 at 05:08:24PM +0100, Stefan Hajnoczi wrote:
> > On Wed, Mar 30, 2022 at 08:53:16AM -0400, Peter Xu wrote:
> > > On Wed, Mar 30, 2022 at 11:04:24AM +0100, Stefan Hajnoczi wrote:
> > > > This makes me wonder whether there is a deeper issue with the
> > > > pci_setup_iommu() API: the lack of per-device cleanup callbacks.
> > > > Per-device IOMMU resources should be freed when a device is hot
> > > > unplugged.
> > > >
> > > > From what I can tell this is not the case today:
> > > >
> > > > - hw/i386/intel_iommu.c:vtd_find_add_as() allocates and adds device
> > > > address spaces but I can't find where they are removed and freed.
> > > > VTDAddressSpace instances pointed to from vtd_bus->dev_as[] are
> > > > leaked.
> > > >
> > > > - hw/i386/amd_iommu.c has similar leaks.
> > >
> > > AFAICT it's because there's no device-specific data cached in the
> > > per-device IOMMU address space, at least so far. IOW, all the data
> > > structures allocated here can be re-used when a new device is plugged in
> > > after the old device unplugged.
> > >
> > > It's definitely not ideal since after unplug (and before a new device
> > > plugged in) the resource is not needed at all so it's kind of wasted, but
> > > it should work functionally. If to achieve that, some iommu_unplug() or
> > > iommu_cleanup() hook sounds reasonable.
> >
> > I guess the question is whether PCI busses can be hotplugged with
> > IOMMUs. If yes, then there is a memory leak that matters for
> > intel_iommu.c and amd_iommu.c.
>
> It can't, and we only support one vIOMMU so far for both (commit
> 1b3bf13890fd849b26). Thanks,
I see, thanks!
Okay, summarizing options for the vfio-user IOMMU:
1. Use the same singleton approach as existing IOMMUs where the
MemoryRegion/AddressSpace are never freed. Don't bother deleting.
2. Keep the approach in this patch where vfio-user code manually calls a
custom delete function (not part of the pci_setup_iommu() API). This
is slightly awkward to do without global state and that's what
started this discussion.
3. Introduce an optional pci_setup_iommu() callback:
typdef void (*PCIIOMMUDeviceUnplug)(PCIBus *bus, void *opaque, int devfn);
Solves the awkwardness of option #2. Not needed by existing IOMMU
devices.
Any preferences anyone?
Stefan
signature.asc
Description: PGP signature
- [PATCH v7 17/17] vfio-user: avocado tests for vfio-user, (continued)
- [PATCH v7 17/17] vfio-user: avocado tests for vfio-user, Jagannathan Raman, 2022/03/25
- [PATCH v7 12/17] vfio-user: IOMMU support for remote device, Jagannathan Raman, 2022/03/25
- Re: [PATCH v7 12/17] vfio-user: IOMMU support for remote device, Stefan Hajnoczi, 2022/03/29
- Re: [PATCH v7 12/17] vfio-user: IOMMU support for remote device, Jag Raman, 2022/03/29
- Re: [PATCH v7 12/17] vfio-user: IOMMU support for remote device, Stefan Hajnoczi, 2022/03/29
- Re: [PATCH v7 12/17] vfio-user: IOMMU support for remote device, Jag Raman, 2022/03/29
- Re: [PATCH v7 12/17] vfio-user: IOMMU support for remote device, Stefan Hajnoczi, 2022/03/30
- Re: [PATCH v7 12/17] vfio-user: IOMMU support for remote device, Peter Xu, 2022/03/30
- Re: [PATCH v7 12/17] vfio-user: IOMMU support for remote device, Stefan Hajnoczi, 2022/03/30
- Re: [PATCH v7 12/17] vfio-user: IOMMU support for remote device, Peter Xu, 2022/03/30
- Re: [PATCH v7 12/17] vfio-user: IOMMU support for remote device,
Stefan Hajnoczi <=
- Re: [PATCH v7 12/17] vfio-user: IOMMU support for remote device, Peter Xu, 2022/03/31
[PATCH v7 13/17] vfio-user: handle DMA mappings, Jagannathan Raman, 2022/03/25
[PATCH v7 14/17] vfio-user: handle PCI BAR accesses, Jagannathan Raman, 2022/03/25
[PATCH v7 15/17] vfio-user: handle device interrupts, Jagannathan Raman, 2022/03/25