On Mon, Jun 17, 2024 at 03:39:45PM -0700, Nicolin Chen wrote: > The IORT doc defines "Number of IDs" ("id_count" in the virt-acpi-build) > to be "the number of IDs in the range minus one". Otherwise, Linux kernel > reports "conflicting mapping for input ID" FW_BUG at the overlapped ID. > > Fixes: 42e0f050e3a5 ("hw/arm/virt-acpi-build: Add IORT support to bypass > SMMUv3") > Signed-off-by: Nicolin Chen <nicol...@nvidia.com> > --- > Changelog > v2: > * Moved "-1" to the same line of id_count calculation > * Added "+1" to the next_range.input_base calculation > v1: > https://lore.kernel.org/all/20240613234802.828265-1-nicol...@nvidia.com/ > > hw/arm/virt-acpi-build.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > index c3ccfef026..631f2c6d04 100644 > --- a/hw/arm/virt-acpi-build.c > +++ b/hw/arm/virt-acpi-build.c > @@ -243,7 +243,8 @@ iort_host_bridges(Object *obj, void *opaque) > > AcpiIortIdMapping idmap = { > .input_base = min_bus << 8, > - .id_count = (max_bus - min_bus + 1) << 8, > + /* id_count is the number of IDs in the range minus one */ > + .id_count = ((max_bus - min_bus + 1) << 8) - 1, > }; > g_array_append_val(idmap_blob, idmap); > } > @@ -298,11 +299,13 @@ build_iort(GArray *table_data, BIOSLinker *linker, > VirtMachineState *vms) > idmap = &g_array_index(smmu_idmaps, AcpiIortIdMapping, i); > > if (next_range.input_base < idmap->input_base) { > - next_range.id_count = idmap->input_base - > next_range.input_base; > + /* id_count is the number of IDs in the range minus one */ > + next_range.id_count = idmap->input_base - > + next_range.input_base - 1; > g_array_append_val(its_idmaps, next_range); > } > > - next_range.input_base = idmap->input_base + idmap->id_count; > + next_range.input_base = idmap->input_base + idmap->id_count + 1; > }
All this has to be written in the way that actually refers to the spec. id_count is nowhere in the spec and one has to know that in the end this is used by build_iort_id_mapping to figure out where this comes from. Not good. I think the best way is to fix build_iort_id_mapping: make it subtract 1 from id_count. Then change text from "Number of IDs" to "Number of IDs - The number of IDs in the range minus one" You should also add the reference to IO Remapping Table document near build_iort_id_mapping, it is currently unclear which table this refers to. Of couse this means the only correct use has to be tweaked so the change to build_iort_id_mapping does not break it: 0xFFFF -> 0x10000 - but that's good for readability, anyway. > /* Append the last RC -> ITS ID mapping */ > -- > 2.43.0