Paths

Table of Contentst

acpi_resource: support multiple IRQs
ClosedPublic
Actions

Authored by val_packett.cool on Jun 5 2020, 12:53 PM.

Details

Reviewers

jhb
andrew
imp

Group Reviewers

Contributor Reviews (src)

Commits

rS366294: acpi_resource: support multiple IRQs

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 Skipped

Unit

Tests Skipped

Event Timeline

val_packett.cool requested review of this revision.Jun 5 2020, 12:53 PM

val_packett.cool created this revision.

val_packett.cool mentioned this in D25157: ahci_generic: add quirk for NXP0004 (NXP Layerscape LX2160A).Jun 5 2020, 8:41 PM

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	This is for ISA DMA channels, and I think you can leave it be as only doing 1.

val_packett.cool updated this revision to Diff 72801.Jun 7 2020, 8:40 PM

val_packett.cool retitled this revision from acpi_resource: support multiple IRQs/DRQs to acpi_resource: support multiple IRQs.

val_packett.cool 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.)

In D25145#554786, @greg_unrelenting.technology wrote:

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.

jhb accepted this revision.Jun 8 2020, 5:29 PM

This revision is now accepted and ready to land.Jun 8 2020, 5:29 PM

ping

Closed by commit rS366294: acpi_resource: support multiple IRQs (authored by manu). · Explain WhySep 30 2020, 5:09 PM

This revision was automatically updated to reflect the committed changes.

manu added a commit: rS366294: acpi_resource: support multiple IRQs.

Revision Contents
Changeset List

Path

Size

sys/

dev/

acpica/

acpi_resource.c

20 lines

Diff 72801

View Options

sys/dev/acpica/acpi_resource.c

acpi_resource: support multiple IRQsClosedPublicActions