RE: [PATCH] memory: Optimize replay of guest mapping

[Duan, Zhenzhong]

>-----Original Message-----
>From: Jason Wang <jasowang@redhat.com>
>Sent: Tuesday, February 14, 2023 2:25 PM
>To: Duan, Zhenzhong <zhenzhong.duan@intel.com>
>Cc: qemu-devel@nongnu.org; mst@redhat.com; peterx@redhat.com;
>pbonzini@redhat.com; richard.henderson@linaro.org; eduardo@habkost.net;
>marcel.apfelbaum@gmail.com; david@redhat.com; philmd@linaro.org
>Subject: Re: [PATCH] memory: Optimize replay of guest mapping
>
>On Tue, Feb 14, 2023 at 11:43 AM Zhenzhong Duan
><zhenzhong.duan@intel.com> wrote:
>>
>> 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.
>>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>> Tested only on x86 with a net card passed to guest, ping/ssh pass.
>>
>>  hw/i386/intel_iommu.c | 2 +-
>>  softmmu/memory.c      | 3 +--
>>  2 files changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index
>> 98a5c304a7d7..6b1de80e8573 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -3831,7 +3831,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, 0, ~0ULL, &info, vtd_as->pasid);
>> +            vtd_page_walk(s, &ce, n->start, n->end, &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
>> 9d64efca269b..f096716e6e78 100644
>> --- a/softmmu/memory.c
>> +++ b/softmmu/memory.c
>> @@ -1923,7 +1923,6 @@ 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 +1935,7 @@ void
>> memory_region_iommu_replay(IOMMUMemoryRegion *iommu_mr,
>IOMMUNotifier
>> *n)
>>
>>      granularity = memory_region_iommu_get_min_page_size(iommu_mr);
>>
>> -    for (addr = 0; addr < memory_region_size(mr); addr += granularity) {
>> +    for (addr = n->start; addr < n->end; addr += granularity) {
>
>Is [n->start, n->end] guaranteed to be the subset of memory_region_size(mr)?

In current implementation it is.
[n->start, n->end] of notifier is derived from iommu memory region's section
which is a subset of iommu memory region itself.

Thanks
Zhenzhong