On 6/27/25 3:57 PM, Eric Auger wrote:
> Hi Gustavo,
>
> On 6/23/25 3:57 PM, Gustavo Romero wrote:
>> When building the Root Complex table, the comment about the code that
> s/table/node? or do you refer to the IORT table?
Reading the IORT spec again, both terminologies are used, ie. node / table
So please ignore my comment
Thanks
Eric
>> maps the RC node to SMMU node is misleading because it reads
>> "RC -> SMMUv3 -> ITS", but the code is only mapping the RCs IDs to the
>> SMMUv3 node. The step of mapping from the SMMUv3 IDs to the ITS Group
>> node is actually defined in another table (in the SMMUv3 node). So
>> change the comment to read "RC -> SMMUv3" instead.
>>
>> Signed-off-by Gustavo Romero <gustavo.rom...@linaro.org>
>> ---
>> hw/arm/virt-acpi-build.c | 25 +++++++++++++++++++------
>> 1 file changed, 19 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index 9eee284c80..e9cd3fb351 100644
>> --- a/hw/arm/virt-acpi-build.c
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -370,7 +370,7 @@ build_iort(GArray *table_data, BIOSLinker *linker,
>> VirtMachineState *vms)
>> /* 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) */
>> + /* Output IORT node is the ITS Group node (the first node) */
>> build_iort_id_mapping(table_data, 0, 0x10000, IORT_NODE_OFFSET);
>> }
>>
>> @@ -407,23 +407,36 @@ build_iort(GArray *table_data, BIOSLinker *linker,
>> VirtMachineState *vms)
>> if (vms->iommu == VIRT_IOMMU_SMMUV3) {
>> AcpiIortIdMapping *range;
>>
>> - /* translated RIDs connect to SMMUv3 node: RC -> SMMUv3 -> ITS */
>> + /*
>> + * Map RIDs (input) from RC to SMMUv3 nodes: RC -> SMMUv3.
>> + *
>> + * N.B.: The mapping from SMMUv3 to ITS Group node (SMMUv3 -> ITS)
>> is
>> + * defined in the SMMUv3 table, where all SMMUv3 IDs are mapped to
>> the
> s/table/node
>> + * ITS Group node.
>> + */
>> for (i = 0; i < smmu_idmaps->len; i++) {
>> range = &g_array_index(smmu_idmaps, AcpiIortIdMapping, i);
>> - /* output IORT node is the smmuv3 node */
>> + /* Output IORT node is the SMMUv3 node. */
>> build_iort_id_mapping(table_data, range->input_base,
>> range->id_count, smmu_offset);
>> }
>>
>> - /* bypassed RIDs connect to ITS group node directly: RC -> ITS */
>> + /*
>> + * Map bypassed (don't go throught the SMMU) RIDs (input) to ITS
>> Group
>> + * node directly: RC -> ITS.
>> + */
>> for (i = 0; i < its_idmaps->len; i++) {
>> range = &g_array_index(its_idmaps, AcpiIortIdMapping, i);
>> - /* output IORT node is the ITS group node (the first node) */
>> + /* Output IORT node is the ITS Group node (the first node). */
>> build_iort_id_mapping(table_data, range->input_base,
>> range->id_count, IORT_NODE_OFFSET);
>> }
>> } else {
>> - /* output IORT node is the ITS group node (the first node) */
>> + /*
>> + * Map all RIDs (input) to ITS Group node directly, since there is
>> no
>> + * SMMU: RC -> ITS.
>> + * Output IORT node is the ITS Group node (the first node).
>> + */
>> build_iort_id_mapping(table_data, 0, 0x10000, IORT_NODE_OFFSET);
>> }
>>
> Besides:
> Reviewed-by: Eric Auger <eric.au...@redhat.com>
> Eric
