On Fri, Jul 14, 2017 at 06:40:34AM +0000, Bharat Bhushan wrote: > Hi Peter, > > > -----Original Message----- > > From: Peter Xu [mailto:pet...@redhat.com] > > Sent: Friday, July 14, 2017 7:48 AM > > To: Eric Auger <eric.au...@redhat.com> > > Cc: eric.auger....@gmail.com; peter.mayd...@linaro.org; > > alex.william...@redhat.com; m...@redhat.com; qemu-...@nongnu.org; > > qemu-devel@nongnu.org; jean-philippe.bruc...@arm.com; > > w...@redhat.com; kevin.t...@intel.com; Bharat Bhushan > > <bharat.bhus...@nxp.com>; marc.zyng...@arm.com; t...@semihalf.com; > > will.dea...@arm.com; drjo...@redhat.com; robin.mur...@arm.com; > > christoffer.d...@linaro.org > > 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 <eric.au...@redhat.com> > > > > [...] > > > > > 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
