[PATCH 0/3] hw/arm/virt_acpi_build: Upgrate the IORT table up to revision E.b

[PATCH 0/3] hw/arm/virt_acpi_build: Upgrate the IORT table up to revision E.b

[Eric Auger]

This series upgrades the ACPI IORT table up to the E.b
specification revision. One of the goal of this upgrade
is to allow the addition of RMR nodes along with the SMMUv3.

It applies on top of Igor's
[PATCH v4 00/35] acpi: refactor error prone build_header() and
packed structures usage in ACPI tables

The latest IORT specification (ARM DEN 0049E.b) can be found at
IO Remapping Table - Platform Design Document
https://developer.arm.com/documentation/den0049/latest/

This series and its dependency can be found at
https://github.com/eauger/qemu.git
branch: igor_acpi_refactoring_v4_dbg2_v3_rmr_v1

Eric Auger (3):
  tests/acpi: Get prepared for IORT E.b revision upgrade
  hw/arm/virt-acpi-build: IORT upgrade up to revision E.b
  tests/acpi: Generate reference blob for IORT rev E.b

 hw/arm/virt-acpi-build.c          |  47 ++++++++++++++++++------------
 tests/data/acpi/virt/IORT         | Bin 124 -> 128 bytes
 tests/data/acpi/virt/IORT.memhp   | Bin 124 -> 128 bytes
 tests/data/acpi/virt/IORT.numamem | Bin 124 -> 128 bytes
 tests/data/acpi/virt/IORT.pxb     | Bin 124 -> 128 bytes
 5 files changed, 28 insertions(+), 19 deletions(-)

-- 
2.26.3

[PATCH 1/3] tests/acpi: Get prepared for IORT E.b revision upgrade

[Eric Auger]

Ignore IORT till reference blob for E.b spec revision gets
added.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 tests/qtest/bios-tables-test-allowed-diff.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..9a5a923d6b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,2 @@
 /* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/virt/IORT",
-- 
2.26.3

[PATCH 2/3] hw/arm/virt-acpi-build: IORT upgrade up to revision E.b

[Eric Auger]

Upgrade the IORT table from B to E.b specification
revision (ARM DEN 0049E.b).

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 hw/arm/virt-acpi-build.c | 47 ++++++++++++++++++++++++----------------
 1 file changed, 28 insertions(+), 19 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 257d0fee17..ce9311ac19 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -241,19 +241,20 @@ static void acpi_dsdt_add_tpm(Aml *scope, VirtMachineState *vms)
 #endif
 
 #define ID_MAPPING_ENTRY_SIZE 20
-#define SMMU_V3_ENTRY_SIZE 60
-#define ROOT_COMPLEX_ENTRY_SIZE 32
+#define SMMU_V3_ENTRY_SIZE 68
+#define ROOT_COMPLEX_ENTRY_SIZE 36
 #define IORT_NODE_OFFSET 48
 
 static void build_iort_id_mapping(GArray *table_data, uint32_t input_base,
                                   uint32_t id_count, uint32_t out_ref)
 {
-    /* Identity RID mapping covering the whole input RID range */
+    /* Table 4 ID mapping format */
     build_append_int_noprefix(table_data, input_base, 4); /* Input base */
     build_append_int_noprefix(table_data, id_count, 4); /* Number of IDs */
     build_append_int_noprefix(table_data, input_base, 4); /* Output base */
     build_append_int_noprefix(table_data, out_ref, 4); /* Output Reference */
-    build_append_int_noprefix(table_data, 0, 4); /* Flags */
+    /* Flags */
+    build_append_int_noprefix(table_data, 0 /* Single mapping */, 4);
 }
 
 struct AcpiIortIdMapping {
@@ -298,7 +299,7 @@ static int iort_idmap_compare(gconstpointer a, gconstpointer b)
 /*
  * Input Output Remapping Table (IORT)
  * Conforms to "IO Remapping Table System Software on ARM Platforms",
- * Document number: ARM DEN 0049B, October 2015
+ * Document number: ARM DEN 0049E, Feb 2021
  */
 static void
 build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
@@ -310,7 +311,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
     GArray *smmu_idmaps = g_array_new(false, true, sizeof(AcpiIortIdMapping));
     GArray *its_idmaps = g_array_new(false, true, sizeof(AcpiIortIdMapping));
 
-    AcpiTable table = { .sig = "IORT", .rev = 0, .oem_id = vms->oem_id,
+    AcpiTable table = { .sig = "IORT", .rev = 3, .oem_id = vms->oem_id,
                         .oem_table_id = vms->oem_table_id };
     /* Table 2 The IORT */
     acpi_table_begin(&table, table_data);
@@ -358,12 +359,12 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
     build_append_int_noprefix(table_data, IORT_NODE_OFFSET, 4);
     build_append_int_noprefix(table_data, 0, 4); /* Reserved */
 
-    /* 3.1.1.3 ITS group node */
+    /* Table 12 ITS Group Format */
     build_append_int_noprefix(table_data, 0 /* ITS Group */, 1); /* Type */
     node_size =  20 /* fixed header size */ + 4 /* 1 GIC ITS Identifier */;
     build_append_int_noprefix(table_data, node_size, 2); /* Length */
-    build_append_int_noprefix(table_data, 0, 1); /* Revision */
-    build_append_int_noprefix(table_data, 0, 4); /* Reserved */
+    build_append_int_noprefix(table_data, 3, 1); /* Revision */
+    build_append_int_noprefix(table_data, 0, 4); /* Identifier */
     build_append_int_noprefix(table_data, 0, 4); /* Number of ID mappings */
     build_append_int_noprefix(table_data, 0, 4); /* Reference to ID Array */
     build_append_int_noprefix(table_data, 1, 4); /* Number of ITSs */
@@ -374,19 +375,19 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
         int irq =  vms->irqmap[VIRT_SMMU] + ARM_SPI_BASE;
 
         smmu_offset = table_data->len - table.table_offset;
-        /* 3.1.1.2 SMMUv3 */
+        /* Table 9 SMMUv3 Format */
         build_append_int_noprefix(table_data, 4 /* SMMUv3 */, 1); /* Type */
         node_size =  SMMU_V3_ENTRY_SIZE + ID_MAPPING_ENTRY_SIZE;
         build_append_int_noprefix(table_data, node_size, 2); /* Length */
-        build_append_int_noprefix(table_data, 0, 1); /* Revision */
-        build_append_int_noprefix(table_data, 0, 4); /* Reserved */
+        build_append_int_noprefix(table_data, 3, 1); /* Revision */
+        build_append_int_noprefix(table_data, 1, 4); /* Identifier */
         build_append_int_noprefix(table_data, 1, 4); /* Number of ID mappings */
         /* Reference to ID Array */
         build_append_int_noprefix(table_data, SMMU_V3_ENTRY_SIZE, 4);
         /* Base address */
         build_append_int_noprefix(table_data, vms->memmap[VIRT_SMMU].base, 8);
         /* Flags */
-        build_append_int_noprefix(table_data, 1 /* COHACC OverrideNote */, 4);
+        build_append_int_noprefix(table_data, 1 /* COHACC Override */, 4);
         build_append_int_noprefix(table_data, 0, 4); /* Reserved */
         build_append_int_noprefix(table_data, 0, 8); /* VATOS address */
         /* Model */
@@ -395,35 +396,43 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
         build_append_int_noprefix(table_data, irq + 1, 4); /* PRI */
         build_append_int_noprefix(table_data, irq + 3, 4); /* GERR */
         build_append_int_noprefix(table_data, irq + 2, 4); /* Sync */
+        build_append_int_noprefix(table_data, 0, 4); /* Proximity domain */
+        /* DeviceID mapping index (ignored since interrupts are GSIV based) */
+        build_append_int_noprefix(table_data, 0, 4);
 
         /* output IORT node is the ITS group node (the first node) */
         build_iort_id_mapping(table_data, 0, 0xFFFF, IORT_NODE_OFFSET);
     }
 
-    /* Table 16 Root Complex Node */
+    /* Table 17 Root Complex Node */
     build_append_int_noprefix(table_data, 2 /* Root complex */, 1); /* Type */
     node_size =  ROOT_COMPLEX_ENTRY_SIZE +
                  ID_MAPPING_ENTRY_SIZE * rc_mapping_count;
     build_append_int_noprefix(table_data, node_size, 2); /* Length */
-    build_append_int_noprefix(table_data, 0, 1); /* Revision */
-    build_append_int_noprefix(table_data, 0, 4); /* Reserved */
+    build_append_int_noprefix(table_data, 3, 1); /* Revision */
+    build_append_int_noprefix(table_data, 2, 4); /* Identifier */
     /* Number of ID mappings */
     build_append_int_noprefix(table_data, rc_mapping_count, 4);
     /* Reference to ID Array */
     build_append_int_noprefix(table_data, ROOT_COMPLEX_ENTRY_SIZE, 4);
 
-    /* Table 13 Memory access properties */
+    /* Table 14 Memory access properties */
     /* CCA: Cache Coherent Attribute */
     build_append_int_noprefix(table_data, 1 /* fully coherent */, 4);
     build_append_int_noprefix(table_data, 0, 1); /* AH: Note Allocation Hints */
     build_append_int_noprefix(table_data, 0, 2); /* Reserved */
-    /* MAF: Note Memory Access Flags */
-    build_append_int_noprefix(table_data, 0x3 /* CCA = CPM = DCAS = 1 */, 1);
+    /* Table 15 Memory Access Flags */
+    build_append_int_noprefix(table_data, 0x3 /* CCA = CPM = DACS = 1 */, 1);
 
     build_append_int_noprefix(table_data, 0, 4); /* ATS Attribute */
     /* MCFG pci_segment */
     build_append_int_noprefix(table_data, 0, 4); /* PCI Segment number */
 
+    /* Memory address size limit */
+    build_append_int_noprefix(table_data, 64, 1);
+
+    build_append_int_noprefix(table_data, 0, 3); /* Reserved */
+
     /* Output Reference */
     if (vms->iommu == VIRT_IOMMU_SMMUV3) {
         AcpiIortIdMapping *range;
-- 
2.26.3

[PATCH 3/3] tests/acpi: Generate reference blob for IORT rev E.b

[Eric Auger]

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 tests/qtest/bios-tables-test-allowed-diff.h |   1 -
 tests/data/acpi/virt/IORT                   | Bin 124 -> 128 bytes
 tests/data/acpi/virt/IORT.memhp             | Bin 124 -> 128 bytes
 tests/data/acpi/virt/IORT.numamem           | Bin 124 -> 128 bytes
 tests/data/acpi/virt/IORT.pxb               | Bin 124 -> 128 bytes
 5 files changed, 1 deletion(-)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index 9a5a923d6b..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,2 +1 @@
 /* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/virt/IORT",
diff --git a/tests/data/acpi/virt/IORT b/tests/data/acpi/virt/IORT
index 521acefe9ba66706c5607321a82d330586f3f280..7ab4fd8107628902a8f05746fb954a8d6af00431 100644
GIT binary patch
literal 128
zcmebD4+?2uU|?Y0;N<V@5v<@85#X!<1dKp25F11@0kHuPgMkDCGmHtMLGmELWWm4;
YQU~IzKxrTaSL*=efWZI%5E^D40Ar&H0RR91

literal 124
zcmebD4+^Pa00MR=e`k+i1*eDrX9XZ&1PX!JAesq?4S*O7Bw!2(4Uz`|CKCt^;wu0#
QRGb+i3L*dhhtM#y0PN=p0RR91

diff --git a/tests/data/acpi/virt/IORT.memhp b/tests/data/acpi/virt/IORT.memhp
index 521acefe9ba66706c5607321a82d330586f3f280..7ab4fd8107628902a8f05746fb954a8d6af00431 100644
GIT binary patch
literal 128
zcmebD4+?2uU|?Y0;N<V@5v<@85#X!<1dKp25F11@0kHuPgMkDCGmHtMLGmELWWm4;
YQU~IzKxrTaSL*=efWZI%5E^D40Ar&H0RR91

literal 124
zcmebD4+^Pa00MR=e`k+i1*eDrX9XZ&1PX!JAesq?4S*O7Bw!2(4Uz`|CKCt^;wu0#
QRGb+i3L*dhhtM#y0PN=p0RR91

diff --git a/tests/data/acpi/virt/IORT.numamem b/tests/data/acpi/virt/IORT.numamem
index 521acefe9ba66706c5607321a82d330586f3f280..7ab4fd8107628902a8f05746fb954a8d6af00431 100644
GIT binary patch
literal 128
zcmebD4+?2uU|?Y0;N<V@5v<@85#X!<1dKp25F11@0kHuPgMkDCGmHtMLGmELWWm4;
YQU~IzKxrTaSL*=efWZI%5E^D40Ar&H0RR91

literal 124
zcmebD4+^Pa00MR=e`k+i1*eDrX9XZ&1PX!JAesq?4S*O7Bw!2(4Uz`|CKCt^;wu0#
QRGb+i3L*dhhtM#y0PN=p0RR91

diff --git a/tests/data/acpi/virt/IORT.pxb b/tests/data/acpi/virt/IORT.pxb
index 521acefe9ba66706c5607321a82d330586f3f280..7ab4fd8107628902a8f05746fb954a8d6af00431 100644
GIT binary patch
literal 128
zcmebD4+?2uU|?Y0;N<V@5v<@85#X!<1dKp25F11@0kHuPgMkDCGmHtMLGmELWWm4;
YQU~IzKxrTaSL*=efWZI%5E^D40Ar&H0RR91

literal 124
zcmebD4+^Pa00MR=e`k+i1*eDrX9XZ&1PX!JAesq?4S*O7Bw!2(4Uz`|CKCt^;wu0#
QRGb+i3L*dhhtM#y0PN=p0RR91

-- 
2.26.3

Re: [PATCH 2/3] hw/arm/virt-acpi-build: IORT upgrade up to revision E.b

[Philippe Mathieu-Daudé]

Hi Eric,

On 9/28/21 19:21, Eric Auger wrote:
> Upgrade the IORT table from B to E.b specification
> revision (ARM DEN 0049E.b).
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>  hw/arm/virt-acpi-build.c | 47 ++++++++++++++++++++++++----------------
>  1 file changed, 28 insertions(+), 19 deletions(-)

> -    /* 3.1.1.3 ITS group node */
> +    /* Table 12 ITS Group Format */
>      build_append_int_noprefix(table_data, 0 /* ITS Group */, 1); /* Type */
>      node_size =  20 /* fixed header size */ + 4 /* 1 GIC ITS Identifier */;
>      build_append_int_noprefix(table_data, node_size, 2); /* Length */
> -    build_append_int_noprefix(table_data, 0, 1); /* Revision */
> -    build_append_int_noprefix(table_data, 0, 4); /* Reserved */
> +    build_append_int_noprefix(table_data, 3, 1); /* Revision */

Why 3? Shouldn't it be 1?

> +    build_append_int_noprefix(table_data, 0, 4); /* Identifier */
>      build_append_int_noprefix(table_data, 0, 4); /* Number of ID mappings */
>      build_append_int_noprefix(table_data, 0, 4); /* Reference to ID Array */
>      build_append_int_noprefix(table_data, 1, 4); /* Number of ITSs */
> @@ -374,19 +375,19 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>          int irq =  vms->irqmap[VIRT_SMMU] + ARM_SPI_BASE;
>  
>          smmu_offset = table_data->len - table.table_offset;
> -        /* 3.1.1.2 SMMUv3 */
> +        /* Table 9 SMMUv3 Format */
>          build_append_int_noprefix(table_data, 4 /* SMMUv3 */, 1); /* Type */
>          node_size =  SMMU_V3_ENTRY_SIZE + ID_MAPPING_ENTRY_SIZE;
>          build_append_int_noprefix(table_data, node_size, 2); /* Length */
> -        build_append_int_noprefix(table_data, 0, 1); /* Revision */
> -        build_append_int_noprefix(table_data, 0, 4); /* Reserved */
> +        build_append_int_noprefix(table_data, 3, 1); /* Revision */

And here 3 -> 4?

> +        build_append_int_noprefix(table_data, 1, 4); /* Identifier */
>          build_append_int_noprefix(table_data, 1, 4); /* Number of ID mappings */
>          /* Reference to ID Array */
>          build_append_int_noprefix(table_data, SMMU_V3_ENTRY_SIZE, 4);
>          /* Base address */
>          build_append_int_noprefix(table_data, vms->memmap[VIRT_SMMU].base, 8);
>          /* Flags */
> -        build_append_int_noprefix(table_data, 1 /* COHACC OverrideNote */, 4);
> +        build_append_int_noprefix(table_data, 1 /* COHACC Override */, 4);
>          build_append_int_noprefix(table_data, 0, 4); /* Reserved */
>          build_append_int_noprefix(table_data, 0, 8); /* VATOS address */
>          /* Model */
> @@ -395,35 +396,43 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>          build_append_int_noprefix(table_data, irq + 1, 4); /* PRI */
>          build_append_int_noprefix(table_data, irq + 3, 4); /* GERR */
>          build_append_int_noprefix(table_data, irq + 2, 4); /* Sync */
> +        build_append_int_noprefix(table_data, 0, 4); /* Proximity domain */
> +        /* DeviceID mapping index (ignored since interrupts are GSIV based) */
> +        build_append_int_noprefix(table_data, 0, 4);
>  
>          /* output IORT node is the ITS group node (the first node) */
>          build_iort_id_mapping(table_data, 0, 0xFFFF, IORT_NODE_OFFSET);
>      }

Also, could the node identifier be a variable we increment?

Re: [PATCH 2/3] hw/arm/virt-acpi-build: IORT upgrade up to revision E.b

[Eric Auger]

Hi Philippe,

On 9/28/21 7:56 PM, Philippe Mathieu-Daudé wrote:
> Hi Eric,
>
> On 9/28/21 19:21, Eric Auger wrote:
>> Upgrade the IORT table from B to E.b specification
>> revision (ARM DEN 0049E.b).
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> ---
>>  hw/arm/virt-acpi-build.c | 47 ++++++++++++++++++++++++----------------
>>  1 file changed, 28 insertions(+), 19 deletions(-)
>> -    /* 3.1.1.3 ITS group node */
>> +    /* Table 12 ITS Group Format */
>>      build_append_int_noprefix(table_data, 0 /* ITS Group */, 1); /* Type */
>>      node_size =  20 /* fixed header size */ + 4 /* 1 GIC ITS Identifier */;
>>      build_append_int_noprefix(table_data, node_size, 2); /* Length */
>> -    build_append_int_noprefix(table_data, 0, 1); /* Revision */
>> -    build_append_int_noprefix(table_data, 0, 4); /* Reserved */
>> +    build_append_int_noprefix(table_data, 3, 1); /* Revision */
> Why 3? Shouldn't it be 1?
you're right, read the SMMUv1 rev and saw it was 3. Thought it was
aligned with the IORT global rev. Will fix here and below
>
>> +    build_append_int_noprefix(table_data, 0, 4); /* Identifier */
>>      build_append_int_noprefix(table_data, 0, 4); /* Number of ID mappings */
>>      build_append_int_noprefix(table_data, 0, 4); /* Reference to ID Array */
>>      build_append_int_noprefix(table_data, 1, 4); /* Number of ITSs */
>> @@ -374,19 +375,19 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>>          int irq =  vms->irqmap[VIRT_SMMU] + ARM_SPI_BASE;
>>  
>>          smmu_offset = table_data->len - table.table_offset;
>> -        /* 3.1.1.2 SMMUv3 */
>> +        /* Table 9 SMMUv3 Format */
>>          build_append_int_noprefix(table_data, 4 /* SMMUv3 */, 1); /* Type */
>>          node_size =  SMMU_V3_ENTRY_SIZE + ID_MAPPING_ENTRY_SIZE;
>>          build_append_int_noprefix(table_data, node_size, 2); /* Length */
>> -        build_append_int_noprefix(table_data, 0, 1); /* Revision */
>> -        build_append_int_noprefix(table_data, 0, 4); /* Reserved */
>> +        build_append_int_noprefix(table_data, 3, 1); /* Revision */
> And here 3 -> 4?
>
>> +        build_append_int_noprefix(table_data, 1, 4); /* Identifier */
>>          build_append_int_noprefix(table_data, 1, 4); /* Number of ID mappings */
>>          /* Reference to ID Array */
>>          build_append_int_noprefix(table_data, SMMU_V3_ENTRY_SIZE, 4);
>>          /* Base address */
>>          build_append_int_noprefix(table_data, vms->memmap[VIRT_SMMU].base, 8);
>>          /* Flags */
>> -        build_append_int_noprefix(table_data, 1 /* COHACC OverrideNote */, 4);
>> +        build_append_int_noprefix(table_data, 1 /* COHACC Override */, 4);
>>          build_append_int_noprefix(table_data, 0, 4); /* Reserved */
>>          build_append_int_noprefix(table_data, 0, 8); /* VATOS address */
>>          /* Model */
>> @@ -395,35 +396,43 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>>          build_append_int_noprefix(table_data, irq + 1, 4); /* PRI */
>>          build_append_int_noprefix(table_data, irq + 3, 4); /* GERR */
>>          build_append_int_noprefix(table_data, irq + 2, 4); /* Sync */
>> +        build_append_int_noprefix(table_data, 0, 4); /* Proximity domain */
>> +        /* DeviceID mapping index (ignored since interrupts are GSIV based) */
>> +        build_append_int_noprefix(table_data, 0, 4);
>>  
>>          /* output IORT node is the ITS group node (the first node) */
>>          build_iort_id_mapping(table_data, 0, 0xFFFF, IORT_NODE_OFFSET);
>>      }
> Also, could the node identifier be a variable we increment?
Yes I will do that.

Thanks!

Eric
>