qemu-arm
[Top][All Lists]
Advanced

[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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]