* [RFC v5 0/3] memory: Delete assertion in memory_region_unregister_iommu_notifier
@ 2020-08-24 10:47 Eugenio Pérez
2020-08-24 10:47 ` [RFC v5 1/3] memory: Rename memory_region_notify_one to memory_region_notify_iommu_one Eugenio Pérez
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Eugenio Pérez @ 2020-08-24 10:47 UTC (permalink / raw)
To: Peter Xu, qemu-devel
Cc: Peter Maydell, Yan Zhao, Eduardo Habkost, Michael S. Tsirkin,
Jason Wang, Juan Quintela, Eric Auger, qemu-arm, Avi Kivity,
Paolo Bonzini, Richard Henderson
I am able to hit this assertion when a Red Hat 7 guest virtio_net device
raises an "Invalidation" of all the TLB entries. This happens in the
guest's startup if 'intel_iommu=on' argument is passed to the guest
kernel and right IOMMU/ATS devices are declared in qemu's command line.
Command line:
/home/qemu/x86_64-softmmu/qemu-system-x86_64 -name \
guest=rhel7-test,debug-threads=on -machine \
pc-q35-5.1,accel=kvm,usb=off,dump-guest-core=off,kernel_irqchip=split \
-cpu \
Broadwell,vme=on,ss=on,vmx=on,f16c=on,rdrand=on,hypervisor=on,arat=on,tsc-adjust=on,umip=on,arch-capabilities=on,xsaveopt=on,pdpe1gb=on,abm=on,skip-l1dfl-vmentry=on,rtm=on,hle=on \
-m 8096 -realtime mlock=off -smp 2,sockets=2,cores=1,threads=1 -uuid \
d022ecbf-679e-4755-87ce-eb87fc5bbc5d -display none -no-user-config \
-nodefaults -rtc base=utc,driftfix=slew -global \
kvm-pit.lost_tick_policy=delay -no-hpet -no-shutdown -global \
ICH9-LPC.disable_s3=1 -global ICH9-LPC.disable_s4=1 -boot strict=on \
-device intel-iommu,intremap=on,device-iotlb=on -device \
pcie-root-port,port=0x8,chassis=1,id=pci.1,bus=pcie.0,multifunction=on,addr=0x1 \
-device \
pcie-root-port,port=0x9,chassis=2,id=pci.2,bus=pcie.0,addr=0x1.0x1 \
-device \
pcie-root-port,port=0xa,chassis=3,id=pci.3,bus=pcie.0,addr=0x1.0x2 \
-device \
pcie-root-port,port=0xb,chassis=4,id=pci.4,bus=pcie.0,addr=0x1.0x3 \
-device \
pcie-root-port,port=0xc,chassis=5,id=pci.5,bus=pcie.0,addr=0x1.0x4 \
-device \
pcie-root-port,port=0xd,chassis=6,id=pci.6,bus=pcie.0,addr=0x1.0x5 \
-device \
pcie-root-port,port=0xe,chassis=7,id=pci.7,bus=pcie.0,addr=0x1.0x6 \
-device qemu-xhci,p2=15,p3=15,id=usb,bus=pci.2,addr=0x0 -device \
virtio-serial-pci,id=virtio-serial0,bus=pci.3,addr=0x0 -drive \
file=/home/virtio-test2.qcow2,format=qcow2,if=none,id=drive-virtio-disk0 \
-device \
virtio-blk-pci,scsi=off,bus=pci.4,addr=0x0,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 \
-netdev tap,id=hostnet0,vhost=on,vhostforce=on -device \
virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:0d:1d:f2,bus=pci.1,addr=0x0,iommu_platform=on,ats=on \
-device virtio-balloon-pci,id=balloon0,bus=pci.5,addr=0x0 -object \
rng-random,id=objrng0,filename=/dev/urandom -device \
virtio-rng-pci,rng=objrng0,id=rng0,bus=pci.6,addr=0x0 -s -msg \
timestamp=on
Full backtrace:
#0 0x00007ffff521370f in raise () at /lib64/libc.so.6
#1 0x00007ffff51fdb25 in abort () at /lib64/libc.so.6
#2 0x00007ffff51fd9f9 in _nl_load_domain.cold.0 () at /lib64/libc.so.6
#3 0x00007ffff520bcc6 in .annobin_assert.c_end () at /lib64/libc.so.6
#4 0x0000555555888171 in memory_region_notify_one
(notifier=0x7ffde0487fa8, entry=0x7ffde5dfe200)
at /home/qemu/memory.c:1918
#5 0x0000555555888247 in memory_region_notify_iommu
(iommu_mr=0x555556f6c0b0, iommu_idx=0, entry=...)
at /home/qemu/memory.c:1941
#6 0x0000555555951c8d in vtd_process_device_iotlb_desc
(s=0x555557609000, inv_desc=0x7ffde5dfe2d0)
at /home/qemu/hw/i386/intel_iommu.c:2468
#7 0x0000555555951e6a in vtd_process_inv_desc (s=0x555557609000)
at /home/qemu/hw/i386/intel_iommu.c:2531
#8 0x0000555555951fa5 in vtd_fetch_inv_desc (s=0x555557609000)
at /home/qemu/hw/i386/intel_iommu.c:2563
#9 0x00005555559520e5 in vtd_handle_iqt_write (s=0x555557609000)
at /home/qemu/hw/i386/intel_iommu.c:2590
#10 0x0000555555952b45 in vtd_mem_write (opaque=0x555557609000,
addr=136, val=2688, size=4)
at /home/qemu/hw/i386/intel_iommu.c:2837
#11 0x0000555555883e17 in memory_region_write_accessor
(mr=0x555557609330, addr=136, value=0x7ffde5dfe478, size=4,
shift=0, mask=4294967295, attrs=...)
at /home/qemu/memory.c:483
#12 0x000055555588401d in access_with_adjusted_size
(addr=136, value=0x7ffde5dfe478, size=4, access_size_min=4,
access_size_max=8,
access_fn=0x555555883d38 <memory_region_write_accessor>,
mr=0x555557609330, attrs=...) at /home/qemu/memory.c:544
#13 0x0000555555886f37 in memory_region_dispatch_write
(mr=0x555557609330, addr=136, data=2688, op=MO_32, attrs=...)
at /home/qemu/memory.c:1476
#14 0x0000555555827a03 in flatview_write_continue
(fv=0x7ffdd8503150, addr=4275634312, attrs=...,
ptr=0x7ffff7ff0028, len=4, addr1=136, l=4,
mr=0x555557609330) at /home/qemu/exec.c:3146
#15 0x0000555555827b48 in flatview_write (fv=0x7ffdd8503150,
addr=4275634312, attrs=..., buf=0x7ffff7ff0028, len=4)
at /home/qemu/exec.c:3186
#16 0x0000555555827e9d in address_space_write
(as=0x5555567ca640 <address_space_memory>,
addr=4275634312,
attrs=..., buf=0x7ffff7ff0028, len=4)
at /home/qemu/exec.c:3277
#17 0x0000555555827f0a in address_space_rw
(as=0x5555567ca640 <address_space_memory>, addr=4275634312,
attrs=..., buf=0x7ffff7ff0028, len=4, is_write=true)
at /home/qemu/exec.c:3287
#18 0x000055555589b633 in kvm_cpu_exec (cpu=0x555556b65640)
at /home/qemu/accel/kvm/kvm-all.c:2511
#19 0x0000555555876ba8 in qemu_kvm_cpu_thread_fn (arg=0x555556b65640)
at /home/qemu/cpus.c:1284
#20 0x0000555555dafff1 in qemu_thread_start (args=0x555556b8c3b0)
at util/qemu-thread-posix.c:521
#21 0x00007ffff55a62de in start_thread () at /lib64/libpthread.so.0
#22 0x00007ffff52d7e83 in clone () at /lib64/libc.so.6
(gdb) frame 4
#4 0x0000555555888171 in memory_region_notify_one
(notifier=0x7ffde0487fa8, entry=0x7ffde5dfe200)
at /home/qemu/memory.c:1918
1918 assert(entry->iova >= notifier->start && entry_end <=
notifier->end);
(gdb) p *entry
$1 = {target_as = 0x555556f6c050, iova = 0, translated_addr = 0,
addr_mask = 18446744073709551615, perm = IOMMU_NONE}
--
Tested with vhost-net, with a linux bridge to forward packets, and
with testpmd in both host & guest for vhostuser.
v5: Skip regular IOTLB notifications in dev_iotlb notifiers
v4: Rename IOMMU_NOTIFIER_IOTLB -> IOMMU_NOTIFIER_DEVIOTLB.
Make vhost-net notifier just IOMMU_NOTIFIER_DEVIOTLB, not
IOMMU_NOTIFIER_UNMAP
v3: Skip the assertion in case notifier is a IOTLB one, since they can manage
arbitrary ranges. Using a flag in the notifier for now, as Peter suggested.
v2: Actually delete assertion instead of just commenting out using C99
Eugenio Pérez (3):
memory: Rename memory_region_notify_one to
memory_region_notify_iommu_one
memory: Skip bad range assertion if notifier is DEVIOTLB type
intel_iommu: Do not notify regular iotlb to device-iotlb notifiers
hw/arm/smmu-common.c | 2 +-
hw/arm/smmuv3.c | 2 +-
hw/i386/intel_iommu.c | 10 ++++++++--
hw/virtio/vhost.c | 2 +-
include/exec/memory.h | 8 +++++---
softmmu/memory.c | 21 ++++++++++++++++-----
6 files changed, 32 insertions(+), 13 deletions(-)
--
2.18.1
^ permalink raw reply [flat|nested] 5+ messages in thread* [RFC v5 1/3] memory: Rename memory_region_notify_one to memory_region_notify_iommu_one 2020-08-24 10:47 [RFC v5 0/3] memory: Delete assertion in memory_region_unregister_iommu_notifier Eugenio Pérez @ 2020-08-24 10:47 ` Eugenio Pérez 2020-08-24 10:47 ` [RFC v5 2/3] memory: Skip bad range assertion if notifier is DEVIOTLB type Eugenio Pérez 2020-08-24 10:47 ` [RFC v5 3/3] intel_iommu: Do not notify regular iotlb to device-iotlb notifiers Eugenio Pérez 2 siblings, 0 replies; 5+ messages in thread From: Eugenio Pérez @ 2020-08-24 10:47 UTC (permalink / raw) To: Peter Xu, qemu-devel Cc: Peter Maydell, Yan Zhao, Eduardo Habkost, Michael S. Tsirkin, Jason Wang, Juan Quintela, Eric Auger, qemu-arm, Avi Kivity, Paolo Bonzini, Richard Henderson Previous name didn't reflect the iommu operation. Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- hw/arm/smmu-common.c | 2 +- hw/arm/smmuv3.c | 2 +- hw/i386/intel_iommu.c | 4 ++-- include/exec/memory.h | 6 +++--- softmmu/memory.c | 6 +++--- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c index e13a5f4a7c..b02ffb8822 100644 --- a/hw/arm/smmu-common.c +++ b/hw/arm/smmu-common.c @@ -396,7 +396,7 @@ static void smmu_unmap_notifier_range(IOMMUNotifier *n) entry.perm = IOMMU_NONE; entry.addr_mask = n->end - n->start; - memory_region_notify_one(n, &entry); + memory_region_notify_iommu_one(n, &entry); } /* Unmap all notifiers attached to @mr */ diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c index 57a79df55b..3bb85ab7e1 100644 --- a/hw/arm/smmuv3.c +++ b/hw/arm/smmuv3.c @@ -838,7 +838,7 @@ static void smmuv3_notify_iova(IOMMUMemoryRegion *mr, entry.addr_mask = (1 << tt->granule_sz) - 1; entry.perm = IOMMU_NONE; - memory_region_notify_one(n, &entry); + memory_region_notify_iommu_one(n, &entry); } /* invalidate an asid/iova tuple in all mr's */ diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index 5284bb68b6..2ad6b9d796 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -3498,7 +3498,7 @@ static void vtd_address_space_unmap(VTDAddressSpace *as, IOMMUNotifier *n) /* This field is meaningless for unmap */ entry.translated_addr = 0; - memory_region_notify_one(n, &entry); + memory_region_notify_iommu_one(n, &entry); start += mask; remain -= mask; @@ -3536,7 +3536,7 @@ static void vtd_address_space_refresh_all(IntelIOMMUState *s) static int vtd_replay_hook(IOMMUTLBEntry *entry, void *private) { - memory_region_notify_one((IOMMUNotifier *)private, entry); + memory_region_notify_iommu_one((IOMMUNotifier *)private, entry); return 0; } diff --git a/include/exec/memory.h b/include/exec/memory.h index 0cfe987ab4..22c5f564d1 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -226,7 +226,7 @@ enum IOMMUMemoryRegionAttr { * The IOMMU implementation must use the IOMMU notifier infrastructure * to report whenever mappings are changed, by calling * memory_region_notify_iommu() (or, if necessary, by calling - * memory_region_notify_one() for each registered notifier). + * memory_region_notify_iommu_one() for each registered notifier). * * Conceptually an IOMMU provides a mapping from input address * to an output TLB entry. If the IOMMU is aware of memory transaction @@ -1274,7 +1274,7 @@ void memory_region_notify_iommu(IOMMUMemoryRegion *iommu_mr, IOMMUTLBEntry entry); /** - * memory_region_notify_one: notify a change in an IOMMU translation + * memory_region_notify_iommu_one: notify a change in an IOMMU translation * entry to a single notifier * * This works just like memory_region_notify_iommu(), but it only @@ -1285,7 +1285,7 @@ void memory_region_notify_iommu(IOMMUMemoryRegion *iommu_mr, * replaces all old entries for the same virtual I/O address range. * Deleted entries have .@perm == 0. */ -void memory_region_notify_one(IOMMUNotifier *notifier, +void memory_region_notify_iommu_one(IOMMUNotifier *notifier, IOMMUTLBEntry *entry); /** diff --git a/softmmu/memory.c b/softmmu/memory.c index 70b93104e8..961c25b42f 100644 --- a/softmmu/memory.c +++ b/softmmu/memory.c @@ -1890,8 +1890,8 @@ void memory_region_unregister_iommu_notifier(MemoryRegion *mr, memory_region_update_iommu_notify_flags(iommu_mr, NULL); } -void memory_region_notify_one(IOMMUNotifier *notifier, - IOMMUTLBEntry *entry) +void memory_region_notify_iommu_one(IOMMUNotifier *notifier, + IOMMUTLBEntry *entry) { IOMMUNotifierFlag request_flags; hwaddr entry_end = entry->iova + entry->addr_mask; @@ -1927,7 +1927,7 @@ void memory_region_notify_iommu(IOMMUMemoryRegion *iommu_mr, IOMMU_NOTIFIER_FOREACH(iommu_notifier, iommu_mr) { if (iommu_notifier->iommu_idx == iommu_idx) { - memory_region_notify_one(iommu_notifier, &entry); + memory_region_notify_iommu_one(iommu_notifier, &entry); } } } -- 2.18.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [RFC v5 2/3] memory: Skip bad range assertion if notifier is DEVIOTLB type 2020-08-24 10:47 [RFC v5 0/3] memory: Delete assertion in memory_region_unregister_iommu_notifier Eugenio Pérez 2020-08-24 10:47 ` [RFC v5 1/3] memory: Rename memory_region_notify_one to memory_region_notify_iommu_one Eugenio Pérez @ 2020-08-24 10:47 ` Eugenio Pérez 2020-08-24 10:47 ` [RFC v5 3/3] intel_iommu: Do not notify regular iotlb to device-iotlb notifiers Eugenio Pérez 2 siblings, 0 replies; 5+ messages in thread From: Eugenio Pérez @ 2020-08-24 10:47 UTC (permalink / raw) To: Peter Xu, qemu-devel Cc: Peter Maydell, Yan Zhao, Eduardo Habkost, Michael S. Tsirkin, Jason Wang, Juan Quintela, Eric Auger, qemu-arm, Avi Kivity, Paolo Bonzini, Richard Henderson I am able to hit this assertion when a Red Hat 7 guest virtio_net device raises an "Invalidation" of all the TLB entries. This happens in the guest's startup if 'intel_iommu=on' argument is passed to the guest kernel and right IOMMU/ATS devices are declared in qemu's command line. Command line: /home/qemu/x86_64-softmmu/qemu-system-x86_64 -name \ guest=rhel7-test,debug-threads=on -machine \ pc-q35-5.1,accel=kvm,usb=off,dump-guest-core=off,kernel_irqchip=split \ -cpu \ Broadwell,vme=on,ss=on,vmx=on,f16c=on,rdrand=on,hypervisor=on,arat=on,tsc-adjust=on,umip=on,arch-capabilities=on,xsaveopt=on,pdpe1gb=on,abm=on,skip-l1dfl-vmentry=on,rtm=on,hle=on \ -m 8096 -realtime mlock=off -smp 2,sockets=2,cores=1,threads=1 -uuid \ d022ecbf-679e-4755-87ce-eb87fc5bbc5d -display none -no-user-config \ -nodefaults -rtc base=utc,driftfix=slew -global \ kvm-pit.lost_tick_policy=delay -no-hpet -no-shutdown -global \ ICH9-LPC.disable_s3=1 -global ICH9-LPC.disable_s4=1 -boot strict=on \ -device intel-iommu,intremap=on,device-iotlb=on -device \ pcie-root-port,port=0x8,chassis=1,id=pci.1,bus=pcie.0,multifunction=on,addr=0x1 \ -device \ pcie-root-port,port=0x9,chassis=2,id=pci.2,bus=pcie.0,addr=0x1.0x1 \ -device \ pcie-root-port,port=0xa,chassis=3,id=pci.3,bus=pcie.0,addr=0x1.0x2 \ -device \ pcie-root-port,port=0xb,chassis=4,id=pci.4,bus=pcie.0,addr=0x1.0x3 \ -device \ pcie-root-port,port=0xc,chassis=5,id=pci.5,bus=pcie.0,addr=0x1.0x4 \ -device \ pcie-root-port,port=0xd,chassis=6,id=pci.6,bus=pcie.0,addr=0x1.0x5 \ -device \ pcie-root-port,port=0xe,chassis=7,id=pci.7,bus=pcie.0,addr=0x1.0x6 \ -device qemu-xhci,p2=15,p3=15,id=usb,bus=pci.2,addr=0x0 -device \ virtio-serial-pci,id=virtio-serial0,bus=pci.3,addr=0x0 -drive \ file=/home/virtio-test2.qcow2,format=qcow2,if=none,id=drive-virtio-disk0 \ -device \ virtio-blk-pci,scsi=off,bus=pci.4,addr=0x0,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 \ -netdev tap,id=hostnet0,vhost=on,vhostforce=on -device \ virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:0d:1d:f2,bus=pci.1,addr=0x0,iommu_platform=on,ats=on \ -device virtio-balloon-pci,id=balloon0,bus=pci.5,addr=0x0 -object \ rng-random,id=objrng0,filename=/dev/urandom -device \ virtio-rng-pci,rng=objrng0,id=rng0,bus=pci.6,addr=0x0 -s -msg \ timestamp=on Full backtrace: #0 0x00007ffff521370f in raise () at /lib64/libc.so.6 #1 0x00007ffff51fdb25 in abort () at /lib64/libc.so.6 #2 0x00007ffff51fd9f9 in _nl_load_domain.cold.0 () at /lib64/libc.so.6 #3 0x00007ffff520bcc6 in .annobin_assert.c_end () at /lib64/libc.so.6 #4 0x0000555555888171 in memory_region_notify_one (notifier=0x7ffde0487fa8, entry=0x7ffde5dfe200) at /home/qemu/memory.c:1918 #5 0x0000555555888247 in memory_region_notify_iommu (iommu_mr=0x555556f6c0b0, iommu_idx=0, entry=...) at /home/qemu/memory.c:1941 #6 0x0000555555951c8d in vtd_process_device_iotlb_desc (s=0x555557609000, inv_desc=0x7ffde5dfe2d0) at /home/qemu/hw/i386/intel_iommu.c:2468 #7 0x0000555555951e6a in vtd_process_inv_desc (s=0x555557609000) at /home/qemu/hw/i386/intel_iommu.c:2531 #8 0x0000555555951fa5 in vtd_fetch_inv_desc (s=0x555557609000) at /home/qemu/hw/i386/intel_iommu.c:2563 #9 0x00005555559520e5 in vtd_handle_iqt_write (s=0x555557609000) at /home/qemu/hw/i386/intel_iommu.c:2590 #10 0x0000555555952b45 in vtd_mem_write (opaque=0x555557609000, addr=136, val=2688, size=4) at /home/qemu/hw/i386/intel_iommu.c:2837 #11 0x0000555555883e17 in memory_region_write_accessor (mr=0x555557609330, addr=136, value=0x7ffde5dfe478, size=4, shift=0, mask=4294967295, attrs=...) at /home/qemu/memory.c:483 #12 0x000055555588401d in access_with_adjusted_size (addr=136, value=0x7ffde5dfe478, size=4, access_size_min=4, access_size_max=8, access_fn=0x555555883d38 <memory_region_write_accessor>, mr=0x555557609330, attrs=...) at /home/qemu/memory.c:544 #13 0x0000555555886f37 in memory_region_dispatch_write (mr=0x555557609330, addr=136, data=2688, op=MO_32, attrs=...) at /home/qemu/memory.c:1476 #14 0x0000555555827a03 in flatview_write_continue (fv=0x7ffdd8503150, addr=4275634312, attrs=..., ptr=0x7ffff7ff0028, len=4, addr1=136, l=4, mr=0x555557609330) at /home/qemu/exec.c:3146 #15 0x0000555555827b48 in flatview_write (fv=0x7ffdd8503150, addr=4275634312, attrs=..., buf=0x7ffff7ff0028, len=4) at /home/qemu/exec.c:3186 #16 0x0000555555827e9d in address_space_write (as=0x5555567ca640 <address_space_memory>, addr=4275634312, attrs=..., buf=0x7ffff7ff0028, len=4) at /home/qemu/exec.c:3277 #17 0x0000555555827f0a in address_space_rw (as=0x5555567ca640 <address_space_memory>, addr=4275634312, attrs=..., buf=0x7ffff7ff0028, len=4, is_write=true) at /home/qemu/exec.c:3287 #18 0x000055555589b633 in kvm_cpu_exec (cpu=0x555556b65640) at /home/qemu/accel/kvm/kvm-all.c:2511 #19 0x0000555555876ba8 in qemu_kvm_cpu_thread_fn (arg=0x555556b65640) at /home/qemu/cpus.c:1284 #20 0x0000555555dafff1 in qemu_thread_start (args=0x555556b8c3b0) at util/qemu-thread-posix.c:521 #21 0x00007ffff55a62de in start_thread () at /lib64/libpthread.so.0 #22 0x00007ffff52d7e83 in clone () at /lib64/libc.so.6 (gdb) frame 4 #4 0x0000555555888171 in memory_region_notify_one (notifier=0x7ffde0487fa8, entry=0x7ffde5dfe200) at /home/qemu/memory.c:1918 1918 assert(entry->iova >= notifier->start && entry_end <= notifier->end); (gdb) p *entry $1 = {target_as = 0x555556f6c050, iova = 0, translated_addr = 0, addr_mask = 18446744073709551615, perm = IOMMU_NONE} -- Tested with vhost-net, with a linux bridge to forward packets, and with testpmd in both host & guest for vhostuser. Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- hw/virtio/vhost.c | 2 +- include/exec/memory.h | 2 ++ softmmu/memory.c | 15 +++++++++++++-- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 1a1384e7a6..6ca168b47e 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -729,7 +729,7 @@ static void vhost_iommu_region_add(MemoryListener *listener, iommu_idx = memory_region_iommu_attrs_to_index(iommu_mr, MEMTXATTRS_UNSPECIFIED); iommu_notifier_init(&iommu->n, vhost_iommu_unmap_notify, - IOMMU_NOTIFIER_UNMAP, + IOMMU_NOTIFIER_DEVIOTLB, section->offset_within_region, int128_get64(end), iommu_idx); diff --git a/include/exec/memory.h b/include/exec/memory.h index 22c5f564d1..b6c3d39b0f 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -87,6 +87,8 @@ typedef enum { IOMMU_NOTIFIER_UNMAP = 0x1, /* Notify entry changes (newly created entries) */ IOMMU_NOTIFIER_MAP = 0x2, + /* Notify changes on device IOTLB entries */ + IOMMU_NOTIFIER_DEVIOTLB = 0x04, } IOMMUNotifierFlag; #define IOMMU_NOTIFIER_ALL (IOMMU_NOTIFIER_MAP | IOMMU_NOTIFIER_UNMAP) diff --git a/softmmu/memory.c b/softmmu/memory.c index 961c25b42f..d75f6fa188 100644 --- a/softmmu/memory.c +++ b/softmmu/memory.c @@ -1895,6 +1895,7 @@ void memory_region_notify_iommu_one(IOMMUNotifier *notifier, { IOMMUNotifierFlag request_flags; hwaddr entry_end = entry->iova + entry->addr_mask; + IOMMUTLBEntry tmp = *entry; /* * Skip the notification if the notification does not overlap @@ -1904,16 +1905,26 @@ void memory_region_notify_iommu_one(IOMMUNotifier *notifier, return; } - assert(entry->iova >= notifier->start && entry_end <= notifier->end); + if (notifier->notifier_flags & IOMMU_NOTIFIER_DEVIOTLB) { + /* Crop (iova, addr_mask) to range */ + tmp.iova = MAX(tmp.iova, notifier->start); + tmp.addr_mask = MIN(entry_end, notifier->end) - tmp.iova; + /* Confirm no underflow */ + assert(MIN(entry_end, notifier->end) >= tmp.iova); + } else { + assert(entry->iova >= notifier->start && entry_end <= notifier->end); + } if (entry->perm & IOMMU_RW) { request_flags = IOMMU_NOTIFIER_MAP; + } else if (notifier->notifier_flags == IOMMU_NOTIFIER_DEVIOTLB) { + request_flags = IOMMU_NOTIFIER_DEVIOTLB; } else { request_flags = IOMMU_NOTIFIER_UNMAP; } if (notifier->notifier_flags & request_flags) { - notifier->notify(notifier, entry); + notifier->notify(notifier, &tmp); } } -- 2.18.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [RFC v5 3/3] intel_iommu: Do not notify regular iotlb to device-iotlb notifiers 2020-08-24 10:47 [RFC v5 0/3] memory: Delete assertion in memory_region_unregister_iommu_notifier Eugenio Pérez 2020-08-24 10:47 ` [RFC v5 1/3] memory: Rename memory_region_notify_one to memory_region_notify_iommu_one Eugenio Pérez 2020-08-24 10:47 ` [RFC v5 2/3] memory: Skip bad range assertion if notifier is DEVIOTLB type Eugenio Pérez @ 2020-08-24 10:47 ` Eugenio Pérez 2020-08-24 16:20 ` Peter Xu 2 siblings, 1 reply; 5+ messages in thread From: Eugenio Pérez @ 2020-08-24 10:47 UTC (permalink / raw) To: Peter Xu, qemu-devel Cc: Peter Maydell, Yan Zhao, Eduardo Habkost, Michael S. Tsirkin, Jason Wang, Juan Quintela, Eric Auger, qemu-arm, Avi Kivity, Paolo Bonzini, Richard Henderson 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(+) 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)) { -- 2.18.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RFC v5 3/3] intel_iommu: Do not notify regular iotlb to device-iotlb notifiers 2020-08-24 10:47 ` [RFC v5 3/3] intel_iommu: Do not notify regular iotlb to device-iotlb notifiers Eugenio Pérez @ 2020-08-24 16:20 ` Peter Xu 0 siblings, 0 replies; 5+ messages in thread From: Peter Xu @ 2020-08-24 16:20 UTC (permalink / raw) To: Eugenio Pérez Cc: Peter Maydell, Yan Zhao, Eduardo Habkost, Juan Quintela, Jason Wang, Michael S. Tsirkin, qemu-devel, Eric Auger, qemu-arm, Avi Kivity, Paolo Bonzini, Richard Henderson 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, -- Peter Xu ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-08-24 16:21 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-08-24 10:47 [RFC v5 0/3] memory: Delete assertion in memory_region_unregister_iommu_notifier Eugenio Pérez 2020-08-24 10:47 ` [RFC v5 1/3] memory: Rename memory_region_notify_one to memory_region_notify_iommu_one Eugenio Pérez 2020-08-24 10:47 ` [RFC v5 2/3] memory: Skip bad range assertion if notifier is DEVIOTLB type Eugenio Pérez 2020-08-24 10:47 ` [RFC v5 3/3] intel_iommu: Do not notify regular iotlb to device-iotlb notifiers Eugenio Pérez 2020-08-24 16:20 ` Peter Xu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).