Re: [PATCH v4] memory: Optimize replay of guest mapping

[Peter Maydell]

On Wed, 19 Apr 2023 at 11:41, Zhenzhong Duan <zhenzhong.duan@intel.com> wrote:
>
> On x86, there are two notifiers registered due to vtd-ir memory
> region splitting the entire address space. During replay of the
> address space for each notifier, the whole address space is
> scanned which is unnecessary. We only need to scan the space
> belong to notifier monitored space.
>
> While on x86 IOMMU memory region spans over entire address space,
> but on some other platforms(e.g. arm mps3-an547), IOMMU memory
> region is only a window in the entire address space. User could
> register a notifier with arbitrary scope beyond IOMMU memory
> region. Though in current implementation replay is only triggered
> by VFIO and dirty page sync with notifiers derived from memory
> region section, but this isn't guaranteed in the future.
>
> So, we replay the intersection part of IOMMU memory region and
> notifier's scope in memory_region_iommu_replay(). Update doc
> comment to match this change.

Hi; I have a couple of minor wording tweaks, and one question
about the docs:

> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 15ade918baa4..61da32d8a428 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -425,12 +425,13 @@ struct IOMMUMemoryRegionClass {
>       * Called to handle memory_region_iommu_replay().
>       *
>       * The default implementation of memory_region_iommu_replay() is to
> -     * call the IOMMU translate method for every page in the address space
> +     * call the IOMMU translate method for every page falling in the
> +     * intersection part of IOMMU memory region and notifier's scope

"falling in the intersection of the IOMMU MemoryRegion and the
MemoryRegion which the notifier was registered for"

>       * with flag == IOMMU_NONE and then call the notifier if translate
>       * returns a valid mapping. If this method is implemented then it
>       * overrides the default behaviour, and must provide the full semantics
>       * of memory_region_iommu_replay(), by calling @notifier for every
> -     * translation present in the IOMMU.
> +     * translation present in the intersection part.

"present in the IOMMU that is within the MemoryRegion the
notifier was registered for."

Question: is it OK for an implementation of this method to call
the notifier for translations that are in the IOMMU and which
are not in the scope of the notifier (ie which are outside
the intersection) ? Or must it specifically restrict itself
to only calling the notifier for translations which are
inside the notifier's range ?

If the latter, we need to check all 4 existing implementations
of this method to ensure that they are not sending notifications
they should not; if the former, we should document that
implementations have that leeway.

thanks
-- PMM