diff mbox

[qemu,v13,15/16] vfio: Move iova_pgsizes from container to guest IOMMU

Message ID 1456823441-46757-16-git-send-email-aik@ozlabs.ru (mailing list archive)
State New, archived
Headers show

Commit Message

Alexey Kardashevskiy March 1, 2016, 9:10 a.m. UTC 
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 <aik@ozlabs.ru>
---
 hw/vfio/common.c              | 16 ++++------------
 include/hw/vfio/vfio-common.h |  2 +-
 2 files changed, 5 insertions(+), 13 deletions(-)

Comments

David Gibson March 3, 2016, 11:22 a.m. UTC | #1 
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 <aik@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;
Alexey Kardashevskiy March 4, 2016, 12:02 a.m. UTC | #2 
On 03/03/2016 10:22 PM, David Gibson wrote:
> 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 <aik@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.


Ah, did not realize that. Then it should be not a move but an additional 
giommu->iova_pgsizes. And this probably answers todo#1 in 16/16 about page 
masks.



>
>> ---
>>   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;
>
diff mbox

Patch

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;