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 <avih...@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 <avih...@nvidia.com>
Signed-off-by: Joao Martins <joao.m.mart...@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.


Reply via email to