On Tue, Mar 01, 2016 at 08:10:40PM +1100, Alexey Kardashevskiy wrote: > The page size is an attribute of an IOMMU, not a container as a container > may contain more just one IOMMU. > > This moves iova_pgsizes from VFIOContainer to VFIOGuestIOMMU. > The following patch will use this. > > This removes iova_pgsizes from Type1 IOMMU as it is not used there anyway > and when it will get guest visible IOMMU, it will use VFIOGuestIOMMU's > iova_pgsizes. > > Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru>
Hmm. This makes an important semantic change which.. I'm not sure is
wrong, but certainly isn't adequately addressed in your commit
message.
The current iova_pgsizes is populated with information about the
*host* IOMMU, whereas you're replacing it with information about the
*guest* IOMMU.
> ---
> hw/vfio/common.c | 16 ++++------------
> include/hw/vfio/vfio-common.h | 2 +-
> 2 files changed, 5 insertions(+), 13 deletions(-)
>
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index f2a03e0..42ef1eb 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -313,9 +313,9 @@ out:
> rcu_read_unlock();
> }
>
> -static hwaddr vfio_container_granularity(VFIOContainer *container)
> +static hwaddr vfio_container_granularity(VFIOGuestIOMMU *giommu)
> {
> - return (hwaddr)1 << ctz64(container->iova_pgsizes);
> + return (hwaddr)1 << ctz64(giommu->iova_pgsizes);
> }
>
> static hwaddr vfio_iommu_page_mask(MemoryRegion *mr)
> @@ -392,12 +392,13 @@ static void vfio_listener_region_add(VFIOMemoryListener
> *vlistener,
> section->offset_within_address_space;
> giommu->container = container;
> giommu->n.notify = vfio_iommu_map_notify;
> + giommu->iova_pgsizes =
> section->mr->iommu_ops->get_page_sizes(section->mr);
> QLIST_INSERT_HEAD(&container->giommu_list, giommu, giommu_next);
>
> memory_region_register_iommu_notifier(giommu->iommu, &giommu->n);
> giommu->iommu->iommu_ops->vfio_notify(section->mr, true);
> memory_region_iommu_replay(giommu->iommu, &giommu->n,
> - vfio_container_granularity(container),
> + vfio_container_granularity(giommu),
> false);
>
> return;
> @@ -743,14 +744,8 @@ static int vfio_connect_container(VFIOGroup *group,
> AddressSpace *as)
> container->min_iova = 0;
> container->max_iova = (hwaddr)-1;
>
> - /* Assume just 4K IOVA page size */
> - container->iova_pgsizes = 0x1000;
> info.argsz = sizeof(info);
> ret = ioctl(fd, VFIO_IOMMU_GET_INFO, &info);
> - /* Ignore errors */
> - if ((ret == 0) && (info.flags & VFIO_IOMMU_INFO_PGSIZES)) {
> - container->iova_pgsizes = info.iova_pgsizes;
> - }
> } else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU) ||
> ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_v2_IOMMU)) {
> struct vfio_iommu_spapr_tce_info info;
> @@ -811,9 +806,6 @@ static int vfio_connect_container(VFIOGroup *group,
> AddressSpace *as)
> }
> container->min_iova = info.dma32_window_start;
> container->max_iova = container->min_iova + info.dma32_window_size -
> 1;
> -
> - /* Assume just 4K IOVA pages for now */
> - container->iova_pgsizes = 0x1000;
> } else {
> error_report("vfio: No available IOMMU models");
> ret = -EINVAL;
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index bcbc5cb..48a1d7f 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -80,7 +80,6 @@ typedef struct VFIOContainer {
> * future
> */
> hwaddr min_iova, max_iova;
> - uint64_t iova_pgsizes;
> QLIST_HEAD(, VFIOGuestIOMMU) giommu_list;
> QLIST_HEAD(, VFIOGroup) group_list;
> QLIST_ENTRY(VFIOContainer) next;
> @@ -90,6 +89,7 @@ typedef struct VFIOGuestIOMMU {
> VFIOContainer *container;
> MemoryRegion *iommu;
> hwaddr offset_within_address_space;
> + uint64_t iova_pgsizes;
> Notifier n;
> QLIST_ENTRY(VFIOGuestIOMMU) giommu_next;
> } VFIOGuestIOMMU;
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
