Page MenuHomeFreeBSD

acpi_iort: fix mapping end calculation
ClosedPublic

Authored by greg_unrelenting.technology on Jun 7 2020, 9:10 PM.

Details

Summary

According to the ARM Design Document "IO Remapping Table Platform" (DEN 0049D), the "Number of IDs" field of the ID mapping format means "The number of IDs in the range minus one".

This means when interpreting that field, we have to *add* one, not subtract one more!

With the current code, a zero in that field (like we can find on the NXP Layerscape LX2160A) would cause the end to be lower than the beginning (!!!) so if the first mapping has that zero, it would always be chosen over the other ones.

Test Plan

Trying to fix PCIe interrupts on the SolidRun HoneyComb LX2K that @dan.kotowski_a9development.com has. (Still not working, but the firmware build used currently has the PCIe OutputReferences pointing at the SMMU, unlike the public source code linked below (!) so my current hypothesis is that in that FW build, only the SMMU would get the interrupts. Still, I think this discovery is worth discussing either way.)

With prints from iort_pci_rc_map:

before:

iort: Found PCI RC node with segment and device ID: mapping 0 out_node_offset was 72, offset is 72. its? 0 smmu3? 0 smmu? 1 | nxtid 6400 rid 256

after:

iort: Found PCI RC node with segment and device ID: mapping 0 out_node_offset was 72, offset is 72. its? 0 smmu3? 0 smmu? 1 | nxtid 6145 rid 256

This is the IORT table:

.PciRcIdMapping = {
  {
    .InputBase = 0x0,
    .NumIds = 0x0,
    .OutputBase = 0x1800,
    .OutputReference = OFFSET_OF (NXP_EFI_ACPI_6_0_IO_REMAPPING_TABLE, ItsNode),
    .Flags = 0,
  },
  {
    .InputBase = 0x100,
    .NumIds = 0x7,
    .OutputBase = 0x1801,
    .OutputReference = OFFSET_OF (NXP_EFI_ACPI_6_0_IO_REMAPPING_TABLE, ItsNode),
    .Flags = 0,
  },
},

RID 256 (0x100 == domain=0, bus=1, slot=0, func=0) clearly should fall into the second mapping (InputBase = 0x100), and get mapped to
0x1801 (OutputBase) + 0x100 (RID) - 0x100 (InputBase), and the log indeed outputs 6145 (0x1801).

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

sys/arm64/acpica/acpi_iort.c
237 ↗(On Diff #72802)

Won't this mean we have an off-by-one error in the opposite direction?

sys/arm64/acpica/acpi_iort.c
237 ↗(On Diff #72802)

Hm, yeah, we actually do lookup with id <= entry->end instead of id < entry->end for some reason. If we change to id < entry->end, it won't. (We could also not do +1 but the comparison being inclusive on both ends seems odd to me)

sys/arm64/acpica/acpi_iort.c
237 ↗(On Diff #72802)

<= allows for entries at the end of the address space while < would require a 65 bit value for a 64 bit address space.

This revision is now accepted and ready to land.Jun 23 2020, 7:36 AM

Already accepted, but just wanted to confirm this solved interrupt mapping problems we were having on LX2160a.

This revision was automatically updated to reflect the committed changes.