[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCHv3 5/7] memory: Allow replay of IOMMU mapping notif
From: |
Laurent Vivier |
Subject: |
Re: [Qemu-ppc] [PATCHv3 5/7] memory: Allow replay of IOMMU mapping notifications |
Date: |
Wed, 30 Sep 2015 10:59:52 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 |
On 30/09/2015 04:13, 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_iommu_replay() function to handle this case. It
> replays any existing mappings in an IOMMU memory region to a specified
> notifier. 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 <address@hidden>
> ---
> include/exec/memory.h | 13 +++++++++++++
> memory.c | 20 ++++++++++++++++++++
> 2 files changed, 33 insertions(+)
>
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 5baaf48..0f07159 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -583,6 +583,19 @@ void memory_region_notify_iommu(MemoryRegion *mr,
> void memory_region_register_iommu_notifier(MemoryRegion *mr, Notifier *n);
>
> /**
> + * memory_region_iommu_replay: replay existing IOMMU translations to
> + * a notifier
> + *
> + * @mr: the memory region to observe
> + * @n: the notifier to which to replay iommu mappings
> + * @granularity: Minimum page granularity to replay notifications for
> + * @is_write: Whether to treat the replay as a translate "write"
> + * through the iommu
> + */
> +void memory_region_iommu_replay(MemoryRegion *mr, Notifier *n,
> + hwaddr granularity, bool is_write);
> +
> +/**
> * memory_region_unregister_iommu_notifier: unregister a notifier for
> * changes to IOMMU translation entries.
> *
> diff --git a/memory.c b/memory.c
> index ef87363..1b03d22 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1403,6 +1403,26 @@ void
> memory_region_register_iommu_notifier(MemoryRegion *mr, Notifier *n)
> notifier_list_add(&mr->iommu_notify, n);
> }
>
> +void memory_region_iommu_replay(MemoryRegion *mr, Notifier *n,
> + hwaddr granularity, bool is_write)
> +{
> + hwaddr addr;
> + IOMMUTLBEntry iotlb;
> +
> + for (addr = 0; addr < memory_region_size(mr); addr += granularity) {
> + iotlb = mr->iommu_ops->translate(mr, addr, is_write);
in iotlb, there is an "address_mask", on spapr, it is copied from
"page_shift", which is SPAPR_TCE_PAGE_SHIFT (12 -> 4k).
At a first glance, we would like to use it to scan the memory region,
but as granularity could be a greater value, I think it is a better choice.
But the question is: why the iotlb page_size is not equal to the
granularity given by VFIO_IOMMU_GET_INFO _IO ?
> + if (iotlb.perm != IOMMU_NONE) {
> + n->notify(n, &iotlb);
> + }
> +
> + /* if (2^64 - MR size) < granularity, it's possible to get an
> + * infinite loop here. This should catch such a wraparound */
> + if ((addr + granularity) < addr) {
> + break;
> + }
> + }
> +}
> +
> void memory_region_unregister_iommu_notifier(Notifier *n)
> {
> notifier_remove(n);
>
As my question is not a bout this particular patch but on another
existing part, I can say:
Reviewed-by: Laurent Vivier <address@hidden>
- [Qemu-ppc] [PATCHv3 1/7] vfio: Remove unneeded union from VFIOContainer, (continued)
- [Qemu-ppc] [PATCHv3 1/7] vfio: Remove unneeded union from VFIOContainer, David Gibson, 2015/10/08
- [Qemu-ppc] [PATCHv3 2/7] vfio: Generalize vfio_listener_region_add failure path, David Gibson, 2015/10/08
- [Qemu-ppc] [PATCHv3 3/7] vfio: Check guest IOVA ranges against host IOMMU capabilities, David Gibson, 2015/10/08
- Re: [Qemu-ppc] [PATCHv3 0/7] VFIO extensions to allow VFIO devices on spapr-pci-host-bridge, Alex Williamson, 2015/10/08
- [Qemu-ppc] [PATCHv3 4/7] vfio: Record host IOMMU's available IO page sizes, David Gibson, 2015/10/08
- [Qemu-ppc] [PATCHv3 5/7] memory: Allow replay of IOMMU mapping notifications, David Gibson, 2015/10/08