[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 8/8] vfio: Create VFIOAddressSpace objects as ne
From: |
David Gibson |
Subject: |
Re: [Qemu-devel] [PATCH 8/8] vfio: Create VFIOAddressSpace objects as needed |
Date: |
Tue, 14 May 2013 11:58:30 +1000 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Mon, May 13, 2013 at 03:33:06PM -0600, Alex Williamson wrote:
> On Mon, 2013-05-13 at 20:54 +1000, David Gibson wrote:
> > So far, VFIO has a notion of different logical DMA address spaces, but
> > only ever uses one (system memory). This patch extends this, creating
> > new VFIOAddressSpace objects as necessary, according to the AddressSpace
> > reported by the PCI subsystem for this device's DMAs.
> >
> > This isn't enough yet to support guest side IOMMUs with VFIO, but it does
> > mean we could now support VFIO devices on, for example, a guest side PCI
> > host bridge which maps system memory at somewhere other than 0 in PCI
> > space.
> >
> > Signed-off-by: David Gibson <address@hidden>
> > ---
> > hw/misc/vfio.c | 36 +++++++++++++++++++++++++++++-------
> > 1 file changed, 29 insertions(+), 7 deletions(-)
> >
> > diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
> > index b1e9220..3850d39 100644
> > --- a/hw/misc/vfio.c
> > +++ b/hw/misc/vfio.c
> > @@ -116,9 +116,10 @@ enum {
> > typedef struct VFIOAddressSpace {
> > AddressSpace *as;
> > QLIST_HEAD(, VFIOContainer) containers;
> > + QLIST_ENTRY(VFIOAddressSpace) list;
> > } VFIOAddressSpace;
> >
> > -static VFIOAddressSpace vfio_address_space_memory;
> > +QLIST_HEAD(, VFIOAddressSpace) vfio_address_spaces;
> >
> > struct VFIOGroup;
> >
> > @@ -2635,6 +2636,23 @@ static void vfio_address_space_init(VFIOAddressSpace
> > *vas, AddressSpace *as)
> > QLIST_INIT(&vas->containers);
> > }
> >
> > +static VFIOAddressSpace *vfio_address_space_get(AddressSpace *as)
>
> vfio_get_address_space is a better match for the rest of the code.
Ok.
> > +{
> > + VFIOAddressSpace *vas;
> > +
> > + QLIST_FOREACH(vas, &vfio_address_spaces, list) {
> > + if (vas->as == as)
> > + return vas;
> > + }
> > +
> > + /* No suitable VFIOAddressSpace, create a new one */
> > + vas = g_malloc0(sizeof(*vas));
> > + vfio_address_space_init(vas, as);
> > + QLIST_INSERT_HEAD(&vfio_address_spaces, vas, list);
>
> Do we still need vfio_address_space_init? Seems like it should be
> rolled in here.
Ah, true. I had some notions of allowing host bridges to statically
allocate a vfio address space as part of their own structure, but this
current code assumes they are malloc()ed by get_address_space, so yes,
I'll fold that in.
> > +
> > + return vas;
> > +}
> > +
> > static int vfio_connect(VFIOGroup *group, VFIOAddressSpace *vas)
> > {
> > VFIOContainer *container;
> > @@ -2727,6 +2745,8 @@ static void vfio_disconnect(VFIOGroup *group)
> > group->container = NULL;
> >
> > if (QLIST_EMPTY(&container->group_list)) {
> > + VFIOAddressSpace *vas = container->vas;
> > +
> > if (container->iommu_data.release) {
> > container->iommu_data.release(container);
> > }
> > @@ -2734,6 +2754,11 @@ static void vfio_disconnect(VFIOGroup *group)
> > DPRINTF("vfio_disconnect: close container->fd\n");
> > close(container->fd);
> > g_free(container);
> > +
> > + if (QLIST_EMPTY(&vas->containers)) {
> > + QLIST_REMOVE(vas, list);
> > + g_free(vas);
> > + }
>
> vfio_put_address_space? Where there's a get...
Fair enough, will revise.
>
> > }
> > }
> >
> > @@ -2984,6 +3009,7 @@ static int vfio_initfn(PCIDevice *pdev)
> > {
> > VFIODevice *pvdev, *vdev = DO_UPCAST(VFIODevice, pdev, pdev);
> > VFIOGroup *group;
> > + VFIOAddressSpace *vas;
> > char path[PATH_MAX], iommu_group_path[PATH_MAX], *group_name;
> > ssize_t len;
> > struct stat st;
> > @@ -3019,12 +3045,9 @@ static int vfio_initfn(PCIDevice *pdev)
> > DPRINTF("%s(%04x:%02x:%02x.%x) group %d\n", __func__,
> > vdev->host.domain,
> > vdev->host.bus, vdev->host.slot, vdev->host.function, groupid);
> >
> > - if (pci_iommu_as(pdev) != &address_space_memory) {
> > - error_report("vfio: DMA address space must be system memory");
> > - return -ENXIO;
> > - }
> > + vas = vfio_address_space_get(pci_iommu_as(pdev));
>
> I don't think the structure malloc'd here will get cleaned up in all
> cases on error. Thanks,
Good point, auditing now..
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: Digital signature
- Re: [Qemu-devel] [PATCH 7/8] vfio: Introduce VFIO address spaces, (continued)
[Qemu-devel] [PATCH 5/8] pci: Introduce helper to retrieve a PCI device's DMA address space, David Gibson, 2013/05/13
[Qemu-devel] [PATCH 8/8] vfio: Create VFIOAddressSpace objects as needed, David Gibson, 2013/05/13
[Qemu-devel] [PATCH 3/8] pci: Rework PCI iommu lifetime assumptions, David Gibson, 2013/05/13
Re: [Qemu-devel] [0/8] RFC: VFIO and guest side IOMMUs, revisited, Paolo Bonzini, 2013/05/13
- Re: [Qemu-devel] [0/8] RFC: VFIO and guest side IOMMUs, revisited, David Gibson, 2013/05/13
- Re: [Qemu-devel] [0/8] RFC: VFIO and guest side IOMMUs, revisited, Paolo Bonzini, 2013/05/13
- Re: [Qemu-devel] [0/8] RFC: VFIO and guest side IOMMUs, revisited, David Gibson, 2013/05/13
- Re: [Qemu-devel] [0/8] RFC: VFIO and guest side IOMMUs, revisited, Paolo Bonzini, 2013/05/14
- Re: [Qemu-devel] [0/8] RFC: VFIO and guest side IOMMUs, revisited, David Gibson, 2013/05/14