[PATCH 1/5] acpi_table: Fix IORT RC node

[PATCH 1/5] acpi_table: Fix IORT RC node

[Patrick Rudolph]

Even though the RC node has the correct size and the ID mappings
are written to the end of the node, the ID 'mapping offset' and
'mapping count' are not written in the IORT RC node header, thus it
looks like that the RC node has no ID mappings.
The Linux kernel doesn't complain about the invalid IORT RC node,
even though the spec says that each RC node must have an ID mapping.
The kernel will fail to use MSI IRQs and fall back to a legacy IRQ
mechanism that's not working either.
Finally it will show strange behaviour around PCI interrupts, making it
hard to trace back to an invalid IORT RC nodes.

Add the missing ID mapping count and mapping offset.

TEST: Fixes IRQ usage of PCI devices on qemu/sbsa-ref.
Fixes: bf5d37662da5 "acpi: acpi_table: Add IORT support"

Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
---
 lib/acpi/acpi_table.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/acpi/acpi_table.c b/lib/acpi/acpi_table.c
index c0ed24984af..0e0a7cc498f 100644
--- a/lib/acpi/acpi_table.c
+++ b/lib/acpi/acpi_table.c
@@ -646,6 +646,8 @@ int acpi_iort_add_rc(struct acpi_ctx *ctx,
 
 	node->type = ACPI_IORT_NODE_PCI_ROOT_COMPLEX;
 	node->revision = 2;
+	node->mapping_count = num_mappings;
+	node->mapping_offset = sizeof(struct acpi_iort_node) + sizeof(struct acpi_iort_rc);
 
 	node->length = sizeof(struct acpi_iort_node);
 	node->length += sizeof(struct acpi_iort_rc);
-- 
2.48.1

[PATCH 2/5] acpi_table: Add asserts in IORT

[Patrick Rudolph]

Check that the provided offsets are really pointing to a node
that have been previously written and are of the correct type.

Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
---
 lib/acpi/acpi_table.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/lib/acpi/acpi_table.c b/lib/acpi/acpi_table.c
index 0e0a7cc498f..97cd8e8ddb0 100644
--- a/lib/acpi/acpi_table.c
+++ b/lib/acpi/acpi_table.c
@@ -635,6 +635,7 @@ int acpi_iort_add_rc(struct acpi_ctx *ctx,
 		     const struct acpi_iort_id_mapping *map)
 {
 	struct acpi_iort_id_mapping *mapping;
+	struct acpi_iort_node *output_node;
 	struct acpi_iort_node *node;
 	struct acpi_iort_rc *rc;
 	int offset;
@@ -661,6 +662,13 @@ int acpi_iort_add_rc(struct acpi_ctx *ctx,
 
 	mapping = (struct acpi_iort_id_mapping *)(rc + 1);
 	for (int i = 0; i < num_mappings; i++) {
+		/* Validate input */
+		output_node = (struct acpi_iort_node *)ctx->tab_start + map[i].output_reference;
+		/* ID mappings can use SMMUs or ITS groups as output references */
+		assert(output_node && ((output_node->type == ACPI_IORT_NODE_ITS_GROUP) ||
+				       (output_node->type == ACPI_IORT_NODE_SMMU) ||
+				       (output_node->type == ACPI_IORT_NODE_SMMU_V3)));
+
 		memcpy(mapping, &map[i], sizeof(struct acpi_iort_id_mapping));
 		mapping++;
 	}
@@ -685,6 +693,7 @@ int acpi_iort_add_smmu_v3(struct acpi_ctx *ctx,
 			  const struct acpi_iort_id_mapping *map)
 {
 	struct acpi_iort_node *node;
+	struct acpi_iort_node *output_node;
 	struct acpi_iort_smmu_v3 *smmu;
 	struct acpi_iort_id_mapping *mapping;
 	int offset;
@@ -718,6 +727,14 @@ int acpi_iort_add_smmu_v3(struct acpi_ctx *ctx,
 
 	mapping = (struct acpi_iort_id_mapping *)(smmu + 1);
 	for (int i = 0; i < num_mappings; i++) {
+		/* Validate input */
+		output_node = (struct acpi_iort_node *)ctx->tab_start + map[i].output_reference;
+		/*
+		 * ID mappings of an SMMUv3 node can only have ITS group nodes
+		 * as output references.
+		 */
+		assert(output_node && output_node->type == ACPI_IORT_NODE_ITS_GROUP);
+
 		memcpy(mapping, &map[i], sizeof(struct acpi_iort_id_mapping));
 		mapping++;
 	}
-- 
2.48.1

[PATCH 3/5] acpi: Clear reserved bits in IORT

[Patrick Rudolph]

The IORT spec says that reserved bits must be set to zero, thus clear
all fields of the struct before starting to fill out non-reserved
fields.

Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
---
 lib/acpi/acpi_table.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/acpi/acpi_table.c b/lib/acpi/acpi_table.c
index 97cd8e8ddb0..fc43862ec37 100644
--- a/lib/acpi/acpi_table.c
+++ b/lib/acpi/acpi_table.c
@@ -615,6 +615,7 @@ int acpi_iort_add_named_component(struct acpi_ctx *ctx,
 	node->length += strlen(device_name) + 1;
 
 	comp = (struct acpi_iort_named_component *)node->node_data;
+	memset(comp, '\0', sizeof(struct acpi_iort_named_component));
 
 	comp->node_flags = node_flags;
 	comp->memory_properties = memory_properties;
@@ -655,6 +656,8 @@ int acpi_iort_add_rc(struct acpi_ctx *ctx,
 	node->length += sizeof(struct acpi_iort_id_mapping) * num_mappings;
 
 	rc = (struct acpi_iort_rc *)node->node_data;
+	memset(rc, '\0', sizeof(struct acpi_iort_rc));
+
 	rc->mem_access_properties = mem_access_properties;
 	rc->ats_attributes = ats_attributes;
 	rc->pci_segment_number = pci_segment_number;
@@ -713,6 +716,7 @@ int acpi_iort_add_smmu_v3(struct acpi_ctx *ctx,
 	node->length += sizeof(struct acpi_iort_id_mapping) * num_mappings;
 
 	smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
+	memset(smmu, '\0', sizeof(struct acpi_iort_smmu_v3));
 
 	smmu->base_address = base_address;
 	smmu->flags = flags;
-- 
2.48.1

[PATCH 4/5] acpi: Conditionally set mapping_offset in IORT

[Patrick Rudolph]

The spec recommends to set the mapping_offset only when there are
ID mappings as indicated by the mapping_count field.

Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
---
 lib/acpi/acpi_table.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/lib/acpi/acpi_table.c b/lib/acpi/acpi_table.c
index fc43862ec37..4ad1f56e961 100644
--- a/lib/acpi/acpi_table.c
+++ b/lib/acpi/acpi_table.c
@@ -649,7 +649,9 @@ int acpi_iort_add_rc(struct acpi_ctx *ctx,
 	node->type = ACPI_IORT_NODE_PCI_ROOT_COMPLEX;
 	node->revision = 2;
 	node->mapping_count = num_mappings;
-	node->mapping_offset = sizeof(struct acpi_iort_node) + sizeof(struct acpi_iort_rc);
+	if (num_mappings)
+		node->mapping_offset = sizeof(struct acpi_iort_node) +
+				       sizeof(struct acpi_iort_rc);
 
 	node->length = sizeof(struct acpi_iort_node);
 	node->length += sizeof(struct acpi_iort_rc);
@@ -709,7 +711,9 @@ int acpi_iort_add_smmu_v3(struct acpi_ctx *ctx,
 	node->type = ACPI_IORT_NODE_SMMU_V3;
 	node->revision = 5;
 	node->mapping_count = num_mappings;
-	node->mapping_offset = sizeof(struct acpi_iort_node) + sizeof(struct acpi_iort_smmu_v3);
+	if (num_mappings)
+		node->mapping_offset = sizeof(struct acpi_iort_node) +
+				       sizeof(struct acpi_iort_smmu_v3);
 
 	node->length = sizeof(struct acpi_iort_node);
 	node->length += sizeof(struct acpi_iort_smmu_v3);
-- 
2.48.1

[PATCH 5/5] test: acpi: Add IORT tests

[Patrick Rudolph]

Add tests for IORT table generation:
- SMMU_V3 node
- RC node

Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
---
 test/dm/acpigen.c | 115 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 115 insertions(+)

diff --git a/test/dm/acpigen.c b/test/dm/acpigen.c
index 23c16bd9866..ee9517f9c29 100644
--- a/test/dm/acpigen.c
+++ b/test/dm/acpigen.c
@@ -1742,3 +1742,118 @@ static int dm_test_acpi_write_tsd_package(struct unit_test_state *uts)
 	return 0;
 }
 DM_TEST(dm_test_acpi_write_tsd_package, 0);
+
+static int dm_test_acpi_iort_smmu_v3(struct unit_test_state *uts)
+{
+	struct acpi_ctx *ctx;
+	int smmu_offset;
+	u8 *ptr;
+
+	ut_assertok(alloc_context(&ctx));
+	ctx->tab_start = ctx->current;
+	acpi_inc(ctx, sizeof(struct acpi_table_iort));
+
+	ptr = acpigen_get_current(ctx);
+
+	smmu_offset = acpi_iort_add_smmu_v3(ctx,
+					    0xaabbccddeeffULL, // Base address
+					    1, // Flags
+					    0xffeeddccaabbULL,  // VATOS address
+					    0,  // SMMUv3 Model
+					    3, // Event
+					    4, // Pri
+					    5, // Gerror
+					    6, // Sync
+					    7,  // Proximity domain
+					    8,  // DevIDMappingIndex
+					    0,
+					    NULL);
+	ut_assert(smmu_offset);
+
+	ut_asserteq(ACPI_IORT_NODE_SMMU_V3, ptr[0]);
+	ut_asserteq(68, get_unaligned((u16 *)(ptr + 1)));
+	ut_asserteq(0, get_unaligned((u16 *)(ptr + 4)));
+	ut_asserteq(0, get_unaligned((u32 *)(ptr + 8)));
+	ut_asserteq(0, get_unaligned((u32 *)(ptr + 12)));
+
+	ut_asserteq_64(0xaabbccddeeffULL, get_unaligned((u64 *)(ptr + 16)));
+	ut_asserteq(1, get_unaligned((u32 *)(ptr + 24)));
+	ut_asserteq(0, get_unaligned((u32 *)(ptr + 28)));
+	ut_asserteq_64(0xffeeddccaabbULL, get_unaligned((u64 *)(ptr + 32)));
+	ut_asserteq(0, get_unaligned((u32 *)(ptr + 40)));
+	ut_asserteq(3, get_unaligned((u32 *)(ptr + 44)));
+	ut_asserteq(4, get_unaligned((u32 *)(ptr + 48)));
+	ut_asserteq(5, get_unaligned((u32 *)(ptr + 52)));
+	ut_asserteq(6, get_unaligned((u32 *)(ptr + 56)));
+	ut_asserteq(7, get_unaligned((u32 *)(ptr + 60)));
+	ut_asserteq(8, get_unaligned((u32 *)(ptr + 64)));
+
+	free_context(&ctx);
+
+	return 0;
+}
+DM_TEST(dm_test_acpi_iort_smmu_v3, 0);
+
+static int dm_test_acpi_iort_rc(struct unit_test_state *uts)
+{
+	struct acpi_ctx *ctx;
+	int its_group_offset, offset;
+	u8 *ptr;
+
+	ut_assertok(alloc_context(&ctx));
+	ctx->tab_start = ctx->current;
+	acpi_inc(ctx, sizeof(struct acpi_table_iort));
+
+	u32 identifiers[] = { 0 };
+
+	its_group_offset = acpi_iort_add_its_group(ctx, ARRAY_SIZE(identifiers),
+						   identifiers);
+
+	ptr = acpigen_get_current(ctx);
+
+	struct acpi_iort_id_mapping map_rc[] = {
+		{0, 0xfff, 0, its_group_offset, 0},
+		{0x1000, 0xffff, 0x1000, its_group_offset, 0}
+	};
+
+	offset = acpi_iort_add_rc(ctx,
+				  0xaabbccddeeffULL, // Mem Access Properties
+				  2, // ATS attributes
+				  3, // PCI segment
+				  4, // Memory address size limit
+				  ARRAY_SIZE(map_rc),
+				  map_rc);
+
+	ut_assert(offset);
+	ut_asserteq(ACPI_IORT_NODE_PCI_ROOT_COMPLEX, ptr[0]);
+	ut_asserteq(36 + ARRAY_SIZE(map_rc) * sizeof(struct acpi_iort_id_mapping),
+		    get_unaligned((u16 *)(ptr + 1)));
+	ut_asserteq(0, get_unaligned((u16 *)(ptr + 4)));
+	ut_asserteq(2, get_unaligned((u32 *)(ptr + 8)));
+	ut_asserteq(36, get_unaligned((u32 *)(ptr + 12)));
+
+	ut_asserteq_64(0xaabbccddeeffULL, get_unaligned((u64 *)(ptr + 16)));
+	ut_asserteq(2, get_unaligned((u32 *)(ptr + 24)));
+	ut_asserteq(3, get_unaligned((u32 *)(ptr + 28)));
+	ut_asserteq(4, ptr[32]);
+	ut_asserteq(0, ptr[33]);
+	ut_asserteq(0, ptr[34]);
+	ut_asserteq(0, ptr[35]);
+
+	ut_asserteq(0, get_unaligned((u32 *)(ptr + 36)));
+	ut_asserteq(0xfff, get_unaligned((u32 *)(ptr + 40)));
+	ut_asserteq(0, get_unaligned((u32 *)(ptr + 44)));
+	ut_asserteq(its_group_offset, get_unaligned((u32 *)(ptr + 48)));
+	ut_asserteq(0, get_unaligned((u32 *)(ptr + 52)));
+
+	ut_asserteq(0x1000, get_unaligned((u32 *)(ptr + 56)));
+	ut_asserteq(0xffff, get_unaligned((u32 *)(ptr + 60)));
+	ut_asserteq(0x1000, get_unaligned((u32 *)(ptr + 64)));
+	ut_asserteq(its_group_offset, get_unaligned((u32 *)(ptr + 68)));
+	ut_asserteq(0, get_unaligned((u32 *)(ptr + 72)));
+
+	free_context(&ctx);
+
+	return 0;
+}
+DM_TEST(dm_test_acpi_iort_rc, 0);
\ No newline at end of file
-- 
2.48.1

Re: [PATCH 1/5] acpi_table: Fix IORT RC node

[Tom Rini]

On Sun, 16 Mar 2025 09:32:52 +0100, Patrick Rudolph wrote:

> Even though the RC node has the correct size and the ID mappings
> are written to the end of the node, the ID 'mapping offset' and
> 'mapping count' are not written in the IORT RC node header, thus it
> looks like that the RC node has no ID mappings.
> The Linux kernel doesn't complain about the invalid IORT RC node,
> even though the spec says that each RC node must have an ID mapping.
> The kernel will fail to use MSI IRQs and fall back to a legacy IRQ
> mechanism that's not working either.
> Finally it will show strange behaviour around PCI interrupts, making it
> hard to trace back to an invalid IORT RC nodes.
> 
> [...]

Applied to u-boot/next, thanks!

[1/5] acpi_table: Fix IORT RC node
      commit: 92d448f4f1ed80dfaa2327eeea9a741717dc3847
[2/5] acpi_table: Add asserts in IORT
      commit: 0ae343239b702336d2c0a4f73a9a953d5a15b2af
[3/5] acpi: Clear reserved bits in IORT
      commit: fe8844f4ad7189a6309976ecc841d55735e3534a
[4/5] acpi: Conditionally set mapping_offset in IORT
      commit: 9c748576402cbc1381498e72800ecaeb99d2b355
[5/5] test: acpi: Add IORT tests
      commit: 636b62c265f8932f12b2fe1ce8b256868d16fbda
-- 
Tom