[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC v2 6/8] virtio-iommu: Implement the translation an
From: |
Bharat Bhushan |
Subject: |
Re: [Qemu-devel] [RFC v2 6/8] virtio-iommu: Implement the translation and commands |
Date: |
Thu, 3 Aug 2017 10:48:15 +0000 |
Hi Eric,
> -----Original Message-----
> From: Auger Eric [mailto:address@hidden
> Sent: Monday, July 31, 2017 6:38 PM
> To: Peter Xu <address@hidden>; Bharat Bhushan
> <address@hidden>
> Cc: address@hidden; address@hidden; address@hidden;
> address@hidden; address@hidden; address@hidden;
> address@hidden; address@hidden; 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
>
> Hi Peter, Bharat,
>
> On 17/07/2017 03:28, Peter Xu wrote:
> > 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; qemu-
> 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"
>
> glib provides g_tree_ref/g_tree_unref. I think the most elegant is to use
> g_tree_ref when adding a device and decrementing when removing a
> device, as suggested by Peter.
>
> "if the reference count drops to 0, all keys and values will be destroyed (if
> destroy functions were specified) and all memory allocated by tree will be
> released."
>
> That way we should be able to remove nr_devices from viommu_as
>
> Bharat, if this breaks your implementation I will let you re-introduce
> nr_devices as part of the VFIO patch.
We need to unmap() everything in physical-iommu when last device is detached,
seems like g_tree_ref/unref() does not give any handle for this. I will go
ahead with adding nr_devices with VFIO integration patches.
Thanks
-Bharat
>
> Thanks
>
> Eric
> >
> > Sorry I didn't read that when posting. It is what I mean. Thanks,
> >
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [RFC v2 6/8] virtio-iommu: Implement the translation and commands,
Bharat Bhushan <=