1. Patchew
  2. QEMU
  3. [PATCH v3 00/15] vfio: VFIO migration support with vIOMMU
  4. View patch

[PATCH v3 10/15] intel-iommu: Implement IOMMU_ATTR_MAX_IOVA get_attr() attribute

Joao Martins posted 15 patches 2 years, 8 months ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Peter Xu <peterx@redhat.com>, Jason Wang <jasowang@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Alex Williamson <alex.williamson@redhat.com>, "Cédric Le Goater" <clg@redhat.com>, David Hildenbrand <david@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>
There is a newer version of this series
[PATCH v3 10/15] intel-iommu: Implement IOMMU_ATTR_MAX_IOVA get_attr() attribute
Posted by Joao Martins 2 years, 8 months ago 
From: Avihai Horon <avihaih@nvidia.com>

Implement get_attr() method and use the address width property to report
the IOMMU_ATTR_MAX_IOVA attribute.

Signed-off-by: Avihai Horon <avihaih@nvidia.com>
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
---
 hw/i386/intel_iommu.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 1906f3a67960..829dd6eadc6c 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -3876,6 +3876,13 @@ static int vtd_iommu_get_attr(IOMMUMemoryRegion *iommu_mr,
         *enabled = s->dma_translation;
         break;
     }
+    case IOMMU_ATTR_MAX_IOVA:
+    {
+        hwaddr *max_iova = data;
+
+        *max_iova = (1ULL << s->aw_bits) - 1;
+        break;
+    }
     default:
         ret = -EINVAL;
         break;
-- 
2.39.3
Re: [PATCH v3 10/15] intel-iommu: Implement IOMMU_ATTR_MAX_IOVA get_attr() attribute
Posted by Philippe Mathieu-Daudé 2 years, 8 months ago 
On 30/5/23 19:59, Joao Martins wrote:
> From: Avihai Horon <avihaih@nvidia.com>
> 
> Implement get_attr() method and use the address width property to report
> the IOMMU_ATTR_MAX_IOVA attribute.
> 
> Signed-off-by: Avihai Horon <avihaih@nvidia.com>
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> ---
>   hw/i386/intel_iommu.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 1906f3a67960..829dd6eadc6c 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -3876,6 +3876,13 @@ static int vtd_iommu_get_attr(IOMMUMemoryRegion *iommu_mr,
>           *enabled = s->dma_translation;
>           break;
>       }
> +    case IOMMU_ATTR_MAX_IOVA:
> +    {
> +        hwaddr *max_iova = data;

Shouldn't we cast to uintptr_t to be safe?

> +        *max_iova = (1ULL << s->aw_bits) - 1;

Alternatively:

            *max_iova = MAKE_64BIT_MASK(0, s->aw_bits);

> +        break;
> +    }
>       default:
>           ret = -EINVAL;
>           break;
Re: [PATCH v3 10/15] intel-iommu: Implement IOMMU_ATTR_MAX_IOVA get_attr() attribute
Posted by Joao Martins 2 years, 8 months ago 

On 30/05/2023 22:45, Philippe Mathieu-Daudé wrote:
> On 30/5/23 19:59, Joao Martins wrote:
>> From: Avihai Horon <avihaih@nvidia.com>
>>
>> Implement get_attr() method and use the address width property to report
>> the IOMMU_ATTR_MAX_IOVA attribute.
>>
>> Signed-off-by: Avihai Horon <avihaih@nvidia.com>
>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>> ---
>>   hw/i386/intel_iommu.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index 1906f3a67960..829dd6eadc6c 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -3876,6 +3876,13 @@ static int vtd_iommu_get_attr(IOMMUMemoryRegion *iommu_mr,
>>           *enabled = s->dma_translation;
>>           break;
>>       }
>> +    case IOMMU_ATTR_MAX_IOVA:
>> +    {
>> +        hwaddr *max_iova = data;
> 
> Shouldn't we cast to uintptr_t to be safe?
> 
Perhaps you mean something like this:

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 829dd6eadc6c..479307f1228f 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -3878,7 +3878,7 @@ static int vtd_iommu_get_attr(IOMMUMemoryRegion *iommu_mr,
     }
     case IOMMU_ATTR_MAX_IOVA:
     {
-        hwaddr *max_iova = data;
+        hwaddr *max_iova = (hwaddr *)(uintptr_t) data;

         *max_iova = (1ULL << s->aw_bits) - 1;
         break;

I guess the thinking is to prevent 32-bit failures.

>> +        *max_iova = (1ULL << s->aw_bits) - 1;
> 
> Alternatively:
> 
>            *max_iova = MAKE_64BIT_MASK(0, s->aw_bits);
> 

I'll switch to your suggestion. Wasn't aware of this macro :)

>> +        break;
>> +    }
>>       default:
>>           ret = -EINVAL;
>>           break;
>
Re: [PATCH v3 10/15] intel-iommu: Implement IOMMU_ATTR_MAX_IOVA get_attr() attribute
Posted by Philippe Mathieu-Daudé 2 years, 8 months ago 
On 31/5/23 11:54, Joao Martins wrote:
> 
> 
> On 30/05/2023 22:45, Philippe Mathieu-Daudé wrote:
>> On 30/5/23 19:59, Joao Martins wrote:
>>> From: Avihai Horon <avihaih@nvidia.com>
>>>
>>> Implement get_attr() method and use the address width property to report
>>> the IOMMU_ATTR_MAX_IOVA attribute.
>>>
>>> Signed-off-by: Avihai Horon <avihaih@nvidia.com>
>>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>>> ---
>>>    hw/i386/intel_iommu.c | 7 +++++++
>>>    1 file changed, 7 insertions(+)


>>> +    case IOMMU_ATTR_MAX_IOVA:
>>> +    {
>>> +        hwaddr *max_iova = data;
>>
>> Shouldn't we cast to uintptr_t to be safe?
>>
> Perhaps you mean something like this:
> 
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 829dd6eadc6c..479307f1228f 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -3878,7 +3878,7 @@ static int vtd_iommu_get_attr(IOMMUMemoryRegion *iommu_mr,
>       }
>       case IOMMU_ATTR_MAX_IOVA:
>       {
> -        hwaddr *max_iova = data;
> +        hwaddr *max_iova = (hwaddr *)(uintptr_t) data;
> 
>           *max_iova = (1ULL << s->aw_bits) - 1;
>           break;
> 
> I guess the thinking is to prevent 32-bit failures.

Exactly.

>>> +        *max_iova = (1ULL << s->aw_bits) - 1;
>>
>> Alternatively:
>>
>>             *max_iova = MAKE_64BIT_MASK(0, s->aw_bits);
>>
> 
> I'll switch to your suggestion. Wasn't aware of this macro :)

Thanks, it is a no-brainer when reviewing.

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.