On Tue, Jun 18, 2024 at 12:04:20PM -0700, Nicolin Chen wrote: > On Tue, Jun 18, 2024 at 05:49:58AM -0400, Michael S. Tsirkin wrote: > > 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); > > > > > > What about other places where id_count is set? > > There are only three places where id_count could be set: > hw/arm/virt-acpi-build.c:296: .id_count = ((max_bus - min_bus > + 1) << 8) - 1, > hw/arm/virt-acpi-build.c:422: next_range.id_count = > idmap->input_base - > hw/arm/virt-acpi-build.c:435: next_range.id_count = 0xFFFF - > next_range.input_base; > > This patch fixes two, while the last one is correct using 0xFFFF. > > > > } > > > > > > - next_range.input_base = idmap->input_base + idmap->id_count; > > > + next_range.input_base = idmap->input_base + idmap->id_count > > > + 1; > > > } > > > > > > > Given this was different previously, did you actually test with multiple > > ranges? > > Tested by creating 5 buses: input_base increases by 0x400 while > id_count=0x2ff (0x300 - 1). ITS results look correct to me: > --------------build_iort: smmu_idmaps > DEBUG: build_iort_id_mapping: input_base=0xec00, id_count=0x2ff, > out_ref=0x48, flags=0 > DEBUG: build_iort_id_mapping: input_base=0xf000, id_count=0x2ff, > out_ref=0xa0, flags=0 > DEBUG: build_iort_id_mapping: input_base=0xf400, id_count=0x2ff, > out_ref=0xf8, flags=0 > DEBUG: build_iort_id_mapping: input_base=0xf800, id_count=0x2ff, > out_ref=0x150, flags=0 > DEBUG: build_iort_id_mapping: input_base=0xfc00, id_count=0x2ff, > out_ref=0x1a8, flags=0 > --------------build_iort: its_idmaps > DEBUG: build_iort_id_mapping: input_base=0x0, id_count=0xebff, out_ref=0x30, > flags=0 > DEBUG: build_iort_id_mapping: input_base=0xef00, id_count=0xff, out_ref=0x30, > flags=0 > DEBUG: build_iort_id_mapping: input_base=0xf300, id_count=0xff, out_ref=0x30, > flags=0 > DEBUG: build_iort_id_mapping: input_base=0xf700, id_count=0xff, out_ref=0x30, > flags=0 > DEBUG: build_iort_id_mapping: input_base=0xfb00, id_count=0xff, out_ref=0x30, > flags=0 > DEBUG: build_iort_id_mapping: input_base=0xff00, id_count=0xff, out_ref=0x30, > flags=0 > > Thanks > Nicolin
Okay. Is it the case that none of these effect the IORT in ./tests/data/acpi/virt/IORT then? -- MST