This patch extends VIRTIO_IOMMU_T_MAP/UNMAP request to
notify registered iommu-notifier. Which will call vfio
notifier to map/unmap region in iommu.
Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com>
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
hw/virtio/trace-events | 2 +
hw/virtio/virtio-iommu.c | 66 +++++++++++++++++++++++++++++++-
include/hw/virtio/virtio-iommu.h | 6 +++
3 files changed, 73 insertions(+), 1 deletion(-)
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index e83500bee9..d94a1cd8a3 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -73,3 +73,5 @@ virtio_iommu_get_domain(uint32_t domain_id) "Alloc domain=%d"
virtio_iommu_put_domain(uint32_t domain_id) "Free domain=%d"
virtio_iommu_translate_out(uint64_t virt_addr, uint64_t phys_addr, uint32_t sid) "0x%"PRIx64" -> 0x%"PRIx64 " for sid=%d"
virtio_iommu_report_fault(uint8_t reason, uint32_t flags, uint32_t endpoint, uint64_t addr) "FAULT reason=%d flags=%d endpoint=%d address =0x%"PRIx64
+virtio_iommu_notify_map(const char *name, uint64_t iova, uint64_t paddr, uint64_t map_size) "mr=%s iova=0x%"PRIx64" pa=0x%" PRIx64" size=0x%"PRIx64
+virtio_iommu_notify_unmap(const char *name, uint64_t iova, uint64_t map_size) "mr=%s iova=0x%"PRIx64" size=0x%"PRIx64
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 4cee8083bc..e51344a53e 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -123,6 +123,38 @@ static gint interval_cmp(gconstpointer a, gconstpointer b, gpointer user_data)
}
}
+static void virtio_iommu_notify_map(IOMMUMemoryRegion *mr, hwaddr iova,
+ hwaddr paddr, hwaddr size)
+{
+ IOMMUTLBEntry entry;
+
+ entry.target_as = &address_space_memory;
+ entry.addr_mask = size - 1;
+
+ entry.iova = iova;
+ trace_virtio_iommu_notify_map(mr->parent_obj.name, iova, paddr, size);
+ entry.perm = IOMMU_RW;
+ entry.translated_addr = paddr;
+
+ memory_region_notify_iommu(mr, 0, entry);
+}
+
+static void virtio_iommu_notify_unmap(IOMMUMemoryRegion *mr, hwaddr iova,
+ hwaddr size)
+{
+ IOMMUTLBEntry entry;
+
+ entry.target_as = &address_space_memory;
+ entry.addr_mask = size - 1;
+
+ entry.iova = iova;
+ trace_virtio_iommu_notify_unmap(mr->parent_obj.name, iova, size);
+ entry.perm = IOMMU_NONE;
+ entry.translated_addr = 0;
+
+ memory_region_notify_iommu(mr, 0, entry);
+}
+
static void virtio_iommu_detach_endpoint_from_domain(VirtIOIOMMUEndpoint *ep)
{
if (!ep->domain) {
@@ -307,9 +339,12 @@ static int virtio_iommu_map(VirtIOIOMMU *s,
uint64_t virt_start = le64_to_cpu(req->virt_start);
uint64_t virt_end = le64_to_cpu(req->virt_end);
uint32_t flags = le32_to_cpu(req->flags);
+ hwaddr size = virt_end - virt_start + 1;
+ VirtioIOMMUNotifierNode *node;
VirtIOIOMMUDomain *domain;
VirtIOIOMMUInterval *interval;
VirtIOIOMMUMapping *mapping;
+ VirtIOIOMMUEndpoint *ep;
if (flags & ~VIRTIO_IOMMU_MAP_F_MASK) {
return VIRTIO_IOMMU_S_INVAL;
@@ -339,9 +374,37 @@ static int virtio_iommu_map(VirtIOIOMMU *s,
g_tree_insert(domain->mappings, interval, mapping);
+ /* All devices in an address-space share mapping */
+ QLIST_FOREACH(node, &s->notifiers_list, next) {
+ QLIST_FOREACH(ep, &domain->endpoint_list, next) {
+ if (ep->id == node->iommu_dev->devfn) {
+ virtio_iommu_notify_map(&node->iommu_dev->iommu_mr,
+ virt_start, phys_start, size);
+ }
+ }
+ }
+
return VIRTIO_IOMMU_S_OK;
}
+static void virtio_iommu_remove_mapping(VirtIOIOMMU *s, VirtIOIOMMUDomain *domain,
+ VirtIOIOMMUInterval *interval)
+{
+ VirtioIOMMUNotifierNode *node;
+ VirtIOIOMMUEndpoint *ep;
+
+ QLIST_FOREACH(node, &s->notifiers_list, next) {
+ QLIST_FOREACH(ep, &domain->endpoint_list, next) {
+ if (ep->id == node->iommu_dev->devfn) {
+ virtio_iommu_notify_unmap(&node->iommu_dev->iommu_mr,
+ interval->low,
+ interval->high - interval->low + 1);
+ }
+ }
+ }
+ g_tree_remove(domain->mappings, (gpointer)(interval));
+}
+
static int virtio_iommu_unmap(VirtIOIOMMU *s,
struct virtio_iommu_req_unmap *req)
{
@@ -368,7 +431,7 @@ static int virtio_iommu_unmap(VirtIOIOMMU *s,
uint64_t current_high = iter_key->high;
if (interval.low <= current_low && interval.high >= current_high) {
- g_tree_remove(domain->mappings, iter_key);
+ virtio_iommu_remove_mapping(s, domain, iter_key);
trace_virtio_iommu_unmap_done(domain_id, current_low, current_high);
} else {
ret = VIRTIO_IOMMU_S_RANGE;
@@ -655,6 +718,7 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
VirtIODevice *vdev = VIRTIO_DEVICE(dev);
VirtIOIOMMU *s = VIRTIO_IOMMU(dev);
+ QLIST_INIT(&s->notifiers_list);
virtio_init(vdev, "virtio-iommu", VIRTIO_ID_IOMMU,
sizeof(struct virtio_iommu_config));
diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
index 6f67f1020a..4539c8ae72 100644
--- a/include/hw/virtio/virtio-iommu.h
+++ b/include/hw/virtio/virtio-iommu.h
@@ -44,6 +44,11 @@ typedef struct IOMMUPciBus {
IOMMUDevice *pbdev[0]; /* Parent array is sparse, so dynamically alloc */
} IOMMUPciBus;
+typedef struct VirtioIOMMUNotifierNode {
+ IOMMUDevice *iommu_dev;
+ QLIST_ENTRY(VirtioIOMMUNotifierNode) next;
+} VirtioIOMMUNotifierNode;
+
typedef struct VirtIOIOMMU {
VirtIODevice parent_obj;
VirtQueue *req_vq;
@@ -56,6 +61,7 @@ typedef struct VirtIOIOMMU {
GTree *domains;
QemuMutex mutex;
GTree *endpoints;
+ QLIST_HEAD(, VirtioIOMMUNotifierNode) notifiers_list;
} VirtIOIOMMU;
#endif
--
2.17.1
Hi Bharat,
On 3/13/20 8:48 AM, Bharat Bhushan wrote:
> This patch extends VIRTIO_IOMMU_T_MAP/UNMAP request to
> notify registered iommu-notifier. Which will call vfio
s/iommu-notifier/iommu-notifiers
> notifier to map/unmap region in iommu.
can be any notifier (vhost/vfio).
>
> Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
> hw/virtio/trace-events | 2 +
> hw/virtio/virtio-iommu.c | 66 +++++++++++++++++++++++++++++++-
> include/hw/virtio/virtio-iommu.h | 6 +++
> 3 files changed, 73 insertions(+), 1 deletion(-)
>
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index e83500bee9..d94a1cd8a3 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -73,3 +73,5 @@ virtio_iommu_get_domain(uint32_t domain_id) "Alloc domain=%d"
> virtio_iommu_put_domain(uint32_t domain_id) "Free domain=%d"
> virtio_iommu_translate_out(uint64_t virt_addr, uint64_t phys_addr, uint32_t sid) "0x%"PRIx64" -> 0x%"PRIx64 " for sid=%d"
> virtio_iommu_report_fault(uint8_t reason, uint32_t flags, uint32_t endpoint, uint64_t addr) "FAULT reason=%d flags=%d endpoint=%d address =0x%"PRIx64
> +virtio_iommu_notify_map(const char *name, uint64_t iova, uint64_t paddr, uint64_t map_size) "mr=%s iova=0x%"PRIx64" pa=0x%" PRIx64" size=0x%"PRIx64
> +virtio_iommu_notify_unmap(const char *name, uint64_t iova, uint64_t map_size) "mr=%s iova=0x%"PRIx64" size=0x%"PRIx64
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index 4cee8083bc..e51344a53e 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -123,6 +123,38 @@ static gint interval_cmp(gconstpointer a, gconstpointer b, gpointer user_data)
> }
> }
>
> +static void virtio_iommu_notify_map(IOMMUMemoryRegion *mr, hwaddr iova,
> + hwaddr paddr, hwaddr size)
> +{
> + IOMMUTLBEntry entry;
> +
> + entry.target_as = &address_space_memory;
> + entry.addr_mask = size - 1;
> +
> + entry.iova = iova;
> + trace_virtio_iommu_notify_map(mr->parent_obj.name, iova, paddr, size);
> + entry.perm = IOMMU_RW;
> + entry.translated_addr = paddr;
> +
> + memory_region_notify_iommu(mr, 0, entry);
> +}
> +
> +static void virtio_iommu_notify_unmap(IOMMUMemoryRegion *mr, hwaddr iova,
> + hwaddr size)
> +{
> + IOMMUTLBEntry entry;
> +
> + entry.target_as = &address_space_memory;
> + entry.addr_mask = size - 1;
> +
> + entry.iova = iova;
> + trace_virtio_iommu_notify_unmap(mr->parent_obj.name, iova, size);
> + entry.perm = IOMMU_NONE;
> + entry.translated_addr = 0;
> +
> + memory_region_notify_iommu(mr, 0, entry);
> +}
> +
> static void virtio_iommu_detach_endpoint_from_domain(VirtIOIOMMUEndpoint *ep)
> {
> if (!ep->domain) {
> @@ -307,9 +339,12 @@ static int virtio_iommu_map(VirtIOIOMMU *s,
> uint64_t virt_start = le64_to_cpu(req->virt_start);
> uint64_t virt_end = le64_to_cpu(req->virt_end);
> uint32_t flags = le32_to_cpu(req->flags);
> + hwaddr size = virt_end - virt_start + 1;
> + VirtioIOMMUNotifierNode *node;
> VirtIOIOMMUDomain *domain;
> VirtIOIOMMUInterval *interval;
> VirtIOIOMMUMapping *mapping;
> + VirtIOIOMMUEndpoint *ep;
>
> if (flags & ~VIRTIO_IOMMU_MAP_F_MASK) {
> return VIRTIO_IOMMU_S_INVAL;
> @@ -339,9 +374,37 @@ static int virtio_iommu_map(VirtIOIOMMU *s,
>
> g_tree_insert(domain->mappings, interval, mapping);
>
> + /* All devices in an address-space share mapping */
> + QLIST_FOREACH(node, &s->notifiers_list, next) {
> + QLIST_FOREACH(ep, &domain->endpoint_list, next) {
> + if (ep->id == node->iommu_dev->devfn) {
> + virtio_iommu_notify_map(&node->iommu_dev->iommu_mr,
> + virt_start, phys_start, size);
> + }
> + }
> + }
> +
> return VIRTIO_IOMMU_S_OK;
> }
>
> +static void virtio_iommu_remove_mapping(VirtIOIOMMU *s, VirtIOIOMMUDomain *domain,
> + VirtIOIOMMUInterval *interval)
> +{
> + VirtioIOMMUNotifierNode *node;
> + VirtIOIOMMUEndpoint *ep;
> +
> + QLIST_FOREACH(node, &s->notifiers_list, next) {
> + QLIST_FOREACH(ep, &domain->endpoint_list, next) {
> + if (ep->id == node->iommu_dev->devfn) {
> + virtio_iommu_notify_unmap(&node->iommu_dev->iommu_mr,
> + interval->low,
> + interval->high - interval->low + 1);
> + }
> + }
> + }
> + g_tree_remove(domain->mappings, (gpointer)(interval));
> +}
What about virtio_iommu_put_domain() where you destroy the mapping
gtree. I guess you also need to send invalidations there?
> +
> static int virtio_iommu_unmap(VirtIOIOMMU *s,
> struct virtio_iommu_req_unmap *req)
> {
> @@ -368,7 +431,7 @@ static int virtio_iommu_unmap(VirtIOIOMMU *s,
> uint64_t current_high = iter_key->high;
>
> if (interval.low <= current_low && interval.high >= current_high) {
> - g_tree_remove(domain->mappings, iter_key);
> + virtio_iommu_remove_mapping(s, domain, iter_key);
> trace_virtio_iommu_unmap_done(domain_id, current_low, current_high);
> } else {
> ret = VIRTIO_IOMMU_S_RANGE;
> @@ -655,6 +718,7 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
> VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> VirtIOIOMMU *s = VIRTIO_IOMMU(dev);
>
> + QLIST_INIT(&s->notifiers_list);
> virtio_init(vdev, "virtio-iommu", VIRTIO_ID_IOMMU,
> sizeof(struct virtio_iommu_config));
>
> diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
> index 6f67f1020a..4539c8ae72 100644
> --- a/include/hw/virtio/virtio-iommu.h
> +++ b/include/hw/virtio/virtio-iommu.h
> @@ -44,6 +44,11 @@ typedef struct IOMMUPciBus {
> IOMMUDevice *pbdev[0]; /* Parent array is sparse, so dynamically alloc */
> } IOMMUPciBus;
>
> +typedef struct VirtioIOMMUNotifierNode {
> + IOMMUDevice *iommu_dev;
> + QLIST_ENTRY(VirtioIOMMUNotifierNode) next;
> +} VirtioIOMMUNotifierNode;
You may use scripts/git.orderfile for a better diff ordering.
> +
> typedef struct VirtIOIOMMU {
> VirtIODevice parent_obj;
> VirtQueue *req_vq;
> @@ -56,6 +61,7 @@ typedef struct VirtIOIOMMU {
> GTree *domains;
> QemuMutex mutex;
> GTree *endpoints;
> + QLIST_HEAD(, VirtioIOMMUNotifierNode) notifiers_list;
See what was done in smmuv3 and intel. We now directly use a list of
IOMMUDevice directly. VirtioIOMMUNotifierNode does not bring anything extra.
> } VirtIOIOMMU;
>
> #endif
>
Thanks
Eric
Hi Eric,
On Fri, Mar 13, 2020 at 7:55 PM Auger Eric <eric.auger@redhat.com> wrote:
>
> Hi Bharat,
> On 3/13/20 8:48 AM, Bharat Bhushan wrote:
> > This patch extends VIRTIO_IOMMU_T_MAP/UNMAP request to
> > notify registered iommu-notifier. Which will call vfio
> s/iommu-notifier/iommu-notifiers
> > notifier to map/unmap region in iommu.
> can be any notifier (vhost/vfio).
> >
> > Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com>
> > Signed-off-by: Eric Auger <eric.auger@redhat.com>
> > ---
> > hw/virtio/trace-events | 2 +
> > hw/virtio/virtio-iommu.c | 66 +++++++++++++++++++++++++++++++-
> > include/hw/virtio/virtio-iommu.h | 6 +++
> > 3 files changed, 73 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> > index e83500bee9..d94a1cd8a3 100644
> > --- a/hw/virtio/trace-events
> > +++ b/hw/virtio/trace-events
> > @@ -73,3 +73,5 @@ virtio_iommu_get_domain(uint32_t domain_id) "Alloc domain=%d"
> > virtio_iommu_put_domain(uint32_t domain_id) "Free domain=%d"
> > virtio_iommu_translate_out(uint64_t virt_addr, uint64_t phys_addr, uint32_t sid) "0x%"PRIx64" -> 0x%"PRIx64 " for sid=%d"
> > virtio_iommu_report_fault(uint8_t reason, uint32_t flags, uint32_t endpoint, uint64_t addr) "FAULT reason=%d flags=%d endpoint=%d address =0x%"PRIx64
> > +virtio_iommu_notify_map(const char *name, uint64_t iova, uint64_t paddr, uint64_t map_size) "mr=%s iova=0x%"PRIx64" pa=0x%" PRIx64" size=0x%"PRIx64
> > +virtio_iommu_notify_unmap(const char *name, uint64_t iova, uint64_t map_size) "mr=%s iova=0x%"PRIx64" size=0x%"PRIx64
> > diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> > index 4cee8083bc..e51344a53e 100644
> > --- a/hw/virtio/virtio-iommu.c
> > +++ b/hw/virtio/virtio-iommu.c
> > @@ -123,6 +123,38 @@ static gint interval_cmp(gconstpointer a, gconstpointer b, gpointer user_data)
> > }
> > }
> >
> > +static void virtio_iommu_notify_map(IOMMUMemoryRegion *mr, hwaddr iova,
> > + hwaddr paddr, hwaddr size)
> > +{
> > + IOMMUTLBEntry entry;
> > +
> > + entry.target_as = &address_space_memory;
> > + entry.addr_mask = size - 1;
> > +
> > + entry.iova = iova;
> > + trace_virtio_iommu_notify_map(mr->parent_obj.name, iova, paddr, size);
> > + entry.perm = IOMMU_RW;
> > + entry.translated_addr = paddr;
> > +
> > + memory_region_notify_iommu(mr, 0, entry);
> > +}
> > +
> > +static void virtio_iommu_notify_unmap(IOMMUMemoryRegion *mr, hwaddr iova,
> > + hwaddr size)
> > +{
> > + IOMMUTLBEntry entry;
> > +
> > + entry.target_as = &address_space_memory;
> > + entry.addr_mask = size - 1;
> > +
> > + entry.iova = iova;
> > + trace_virtio_iommu_notify_unmap(mr->parent_obj.name, iova, size);
> > + entry.perm = IOMMU_NONE;
> > + entry.translated_addr = 0;
> > +
> > + memory_region_notify_iommu(mr, 0, entry);
> > +}
> > +
> > static void virtio_iommu_detach_endpoint_from_domain(VirtIOIOMMUEndpoint *ep)
> > {
> > if (!ep->domain) {
> > @@ -307,9 +339,12 @@ static int virtio_iommu_map(VirtIOIOMMU *s,
> > uint64_t virt_start = le64_to_cpu(req->virt_start);
> > uint64_t virt_end = le64_to_cpu(req->virt_end);
> > uint32_t flags = le32_to_cpu(req->flags);
> > + hwaddr size = virt_end - virt_start + 1;
> > + VirtioIOMMUNotifierNode *node;
> > VirtIOIOMMUDomain *domain;
> > VirtIOIOMMUInterval *interval;
> > VirtIOIOMMUMapping *mapping;
> > + VirtIOIOMMUEndpoint *ep;
> >
> > if (flags & ~VIRTIO_IOMMU_MAP_F_MASK) {
> > return VIRTIO_IOMMU_S_INVAL;
> > @@ -339,9 +374,37 @@ static int virtio_iommu_map(VirtIOIOMMU *s,
> >
> > g_tree_insert(domain->mappings, interval, mapping);
> >
> > + /* All devices in an address-space share mapping */
> > + QLIST_FOREACH(node, &s->notifiers_list, next) {
> > + QLIST_FOREACH(ep, &domain->endpoint_list, next) {
> > + if (ep->id == node->iommu_dev->devfn) {
> > + virtio_iommu_notify_map(&node->iommu_dev->iommu_mr,
> > + virt_start, phys_start, size);
> > + }
> > + }
> > + }
> > +
> > return VIRTIO_IOMMU_S_OK;
> > }
> >
> > +static void virtio_iommu_remove_mapping(VirtIOIOMMU *s, VirtIOIOMMUDomain *domain,
> > + VirtIOIOMMUInterval *interval)
> > +{
> > + VirtioIOMMUNotifierNode *node;
> > + VirtIOIOMMUEndpoint *ep;
> > +
> > + QLIST_FOREACH(node, &s->notifiers_list, next) {
> > + QLIST_FOREACH(ep, &domain->endpoint_list, next) {
> > + if (ep->id == node->iommu_dev->devfn) {
> > + virtio_iommu_notify_unmap(&node->iommu_dev->iommu_mr,
> > + interval->low,
> > + interval->high - interval->low + 1);
> > + }
> > + }
> > + }
> > + g_tree_remove(domain->mappings, (gpointer)(interval));
> > +}
> What about virtio_iommu_put_domain() where you destroy the mapping
> gtree. I guess you also need to send invalidations there?
In virtio_iommu_put_domain(), before destroying domain->mappings we
are calling virtio_iommu_detach_endpoint_from_domain(), which send
invalidations, no ?
> > +
> > static int virtio_iommu_unmap(VirtIOIOMMU *s,
> > struct virtio_iommu_req_unmap *req)
> > {
> > @@ -368,7 +431,7 @@ static int virtio_iommu_unmap(VirtIOIOMMU *s,
> > uint64_t current_high = iter_key->high;
> >
> > if (interval.low <= current_low && interval.high >= current_high) {
> > - g_tree_remove(domain->mappings, iter_key);
> > + virtio_iommu_remove_mapping(s, domain, iter_key);
> > trace_virtio_iommu_unmap_done(domain_id, current_low, current_high);
> > } else {
> > ret = VIRTIO_IOMMU_S_RANGE;
> > @@ -655,6 +718,7 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
> > VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> > VirtIOIOMMU *s = VIRTIO_IOMMU(dev);
> >
> > + QLIST_INIT(&s->notifiers_list);
> > virtio_init(vdev, "virtio-iommu", VIRTIO_ID_IOMMU,
> > sizeof(struct virtio_iommu_config));
> >
> > diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
> > index 6f67f1020a..4539c8ae72 100644
> > --- a/include/hw/virtio/virtio-iommu.h
> > +++ b/include/hw/virtio/virtio-iommu.h
> > @@ -44,6 +44,11 @@ typedef struct IOMMUPciBus {
> > IOMMUDevice *pbdev[0]; /* Parent array is sparse, so dynamically alloc */
> > } IOMMUPciBus;
> >
> > +typedef struct VirtioIOMMUNotifierNode {
> > + IOMMUDevice *iommu_dev;
> > + QLIST_ENTRY(VirtioIOMMUNotifierNode) next;
> > +} VirtioIOMMUNotifierNode;
> You may use scripts/git.orderfile for a better diff ordering.
ok, run "git config diff.orderFile scripts/git.orderfile"
> > +
> > typedef struct VirtIOIOMMU {
> > VirtIODevice parent_obj;
> > VirtQueue *req_vq;
> > @@ -56,6 +61,7 @@ typedef struct VirtIOIOMMU {
> > GTree *domains;
> > QemuMutex mutex;
> > GTree *endpoints;
> > + QLIST_HEAD(, VirtioIOMMUNotifierNode) notifiers_list;
> See what was done in smmuv3 and intel. We now directly use a list of
> IOMMUDevice directly. VirtioIOMMUNotifierNode does not bring anything extra.
ok,
Thanks
-Bharat
> > } VirtIOIOMMU;
> >
> > #endif
> >
>
> Thanks
>
> Eric
>
Hi Bharat,
On 3/16/20 7:36 AM, Bharat Bhushan wrote:
> Hi Eric,
>
> On Fri, Mar 13, 2020 at 7:55 PM Auger Eric <eric.auger@redhat.com> wrote:
>>
>> Hi Bharat,
>> On 3/13/20 8:48 AM, Bharat Bhushan wrote:
>>> This patch extends VIRTIO_IOMMU_T_MAP/UNMAP request to
>>> notify registered iommu-notifier. Which will call vfio
>> s/iommu-notifier/iommu-notifiers
>>> notifier to map/unmap region in iommu.
>> can be any notifier (vhost/vfio).
>>>
>>> Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com>
>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>> ---
>>> hw/virtio/trace-events | 2 +
>>> hw/virtio/virtio-iommu.c | 66 +++++++++++++++++++++++++++++++-
>>> include/hw/virtio/virtio-iommu.h | 6 +++
>>> 3 files changed, 73 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
>>> index e83500bee9..d94a1cd8a3 100644
>>> --- a/hw/virtio/trace-events
>>> +++ b/hw/virtio/trace-events
>>> @@ -73,3 +73,5 @@ virtio_iommu_get_domain(uint32_t domain_id) "Alloc domain=%d"
>>> virtio_iommu_put_domain(uint32_t domain_id) "Free domain=%d"
>>> virtio_iommu_translate_out(uint64_t virt_addr, uint64_t phys_addr, uint32_t sid) "0x%"PRIx64" -> 0x%"PRIx64 " for sid=%d"
>>> virtio_iommu_report_fault(uint8_t reason, uint32_t flags, uint32_t endpoint, uint64_t addr) "FAULT reason=%d flags=%d endpoint=%d address =0x%"PRIx64
>>> +virtio_iommu_notify_map(const char *name, uint64_t iova, uint64_t paddr, uint64_t map_size) "mr=%s iova=0x%"PRIx64" pa=0x%" PRIx64" size=0x%"PRIx64
>>> +virtio_iommu_notify_unmap(const char *name, uint64_t iova, uint64_t map_size) "mr=%s iova=0x%"PRIx64" size=0x%"PRIx64
>>> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
>>> index 4cee8083bc..e51344a53e 100644
>>> --- a/hw/virtio/virtio-iommu.c
>>> +++ b/hw/virtio/virtio-iommu.c
>>> @@ -123,6 +123,38 @@ static gint interval_cmp(gconstpointer a, gconstpointer b, gpointer user_data)
>>> }
>>> }
>>>
>>> +static void virtio_iommu_notify_map(IOMMUMemoryRegion *mr, hwaddr iova,
>>> + hwaddr paddr, hwaddr size)
>>> +{
>>> + IOMMUTLBEntry entry;
>>> +
>>> + entry.target_as = &address_space_memory;
>>> + entry.addr_mask = size - 1;
>>> +
>>> + entry.iova = iova;
>>> + trace_virtio_iommu_notify_map(mr->parent_obj.name, iova, paddr, size);
>>> + entry.perm = IOMMU_RW;
>>> + entry.translated_addr = paddr;
>>> +
>>> + memory_region_notify_iommu(mr, 0, entry);
>>> +}
>>> +
>>> +static void virtio_iommu_notify_unmap(IOMMUMemoryRegion *mr, hwaddr iova,
>>> + hwaddr size)
>>> +{
>>> + IOMMUTLBEntry entry;
>>> +
>>> + entry.target_as = &address_space_memory;
>>> + entry.addr_mask = size - 1;
>>> +
>>> + entry.iova = iova;
>>> + trace_virtio_iommu_notify_unmap(mr->parent_obj.name, iova, size);
>>> + entry.perm = IOMMU_NONE;
>>> + entry.translated_addr = 0;
>>> +
>>> + memory_region_notify_iommu(mr, 0, entry);
>>> +}
>>> +
>>> static void virtio_iommu_detach_endpoint_from_domain(VirtIOIOMMUEndpoint *ep)
>>> {
>>> if (!ep->domain) {
>>> @@ -307,9 +339,12 @@ static int virtio_iommu_map(VirtIOIOMMU *s,
>>> uint64_t virt_start = le64_to_cpu(req->virt_start);
>>> uint64_t virt_end = le64_to_cpu(req->virt_end);
>>> uint32_t flags = le32_to_cpu(req->flags);
>>> + hwaddr size = virt_end - virt_start + 1;
>>> + VirtioIOMMUNotifierNode *node;
>>> VirtIOIOMMUDomain *domain;
>>> VirtIOIOMMUInterval *interval;
>>> VirtIOIOMMUMapping *mapping;
>>> + VirtIOIOMMUEndpoint *ep;
>>>
>>> if (flags & ~VIRTIO_IOMMU_MAP_F_MASK) {
>>> return VIRTIO_IOMMU_S_INVAL;
>>> @@ -339,9 +374,37 @@ static int virtio_iommu_map(VirtIOIOMMU *s,
>>>
>>> g_tree_insert(domain->mappings, interval, mapping);
>>>
>>> + /* All devices in an address-space share mapping */
>>> + QLIST_FOREACH(node, &s->notifiers_list, next) {
>>> + QLIST_FOREACH(ep, &domain->endpoint_list, next) {
>>> + if (ep->id == node->iommu_dev->devfn) {
>>> + virtio_iommu_notify_map(&node->iommu_dev->iommu_mr,
>>> + virt_start, phys_start, size);
>>> + }
>>> + }
>>> + }
>>> +
>>> return VIRTIO_IOMMU_S_OK;
>>> }
>>>
>>> +static void virtio_iommu_remove_mapping(VirtIOIOMMU *s, VirtIOIOMMUDomain *domain,
>>> + VirtIOIOMMUInterval *interval)
>>> +{
>>> + VirtioIOMMUNotifierNode *node;
>>> + VirtIOIOMMUEndpoint *ep;
>>> +
>>> + QLIST_FOREACH(node, &s->notifiers_list, next) {
>>> + QLIST_FOREACH(ep, &domain->endpoint_list, next) {
>>> + if (ep->id == node->iommu_dev->devfn) {
>>> + virtio_iommu_notify_unmap(&node->iommu_dev->iommu_mr,
>>> + interval->low,
>>> + interval->high - interval->low + 1);
>>> + }
>>> + }
>>> + }
>>> + g_tree_remove(domain->mappings, (gpointer)(interval));
>>> +}
>> What about virtio_iommu_put_domain() where you destroy the mapping
>> gtree. I guess you also need to send invalidations there?
>
> In virtio_iommu_put_domain(), before destroying domain->mappings we
> are calling virtio_iommu_detach_endpoint_from_domain(), which send
> invalidations, no ?
Yes I noticed that later as it is in patch 3/5 (this is the comment you
did not get)
>
>>> +
>>> static int virtio_iommu_unmap(VirtIOIOMMU *s,
>>> struct virtio_iommu_req_unmap *req)
>>> {
>>> @@ -368,7 +431,7 @@ static int virtio_iommu_unmap(VirtIOIOMMU *s,
>>> uint64_t current_high = iter_key->high;
>>>
>>> if (interval.low <= current_low && interval.high >= current_high) {
>>> - g_tree_remove(domain->mappings, iter_key);
>>> + virtio_iommu_remove_mapping(s, domain, iter_key);
>>> trace_virtio_iommu_unmap_done(domain_id, current_low, current_high);
>>> } else {
>>> ret = VIRTIO_IOMMU_S_RANGE;
>>> @@ -655,6 +718,7 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
>>> VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>>> VirtIOIOMMU *s = VIRTIO_IOMMU(dev);
>>>
>>> + QLIST_INIT(&s->notifiers_list);
>>> virtio_init(vdev, "virtio-iommu", VIRTIO_ID_IOMMU,
>>> sizeof(struct virtio_iommu_config));
>>>
>>> diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
>>> index 6f67f1020a..4539c8ae72 100644
>>> --- a/include/hw/virtio/virtio-iommu.h
>>> +++ b/include/hw/virtio/virtio-iommu.h
>>> @@ -44,6 +44,11 @@ typedef struct IOMMUPciBus {
>>> IOMMUDevice *pbdev[0]; /* Parent array is sparse, so dynamically alloc */
>>> } IOMMUPciBus;
>>>
>>> +typedef struct VirtioIOMMUNotifierNode {
>>> + IOMMUDevice *iommu_dev;
>>> + QLIST_ENTRY(VirtioIOMMUNotifierNode) next;
>>> +} VirtioIOMMUNotifierNode;
>> You may use scripts/git.orderfile for a better diff ordering.
>
> ok, run "git config diff.orderFile scripts/git.orderfile"
>
>>> +
>>> typedef struct VirtIOIOMMU {
>>> VirtIODevice parent_obj;
>>> VirtQueue *req_vq;
>>> @@ -56,6 +61,7 @@ typedef struct VirtIOIOMMU {
>>> GTree *domains;
>>> QemuMutex mutex;
>>> GTree *endpoints;
>>> + QLIST_HEAD(, VirtioIOMMUNotifierNode) notifiers_list;
>> See what was done in smmuv3 and intel. We now directly use a list of
>> IOMMUDevice directly. VirtioIOMMUNotifierNode does not bring anything extra.
>
> ok,
>
> Thanks
> -Bharat
>
>>> } VirtIOIOMMU;
>>>
>>> #endif
>>>
>>
>> Thanks
>>
>> Eric
>>
>
© 2016 - 2026 Red Hat, Inc.