Page MenuHomeFreeBSD

acpi_resource: support multiple IRQs
ClosedPublic

Authored by greg_unrelenting.technology on Jun 5 2020, 12:53 PM.

Details

Summary

Apparently since forever, FreeBSD has only supported one interrupt resource per device in ACPI, and failed rather quietly (not pointing to the reason) when there are multiple interrupts listed for a device.

The NXP Layerscape LX2160A (SolidRun HoneyComb LX2K etc.) has the following DSDT entries for the AHCI SATA controllers:

https://github.com/SolidRun/edk2-platforms/blob/eec706c2d693be0b3793d9180e7d1a4813a526cf/Silicon/NXP/LX2160A/AcpiTables/Dsdt/Sata.asl

Device(SAT0) {
  ...
  Name(_CRS, ResourceTemplate() {
    Memory32Fixed(ReadWrite, SATA0_BASE, SATA_LEN)
    Interrupt(ResourceConsumer, Level, ActiveHigh, Exclusive)
     {
       SATA0_IT_1, SATA0_IT_2, SATA0_IT_3
     }
  ...

And they were mysteriously failing to attach. Took me quite some time to dig deep enough to find this.

Test Plan

Tested by @dan.kotowski_a9development.com on the NXP Layerscape LX2160A

ahci0: <AHCI SATA controller> iomem 0x3200000-0x320ffff,0x700100520-0x700100523 irq 10,11,12 on acpi0
ahci1: <AHCI SATA controller> iomem 0x3210000-0x321ffff irq 13,14,15 on acpi0
ahci2: <AHCI SATA controller> iomem 0x3220000-0x322ffff irq 16,17,18 on acpi0
ahci3: <AHCI SATA controller> iomem 0x3230000-0x323ffff irq 19,20,21 on acpi0

Full dmesg with a working ada0 disk (and with lots of PCIe interrupt debug spam, as we're debugging that too): https://gist.github.com/agrajag9/749f504dcac741e902f87c2ea03ae9ac

The driver only allocates the first interrupt on this machine, so if there's something wrong with this patch as is, just returning the first interrupt with a warning instead of failing would also be acceptable.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

On the surface I think this is ok, but you should really be checking to see what the AHCI on ACPI spec says about how to handle multiple available interrupts.

As noted, I would perhaps leave the DMA thing as-is and just adjust interrupt resource handling for now. I don't think we are likely to see DMA channels on anything non-ancient at this point.

sys/dev/acpica/acpi_resource.c
661 ↗(On Diff #72712)

This is for ISA DMA channels, and I think you can leave it be as only doing 1.

greg_unrelenting.technology retitled this revision from acpi_resource: support multiple IRQs/DRQs to acpi_resource: support multiple IRQs.
greg_unrelenting.technology marked an inline comment as done.

you should really be checking to see what the AHCI on ACPI spec says about how to handle multiple available interrupts

I can't find any such spec on google.
I doubt that it exists, the fact that the DSDT definition uses a PCI class code (_CLS) instead of a _CID for matching strongly hints at it being an ad-hoc "fake PCI" type of thing :)

Our AHCI driver supports one-interrupt-per-channel just fine.
(On this hardware though, there's only one active channel per controller, so these extra interrupts are just unused.)

you should really be checking to see what the AHCI on ACPI spec says about how to handle multiple available interrupts

I can't find any such spec on google.
I doubt that it exists, the fact that the DSDT definition uses a PCI class code (_CLS) instead of a _CID for matching strongly hints at it being an ad-hoc "fake PCI" type of thing :)

Our AHCI driver supports one-interrupt-per-channel just fine.
(On this hardware though, there's only one active channel per controller, so these extra interrupts are just unused.)

Yes, my guess is it is enabling MSI (or MSI-X). The only issue is that some devices don't route all events to the first MSI interrupt when multiple messages are available, so we should make sure achi(4) treats this case the same as if MSI were enabled with N interrupts is all. The ACPI patch is fine though.

This revision is now accepted and ready to land.Jun 8 2020, 5:29 PM
This revision was automatically updated to reflect the committed changes.