| Message ID | 20200824104738.20664-4-eperezma@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | memory: Delete assertion in memory_region_unregister_iommu_notifier | expand |
On Mon, Aug 24, 2020 at 12:47:38PM +0200, Eugenio Pérez wrote: > This improves performance in case of netperf with vhost-net: > * TCP_STREAM: From 1374.81Mbit/s to 1845Mbit/s (37%) > * TCP_RR: From 6559.36 trans/s to 7916.29/s (21%) > * UDP_RR: From 6705.39 trans/s to 8199.39/s (22%) > * UDP_STREAM: No change observed (not significant 0.1% improvement) Good to know that we can get such a perf boost by removing the extra invalidations! > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > --- > hw/i386/intel_iommu.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c > index 2ad6b9d796..d539a9f281 100644 > --- a/hw/i386/intel_iommu.c > +++ b/hw/i386/intel_iommu.c > @@ -1959,6 +1959,12 @@ static void vtd_iotlb_domain_invalidate(IntelIOMMUState *s, uint16_t domain_id) > vtd_iommu_unlock(s); > > QLIST_FOREACH(vtd_as, &s->vtd_as_with_notifiers, next) { > + if (vtd_as->iommu.iommu_notify_flags & IOMMU_NOTIFIER_DEVIOTLB) { > + /* If IOMMU memory region is DEVICE IOTLB type, it does not make > + * sense to send regular IOMMU notifications. */ > + continue; > + } Though here IMHO a cleaner approach (rather than checking explicitly against DEVIOTLB flag) is to add a notification type into IOMMUTLBEntry. Then for domain invalidation, the caller is responsible to pass the type IOMMU_NOTIFIER_UNMAP into the type field. memory_region_notify_iommu_one() will automatically skip the message if not registerd by the notifier. Also, it would be nice to have this new type before or when introducing the DEVIOTLB message, otherwise the DEVIOTLB patch can be still slightly confusing by itself when notified as UNMAP. So here's some patch layout I'm thinking: - patch 1: rename function, we can keep it - patch 2: introduce "type" for IOMMUTLBEntry, so far we only have MAP/UNMAP. Modify all callers of memory_region_notify_iommu*() to fill in the type correctly into IOMMUTLBEntry with map/unmap. Won't hurt if we still keep filling in UNMAP even for DEVIOTLB since that's after all the same old behavior. - patch 3: introduce DEVIOTLB, switch the type field of dev-iotlb notifications to the new type and let vhost registers only to that. - patch 4: at last, fix the assertion since we've got the DEVIOTLB messages. Would this be slightly cleaner? Thanks,
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index 2ad6b9d796..d539a9f281 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -1959,6 +1959,12 @@ static void vtd_iotlb_domain_invalidate(IntelIOMMUState *s, uint16_t domain_id) vtd_iommu_unlock(s); QLIST_FOREACH(vtd_as, &s->vtd_as_with_notifiers, next) { + if (vtd_as->iommu.iommu_notify_flags & IOMMU_NOTIFIER_DEVIOTLB) { + /* If IOMMU memory region is DEVICE IOTLB type, it does not make + * sense to send regular IOMMU notifications. */ + continue; + } + if (!vtd_dev_to_context_entry(s, pci_bus_num(vtd_as->bus), vtd_as->devfn, &ce) && domain_id == vtd_get_domain_id(s, &ce)) {
This improves performance in case of netperf with vhost-net: * TCP_STREAM: From 1374.81Mbit/s to 1845Mbit/s (37%) * TCP_RR: From 6559.36 trans/s to 7916.29/s (21%) * UDP_RR: From 6705.39 trans/s to 8199.39/s (22%) * UDP_STREAM: No change observed (not significant 0.1% improvement) Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- hw/i386/intel_iommu.c | 6 ++++++ 1 file changed, 6 insertions(+)