[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v7 3/5] virtio-iommu: Call iommu notifier for attach/detach
From: |
Auger Eric |
Subject: |
Re: [PATCH v7 3/5] virtio-iommu: Call iommu notifier for attach/detach |
Date: |
Mon, 16 Mar 2020 08:32:37 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 |
Hi Bharat,
On 3/16/20 7:41 AM, Bharat Bhushan wrote:
> Hi Eric,
>
> On Fri, Mar 13, 2020 at 8:11 PM Auger Eric <address@hidden> wrote:
>>
>> Hi Bharat
>>
>> On 3/13/20 8:48 AM, Bharat Bhushan wrote:
>>> iommu-notifier are called when a device is attached
>> IOMMU notifiers
>>> or detached to as address-space.
>>> This is needed for VFIO.
>> and vhost for detach
>>>
>>> Signed-off-by: Bharat Bhushan <address@hidden>
>>> ---
>>> hw/virtio/virtio-iommu.c | 47 ++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 47 insertions(+)
>>>
>>> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
>>> index e51344a53e..2006f72901 100644
>>> --- a/hw/virtio/virtio-iommu.c
>>> +++ b/hw/virtio/virtio-iommu.c
>>> @@ -49,6 +49,7 @@ typedef struct VirtIOIOMMUEndpoint {
>>> uint32_t id;
>>> VirtIOIOMMUDomain *domain;
>>> QLIST_ENTRY(VirtIOIOMMUEndpoint) next;
>>> + VirtIOIOMMU *viommu;
>> This needs specal care on post-load. When migrating the EPs, only the id
>> is migrated. On post-load you need to set viommu as it is done for
>> domain. migration is allowed with vhost.
>
> ok, I have not tried vhost/migration. Below change set viommu when
> reconstructing endpoint.
Yes I think this should be OK.
By the end I did the series a try with vhost/vfio. with vhost it works
(not with recent kernel though, but the issue may be related to kernel).
With VFIO however it does not for me.
First issue is: your guest can use 4K page and your host can use 64KB
pages. In that case VFIO_DMA_MAP will fail with -EINVAL. We must devise
a way to pass the host settings to the VIRTIO-IOMMU device.
Even with 64KB pages, it did not work for me. I have obviously not the
storm of VFIO_DMA_MAP failures but I have some, most probably due to
some wrong notifications somewhere. I will try to investigate on my side.
Did you test with VFIO on your side?
Thanks
Eric
>
> @@ -984,6 +973,7 @@ static gboolean reconstruct_endpoints(gpointer
> key, gpointer value,
>
> QLIST_FOREACH(iter, &d->endpoint_list, next) {
> iter->domain = d;
> + iter->viommu = s;
> g_tree_insert(s->endpoints, GUINT_TO_POINTER(iter->id), iter);
> }
> return false; /* continue the domain traversal */
>
>>> } VirtIOIOMMUEndpoint;
>>>
>>> typedef struct VirtIOIOMMUInterval {
>>> @@ -155,8 +156,44 @@ static void
>>> virtio_iommu_notify_unmap(IOMMUMemoryRegion *mr, hwaddr iova,
>>> memory_region_notify_iommu(mr, 0, entry);
>>> }
>>>
>>> +static gboolean virtio_iommu_mapping_unmap(gpointer key, gpointer value,
>>> + gpointer data)
>>> +{
>>> + VirtIOIOMMUInterval *interval = (VirtIOIOMMUInterval *) key;
>>> + IOMMUMemoryRegion *mr = (IOMMUMemoryRegion *) data;
>>> +
>>> + virtio_iommu_notify_unmap(mr, interval->low,
>>> + interval->high - interval->low + 1);
>>> +
>>> + return false;
>>> +}
>>> +
>>> +static gboolean virtio_iommu_mapping_map(gpointer key, gpointer value,
>>> + gpointer data)
>>> +{
>>> + VirtIOIOMMUMapping *mapping = (VirtIOIOMMUMapping *) value;
>>> + VirtIOIOMMUInterval *interval = (VirtIOIOMMUInterval *) key;
>>> + IOMMUMemoryRegion *mr = (IOMMUMemoryRegion *) data;
>>> +
>>> + virtio_iommu_notify_map(mr, interval->low, mapping->phys_addr,
>>> + interval->high - interval->low + 1);
>>> +
>>> + return false;
>>> +}
>>> +
>>> static void virtio_iommu_detach_endpoint_from_domain(VirtIOIOMMUEndpoint
>>> *ep)
>>> {
>>> + VirtioIOMMUNotifierNode *node;
>>> + VirtIOIOMMU *s = ep->viommu;
>>> + VirtIOIOMMUDomain *domain = ep->domain;
>>> +
>>> + QLIST_FOREACH(node, &s->notifiers_list, next) {
>>> + if (ep->id == node->iommu_dev->devfn) {
>>> + g_tree_foreach(domain->mappings, virtio_iommu_mapping_unmap,
>>> + &node->iommu_dev->iommu_mr);
>> I understand this should fo the job for domain removal
>
> did not get the comment, are you saying we should do this on domain removal?
see my reply on 2/5
Note the above code should be moved after the check of !ep->domain below
>
>>> + }
>>> + }
>>> +
>>> if (!ep->domain) {
>>> return;
>>> }
>>> @@ -178,6 +215,7 @@ static VirtIOIOMMUEndpoint
>>> *virtio_iommu_get_endpoint(VirtIOIOMMU *s,
>>> }
>>> ep = g_malloc0(sizeof(*ep));
>>> ep->id = ep_id;
>>> + ep->viommu = s;
>>> trace_virtio_iommu_get_endpoint(ep_id);
>>> g_tree_insert(s->endpoints, GUINT_TO_POINTER(ep_id), ep);
>>> return ep;
>>> @@ -272,6 +310,7 @@ static int virtio_iommu_attach(VirtIOIOMMU *s,
>>> {
>>> uint32_t domain_id = le32_to_cpu(req->domain);
>>> uint32_t ep_id = le32_to_cpu(req->endpoint);
>>> + VirtioIOMMUNotifierNode *node;
>>> VirtIOIOMMUDomain *domain;
>>> VirtIOIOMMUEndpoint *ep;
>>>
>>> @@ -299,6 +338,14 @@ static int virtio_iommu_attach(VirtIOIOMMU *s,
>>>
>>> ep->domain = domain;
>>>
>>> + /* Replay existing address space mappings on the associated memory
>>> region */
>> maybe use the "domain" terminology here.
>
> ok,
>
> Thanks
> -Bharat
>
>>> + QLIST_FOREACH(node, &s->notifiers_list, next) {
>>> + if (ep_id == node->iommu_dev->devfn) {
>>> + g_tree_foreach(domain->mappings, virtio_iommu_mapping_map,
>>> + &node->iommu_dev->iommu_mr);
>>> + }
>>> + }
>>> +
>>> return VIRTIO_IOMMU_S_OK;
>>> }
>>>
>>>
>> Thanks
>>
>> Eric
>>
>
- [PATCH v7 0/5] virtio-iommu: VFIO integration, Bharat Bhushan, 2020/03/13
- [PATCH v7 1/5] hw/vfio/common: Remove error print on mmio region translation by viommu, Bharat Bhushan, 2020/03/13
- [PATCH v7 3/5] virtio-iommu: Call iommu notifier for attach/detach, Bharat Bhushan, 2020/03/13
- Re: [PATCH v7 3/5] virtio-iommu: Call iommu notifier for attach/detach, Auger Eric, 2020/03/13
- Re: [PATCH v7 3/5] virtio-iommu: Call iommu notifier for attach/detach, Bharat Bhushan, 2020/03/16
- Re: [PATCH v7 3/5] virtio-iommu: Call iommu notifier for attach/detach,
Auger Eric <=
- Re: [PATCH v7 3/5] virtio-iommu: Call iommu notifier for attach/detach, Bharat Bhushan, 2020/03/16
- Re: [PATCH v7 3/5] virtio-iommu: Call iommu notifier for attach/detach, Bharat Bhushan, 2020/03/16
- Re: [PATCH v7 3/5] virtio-iommu: Call iommu notifier for attach/detach, Auger Eric, 2020/03/16
- Re: [PATCH v7 3/5] virtio-iommu: Call iommu notifier for attach/detach, Bharat Bhushan, 2020/03/16
- Re: [PATCH v7 3/5] virtio-iommu: Call iommu notifier for attach/detach, Jean-Philippe Brucker, 2020/03/16
- Re: [PATCH v7 3/5] virtio-iommu: Call iommu notifier for attach/detach, Bharat Bhushan, 2020/03/17
- Re: [PATCH v7 3/5] virtio-iommu: Call iommu notifier for attach/detach, Auger Eric, 2020/03/17
- Re: [PATCH v7 3/5] virtio-iommu: Call iommu notifier for attach/detach, Jean-Philippe Brucker, 2020/03/17
- Re: [PATCH v7 3/5] virtio-iommu: Call iommu notifier for attach/detach, Bharat Bhushan, 2020/03/17
- Re: [PATCH v7 3/5] virtio-iommu: Call iommu notifier for attach/detach, Jean-Philippe Brucker, 2020/03/17