Re: [Bug 215560] New: _PRS/_SRS methods should be optional

[Bjorn Helgaas <helgaas@kernel.org>]

On Thu, Feb 03, 2022 at 06:12:10PM +0100, Pierre Gondois wrote:
> On 2/3/22 5:32 PM, Bjorn Helgaas wrote:
> > On Thu, Feb 03, 2022 at 10:10:19AM +0100, Pierre Gondois wrote:
> > > On 2/2/22 6:42 PM, Bjorn Helgaas wrote:
> > > > On Wed, Feb 02, 2022 at 10:20:44AM +0000, bugzilla-daemon@kernel.org wrote:
> > > > > https://bugzilla.kernel.org/show_bug.cgi?id=215560
> > > > > 
> > > > > The PCI legacy interrupts can be described with link devices, cf ACPI 6.4,
> > > > > s6.2.13 "_PRT (PCI Routing Table)".
> > > > > Link devices can have optional _SRS/_PRS methods to set the interrupt.
> > > > > ...
> > 
> > > > > However, _PRS/_SRS methods are checked in drivers/acpi/pci_link.c, and the
> > > > > driver aborts if they are absent.
> > > > > E.g.: When _PRS is missing:
> > > > > ACPI: \_SB_.PCI0.LNKA: _CRS 36 not found in _PRS
> > > > > ACPI: \_SB_.PCI0.LNKA: No IRQ available. Try pci=noacpi or acpi=off
> > > > 
> > > > I assume this bug report is because something isn't working.  Can
> > > > you update the bugzilla with a note about what specifically isn't
> > > > working and also attach a complete dmesg log and acpidump?
> > > 
> > > The question arose while writing link devices code, so there is no
> > > platform with missing _PRS/_SRS methods that I know.
> > > 
> > > The question was more about spec compliance and the necessity to
> > > have these methods when legacy interrupts are not configurable.  The
> > > message above (_CRS XXX not found in _PRS) can be generated for a
> > > Juno for instance, and the ACPI tables are at:
> > > https://github.com/tianocore/edk2-platforms/blob/master/Platform/ARM/JunoPkg/AcpiTables/AcpiSsdtRootPci.asl
> > > The ACPI table need to be modified (remove _PRS and set _CRS
> > > correctly).
> > > 
> > > If the conclusion is that _PRS/_SRS are mandatory, even for
> > > hard-wired interrupts, then the bugzilla can be closed.
> > 
> > OK, so if I understand correctly, you're using Interrupt Link devices
> > not because it's possible to connect a PCI interrupt to one of several
> > inputs on the interrupt controller, but because the PCI default of
> > "level triggered, active low" is not compatible with GICv2.
> > 
> > The Interrupt Link device gives you a chance to specify "level
> > triggered, active *high*".  If you used a Source of 0 (where there
> > is no Interrupt Link), there would be no way to specify that.
> > 
> > Since this use of Interrupt Links only conveys triggering information
> > and nothing is configurable, I think the OS should get that info from
> > _CRS, and _PRS and _SRS should not be required.
> > 
> > Alex made a change [1] along that line a while ago, but maybe there's
> > more we should do.
> > 
> > Bjorn
> > 
> > [1] https://git.kernel.org/linus/92d1b381f677
> 
> Yes, this is exactly the situation.
> 
> The interrupt advertised in _CRS is checked to be in _PRS at:
> https://github.com/torvalds/linux/blob/26291c54e111ff6ba87a164d85d4a4e134b7315c/drivers/acpi/pci_link.c#L549
> and the _SRS method is also evaluated.
> 
> I can submit a patch if necessary,

That would be awesome.  Thanks for pushing on this!

Bjorn