From: Xingang Wang <wangxingang5@huawei.com>

This add explicit IORT idmap info according to pci root bus number
range, and only add smmu idmap for those which does not bypass iommu.

For idmap directly to ITS node, this split the whole RID mapping to
smmu idmap and its idmap. So this should cover the whole idmap for
through/bypass SMMUv3 node.

Signed-off-by: Xingang Wang <wangxingang5@huawei.com>
---
 hw/arm/virt-acpi-build.c | 135 +++++++++++++++++++++++++++++++++------
 1 file changed, 116 insertions(+), 19 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 60fe2e65a7..f63a57dcfa 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -44,6 +44,7 @@
 #include "hw/acpi/tpm.h"
 #include "hw/pci/pcie_host.h"
 #include "hw/pci/pci.h"
+#include "hw/pci/pci_bus.h"
 #include "hw/pci-host/gpex.h"
 #include "hw/arm/virt.h"
 #include "hw/mem/nvdimm.h"
@@ -237,16 +238,82 @@ static void acpi_dsdt_add_tpm(Aml *scope, VirtMachineState *vms)
     aml_append(scope, dev);
 }
 
+/* Build the iort ID mapping to SMMUv3 for a given PCI host bridge */
+static int
+iort_host_bridges(Object *obj, void *opaque)
+{
+    GArray *idmap_blob = opaque;
+
+    if (object_dynamic_cast(obj, TYPE_PCI_HOST_BRIDGE)) {
+        PCIBus *bus = PCI_HOST_BRIDGE(obj)->bus;
+
+        if (bus && !pci_bus_bypass_iommu(bus)) {
+            int min_bus, max_bus;
+            pci_bus_range(bus, &min_bus, &max_bus);
+
+            AcpiIortIdMapping idmap = {
+                .input_base = min_bus << 8,
+                .id_count = (max_bus - min_bus + 1) << 8,
+            };
+            g_array_append_val(idmap_blob, idmap);
+        }
+    }
+
+    return 0;
+}
+
+static int iort_idmap_compare(gconstpointer a, gconstpointer b)
+{
+    AcpiIortIdMapping *idmap_a = (AcpiIortIdMapping *)a;
+    AcpiIortIdMapping *idmap_b = (AcpiIortIdMapping *)b;
+
+    return idmap_a->input_base - idmap_b->input_base;
+}
+
 static void
 build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
 {
-    int nb_nodes, iort_start = table_data->len;
-    AcpiIortIdMapping *idmap;
+    int i, nb_nodes, rc_map_count, iort_start = table_data->len;
+    AcpiIortIdMapping *idmap, *range;
     AcpiIortItsGroup *its;
     AcpiIortTable *iort;
     AcpiIortSmmu3 *smmu;
     size_t node_size, iort_node_offset, iort_length, smmu_offset = 0;
     AcpiIortRC *rc;
+    GArray *smmu_idmap_ranges =
+        g_array_new(false, true, sizeof(AcpiIortIdMapping));
+    GArray *its_idmap_ranges =
+        g_array_new(false, true, sizeof(AcpiIortIdMapping));
+
+    object_child_foreach_recursive(object_get_root(),
+                                   iort_host_bridges, smmu_idmap_ranges);
+
+    g_array_sort(smmu_idmap_ranges, iort_idmap_compare);
+
+    AcpiIortIdMapping next_range = {
+        .input_base = 0,
+    };
+
+    /*
+     * Build the iort ID mapping to ITS directly,
+     * split the whole RID input range by RID mapping to SMMU node
+     */
+    for (i = 0; i < smmu_idmap_ranges->len; i++) {
+        idmap = &g_array_index(smmu_idmap_ranges, AcpiIortIdMapping, i);
+
+        if (next_range.input_base < idmap->input_base) {
+            next_range.id_count = idmap->input_base - next_range.input_base;
+            g_array_append_val(its_idmap_ranges, next_range);
+        }
+
+        next_range.input_base = idmap->input_base + idmap->id_count;
+    }
+
+    /* Append the last ITS ID mapping */
+    if (next_range.input_base < 0xFFFF) {
+        next_range.id_count = 0xFFFF - next_range.input_base;
+        g_array_append_val(its_idmap_ranges, next_range);
+    }
 
     iort = acpi_data_push(table_data, sizeof(*iort));
 
@@ -280,13 +347,13 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
 
         /* SMMUv3 node */
         smmu_offset = iort_node_offset + node_size;
-        node_size = sizeof(*smmu) + sizeof(*idmap);
+        node_size = sizeof(*smmu) + sizeof(*idmap) * smmu_idmap_ranges->len;
         iort_length += node_size;
         smmu = acpi_data_push(table_data, node_size);
 
         smmu->type = ACPI_IORT_NODE_SMMU_V3;
         smmu->length = cpu_to_le16(node_size);
-        smmu->mapping_count = cpu_to_le32(1);
+        smmu->mapping_count = cpu_to_le32(smmu_idmap_ranges->len);
         smmu->mapping_offset = cpu_to_le32(sizeof(*smmu));
         smmu->base_address = cpu_to_le64(vms->memmap[VIRT_SMMU].base);
         smmu->flags = cpu_to_le32(ACPI_IORT_SMMU_V3_COHACC_OVERRIDE);
@@ -295,23 +362,32 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
         smmu->sync_gsiv = cpu_to_le32(irq + 2);
         smmu->gerr_gsiv = cpu_to_le32(irq + 3);
 
-        /* Identity RID mapping covering the whole input RID range */
-        idmap = &smmu->id_mapping_array[0];
-        idmap->input_base = 0;
-        idmap->id_count = cpu_to_le32(0xFFFF);
-        idmap->output_base = 0;
         /* output IORT node is the ITS group node (the first node) */
-        idmap->output_reference = cpu_to_le32(iort_node_offset);
+        for (i = 0; i < smmu_idmap_ranges->len; i++) {
+            idmap = &smmu->id_mapping_array[i];
+            range = &g_array_index(smmu_idmap_ranges, AcpiIortIdMapping, i);
+
+            idmap->input_base = cpu_to_le32(range->input_base);
+            idmap->id_count = cpu_to_le32(range->id_count);
+            idmap->output_base = cpu_to_le32(range->input_base);
+            idmap->output_reference = cpu_to_le32(iort_node_offset);
+        }
     }
 
     /* Root Complex Node */
-    node_size = sizeof(*rc) + sizeof(*idmap);
+    if (vms->iommu == VIRT_IOMMU_SMMUV3) {
+        rc_map_count = smmu_idmap_ranges->len + its_idmap_ranges->len;
+    } else {
+        rc_map_count = 1;
+    }
+
+    node_size = sizeof(*rc) + sizeof(*idmap) * rc_map_count;
     iort_length += node_size;
     rc = acpi_data_push(table_data, node_size);
 
     rc->type = ACPI_IORT_NODE_PCI_ROOT_COMPLEX;
     rc->length = cpu_to_le16(node_size);
-    rc->mapping_count = cpu_to_le32(1);
+    rc->mapping_count = cpu_to_le32(rc_map_count);
     rc->mapping_offset = cpu_to_le32(sizeof(*rc));
 
     /* fully coherent device */
@@ -319,20 +395,41 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
     rc->memory_properties.memory_flags = 0x3; /* CCA = CPM = DCAS = 1 */
     rc->pci_segment_number = 0; /* MCFG pci_segment */
 
-    /* Identity RID mapping covering the whole input RID range */
-    idmap = &rc->id_mapping_array[0];
-    idmap->input_base = 0;
-    idmap->id_count = cpu_to_le32(0xFFFF);
-    idmap->output_base = 0;
-
     if (vms->iommu == VIRT_IOMMU_SMMUV3) {
         /* output IORT node is the smmuv3 node */
-        idmap->output_reference = cpu_to_le32(smmu_offset);
+        for (i = 0; i < smmu_idmap_ranges->len; i++) {
+            idmap = &rc->id_mapping_array[i];
+            range = &g_array_index(smmu_idmap_ranges, AcpiIortIdMapping, i);
+
+            idmap->input_base = cpu_to_le32(range->input_base);
+            idmap->id_count = cpu_to_le32(range->id_count);
+            idmap->output_base = cpu_to_le32(range->input_base);
+            idmap->output_reference = cpu_to_le32(smmu_offset);
+        }
+
+        /* output IORT node is the ITS group node (the first node) */
+        for (i = 0; i < its_idmap_ranges->len; i++) {
+            idmap = &rc->id_mapping_array[smmu_idmap_ranges->len + i];
+            range = &g_array_index(its_idmap_ranges, AcpiIortIdMapping, i);
+
+            idmap->input_base = cpu_to_le32(range->input_base);
+            idmap->id_count = cpu_to_le32(range->id_count);
+            idmap->output_base = cpu_to_le32(range->input_base);
+            idmap->output_reference = cpu_to_le32(iort_node_offset);
+        }
     } else {
+        /* Identity RID mapping covering the whole input RID range */
+        idmap = &rc->id_mapping_array[0];
+        idmap->input_base = cpu_to_le32(0);
+        idmap->id_count = cpu_to_le32(0xFFFF);
+        idmap->output_base = cpu_to_le32(0);
         /* output IORT node is the ITS group node (the first node) */
         idmap->output_reference = cpu_to_le32(iort_node_offset);
     }
 
+    g_array_free(smmu_idmap_ranges, true);
+    g_array_free(its_idmap_ranges, true);
+
     /*
      * Update the pointer address in case table_data->data moves during above
      * acpi_data_push operations.
-- 
2.19.1

Hi Xingang,

On 5/25/21 5:50 AM, Wang Xingang wrote:
> From: Xingang Wang <wangxingang5@huawei.com>
>
> This add explicit IORT idmap info according to pci root bus number
> range, and only add smmu idmap for those which does not bypass iommu.
>
> For idmap directly to ITS node, this split the whole RID mapping to
> smmu idmap and its idmap. So this should cover the whole idmap for
> through/bypass SMMUv3 node.
>
> Signed-off-by: Xingang Wang <wangxingang5@huawei.com>
> ---
>  hw/arm/virt-acpi-build.c | 135 +++++++++++++++++++++++++++++++++------
>  1 file changed, 116 insertions(+), 19 deletions(-)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 60fe2e65a7..f63a57dcfa 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -44,6 +44,7 @@
>  #include "hw/acpi/tpm.h"
>  #include "hw/pci/pcie_host.h"
>  #include "hw/pci/pci.h"
> +#include "hw/pci/pci_bus.h"
>  #include "hw/pci-host/gpex.h"
>  #include "hw/arm/virt.h"
>  #include "hw/mem/nvdimm.h"
> @@ -237,16 +238,82 @@ static void acpi_dsdt_add_tpm(Aml *scope, VirtMachineState *vms)
>      aml_append(scope, dev);
>  }
>  
> +/* Build the iort ID mapping to SMMUv3 for a given PCI host bridge */
> +static int
> +iort_host_bridges(Object *obj, void *opaque)
> +{
> +    GArray *idmap_blob = opaque;
> +
> +    if (object_dynamic_cast(obj, TYPE_PCI_HOST_BRIDGE)) {
> +        PCIBus *bus = PCI_HOST_BRIDGE(obj)->bus;
> +
> +        if (bus && !pci_bus_bypass_iommu(bus)) {
> +            int min_bus, max_bus;
extra line needed
> +            pci_bus_range(bus, &min_bus, &max_bus);
> +
> +            AcpiIortIdMapping idmap = {
> +                .input_base = min_bus << 8,
> +                .id_count = (max_bus - min_bus + 1) << 8,
> +            };
> +            g_array_append_val(idmap_blob, idmap);
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +static int iort_idmap_compare(gconstpointer a, gconstpointer b)
> +{
> +    AcpiIortIdMapping *idmap_a = (AcpiIortIdMapping *)a;
> +    AcpiIortIdMapping *idmap_b = (AcpiIortIdMapping *)b;
> +
> +    return idmap_a->input_base - idmap_b->input_base;
> +}
> +
>  static void
>  build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>  {
> -    int nb_nodes, iort_start = table_data->len;
> -    AcpiIortIdMapping *idmap;
> +    int i, nb_nodes, rc_map_count, iort_start = table_data->len;
> +    AcpiIortIdMapping *idmap, *range;
>      AcpiIortItsGroup *its;
>      AcpiIortTable *iort;
>      AcpiIortSmmu3 *smmu;
>      size_t node_size, iort_node_offset, iort_length, smmu_offset = 0;
>      AcpiIortRC *rc;
> +    GArray *smmu_idmap_ranges =
> +        g_array_new(false, true, sizeof(AcpiIortIdMapping));
> +    GArray *its_idmap_ranges =
> +        g_array_new(false, true, sizeof(AcpiIortIdMapping));
> +
> +    object_child_foreach_recursive(object_get_root(),
> +                                   iort_host_bridges, smmu_idmap_ranges);
> +
> +    g_array_sort(smmu_idmap_ranges, iort_idmap_compare);
> +
> +    AcpiIortIdMapping next_range = {
> +        .input_base = 0,
> +    };
> +
> +    /*
> +     * Build the iort ID mapping to ITS directly,
> +     * split the whole RID input range by RID mapping to SMMU node
> +     */
> +    for (i = 0; i < smmu_idmap_ranges->len; i++) {
> +        idmap = &g_array_index(smmu_idmap_ranges, AcpiIortIdMapping, i);
> +
> +        if (next_range.input_base < idmap->input_base) {
> +            next_range.id_count = idmap->input_base - next_range.input_base;
> +            g_array_append_val(its_idmap_ranges, next_range);
> +        }
> +
> +        next_range.input_base = idmap->input_base + idmap->id_count;
> +    }
> +
> +    /* Append the last ITS ID mapping */
> +    if (next_range.input_base < 0xFFFF) {
> +        next_range.id_count = 0xFFFF - next_range.input_base;
> +        g_array_append_val(its_idmap_ranges, next_range);
> +    }
>  
>      iort = acpi_data_push(table_data, sizeof(*iort));
>  
> @@ -280,13 +347,13 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>  
>          /* SMMUv3 node */
>          smmu_offset = iort_node_offset + node_size;
> -        node_size = sizeof(*smmu) + sizeof(*idmap);
> +        node_size = sizeof(*smmu) + sizeof(*idmap) * smmu_idmap_ranges->len;
>          iort_length += node_size;
>          smmu = acpi_data_push(table_data, node_size);
>  
>          smmu->type = ACPI_IORT_NODE_SMMU_V3;
>          smmu->length = cpu_to_le16(node_size);
> -        smmu->mapping_count = cpu_to_le32(1);
> +        smmu->mapping_count = cpu_to_le32(smmu_idmap_ranges->len);
>          smmu->mapping_offset = cpu_to_le32(sizeof(*smmu));
>          smmu->base_address = cpu_to_le64(vms->memmap[VIRT_SMMU].base);
>          smmu->flags = cpu_to_le32(ACPI_IORT_SMMU_V3_COHACC_OVERRIDE);
> @@ -295,23 +362,32 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>          smmu->sync_gsiv = cpu_to_le32(irq + 2);
>          smmu->gerr_gsiv = cpu_to_le32(irq + 3);
>  
> -        /* Identity RID mapping covering the whole input RID range */
> -        idmap = &smmu->id_mapping_array[0];
> -        idmap->input_base = 0;
> -        idmap->id_count = cpu_to_le32(0xFFFF);
> -        idmap->output_base = 0;
>          /* output IORT node is the ITS group node (the first node) */
> -        idmap->output_reference = cpu_to_le32(iort_node_offset);
> +        for (i = 0; i < smmu_idmap_ranges->len; i++) {
> +            idmap = &smmu->id_mapping_array[i];
> +            range = &g_array_index(smmu_idmap_ranges, AcpiIortIdMapping, i);
> +
> +            idmap->input_base = cpu_to_le32(range->input_base);
> +            idmap->id_count = cpu_to_le32(range->id_count);
> +            idmap->output_base = cpu_to_le32(range->input_base);
> +            idmap->output_reference = cpu_to_le32(iort_node_offset);
> +        }
I don't really get this extra complexity. Can't the SMMU -> ITS mapping
be a direct mapping covering the whole range of RIDs.
Do you really need to match the input ID range? I don't think so.

Bypassed RIDs should only affect RC mappings to me.
>      }
>  
>      /* Root Complex Node */
> -    node_size = sizeof(*rc) + sizeof(*idmap);
> +    if (vms->iommu == VIRT_IOMMU_SMMUV3) {
> +        rc_map_count = smmu_idmap_ranges->len + its_idmap_ranges->len;
> +    } else {
> +        rc_map_count = 1;
> +    }
> +
> +    node_size = sizeof(*rc) + sizeof(*idmap) * rc_map_count;
>      iort_length += node_size;
>      rc = acpi_data_push(table_data, node_size);
>  
>      rc->type = ACPI_IORT_NODE_PCI_ROOT_COMPLEX;
>      rc->length = cpu_to_le16(node_size);
> -    rc->mapping_count = cpu_to_le32(1);
> +    rc->mapping_count = cpu_to_le32(rc_map_count);
>      rc->mapping_offset = cpu_to_le32(sizeof(*rc));
>  
>      /* fully coherent device */
> @@ -319,20 +395,41 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>      rc->memory_properties.memory_flags = 0x3; /* CCA = CPM = DCAS = 1 */
>      rc->pci_segment_number = 0; /* MCFG pci_segment */
>  
> -    /* Identity RID mapping covering the whole input RID range */
> -    idmap = &rc->id_mapping_array[0];
> -    idmap->input_base = 0;
> -    idmap->id_count = cpu_to_le32(0xFFFF);
> -    idmap->output_base = 0;
> -
>      if (vms->iommu == VIRT_IOMMU_SMMUV3) {
>          /* output IORT node is the smmuv3 node */
maybe add a comment saying translated RIDs connect to SMMU node
> -        idmap->output_reference = cpu_to_le32(smmu_offset);
> +        for (i = 0; i < smmu_idmap_ranges->len; i++) {
> +            idmap = &rc->id_mapping_array[i];
> +            range = &g_array_index(smmu_idmap_ranges, AcpiIortIdMapping, i);
> +
> +            idmap->input_base = cpu_to_le32(range->input_base);
> +            idmap->id_count = cpu_to_le32(range->id_count);
> +            idmap->output_base = cpu_to_le32(range->input_base);
> +            idmap->output_reference = cpu_to_le32(smmu_offset);
> +        }
> +
add comment saying bypassed RIDs connect to ITS directly?
> +        /* output IORT node is the ITS group node (the first node) */
> +        for (i = 0; i < its_idmap_ranges->len; i++) {
> +            idmap = &rc->id_mapping_array[smmu_idmap_ranges->len + i];
> +            range = &g_array_index(its_idmap_ranges, AcpiIortIdMapping, i);
> +
> +            idmap->input_base = cpu_to_le32(range->input_base);
> +            idmap->id_count = cpu_to_le32(range->id_count);
> +            idmap->output_base = cpu_to_le32(range->input_base);
> +            idmap->output_reference = cpu_to_le32(iort_node_offset);
> +        }
>      } else {
> +        /* Identity RID mapping covering the whole input RID range */
> +        idmap = &rc->id_mapping_array[0];
> +        idmap->input_base = cpu_to_le32(0);
> +        idmap->id_count = cpu_to_le32(0xFFFF);
> +        idmap->output_base = cpu_to_le32(0);
>          /* output IORT node is the ITS group node (the first node) */
>          idmap->output_reference = cpu_to_le32(iort_node_offset);
>      }
>  
> +    g_array_free(smmu_idmap_ranges, true);
> +    g_array_free(its_idmap_ranges, true);
> +
>      /*
>       * Update the pointer address in case table_data->data moves during above
>       * acpi_data_push operations.
Thanks

Eric

Hi Eric,

On 2021/6/2 22:21, Eric Auger wrote:
> Hi Xingang,
> 
> On 5/25/21 5:50 AM, Wang Xingang wrote:
>> From: Xingang Wang <wangxingang5@huawei.com>
>>
>> This add explicit IORT idmap info according to pci root bus number
>> range, and only add smmu idmap for those which does not bypass iommu.
>>
>> For idmap directly to ITS node, this split the whole RID mapping to
>> smmu idmap and its idmap. So this should cover the whole idmap for
>> through/bypass SMMUv3 node.
>>
>> Signed-off-by: Xingang Wang <wangxingang5@huawei.com>
>> ---
>>   hw/arm/virt-acpi-build.c | 135 +++++++++++++++++++++++++++++++++------
>>   1 file changed, 116 insertions(+), 19 deletions(-)
>>
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index 60fe2e65a7..f63a57dcfa 100644
>> --- a/hw/arm/virt-acpi-build.c
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -44,6 +44,7 @@
>>   #include "hw/acpi/tpm.h"
>>   #include "hw/pci/pcie_host.h"
>>   #include "hw/pci/pci.h"
>> +#include "hw/pci/pci_bus.h"
>>   #include "hw/pci-host/gpex.h"
>>   #include "hw/arm/virt.h"
>>   #include "hw/mem/nvdimm.h"
>> @@ -237,16 +238,82 @@ static void acpi_dsdt_add_tpm(Aml *scope, VirtMachineState *vms)
>>       aml_append(scope, dev);
>>   }
>>   
>> +/* Build the iort ID mapping to SMMUv3 for a given PCI host bridge */
>> +static int
>> +iort_host_bridges(Object *obj, void *opaque)
>> +{
>> +    GArray *idmap_blob = opaque;
>> +
>> +    if (object_dynamic_cast(obj, TYPE_PCI_HOST_BRIDGE)) {
>> +        PCIBus *bus = PCI_HOST_BRIDGE(obj)->bus;
>> +
>> +        if (bus && !pci_bus_bypass_iommu(bus)) {
>> +            int min_bus, max_bus;
> extra line needed
Done, Thanks.
>> +            pci_bus_range(bus, &min_bus, &max_bus);
>> +
>> +            AcpiIortIdMapping idmap = {
>> +                .input_base = min_bus << 8,
>> +                .id_count = (max_bus - min_bus + 1) << 8,
>> +            };
>> +            g_array_append_val(idmap_blob, idmap);
>> +        }
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int iort_idmap_compare(gconstpointer a, gconstpointer b)
>> +{
>> +    AcpiIortIdMapping *idmap_a = (AcpiIortIdMapping *)a;
>> +    AcpiIortIdMapping *idmap_b = (AcpiIortIdMapping *)b;
>> +
>> +    return idmap_a->input_base - idmap_b->input_base;
>> +}
>> +
>>   static void
>>   build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>>   {
>> -    int nb_nodes, iort_start = table_data->len;
>> -    AcpiIortIdMapping *idmap;
>> +    int i, nb_nodes, rc_map_count, iort_start = table_data->len;
>> +    AcpiIortIdMapping *idmap, *range;
>>       AcpiIortItsGroup *its;
>>       AcpiIortTable *iort;
>>       AcpiIortSmmu3 *smmu;
>>       size_t node_size, iort_node_offset, iort_length, smmu_offset = 0;
>>       AcpiIortRC *rc;
>> +    GArray *smmu_idmap_ranges =
>> +        g_array_new(false, true, sizeof(AcpiIortIdMapping));
>> +    GArray *its_idmap_ranges =
>> +        g_array_new(false, true, sizeof(AcpiIortIdMapping));
>> +
>> +    object_child_foreach_recursive(object_get_root(),
>> +                                   iort_host_bridges, smmu_idmap_ranges);
>> +
>> +    g_array_sort(smmu_idmap_ranges, iort_idmap_compare);
>> +
>> +    AcpiIortIdMapping next_range = {
>> +        .input_base = 0,
>> +    };
>> +
>> +    /*
>> +     * Build the iort ID mapping to ITS directly,
>> +     * split the whole RID input range by RID mapping to SMMU node
>> +     */
>> +    for (i = 0; i < smmu_idmap_ranges->len; i++) {
>> +        idmap = &g_array_index(smmu_idmap_ranges, AcpiIortIdMapping, i);
>> +
>> +        if (next_range.input_base < idmap->input_base) {
>> +            next_range.id_count = idmap->input_base - next_range.input_base;
>> +            g_array_append_val(its_idmap_ranges, next_range);
>> +        }
>> +
>> +        next_range.input_base = idmap->input_base + idmap->id_count;
>> +    }
>> +
>> +    /* Append the last ITS ID mapping */
>> +    if (next_range.input_base < 0xFFFF) {
>> +        next_range.id_count = 0xFFFF - next_range.input_base;
>> +        g_array_append_val(its_idmap_ranges, next_range);
>> +    }
>>   
>>       iort = acpi_data_push(table_data, sizeof(*iort));
>>   
>> @@ -280,13 +347,13 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>>   
>>           /* SMMUv3 node */
>>           smmu_offset = iort_node_offset + node_size;
>> -        node_size = sizeof(*smmu) + sizeof(*idmap);
>> +        node_size = sizeof(*smmu) + sizeof(*idmap) * smmu_idmap_ranges->len;
>>           iort_length += node_size;
>>           smmu = acpi_data_push(table_data, node_size);
>>   
>>           smmu->type = ACPI_IORT_NODE_SMMU_V3;
>>           smmu->length = cpu_to_le16(node_size);
>> -        smmu->mapping_count = cpu_to_le32(1);
>> +        smmu->mapping_count = cpu_to_le32(smmu_idmap_ranges->len);
>>           smmu->mapping_offset = cpu_to_le32(sizeof(*smmu));
>>           smmu->base_address = cpu_to_le64(vms->memmap[VIRT_SMMU].base);
>>           smmu->flags = cpu_to_le32(ACPI_IORT_SMMU_V3_COHACC_OVERRIDE);
>> @@ -295,23 +362,32 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>>           smmu->sync_gsiv = cpu_to_le32(irq + 2);
>>           smmu->gerr_gsiv = cpu_to_le32(irq + 3);
>>   
>> -        /* Identity RID mapping covering the whole input RID range */
>> -        idmap = &smmu->id_mapping_array[0];
>> -        idmap->input_base = 0;
>> -        idmap->id_count = cpu_to_le32(0xFFFF);
>> -        idmap->output_base = 0;
>>           /* output IORT node is the ITS group node (the first node) */
>> -        idmap->output_reference = cpu_to_le32(iort_node_offset);
>> +        for (i = 0; i < smmu_idmap_ranges->len; i++) {
>> +            idmap = &smmu->id_mapping_array[i];
>> +            range = &g_array_index(smmu_idmap_ranges, AcpiIortIdMapping, i);
>> +
>> +            idmap->input_base = cpu_to_le32(range->input_base);
>> +            idmap->id_count = cpu_to_le32(range->id_count);
>> +            idmap->output_base = cpu_to_le32(range->input_base);
>> +            idmap->output_reference = cpu_to_le32(iort_node_offset);
>> +        }
> I don't really get this extra complexity. Can't the SMMU -> ITS mapping
> be a direct mapping covering the whole range of RIDs.
> Do you really need to match the input ID range? I don't think so.
> 
> Bypassed RIDs should only affect RC mappings to me.

Thanks, I will simplify this, only one idmap covering the whole RIDs is
needed here. I have retested and it worked.

>>       }
>>   
>>       /* Root Complex Node */
>> -    node_size = sizeof(*rc) + sizeof(*idmap);
>> +    if (vms->iommu == VIRT_IOMMU_SMMUV3) {
>> +        rc_map_count = smmu_idmap_ranges->len + its_idmap_ranges->len;
>> +    } else {
>> +        rc_map_count = 1;
>> +    }
>> +
>> +    node_size = sizeof(*rc) + sizeof(*idmap) * rc_map_count;
>>       iort_length += node_size;
>>       rc = acpi_data_push(table_data, node_size);
>>   
>>       rc->type = ACPI_IORT_NODE_PCI_ROOT_COMPLEX;
>>       rc->length = cpu_to_le16(node_size);
>> -    rc->mapping_count = cpu_to_le32(1);
>> +    rc->mapping_count = cpu_to_le32(rc_map_count);
>>       rc->mapping_offset = cpu_to_le32(sizeof(*rc));
>>   
>>       /* fully coherent device */
>> @@ -319,20 +395,41 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>>       rc->memory_properties.memory_flags = 0x3; /* CCA = CPM = DCAS = 1 */
>>       rc->pci_segment_number = 0; /* MCFG pci_segment */
>>   
>> -    /* Identity RID mapping covering the whole input RID range */
>> -    idmap = &rc->id_mapping_array[0];
>> -    idmap->input_base = 0;
>> -    idmap->id_count = cpu_to_le32(0xFFFF);
>> -    idmap->output_base = 0;
>> -
>>       if (vms->iommu == VIRT_IOMMU_SMMUV3) {
>>           /* output IORT node is the smmuv3 node */
> maybe add a comment saying translated RIDs connect to SMMU node
Done.
>> -        idmap->output_reference = cpu_to_le32(smmu_offset);
>> +        for (i = 0; i < smmu_idmap_ranges->len; i++) {
>> +            idmap = &rc->id_mapping_array[i];
>> +            range = &g_array_index(smmu_idmap_ranges, AcpiIortIdMapping, i);
>> +
>> +            idmap->input_base = cpu_to_le32(range->input_base);
>> +            idmap->id_count = cpu_to_le32(range->id_count);
>> +            idmap->output_base = cpu_to_le32(range->input_base);
>> +            idmap->output_reference = cpu_to_le32(smmu_offset);
>> +        }
>> +
> add comment saying bypassed RIDs connect to ITS directly?
Done.
>> +        /* output IORT node is the ITS group node (the first node) */
>> +        for (i = 0; i < its_idmap_ranges->len; i++) {
>> +            idmap = &rc->id_mapping_array[smmu_idmap_ranges->len + i];
>> +            range = &g_array_index(its_idmap_ranges, AcpiIortIdMapping, i);
>> +
>> +            idmap->input_base = cpu_to_le32(range->input_base);
>> +            idmap->id_count = cpu_to_le32(range->id_count);
>> +            idmap->output_base = cpu_to_le32(range->input_base);
>> +            idmap->output_reference = cpu_to_le32(iort_node_offset);
>> +        }
>>       } else {
>> +        /* Identity RID mapping covering the whole input RID range */
>> +        idmap = &rc->id_mapping_array[0];
>> +        idmap->input_base = cpu_to_le32(0);
>> +        idmap->id_count = cpu_to_le32(0xFFFF);
>> +        idmap->output_base = cpu_to_le32(0);
>>           /* output IORT node is the ITS group node (the first node) */
>>           idmap->output_reference = cpu_to_le32(iort_node_offset);
>>       }
>>   
>> +    g_array_free(smmu_idmap_ranges, true);
>> +    g_array_free(its_idmap_ranges, true);
>> +
>>       /*
>>        * Update the pointer address in case table_data->data moves during above
>>        * acpi_data_push operations.
> Thanks
> 
> Eric
> 
> .
> 

Thanks

Xingang

.