commit "acpi: LNKS is not needed" breaks FreeBSD boot

[Marcelo Tosatti]

Since FreeBSD treats IRQ 9 polarity as active low
without the Interrupt() entry. Please revert.

[Kevin O'Connor]

On Fri, Dec 07, 2012 at 11:03:56PM -0200, Marcelo Tosatti wrote:
> Since FreeBSD treats IRQ 9 polarity as active low
> without the Interrupt() entry. Please revert.

Sigh.  Unfortunately, that code has gotten refactored a bit and isn't
as simple as just reverting it.  Paolo - what was the reason behind
removing LNKS in 4540409d19?

-Kevin

[Paolo Bonzini]

Il 11/12/2012 06:05, Kevin O'Connor ha scritto:
> On Fri, Dec 07, 2012 at 11:03:56PM -0200, Marcelo Tosatti wrote:
> > Since FreeBSD treats IRQ 9 polarity as active low
> > without the Interrupt() entry. Please revert.
>
> Sigh.  Unfortunately, that code has gotten refactored a bit and isn't
> as simple as just reverting it.  Paolo - what was the reason behind
> removing LNKS in 4540409d19?

It's in the commit message.  LNKS basically worked by chance.

LNKS is a bit strange in that it reuses the same PIIX register as LNKA,
    but has a different interrupt.  This means that the _CRS it returns will
    not be one of the possible resources from _PRS.  This shows up in the
    Linux boot logs as

ACPI: PCI Interrupt Link [LNKS] (IRQs 9) *0

Also, _DIS would not really disable the interrupt, and there was no _SRS
method.  It just happens to work because Linux special-cases the SCI all
over the place.  Linux is really not setting up the link at all.

Unfortunately, the code after the patch is also against the spec, it just
happens to trigger another special casing of SCI.  In fact, numeric _PRT
entries are handled the same in Linux and FreeBSD, as active-low.  Here
is the Linux code:

int triggering = ACPI_LEVEL_SENSITIVE;
        int polarity = ACPI_ACTIVE_LOW;

...

if (entry) {
                if (entry->link)
                        gsi = acpi_pci_link_allocate_irq(entry->link,
                                                         entry->index,
                                                         &triggering, &polarity,
                                                         &link);
                else
                        gsi = entry->index;

rc = acpi_register_gsi(&dev->dev, gsi, triggering, polarity);

but Linux doesn't fail because of this in arch/x86/kernel/acpi/boot.c:

/* Don't set up the ACPI SCI because it's already set up */
        if (acpi_gbl_FADT.sci_interrupt == gsi)
                return gsi;

thus in Linux the SCI interrupt is set up based on the override in the MADT.
(BTW, this second snippet is actually wrong and it should check against
acpi_sci_override_gsi, because the GSI for the MADT could also *remap*
the SCI interrupt specified in the FADT).

Anyway, I'll work on a revert.

Paolo