[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [Qemu-devel] [RFC v2 6/8] virtio-iommu: Implement the tra
From: |
Peter Xu |
Subject: |
Re: [Qemu-arm] [Qemu-devel] [RFC v2 6/8] virtio-iommu: Implement the translation and commands |
Date: |
Mon, 17 Jul 2017 09:28:09 +0800 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Fri, Jul 14, 2017 at 06:40:34AM +0000, Bharat Bhushan wrote:
> Hi Peter,
>
> > -----Original Message-----
> > From: Peter Xu [mailto:address@hidden
> > Sent: Friday, July 14, 2017 7:48 AM
> > To: Eric Auger <address@hidden>
> > Cc: address@hidden; address@hidden;
> > address@hidden; address@hidden; address@hidden;
> > address@hidden; address@hidden;
> > address@hidden; address@hidden; Bharat Bhushan
> > <address@hidden>; address@hidden; address@hidden;
> > address@hidden; address@hidden; address@hidden;
> > address@hidden
> > Subject: Re: [Qemu-devel] [RFC v2 6/8] virtio-iommu: Implement the
> > translation and commands
> >
> > On Wed, Jun 07, 2017 at 06:01:25PM +0200, Eric Auger wrote:
> > > This patch adds the actual implementation for the translation routine
> > > and the virtio-iommu commands.
> > >
> > > Signed-off-by: Eric Auger <address@hidden>
> >
> > [...]
> >
> > > static int virtio_iommu_attach(VirtIOIOMMU *s,
> > > struct virtio_iommu_req_attach *req)
> > > @@ -95,10 +135,34 @@ static int virtio_iommu_attach(VirtIOIOMMU *s,
> > > uint32_t asid = le32_to_cpu(req->address_space);
> > > uint32_t devid = le32_to_cpu(req->device);
> > > uint32_t reserved = le32_to_cpu(req->reserved);
> > > + viommu_as *as;
> > > + viommu_dev *dev;
> > >
> > > trace_virtio_iommu_attach(asid, devid, reserved);
> > >
> > > - return VIRTIO_IOMMU_S_UNSUPP;
> > > + dev = g_tree_lookup(s->devices, GUINT_TO_POINTER(devid));
> > > + if (dev) {
> > > + return -1;
> > > + }
> > > +
> > > + as = g_tree_lookup(s->address_spaces, GUINT_TO_POINTER(asid));
> > > + if (!as) {
> > > + as = g_malloc0(sizeof(*as));
> > > + as->id = asid;
> > > + as->mappings = g_tree_new_full((GCompareDataFunc)interval_cmp,
> > > + NULL, NULL,
> > > (GDestroyNotify)g_free);
> > > + g_tree_insert(s->address_spaces, GUINT_TO_POINTER(asid), as);
> > > + trace_virtio_iommu_new_asid(asid);
> > > + }
> > > +
> > > + dev = g_malloc0(sizeof(*dev));
> > > + dev->as = as;
> > > + dev->id = devid;
> > > + as->nr_devices++;
> > > + trace_virtio_iommu_new_devid(devid);
> > > + g_tree_insert(s->devices, GUINT_TO_POINTER(devid), dev);
> >
> > Here do we need to record something like a refcount for address space?
> > Since...
>
> We are using "nr_devices" as number of devices attached to an address-space
>
> >
> > > +
> > > + return VIRTIO_IOMMU_S_OK;
> > > }
> > >
> > > static int virtio_iommu_detach(VirtIOIOMMU *s, @@ -106,10 +170,13 @@
> > > static int virtio_iommu_detach(VirtIOIOMMU *s, {
> > > uint32_t devid = le32_to_cpu(req->device);
> > > uint32_t reserved = le32_to_cpu(req->reserved);
> > > + int ret;
> > >
> > > trace_virtio_iommu_detach(devid, reserved);
> > >
> > > - return VIRTIO_IOMMU_S_UNSUPP;
> > > + ret = g_tree_remove(s->devices, GUINT_TO_POINTER(devid));
> >
> > ... here when detach, imho we should check the refcount: if there is no
> > device using specific address space, we should release the address space as
> > well.
> >
> > Otherwise there would have no way to destroy an address space?
>
>
> Here if nr_devices == 0 then release the address space, is that ok?
>
> This is how I implemented as part of VFIO integration over this patch series.
> "[RFC PATCH 2/2] virtio-iommu: vfio integration with virtio-iommu"
Sorry I didn't read that when posting. It is what I mean. Thanks,
--
Peter Xu