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?