Re: [PATCH v4 06/15] intel-iommu: Implement get_attr() method

[Duan, Zhenzhong]

On 6/23/2023 5:48 AM, Joao Martins wrote:

Implement IOMMU MR get_attr() method and use the dma_translation
property to report the IOMMU_ATTR_DMA_TRANSLATION attribute.
Additionally add the necessary get_iommu_attr into the PCIIOMMUOps to
support pci_device_iommu_get_attr().

The callback in there acts as a IOMMU-specific address space walker
which will call get_attr in the IOMMUMemoryRegion backing the device to
fetch the desired attribute.

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

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 1606d1b952d0..ed2a46e008df 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -3861,6 +3861,29 @@ static void vtd_iommu_replay(IOMMUMemoryRegion 
*iommu_mr, IOMMUNotifier *n)
      return;
  }

+static int vtd_iommu_get_attr(IOMMUMemoryRegion *iommu_mr,

+                              enum IOMMUMemoryRegionAttr attr, void *data)
+{
+    VTDAddressSpace *vtd_as = container_of(iommu_mr, VTDAddressSpace, iommu);
+    IntelIOMMUState *s = vtd_as->iommu_state;
+    int ret = 0;
+
+    switch (attr) {
+    case IOMMU_ATTR_DMA_TRANSLATION:
+    {
+        bool *enabled = (bool *)(uintptr_t) data;
+
+        *enabled = s->dma_translation;
+        break;
+    }
+    default:
+        ret = -EINVAL;
+        break;
+    }
+
+    return ret;
+}
+
  /* Do the initialization. It will also be called when reset, so pay
   * attention when adding new initialization stuff.
   */
@@ -4032,8 +4055,24 @@ static AddressSpace *vtd_host_dma_iommu(PCIBus *bus, 
void *opaque, int devfn)
      return &vtd_as->as;
  }

+static int vtd_get_iommu_attr(PCIBus *bus, void *opaque, int32_t devfn,

+                              enum IOMMUMemoryRegionAttr attr, void *data)
+{
+    IntelIOMMUState *s = opaque;
+    VTDAddressSpace *vtd_as;
+
+    assert(0 <= devfn && devfn < PCI_DEVFN_MAX);
+
+    vtd_as = vtd_find_add_as(s, bus, devfn, PCI_NO_PASID);
+    if (!vtd_as)
+       return -EINVAL;
+
+    return memory_region_iommu_get_attr(&vtd_as->iommu, attr, data);
+}
+
  static PCIIOMMUOps vtd_iommu_ops = {
      .get_address_space = vtd_host_dma_iommu,
+    .get_iommu_attr = vtd_get_iommu_attr,
  };

static bool vtd_decide_config(IntelIOMMUState *s, Error **errp)

@@ -4197,6 +4236,7 @@ static void 
vtd_iommu_memory_region_class_init(ObjectClass *klass,
      imrc->translate = vtd_iommu_translate;
      imrc->notify_flag_changed = vtd_iommu_notify_flag_changed;
      imrc->replay = vtd_iommu_replay;
+    imrc->get_attr = vtd_iommu_get_attr;

Do we have other user of imrc->get_attr? If not, what about squashing

vtd_iommu_get_attr into vtd_get_iommu_attr and have imrc->get_attr removed?

Thanks
Zhenzhong

  }

static const TypeInfo vtd_iommu_memory_region_info = {