Re: [PULL 36/53] memory: Optimize replay of guest mapping

[Peter Maydell]

On Thu, 2 Mar 2023 at 08:26, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> From: Zhenzhong Duan <zhenzhong.duan@intel.com>
>
> On x86, there are two notifiers registered due to vtd-ir memory region
> splitting the whole address space. During replay of the address space
> for each notifier, the whole address space is scanned which is
> unnecessory.
>
> We only need to scan the space belong to notifier montiored space.
>
> Assert when notifier is used to monitor beyond iommu memory region's
> address space.

Hi. This patch seems to have regressed the mps3-an547 board,
which now asserts on startup:

$ ./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.
Aborted (core dumped)

(reported under https://gitlab.com/qemu-project/qemu/-/issues/1488)

Since this commit says it's just an optimization, for the 8.0
release can we simply revert it without breaking anything?

> diff --git a/softmmu/memory.c b/softmmu/memory.c
> index 9d64efca26..da7d846619 100644
> --- a/softmmu/memory.c
> +++ b/softmmu/memory.c
> @@ -1900,6 +1900,7 @@ 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));

In the mps3-an547 case we assert here because n->end is -1.
This is because tcg_register_iommu_notifier() registers an iommu
notifier that covers the entire address space:

        iommu_notifier_init(&notifier->n,
                            tcg_iommu_unmap_notify,
                            IOMMU_NOTIFIER_UNMAP,
                            0,
                            HWADDR_MAX,
                            iommu_idx);
        memory_region_register_iommu_notifier(notifier->mr, &notifier->n,
                                              &error_fatal);

thanks
-- PMM