| Message ID | 1439963634-12006-1-git-send-email-jiang.liu@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
Hi, On Wed, Aug 19, 2015 at 7:53 AM, Jiang Liu <jiang.liu@linux.intel.com> wrote: > Nick Meier reported a regression with HyperV that " > After rebooting the VM, the following messages are logged in syslog > when trying to load the tulip driver: > tulip: Linux Tulip drivers version 1.1.15 (Feb 27, 2007) > tulip: 0000:00:0a.0: PCI INT A: failed to register GSI > tulip: Cannot enable tulip board #0, aborting > tulip: probe of 0000:00:0a.0 failed with error -16 > Errors occur in 3.19.0 kernel > Works in 3.17 kernel. > " > > According to the ACPI dump file posted by Nick at > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1440072 > > The ACPI MADT table includes an interrupt source overridden entry for > ACPI SCI: > [236h 0566 1] Subtable Type : 02 <Interrupt Source Override> > [237h 0567 1] Length : 0A > [238h 0568 1] Bus : 00 > [239h 0569 1] Source : 09 > [23Ah 0570 4] Interrupt : 00000009 > [23Eh 0574 2] Flags (decoded below) : 000D > Polarity : 1 > Trigger Mode : 3 > > And in DSDT table, we have _PRT method to define PCI interrupts, which > eventually goes to: > Name (PRSA, ResourceTemplate () > { > IRQ (Level, ActiveLow, Shared, ) > {3,4,5,7,9,10,11,12,14,15} > }) > Name (PRSB, ResourceTemplate () > { > IRQ (Level, ActiveLow, Shared, ) > {3,4,5,7,9,10,11,12,14,15} > }) > Name (PRSC, ResourceTemplate () > { > IRQ (Level, ActiveLow, Shared, ) > {3,4,5,7,9,10,11,12,14,15} > }) > Name (PRSD, ResourceTemplate () > { > IRQ (Level, ActiveLow, Shared, ) > {3,4,5,7,9,10,11,12,14,15} > }) > > According to the MADT and DSDT tables above, IRQ 9 may be used for: > 1) ACPI SCI in level, high mode > 2) PCI legacy IRQ in level, low mode > So there's a possible conflict in polarity setting for IRQ 9. > > Prior to commit cd68f6bd53cf ("x86, irq, acpi: Get rid of special > handling of GSI for ACPI SCI"), ACPI SCI is handled specially and > there's no check for conflicts between ACPI SCI and PCI legagy IRQ. > And it seems that the HyperV hypervisor doesn't make use of the > polarity configuration in IOAPIC entry, so it just works. That likely means the previous behavior was the same as (or at least analogous to) what happens on Windows which is what the firmware has been tested against and that's why the bug in it has not been caught. That in turn indicates that there may be more systems having this kind of problems, possibly from other vendors too, so I'm wondering if we can do something more generic than using a quirk here? Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2015/8/19 14:04, Rafael J. Wysocki wrote: > Hi, > > On Wed, Aug 19, 2015 at 7:53 AM, Jiang Liu <jiang.liu@linux.intel.com> wrote: >> Nick Meier reported a regression with HyperV that " >> After rebooting the VM, the following messages are logged in syslog >> when trying to load the tulip driver: >> tulip: Linux Tulip drivers version 1.1.15 (Feb 27, 2007) >> tulip: 0000:00:0a.0: PCI INT A: failed to register GSI >> tulip: Cannot enable tulip board #0, aborting >> tulip: probe of 0000:00:0a.0 failed with error -16 >> Errors occur in 3.19.0 kernel >> Works in 3.17 kernel. >> " >> >> According to the ACPI dump file posted by Nick at >> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1440072 >> >> The ACPI MADT table includes an interrupt source overridden entry for >> ACPI SCI: >> [236h 0566 1] Subtable Type : 02 <Interrupt Source Override> >> [237h 0567 1] Length : 0A >> [238h 0568 1] Bus : 00 >> [239h 0569 1] Source : 09 >> [23Ah 0570 4] Interrupt : 00000009 >> [23Eh 0574 2] Flags (decoded below) : 000D >> Polarity : 1 >> Trigger Mode : 3 >> >> And in DSDT table, we have _PRT method to define PCI interrupts, which >> eventually goes to: >> Name (PRSA, ResourceTemplate () >> { >> IRQ (Level, ActiveLow, Shared, ) >> {3,4,5,7,9,10,11,12,14,15} >> }) >> Name (PRSB, ResourceTemplate () >> { >> IRQ (Level, ActiveLow, Shared, ) >> {3,4,5,7,9,10,11,12,14,15} >> }) >> Name (PRSC, ResourceTemplate () >> { >> IRQ (Level, ActiveLow, Shared, ) >> {3,4,5,7,9,10,11,12,14,15} >> }) >> Name (PRSD, ResourceTemplate () >> { >> IRQ (Level, ActiveLow, Shared, ) >> {3,4,5,7,9,10,11,12,14,15} >> }) >> >> According to the MADT and DSDT tables above, IRQ 9 may be used for: >> 1) ACPI SCI in level, high mode >> 2) PCI legacy IRQ in level, low mode >> So there's a possible conflict in polarity setting for IRQ 9. >> >> Prior to commit cd68f6bd53cf ("x86, irq, acpi: Get rid of special >> handling of GSI for ACPI SCI"), ACPI SCI is handled specially and >> there's no check for conflicts between ACPI SCI and PCI legagy IRQ. >> And it seems that the HyperV hypervisor doesn't make use of the >> polarity configuration in IOAPIC entry, so it just works. > > That likely means the previous behavior was the same as (or at least > analogous to) what happens on Windows which is what the firmware has > been tested against and that's why the bug in it has not been caught. > That in turn indicates that there may be more systems having this kind > of problems, possibly from other vendors too, so I'm wondering if we > can do something more generic than using a quirk here? Hi Rafael, Good question. I was thought only virtual machines may generate such fake ACPI tables with conflicting pin attribute settings. For bare metal systems, I guess BIOS writers should report the correct hardware configurations, otherwise wrong pin attributes may cause hardware malfunction. If that's not the case, we may try to revert cd68f6bd53cf instead. Thanks! Gerry -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Wed, Aug 19, 2015 at 8:26 AM, Jiang Liu <jiang.liu@linux.intel.com> wrote: > On 2015/8/19 14:04, Rafael J. Wysocki wrote: >> Hi, >> >> On Wed, Aug 19, 2015 at 7:53 AM, Jiang Liu <jiang.liu@linux.intel.com> wrote: >>> Nick Meier reported a regression with HyperV that " >>> After rebooting the VM, the following messages are logged in syslog >>> when trying to load the tulip driver: >>> tulip: Linux Tulip drivers version 1.1.15 (Feb 27, 2007) >>> tulip: 0000:00:0a.0: PCI INT A: failed to register GSI >>> tulip: Cannot enable tulip board #0, aborting >>> tulip: probe of 0000:00:0a.0 failed with error -16 >>> Errors occur in 3.19.0 kernel >>> Works in 3.17 kernel. >>> " >>> >>> According to the ACPI dump file posted by Nick at >>> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1440072 >>> >>> The ACPI MADT table includes an interrupt source overridden entry for >>> ACPI SCI: >>> [236h 0566 1] Subtable Type : 02 <Interrupt Source Override> >>> [237h 0567 1] Length : 0A >>> [238h 0568 1] Bus : 00 >>> [239h 0569 1] Source : 09 >>> [23Ah 0570 4] Interrupt : 00000009 >>> [23Eh 0574 2] Flags (decoded below) : 000D >>> Polarity : 1 >>> Trigger Mode : 3 >>> >>> And in DSDT table, we have _PRT method to define PCI interrupts, which >>> eventually goes to: >>> Name (PRSA, ResourceTemplate () >>> { >>> IRQ (Level, ActiveLow, Shared, ) >>> {3,4,5,7,9,10,11,12,14,15} >>> }) >>> Name (PRSB, ResourceTemplate () >>> { >>> IRQ (Level, ActiveLow, Shared, ) >>> {3,4,5,7,9,10,11,12,14,15} >>> }) >>> Name (PRSC, ResourceTemplate () >>> { >>> IRQ (Level, ActiveLow, Shared, ) >>> {3,4,5,7,9,10,11,12,14,15} >>> }) >>> Name (PRSD, ResourceTemplate () >>> { >>> IRQ (Level, ActiveLow, Shared, ) >>> {3,4,5,7,9,10,11,12,14,15} >>> }) >>> >>> According to the MADT and DSDT tables above, IRQ 9 may be used for: >>> 1) ACPI SCI in level, high mode >>> 2) PCI legacy IRQ in level, low mode >>> So there's a possible conflict in polarity setting for IRQ 9. >>> >>> Prior to commit cd68f6bd53cf ("x86, irq, acpi: Get rid of special >>> handling of GSI for ACPI SCI"), ACPI SCI is handled specially and >>> there's no check for conflicts between ACPI SCI and PCI legagy IRQ. >>> And it seems that the HyperV hypervisor doesn't make use of the >>> polarity configuration in IOAPIC entry, so it just works. >> >> That likely means the previous behavior was the same as (or at least >> analogous to) what happens on Windows which is what the firmware has >> been tested against and that's why the bug in it has not been caught. >> That in turn indicates that there may be more systems having this kind >> of problems, possibly from other vendors too, so I'm wondering if we >> can do something more generic than using a quirk here? > Hi Rafael, > Good question. I was thought only virtual machines may generate > such fake ACPI tables with conflicting pin attribute settings. For bare > metal systems, I guess BIOS writers should report the correct hardware > configurations, otherwise wrong pin attributes may cause hardware > malfunction. > If that's not the case, we may try to revert cd68f6bd53cf Well, the regression at hand has just shown that the assertion in the changelog of that commit ("no need for for special treatment for GSI used by ACPI SCI") does not really hold. So, if the only motivation for it was to get rid of one extra check in mp_unregister_gsi() (mp_register_gsi() still needs to check if it is dealing with the SCI anyway), I'd vote for reverting it. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2015/8/19 14:45, Rafael J. Wysocki wrote: > Hi, > > On Wed, Aug 19, 2015 at 8:26 AM, Jiang Liu <jiang.liu@linux.intel.com> wrote: >> On 2015/8/19 14:04, Rafael J. Wysocki wrote: >>> Hi, >>> >>> On Wed, Aug 19, 2015 at 7:53 AM, Jiang Liu <jiang.liu@linux.intel.com> wrote: >>>> Nick Meier reported a regression with HyperV that " >>>> After rebooting the VM, the following messages are logged in syslog >>>> when trying to load the tulip driver: >>>> tulip: Linux Tulip drivers version 1.1.15 (Feb 27, 2007) >>>> tulip: 0000:00:0a.0: PCI INT A: failed to register GSI >>>> tulip: Cannot enable tulip board #0, aborting >>>> tulip: probe of 0000:00:0a.0 failed with error -16 >>>> Errors occur in 3.19.0 kernel >>>> Works in 3.17 kernel. >>>> " >>>> >>>> According to the ACPI dump file posted by Nick at >>>> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1440072 >>>> >>>> The ACPI MADT table includes an interrupt source overridden entry for >>>> ACPI SCI: >>>> [236h 0566 1] Subtable Type : 02 <Interrupt Source Override> >>>> [237h 0567 1] Length : 0A >>>> [238h 0568 1] Bus : 00 >>>> [239h 0569 1] Source : 09 >>>> [23Ah 0570 4] Interrupt : 00000009 >>>> [23Eh 0574 2] Flags (decoded below) : 000D >>>> Polarity : 1 >>>> Trigger Mode : 3 >>>> >>>> And in DSDT table, we have _PRT method to define PCI interrupts, which >>>> eventually goes to: >>>> Name (PRSA, ResourceTemplate () >>>> { >>>> IRQ (Level, ActiveLow, Shared, ) >>>> {3,4,5,7,9,10,11,12,14,15} >>>> }) >>>> Name (PRSB, ResourceTemplate () >>>> { >>>> IRQ (Level, ActiveLow, Shared, ) >>>> {3,4,5,7,9,10,11,12,14,15} >>>> }) >>>> Name (PRSC, ResourceTemplate () >>>> { >>>> IRQ (Level, ActiveLow, Shared, ) >>>> {3,4,5,7,9,10,11,12,14,15} >>>> }) >>>> Name (PRSD, ResourceTemplate () >>>> { >>>> IRQ (Level, ActiveLow, Shared, ) >>>> {3,4,5,7,9,10,11,12,14,15} >>>> }) >>>> >>>> According to the MADT and DSDT tables above, IRQ 9 may be used for: >>>> 1) ACPI SCI in level, high mode >>>> 2) PCI legacy IRQ in level, low mode >>>> So there's a possible conflict in polarity setting for IRQ 9. >>>> >>>> Prior to commit cd68f6bd53cf ("x86, irq, acpi: Get rid of special >>>> handling of GSI for ACPI SCI"), ACPI SCI is handled specially and >>>> there's no check for conflicts between ACPI SCI and PCI legagy IRQ. >>>> And it seems that the HyperV hypervisor doesn't make use of the >>>> polarity configuration in IOAPIC entry, so it just works. >>> >>> That likely means the previous behavior was the same as (or at least >>> analogous to) what happens on Windows which is what the firmware has >>> been tested against and that's why the bug in it has not been caught. >>> That in turn indicates that there may be more systems having this kind >>> of problems, possibly from other vendors too, so I'm wondering if we >>> can do something more generic than using a quirk here? >> Hi Rafael, >> Good question. I was thought only virtual machines may generate >> such fake ACPI tables with conflicting pin attribute settings. For bare >> metal systems, I guess BIOS writers should report the correct hardware >> configurations, otherwise wrong pin attributes may cause hardware >> malfunction. >> If that's not the case, we may try to revert cd68f6bd53cf > > Well, the regression at hand has just shown that the assertion in the > changelog of that commit ("no need for for special treatment for GSI > used by ACPI SCI") does not really hold. So, if the only motivation > for it was to get rid of one extra check in mp_unregister_gsi() > (mp_register_gsi() still needs to check if it is dealing with the SCI > anyway), I'd vote for reverting it. Hi Rafael, The motivation is to treat SCI as normal IOAPIC interrupt so we could enforce stricter pin attribute checking. Now it does reveal flaws in ACPI BIOS implementations, but we ran into trouble about how to handle those flaws:( Thanks! Gerry ACPI bios with flaws:( > > Thanks, > Rafael > -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 19 Aug 2015, Jiang Liu wrote: > On 2015/8/19 14:45, Rafael J. Wysocki wrote: > > Well, the regression at hand has just shown that the assertion in the > > changelog of that commit ("no need for for special treatment for GSI > > used by ACPI SCI") does not really hold. So, if the only motivation > > for it was to get rid of one extra check in mp_unregister_gsi() > > (mp_register_gsi() still needs to check if it is dealing with the SCI > > anyway), I'd vote for reverting it. > Hi Rafael, > The motivation is to treat SCI as normal IOAPIC interrupt so > we could enforce stricter pin attribute checking. Now it does reveal > flaws in ACPI BIOS implementations, but we ran into trouble about how to > handle those flaws:( The intent of this change is entirely correct, though it seems that reality of ACPI is just different. To be on the safe side of things, I agree with Rafael that we should revert that patch instead of introducing a single platform quirk. What we can do is to emit a pr_warn() if we detect inconsistency of the SCI configuration. That way we might be able to gather information how wide spread this disease is. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 19 Aug 2015, Thomas Gleixner wrote: > On Wed, 19 Aug 2015, Jiang Liu wrote: > > On 2015/8/19 14:45, Rafael J. Wysocki wrote: > > > Well, the regression at hand has just shown that the assertion in the > > > changelog of that commit ("no need for for special treatment for GSI > > > used by ACPI SCI") does not really hold. So, if the only motivation > > > for it was to get rid of one extra check in mp_unregister_gsi() > > > (mp_register_gsi() still needs to check if it is dealing with the SCI > > > anyway), I'd vote for reverting it. > > Hi Rafael, > > The motivation is to treat SCI as normal IOAPIC interrupt so > > we could enforce stricter pin attribute checking. Now it does reveal > > flaws in ACPI BIOS implementations, but we ran into trouble about how to > > handle those flaws:( > > The intent of this change is entirely correct, though it seems that > reality of ACPI is just different. > > To be on the safe side of things, I agree with Rafael that we should > revert that patch instead of introducing a single platform quirk. Jiang, can you please prepare a revert patch for this? Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2015/8/19 16:40, Thomas Gleixner wrote: > On Wed, 19 Aug 2015, Thomas Gleixner wrote: >> On Wed, 19 Aug 2015, Jiang Liu wrote: >>> On 2015/8/19 14:45, Rafael J. Wysocki wrote: >>>> Well, the regression at hand has just shown that the assertion in the >>>> changelog of that commit ("no need for for special treatment for GSI >>>> used by ACPI SCI") does not really hold. So, if the only motivation >>>> for it was to get rid of one extra check in mp_unregister_gsi() >>>> (mp_register_gsi() still needs to check if it is dealing with the SCI >>>> anyway), I'd vote for reverting it. >>> Hi Rafael, >>> The motivation is to treat SCI as normal IOAPIC interrupt so >>> we could enforce stricter pin attribute checking. Now it does reveal >>> flaws in ACPI BIOS implementations, but we ran into trouble about how to >>> handle those flaws:( >> >> The intent of this change is entirely correct, though it seems that >> reality of ACPI is just different. >> >> To be on the safe side of things, I agree with Rafael that we should >> revert that patch instead of introducing a single platform quirk. > > Jiang, > > can you please prepare a revert patch for this? Sure, will send out revert patch after basic tests. -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2015/8/19 16:40, Thomas Gleixner wrote: > On Wed, 19 Aug 2015, Thomas Gleixner wrote: >> On Wed, 19 Aug 2015, Jiang Liu wrote: >>> On 2015/8/19 14:45, Rafael J. Wysocki wrote: >>>> Well, the regression at hand has just shown that the assertion in the >>>> changelog of that commit ("no need for for special treatment for GSI >>>> used by ACPI SCI") does not really hold. So, if the only motivation >>>> for it was to get rid of one extra check in mp_unregister_gsi() >>>> (mp_register_gsi() still needs to check if it is dealing with the SCI >>>> anyway), I'd vote for reverting it. >>> Hi Rafael, >>> The motivation is to treat SCI as normal IOAPIC interrupt so >>> we could enforce stricter pin attribute checking. Now it does reveal >>> flaws in ACPI BIOS implementations, but we ran into trouble about how to >>> handle those flaws:( >> >> The intent of this change is entirely correct, though it seems that >> reality of ACPI is just different. >> >> To be on the safe side of things, I agree with Rafael that we should >> revert that patch instead of introducing a single platform quirk. > > Jiang, > > can you please prepare a revert patch for this? Hi Rafael and Thomas, I have tried to revert commit cd68f6bd53cf, but found it's not an easy task now. When converting to hierarchical irqdomain, the IOAPIC internal and interfaces have changed much, and seems no easy way to revert cd68f6bd53cf. There may be three possible solutions here: 1) use quirk to correct SCI polarity, as the patch does. 2) change IOAPIC interfaces to provide a special way to handle SCI interrupt. 3) change drivers/acpi/pci_link.c to penalize SCI IRQ so it won't be used for PCI IRQ if SCI polarity conflicts with PCI IRQ polarity. What's your thoughts here? Thanks! Gerry -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 20 Aug 2015, Jiang Liu wrote: > On 2015/8/19 16:40, Thomas Gleixner wrote: > > On Wed, 19 Aug 2015, Thomas Gleixner wrote: > >> On Wed, 19 Aug 2015, Jiang Liu wrote: > >>> On 2015/8/19 14:45, Rafael J. Wysocki wrote: > >>>> Well, the regression at hand has just shown that the assertion in the > >>>> changelog of that commit ("no need for for special treatment for GSI > >>>> used by ACPI SCI") does not really hold. So, if the only motivation > >>>> for it was to get rid of one extra check in mp_unregister_gsi() > >>>> (mp_register_gsi() still needs to check if it is dealing with the SCI > >>>> anyway), I'd vote for reverting it. > >>> Hi Rafael, > >>> The motivation is to treat SCI as normal IOAPIC interrupt so > >>> we could enforce stricter pin attribute checking. Now it does reveal > >>> flaws in ACPI BIOS implementations, but we ran into trouble about how to > >>> handle those flaws:( > >> > >> The intent of this change is entirely correct, though it seems that > >> reality of ACPI is just different. > >> > >> To be on the safe side of things, I agree with Rafael that we should > >> revert that patch instead of introducing a single platform quirk. > > > > Jiang, > > > > can you please prepare a revert patch for this? > Hi Rafael and Thomas, > I have tried to revert commit cd68f6bd53cf, but found > it's not an easy task now. That's what I feared > When converting to hierarchical irqdomain, the IOAPIC > internal and interfaces have changed much, and seems no easy > way to revert cd68f6bd53cf. There may be three possible solutions > here: > 1) use quirk to correct SCI polarity, as the patch does. > 2) change IOAPIC interfaces to provide a special way to > handle SCI interrupt. > 3) change drivers/acpi/pci_link.c to penalize SCI IRQ so it > won't be used for PCI IRQ if SCI polarity conflicts with > PCI IRQ polarity. Stupid question. Is the SCI polarity ever the opposite of PCI polarity? I.e. is such a ACPI override valid at all? Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2015/8/20 17:15, Thomas Gleixner wrote: > On Thu, 20 Aug 2015, Jiang Liu wrote: >> On 2015/8/19 16:40, Thomas Gleixner wrote: >>> On Wed, 19 Aug 2015, Thomas Gleixner wrote: >>>> On Wed, 19 Aug 2015, Jiang Liu wrote: >>>>> On 2015/8/19 14:45, Rafael J. Wysocki wrote: >>>>>> Well, the regression at hand has just shown that the assertion in the >>>>>> changelog of that commit ("no need for for special treatment for GSI >>>>>> used by ACPI SCI") does not really hold. So, if the only motivation >>>>>> for it was to get rid of one extra check in mp_unregister_gsi() >>>>>> (mp_register_gsi() still needs to check if it is dealing with the SCI >>>>>> anyway), I'd vote for reverting it. >>>>> Hi Rafael, >>>>> The motivation is to treat SCI as normal IOAPIC interrupt so >>>>> we could enforce stricter pin attribute checking. Now it does reveal >>>>> flaws in ACPI BIOS implementations, but we ran into trouble about how to >>>>> handle those flaws:( >>>> >>>> The intent of this change is entirely correct, though it seems that >>>> reality of ACPI is just different. >>>> >>>> To be on the safe side of things, I agree with Rafael that we should >>>> revert that patch instead of introducing a single platform quirk. >>> >>> Jiang, >>> >>> can you please prepare a revert patch for this? >> Hi Rafael and Thomas, >> I have tried to revert commit cd68f6bd53cf, but found >> it's not an easy task now. > > That's what I feared > >> When converting to hierarchical irqdomain, the IOAPIC >> internal and interfaces have changed much, and seems no easy >> way to revert cd68f6bd53cf. There may be three possible solutions >> here: >> 1) use quirk to correct SCI polarity, as the patch does. >> 2) change IOAPIC interfaces to provide a special way to >> handle SCI interrupt. >> 3) change drivers/acpi/pci_link.c to penalize SCI IRQ so it >> won't be used for PCI IRQ if SCI polarity conflicts with >> PCI IRQ polarity. > > Stupid question. Is the SCI polarity ever the opposite of PCI > polarity? I.e. is such a ACPI override valid at all? Hi Thomas, I have analyzed another system which works: 1) SCI(IRQ9) works in level, high mode (so such an ACPI override is valid) 2) there's a flag to detect whether system works in PIC or APIC mode. 3) IRQ9 may be used for PCI IRQ if system works in PIC mode. 4) IRQ9 won't be used for PCI IRQ if system works in APIC mode. Based on the above observation, I feel solution 3) may be the best one. Thanks! Gerry -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 20 Aug 2015, Jiang Liu wrote: > On 2015/8/20 17:15, Thomas Gleixner wrote: > > On Thu, 20 Aug 2015, Jiang Liu wrote: > >> When converting to hierarchical irqdomain, the IOAPIC > >> internal and interfaces have changed much, and seems no easy > >> way to revert cd68f6bd53cf. There may be three possible solutions > >> here: > >> 1) use quirk to correct SCI polarity, as the patch does. > >> 2) change IOAPIC interfaces to provide a special way to > >> handle SCI interrupt. > >> 3) change drivers/acpi/pci_link.c to penalize SCI IRQ so it > >> won't be used for PCI IRQ if SCI polarity conflicts with > >> PCI IRQ polarity. > > > > Stupid question. Is the SCI polarity ever the opposite of PCI > > polarity? I.e. is such a ACPI override valid at all? > Hi Thomas, > I have analyzed another system which works: > 1) SCI(IRQ9) works in level, high mode (so such an ACPI override is valid) > 2) there's a flag to detect whether system works in PIC or APIC mode. > 3) IRQ9 may be used for PCI IRQ if system works in PIC mode. Why? > 4) IRQ9 won't be used for PCI IRQ if system works in APIC mode. > Based on the above observation, I feel solution 3) may be the best one. Can you whip up a patch and test it on hyperv and maybe other machines which have that SCI override. Thanks tglx -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c index e49ee24da85e..47d95a86d56d 100644 --- a/arch/x86/kernel/acpi/boot.c +++ b/arch/x86/kernel/acpi/boot.c @@ -1308,6 +1308,13 @@ static int __init dmi_ignore_irq0_timer_override(const struct dmi_system_id *d) return 0; } +static int __init acpi_force_hyperv_sci_attr(const struct dmi_system_id *d) +{ + acpi_sci_flags = ACPI_MADT_POLARITY_ACTIVE_LOW | + (acpi_sci_flags & ~ACPI_MADT_POLARITY_MASK); + return 0; +} + /* * ACPI offers an alternative platform interface model that removes * ACPI hardware requirements for platforms that do not implement @@ -1458,6 +1465,14 @@ static struct dmi_system_id __initdata acpi_dmi_table_late[] = { DMI_MATCH(DMI_PRODUCT_NAME, "AMILO PRO V2030"), }, }, + { + .callback = acpi_force_hyperv_sci_attr, + .ident = "HyperV", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"), + DMI_MATCH(DMI_PRODUCT_NAME, "Virtual Machine"), + }, + }, {} };
Nick Meier reported a regression with HyperV that " After rebooting the VM, the following messages are logged in syslog when trying to load the tulip driver: tulip: Linux Tulip drivers version 1.1.15 (Feb 27, 2007) tulip: 0000:00:0a.0: PCI INT A: failed to register GSI tulip: Cannot enable tulip board #0, aborting tulip: probe of 0000:00:0a.0 failed with error -16 Errors occur in 3.19.0 kernel Works in 3.17 kernel. " According to the ACPI dump file posted by Nick at https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1440072 The ACPI MADT table includes an interrupt source overridden entry for ACPI SCI: [236h 0566 1] Subtable Type : 02 <Interrupt Source Override> [237h 0567 1] Length : 0A [238h 0568 1] Bus : 00 [239h 0569 1] Source : 09 [23Ah 0570 4] Interrupt : 00000009 [23Eh 0574 2] Flags (decoded below) : 000D Polarity : 1 Trigger Mode : 3 And in DSDT table, we have _PRT method to define PCI interrupts, which eventually goes to: Name (PRSA, ResourceTemplate () { IRQ (Level, ActiveLow, Shared, ) {3,4,5,7,9,10,11,12,14,15} }) Name (PRSB, ResourceTemplate () { IRQ (Level, ActiveLow, Shared, ) {3,4,5,7,9,10,11,12,14,15} }) Name (PRSC, ResourceTemplate () { IRQ (Level, ActiveLow, Shared, ) {3,4,5,7,9,10,11,12,14,15} }) Name (PRSD, ResourceTemplate () { IRQ (Level, ActiveLow, Shared, ) {3,4,5,7,9,10,11,12,14,15} }) According to the MADT and DSDT tables above, IRQ 9 may be used for: 1) ACPI SCI in level, high mode 2) PCI legacy IRQ in level, low mode So there's a possible conflict in polarity setting for IRQ 9. Prior to commit cd68f6bd53cf ("x86, irq, acpi: Get rid of special handling of GSI for ACPI SCI"), ACPI SCI is handled specially and there's no check for conflicts between ACPI SCI and PCI legagy IRQ. And it seems that the HyperV hypervisor doesn't make use of the polarity configuration in IOAPIC entry, so it just works. Commit cd68f6bd53cf gets rid of the specially handling of ACPI SCI, and then the pin attribute checking logic discloses the conflicts between ACPI SCI and PCI legacy IRQ on HyperV virtual machine, and rejects the request to assign IRQ9 to PCI devices. Since HyperV doesn't make use of the IOAPIC polarity setting, add a quirk to enforce ACPI SCI as level, low on HyperV guests, so IRQ9 could be used for both ACPI SCI and PCI legacy IRQ. Nick reports the proposed patch fixes the regression as " Applied the above proposed patch with the DMI values substituted. The tulip driver loaded, and an address was assigned via DHCP. " Please refer to following links for more information: https://bugzilla.kernel.org/show_bug.cgi?id=101301 https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1440072 Fixes: cd68f6bd53cf ("x86, irq, acpi: Get rid of special handling of GSI for ACPI SCI") Reported-and-tested-by: Nick Meier <nmeier@microsoft.com> Cc: <stable@vger.kernel.org> # 3.19 Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com> --- Hi Thomas, I have refined the commit message to explain why it works with the old code. Thanks! Gerry --- arch/x86/kernel/acpi/boot.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)