| Message ID | 917c1c552b2d1b732f9a86c6a90684c3a5e4cada.1680640587.git.mst@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Revert "memory: Optimize replay of guest mapping" | expand |
On 4/4/23 22:37, Michael S. Tsirkin wrote: > From: Peter Maydell <peter.maydell@linaro.org> > > This reverts commit 6da24341866fa940fd7d575788a2319514941c77 > ("memory: Optimize replay of guest mapping"). > > This change breaks the mps3-an547 board under TCG (and > probably other TCG boards using an IOMMU), which now > assert: > > $ ./build/x86/qemu-system-arm --machine mps3-an547 -serial stdio > -kernel /tmp/an547-mwe/build/test.elf > qemu-system-arm: ../../softmmu/memory.c:1903: > memory_region_register_iommu_notifier: Assertion `n->end <= > memory_region_size(mr)' failed. > > This is because tcg_register_iommu_notifier() registers > an IOMMU notifier which covers the entire address space, > so the assertion added in this commit is not correct. > > For the 8.0 release, just revert this commit as it is > only an optimization. > > Fixes: 6da24341866f ("memory: Optimize replay of guest mapping") > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > > completely untested but Pater asked help in sending this. > > hw/i386/intel_iommu.c | 2 +- > softmmu/memory.c | 4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
On Tue, 4 Apr 2023 at 23:04, Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > > On 4/4/23 22:37, Michael S. Tsirkin wrote: > > From: Peter Maydell <peter.maydell@linaro.org> > > > > This reverts commit 6da24341866fa940fd7d575788a2319514941c77 > > ("memory: Optimize replay of guest mapping"). > > > > This change breaks the mps3-an547 board under TCG (and > > probably other TCG boards using an IOMMU), which now > > assert: > > > > $ ./build/x86/qemu-system-arm --machine mps3-an547 -serial stdio > > -kernel /tmp/an547-mwe/build/test.elf > > qemu-system-arm: ../../softmmu/memory.c:1903: > > memory_region_register_iommu_notifier: Assertion `n->end <= > > memory_region_size(mr)' failed. > > > > This is because tcg_register_iommu_notifier() registers > > an IOMMU notifier which covers the entire address space, > > so the assertion added in this commit is not correct. > > > > For the 8.0 release, just revert this commit as it is > > only an optimization. > > > > Fixes: 6da24341866f ("memory: Optimize replay of guest mapping") > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > --- > > > > completely untested but Pater asked help in sending this. > > > > hw/i386/intel_iommu.c | 2 +- > > softmmu/memory.c | 4 ++-- > > 2 files changed, 3 insertions(+), 3 deletions(-) > > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Thanks; I have applied this to master to fix the assertion failure for rc3. -- PMM
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index faade7def8..a62896759c 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -3850,7 +3850,7 @@ static void vtd_iommu_replay(IOMMUMemoryRegion *iommu_mr, IOMMUNotifier *n) .domain_id = vtd_get_domain_id(s, &ce, vtd_as->pasid), }; - vtd_page_walk(s, &ce, n->start, n->end, &info, vtd_as->pasid); + vtd_page_walk(s, &ce, 0, ~0ULL, &info, vtd_as->pasid); } } else { trace_vtd_replay_ce_invalid(bus_n, PCI_SLOT(vtd_as->devfn), diff --git a/softmmu/memory.c b/softmmu/memory.c index 5305aca7ca..b1a6cae6f5 100644 --- a/softmmu/memory.c +++ b/softmmu/memory.c @@ -1900,7 +1900,6 @@ int memory_region_register_iommu_notifier(MemoryRegion *mr, iommu_mr = IOMMU_MEMORY_REGION(mr); assert(n->notifier_flags != IOMMU_NOTIFIER_NONE); assert(n->start <= n->end); - assert(n->end <= memory_region_size(mr)); assert(n->iommu_idx >= 0 && n->iommu_idx < memory_region_iommu_num_indexes(iommu_mr)); @@ -1924,6 +1923,7 @@ uint64_t memory_region_iommu_get_min_page_size(IOMMUMemoryRegion *iommu_mr) void memory_region_iommu_replay(IOMMUMemoryRegion *iommu_mr, IOMMUNotifier *n) { + MemoryRegion *mr = MEMORY_REGION(iommu_mr); IOMMUMemoryRegionClass *imrc = IOMMU_MEMORY_REGION_GET_CLASS(iommu_mr); hwaddr addr, granularity; IOMMUTLBEntry iotlb; @@ -1936,7 +1936,7 @@ void memory_region_iommu_replay(IOMMUMemoryRegion *iommu_mr, IOMMUNotifier *n) granularity = memory_region_iommu_get_min_page_size(iommu_mr); - for (addr = n->start; addr < n->end; addr += granularity) { + for (addr = 0; addr < memory_region_size(mr); addr += granularity) { iotlb = imrc->translate(iommu_mr, addr, IOMMU_NONE, n->iommu_idx); if (iotlb.perm != IOMMU_NONE) { n->notify(n, &iotlb);