Re: kern/47648: NetBSD 6.1_RC1 ACPI interupt routing problem + other ACPI lossage

[Reinoud Zandijk <reinoud%NetBSD.org@localhost>]

On Mon, Mar 18, 2013 at 10:10:02AM -0700, Chuck Silvers wrote:
> actually I thought of a simpler hack than I was thinking of before,
> please try the attached patch.
> 
> if that works then there's other hacky code (from before I got involved)
> that can be removed too.

The patch workes and fixes the problem! I get a neat 100 interrupts/sec and my
impression is that it is smoother than before, but thats most likely my
imagination :)

Would you like to commit it to -current en to netbsd-6? Maybe it can be pulled
up before 6.1 comes out?

Thanks a lot,
Reinoud


> Index: src/sys/arch/x86/x86/mpacpi.c
> ===================================================================
> RCS file: /home/chs/netbsd/cvs/src/sys/arch/x86/x86/mpacpi.c,v
> retrieving revision 1.96
> diff -u -p -r1.96 mpacpi.c
> --- src/sys/arch/x86/x86/mpacpi.c     3 Oct 2012 17:04:25 -0000       1.96
> +++ src/sys/arch/x86/x86/mpacpi.c     18 Mar 2013 16:49:06 -0000
> @@ -942,13 +942,6 @@ mpacpi_find_interrupts(void *self)
>               printf("mpacpi: %d PCI busses\n", mpacpi_npci);
>  #endif
>       mpacpi_config_irouting(acpi);
> -#if NIOAPIC > 0
> -     /*
> -      * XXX fix up the SCI interrupt polarity.
> -      * it's installed before we have parsed the MADT.
> -      */
> -     ioapic_reenable();
> -#endif
>       if (mp_verbose)
>               for (i = 0; i < mp_nintr; i++)
>                       mpacpi_print_intr(&mp_intrs[i]);
> Index: src/sys/arch/x86/acpi/acpi_machdep.c
> ===================================================================
> RCS file: /home/chs/netbsd/cvs/src/sys/arch/x86/acpi/acpi_machdep.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 acpi_machdep.c
> --- src/sys/arch/x86/acpi/acpi_machdep.c      23 Sep 2012 00:31:05 -0000      
> 1.4
> +++ src/sys/arch/x86/acpi/acpi_machdep.c      18 Mar 2013 17:05:17 -0000
> @@ -93,6 +93,30 @@ acpi_md_OsGetRootPointer(void)
>       return PhysicalAddress;
>  }
>  
> +
> +struct acpi_md_sci {
> +     int pin;
> +     int flags;
> +};
> +
> +static ACPI_STATUS
> +acpi_md_findsci(ACPI_SUBTABLE_HEADER *hdrp, void *aux)
> +{
> +     ACPI_MADT_INTERRUPT_OVERRIDE *iop;
> +     struct acpi_md_sci *scip;
> +
> +     if (hdrp->Type != ACPI_MADT_TYPE_INTERRUPT_OVERRIDE) {
> +             return AE_OK;
> +     }
> +
> +     iop = (void *)hdrp;
> +     if (iop->SourceIrq == AcpiGbl_FADT.SciInterrupt) {
> +             scip->pin = iop->GlobalIrq;
> +             scip->flags = iop->IntiFlags;
> +     }
> +     return AE_OK;
> +}
> +
>  ACPI_STATUS
>  acpi_md_OsInstallInterruptHandler(uint32_t InterruptNumber,
>      ACPI_OSD_HANDLER ServiceRoutine, void *Context, void **cookiep)
> @@ -101,37 +125,50 @@ acpi_md_OsInstallInterruptHandler(uint32
>       struct pic *pic;
>  #if NIOAPIC > 0
>       struct ioapic_softc *sc;
> +     struct acpi_md_sci sci;
>  #endif
> -     int irq, pin, trigger;
> +     int irq, pin, trigger, redir, mpflags;
> +
> +     /*
> +      * The SCI defaults to level-triggered active-low.
> +      */
> +
> +     trigger = IST_LEVEL;
> +     mpflags = (MPS_INTTR_LEVEL << 2) | MPS_INTPO_ACTLO;
> +     redir = IOAPIC_REDLO_LEVEL | IOAPIC_REDLO_ACTLO;
>  
>  #if NIOAPIC > 0
> +
>       /*
> -      * Can only match on ACPI global interrupt numbers if the ACPI
> -      * interrupt info was extracted, which is in the ACPI case.
> +      * Apply any MADT override setting.
>        */
> -     if (mpacpi_sci_override != NULL) {
> -             pic = mpacpi_sci_override->ioapic;
> -             pin = mpacpi_sci_override->ioapic_pin;
> -             if (mpacpi_sci_override->redir & IOAPIC_REDLO_LEVEL)
> -                     trigger = IST_LEVEL;
> -             else
> +
> +     if (acpi_madt_map() != AE_OK) {
> +             return AE_NO_ACPI_TABLES;
> +     }
> +     sci.pin = -1;
> +     acpi_madt_walk(acpi_md_findsci, &sci);
> +     acpi_madt_unmap();
> +
> +     if (sci.pin != -1) {
> +             InterruptNumber = sci.pin;
> +             if ((sci.flags & ACPI_MADT_POLARITY_MASK) ==
> +                 ACPI_MADT_POLARITY_ACTIVE_HIGH) {
> +                     mpflags &= ~MPS_INTPO_ACTLO;
> +                     redir &= ~IOAPIC_REDLO_ACTLO;
> +             }
> +             if ((sci.flags & ACPI_MADT_TRIGGER_MASK) ==
> +                 ACPI_MADT_TRIGGER_EDGE) {
>                       trigger = IST_EDGE;
> -             if (pic->pic_type == PIC_IOAPIC)
> -                     irq = -1;
> -             else
> -                     irq = (int)InterruptNumber;
> -             goto sci_override;
> +                     mpflags &= ~(MPS_INTTR_LEVEL << 2);
> +                     redir &= ~IOAPIC_REDLO_LEVEL;
> +             }
>       }
> -#endif
>  
>       /*
> -      * There was no ACPI interrupt source override,
> -      *
> -      * If the interrupt is handled via IOAPIC, mark it
> -      * as level-triggered, active low in the table.
> +      * If the interrupt is handled via IOAPIC, update the map.
>        */
>  
> -#if NIOAPIC > 0
>       sc = ioapic_find_bybase(InterruptNumber);
>       if (sc != NULL) {
>               pic = &sc->sc_pic;
> @@ -146,9 +183,12 @@ acpi_md_OsInstallInterruptHandler(uint32
>  
>               mip = sc->sc_pins[pin].ip_map;
>               if (mip) {
> -                     mip->flags &= ~3;
> -                     mip->flags |= MPS_INTPO_ACTLO;
> -                     mip->redir |= IOAPIC_REDLO_ACTLO;
> +                     mip->flags &= ~(ACPI_MADT_TRIGGER_MASK |
> +                                     ACPI_MADT_POLARITY_MASK);
> +                     mip->flags |= mpflags;
> +                     mip->redir &= ~(IOAPIC_REDLO_LEVEL |
> +                                     IOAPIC_REDLO_ACTLO);
> +                     mip->redir |= redir;
>               }
>       } else
>  #endif
> @@ -157,12 +197,6 @@ acpi_md_OsInstallInterruptHandler(uint32
>               irq = pin = (int)InterruptNumber;
>       }
>  
> -     trigger = IST_LEVEL;
> -
> -#if NIOAPIC > 0
> -sci_override:
> -#endif
> -
>       /*
>        * XXX probably, IPL_BIO is enough.
>        */