On 06/06/2016 03:32 AM, Po Liu wrote: > Hi Bjorn, > I confirm we met same problem with KeyStone base on DesignWare design. > > > Best regards, > Liu Po > >> -----Original Message----- >> From: Bjorn Helgaas [mailto:helgaas@xxxxxxxxxx] >> Sent: Saturday, June 04, 2016 11:49 AM >> To: Murali Karicheri >> Cc: Po Liu; linux-pci@xxxxxxxxxxxxxxx; linux-arm- >> kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; >> devicetree@xxxxxxxxxxxxxxx; Arnd Bergmann; Roy Zang; Marc Zyngier; >> Stuart Yoder; Yang-Leo Li; Minghuan Lian; Bjorn Helgaas; Shawn Guo; >> Mingkai Hu; Rob Herring >> Subject: Re: [PATCH 2/2] aer: add support aer interrupt with none >> MSI/MSI-X/INTx mode >> >> On Fri, Jun 03, 2016 at 01:31:11PM -0400, Murali Karicheri wrote: >> > Po, >> > >> > Sorry to hijack your discussion, but the problem seems to be same for >> > Keystone PCI controller which is also designware (old version) based. >> > >> > On 06/03/2016 12:09 AM, Bjorn Helgaas wrote: >> > > On Thu, Jun 02, 2016 at 11:37:28AM -0400, Murali Karicheri wrote: >> > >> On 06/02/2016 09:55 AM, Bjorn Helgaas wrote: >> > >>> On Thu, Jun 02, 2016 at 05:01:19AM +0000, Po Liu wrote: >> > >>>>> -----Original Message----- >> > >>>>> From: Bjorn Helgaas [mailto:helgaas@xxxxxxxxxx] >> > >>>>> Sent: Thursday, June 02, 2016 11:48 AM >> > >>>>> To: Po Liu >> > >>>>> Cc: linux-pci@xxxxxxxxxxxxxxx; >> > >>>>> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; >> > >>>>> linux-kernel@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; Arnd >> > >>>>> Bergmann; Roy Zang; Marc Zyngier; Stuart Yoder; Yang-Leo Li; >> > >>>>> Minghuan Lian; Bjorn Helgaas; Shawn Guo; Mingkai Hu; Rob >> > >>>>> Herring >> > >>>>> Subject: Re: [PATCH 2/2] aer: add support aer interrupt with >> > >>>>> none MSI/MSI-X/INTx mode >> > >>>>> >> > >>>>> [+cc Rob] >> > >>>>> >> > >>>>> Hi Po, >> > >>>>> >> > >>>>> On Thu, May 26, 2016 at 02:00:06PM +0800, Po Liu wrote: >> > >>>>> > On some platforms, root port doesn't support MSI/MSI-X/INTx >> in RC mode. >> > >>>>> > When chip support the aer interrupt with none MSI/MSI-X/INTx >> > >>>>> mode, > maybe there is interrupt line for aer pme etc. Search >> > >>>>> the interrupt > number in the fdt file. >> > >>>>> >> > >>>>> My understanding is that AER interrupt signaling can be done >> > >>>>> via INTx, MSI, or MSI-X (PCIe spec r3.0, sec 6.2.4.1.2). >> > >>>>> Apparently your device doesn't support MSI or MSI-X. Are you >> > >>>>> saying it doesn't support INTx either? How is the interrupt >> you're requesting here different from INTx? >> > >>>> >> > >>>> Layerscape use none of MSI or MSI-X or INTx to indicate the >> > >>>> devices or root error in RC mode. But use an independent SPI >> > >>>> interrupt(arm interrupt controller) line. >> > >>> >> > >>> The Root Port is a PCI device and should follow the normal PCI >> > >>> rules for interrupts. As far as I understand, that means it >> > >>> should use MSI, MSI-X, or INTx. If your Root Port doesn't use MSI >> > >>> or MSI-X, it should use INTx, the PCI_INTERRUPT_PIN register >> > >>> should tell us which (INTA/ INTB/etc.), and >> PCI_COMMAND_INTX_DISABLE should work to disable it. >> > >>> That's all from the PCI point of view, of course. >> > >> >> > >> I am faced with the same issue on Keystone PCI hardware and it has >> > >> been on my TODO list for quite some time. Keystone PCI hardware >> > >> also doesn't use MSI or MSI-X or INTx for reporting errors received >> > >> at the root port, but use a platform interrupt instead (not >> > >> complaint to PCI standard as per PCI base spec). So I would need >> > >> similar change to have the error interrupt passed to the aer >> > >> driver. So there are hardware out there like Keystone which >> requires to support this through platform IRQ. >> > > >> > > This is not a new area of the spec, and it's hard for me to believe >> > > that these two new PCIe controllers are both broken the same way >> > > (although I guess both are DesignWare-based, so maybe this is the >> > > same underlying problem in both cases?). I think it's more likely >> > > that we just haven't figured out the right way to describe this in >> the DT. >> > >> > Keystone is using an older version of the designware IP and it >> > implements all of the interrupts in the application register space >> > unlike other newer version of the hardware. So I assume, the version >> > used on Layerscape is also an older version and the both have same >> > issue in terms of non standard platform interrupt used for error >> reporting. >> > >> > > I assume you have a Root Port with an AER capability, no MSI >> > > capability, and no MSI-X capability, right? >> > >> > Has AER capability and both MSI and INTx (legacy) capability >> > >> > > What does its Interrupt >> > > Pin register contain? If it's zero, it doesn't use INTx either, so >> > > according to the spec it should generate no interrupts. >> > > >> > At address offset 0x3C by default has a value of 1, but it is writable >> > by software. So default is INTx A. >> >> 0x3c is the Interrupt *Line*, which is read/write. The Interrupt >> *Pin* is at 0x3d and should be read-only. >> You are right. But default is 1 at this address. >> Does your Keystone driver support MSI? If so, since your Root Port >> supports MSI, I would think we would use that by default, and the INTx >> stuff wouldn't even matter. > > Layerscape is also shows "Both message signaled interrupts (MSI) and legacy INTx are supported." > But both of them not work for AER interrupt when devices or root port report aer error. > But another GIC interrupt line do. Same with Keystone. Even though both MSI and INTx are supported error interrupt at root port is reported on a different interrupt line than MSI/INTx. So for Power Management event interrupt is also different line. Murali > >> >> > > But if Interrupt Pin is non-zero, that means the Root Port should be >> > > able to generate virtual INTx interrupts. Presumably the Root >> > > Complex connects those interrupts to something; maybe to your >> > > platform interrupt? >> > >> > Probably that is what is happening. Both Power management and Error >> > interrupts are raised on platform interrupt lines. >> > >> > 12 Error Interrupts >> > >> > [0] System error (OR of fatal, nonfatal, correctable errors) (RC mode >> > only) [1] PCIe fatal error (RC mode only) [2] PCIe non-fatal error (RC >> > mode only) [3] PCIe correctable error (RC mode only) [4] AXI Error due >> > to fatal condition in AXI bridge (EP/RC modes) [5] PCIe advanced error >> > (RC mode only) >> > >> > 13 Power management and reset event interrupts >> > >> > [0] Power management turn-off message interrupt (EP mode only) [1] >> > Power management ack message interrupt (RC mode only) [2] Power >> > management event interrupt (RC mode only) [3] Link request reset >> > interrupt (hot reset or link down) (RC mode only) >> > >> > > PCI doesn't say anything about an interrupt after it leaves the Root >> > > Complex, so the fact that it's connected to a "platform interrupt" >> > > or "SPI interrupt" or "IOAPIC interrupt" doesn't make it non- >> compliant. >> > > Shouldn't we be able to use the interrupt-map and related DT >> > > properties to express the fact that Root Port virtual INTx is routed >> > > to platform interrupt Y, even without any special-case code in >> > > portdrv? >> > >> > My understanding is if RC also raise interrupt on INTx, then below map >> > make sense, where either EP or RC can raise interrupt and the line >> > will be muxed for interrupt from EP or RC port. >> >> I'm sorry, I didn't quite catch your meaning here, so let me try to >> clarify some terminology. Maybe we'll eventually blunder into a common >> understanding :) >> >> INTx is a PCI concept and only means something in the PCI hierarchy. >> The RC would *receive* virtual INTx interrupts from the PCI hierarchy >> and turn them into some platform-specific interrupt (not INTx) on the >> upstream side. >> >> So strictly speaking, the RC might raise platform-specific interrupts >> when it receives INTx interrupts, but it would not raise any INTx >> interrupts itself. >> >> > Here is the DT entry in PCIE keystone for Legacy interrupt mapping to >> > host interrupt. >> > #interrupt-cells = <1>; >> > interrupt-map-mask = <0 0 0 7>; >> > interrupt-map = <0 0 0 1 &pcie_intc0 0>, /* >> INT A */ >> > <0 0 0 2 &pcie_intc0 1>, /* >> INT B */ >> > <0 0 0 3 &pcie_intc0 2>, /* >> INT C */ >> > <0 0 0 4 &pcie_intc0 3>; /* >> > INT D */ >> >> If I understand correctly, this is the mapping from the PCI world (INTA, >> INTB, etc.) to the platform-specific world (pcie_intc0 0, etc.) >> >> If a Root Port raises a virtual INTA, the RC should turn it into the >> corresponding platform interrupt, i.e., GIC_SPI 48 from the description >> below. >> >> > And then >> > >> > pcie_intc0: legacy-interrupt-controller { >> > interrupt-controller; >> > #interrupt-cells = <1>; >> > interrupt-parent = <&gic>; >> > interrupts = <GIC_SPI 48 >> IRQ_TYPE_EDGE_RISING>, >> > <GIC_SPI 49 >> IRQ_TYPE_EDGE_RISING>, >> > <GIC_SPI 50 >> IRQ_TYPE_EDGE_RISING>, >> > <GIC_SPI 51 >> IRQ_TYPE_EDGE_RISING>; >> > }; >> > >> > So if RC interrupt for Power management and Error interrupt is a >> > separate line, how can software describe that using the above >> interrupt map? >> >> I don't know anything about interrupts the RC might generate on its own >> behalf. A lot of the RC behavior is not specified by the PCIe spec >> because the RC is on the border between the upstream platform-specific >> stuff and the downstream PCIe stuff. Is there something in the PCIe >> spec about the power management and error interrupts you're talking >> about? Of maybe you can point me to a section of the Keystone spec that >> talks about interrupts generated by the RC? > > Below is one of the PCIE controller interrupts list: > > 142 PEX1 INTA > 143 PEX1 INTB > 144 PEX1 INTC > 145 PEX1 INTD > 146-147 Reserved > 148 PEX1 MSI > 149 PEX1 PME > 150 PEX1 CFG err interrupt > > Only the "150 PEX1 CFG err interrupt" routing to the aer interrupt. > > >> >> In any event, the AER interrupts we're looking for in portdrv are from >> the Root Port, not from the RC. For INTx, my understanding is that the >> RC *transforms* virtual INTx messages from the Root Port in the PCIe >> domain into GIC transactions in the platform domain. >> >> Bjorn -- Murali Karicheri Linux Kernel, Keystone -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
