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
>