[PATCH] Revert "memory: Optimize replay of guest mapping"

[PATCH] Revert "memory: Optimize replay of guest mapping"

[Michael S. Tsirkin]

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(-)

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);
-- 
MST

Re: [PATCH] Revert "memory: Optimize replay of guest mapping"

[Philippe Mathieu-Daudé]

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>

Re: [PATCH] Revert "memory: Optimize replay of guest mapping"

[Peter Maydell]

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