From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55253) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zen18-0001AW-Ty for qemu-devel@nongnu.org; Wed, 23 Sep 2015 12:35:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zen14-0008J3-B5 for qemu-devel@nongnu.org; Wed, 23 Sep 2015 12:35:38 -0400 References: <1442495357-26547-1-git-send-email-david@gibson.dropbear.id.au> <1442495357-26547-6-git-send-email-david@gibson.dropbear.id.au> <56028192.8060001@redhat.com> From: Laurent Vivier Message-ID: <5602D4D0.6020202@redhat.com> Date: Wed, 23 Sep 2015 18:35:28 +0200 MIME-Version: 1.0 In-Reply-To: <56028192.8060001@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH 05/10] memory: Allow replay of IOMMU mapping notifications List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Thomas Huth , David Gibson , alex.williamson@redhat.com, aik@ozlabs.ru, gwshan@linux.vnet.ibm.com Cc: pbonzini@redhat.com, qemu-ppc@nongnu.org, qemu-devel@nongnu.org On 23/09/2015 12:40, Thomas Huth wrote: > On 17/09/15 15:09, David Gibson wrote: >> When we have guest visible IOMMUs, we allow notifiers to be registered >> which will be informed of all changes to IOMMU mappings. This is used by >> vfio to keep the host IOMMU mappings in sync with guest IOMMU mappings. >> >> However, unlike with a memory region listener, an iommu notifier won't be >> told about any mappings which already exist in the (guest) IOMMU at the >> time it is registered. This can cause problems if hotplugging a VFIO >> device onto a guest bus which had existing guest IOMMU mappings, but didn't >> previously have an VFIO devices (and hence no host IOMMU mappings). >> >> This adds a memory_region_register_iommu_notifier_replay() function to >> handle this case. As well as registering the new notifier it replays >> existing mappings. Because the IOMMU memory region doesn't internally >> remember the granularity of the guest IOMMU it has a small hack where the >> caller must specify a granularity at which to replay mappings. >> >> If there are finer mappings in the guest IOMMU these will be reported in >> the iotlb structures passed to the notifier which it must handle (probably >> causing it to flag an error). This isn't new - the VFIO iommu notifier >> must already handle notifications about guest IOMMU mappings too short >> for it to represent in the host IOMMU. >> >> Signed-off-by: David Gibson >> --- >> include/exec/memory.h | 16 ++++++++++++++++ >> memory.c | 18 ++++++++++++++++++ >> 2 files changed, 34 insertions(+) > >> diff --git a/memory.c b/memory.c >> index 0d8b2d9..6b5a2f1 100644 >> --- a/memory.c >> +++ b/memory.c >> @@ -1403,6 +1403,24 @@ void memory_region_register_iommu_notifier(MemoryRegion *mr, Notifier *n) >> notifier_list_add(&mr->iommu_notify, n); >> } >> >> +void memory_region_register_iommu_notifier_replay(MemoryRegion *mr, Notifier *n, >> + hwaddr granularity, bool is_write) > > granularity itself is not an address, but a size, isn't it? So using > "hwaddr" sounds wrong here. I disagree: in memory.c, all size are defined with hwaddr. >> +{ >> + hwaddr addr; > > dma_addr_t ? > >> + IOMMUTLBEntry iotlb; >> + >> + memory_region_register_iommu_notifier(mr, n); >> + >> + for (addr = 0; >> + int128_lt(int128_make64(addr), mr->size); >> + addr += granularity) { >> + >> + iotlb = mr->iommu_ops->translate(mr, addr, is_write); >> + if (iotlb.perm != IOMMU_NONE) >> + n->notify(n, &iotlb); > > Missing curly braces. > >> + } >> +} >> + > > Thomas >