Hi everyone, First of all I have to admire your work maintaining so many systems and architectures and in particular the mvebu platform and I would like to offer my help for testing the patches with your guidance. I have essentially five working Armada A388 systems, one from solidrun and the other four are my own creation, a mini-DTX board that I designed and built for my home use, although at some point I planned to sell those boards, but it did workout due to my PhD that consumed too much time. I am now planning to do some NAS units with it and possibly sell them. In any way, long story short I've read in some threads about mvebu that PCIe is broken, and I say that it is not, the secret to have PCIe working is to use a good u-boot bootloader. Many recent u-boot versions cause the PCIe not to work on boot and I haven't figured out why yet. In fact my A388 systems are working with the amdgpu driver and an AMD Polaris, a RX 550 with 4GB VRAM. I have been succesfully using PCIe with this old u-boot version: https://github.com/SolidRun/u-boot-armada38x repo and branch u-boot-2013.01-15t1-clearfog. The system has 2GB of RAM and I am able to use the Ubuntu desktop and even navigate the web with Firefox, or watch 4K H265 videos with Kodi. The amdgpu driver works stable for several weeks in a row of power-up time, although since a year ago there is a regression in drm_buddy that freezes 32-bits systems on boot, but I have reverted that patch locally and have it working with kernel Vanilla 6.9.6. Yes, I know I should have reported it to the amdgpu team, but I have been very busy during this period. I hope to do it soon, would like to debug it better even with OpenOCD and a remote GDB for the kernel code. Please let me know if I can be of help. Thanks & best regards, Luis Mendes On Wed, Feb 26, 2025 at 9:50 PM Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote: > > On Thu, Jun 23, 2022 at 09:31:40PM +0100, Marc Zyngier wrote: > > On Thu, 23 Jun 2022 17:49:42 +0100, > > Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote: > > > On Thu, Jun 23, 2022 at 06:32:40PM +0200, Pali Rohár wrote: > > > > On Thursday 23 June 2022 11:27:47 Bjorn Helgaas wrote: > > > > > On Tue, May 24, 2022 at 02:28:17PM +0200, Pali Rohár wrote: > > > > > > Same as in commit a3b69dd0ad62 ("Revert "PCI: aardvark: > > > > > > Rewrite IRQ code to chained IRQ handler"") for pci-aardvark > > > > > > driver, use devm_request_irq() instead of chained IRQ > > > > > > handler in pci-mvebu.c driver. > > > > > > > > > > > > This change fixes affinity support and allows to pin > > > > > > interrupts from different PCIe controllers to different CPU > > > > > > cores. > > > > > > > > > > Several other drivers use irq_set_chained_handler_and_data(). > > > > > Do any of them need similar changes? > > > > > > > > I do not know. This needs testing on HW which use those other > > > > drivers. > > > > > > > > > The commit log suggests that using chained IRQ handlers breaks > > > > > affinity support. But perhaps that's not the case and the > > > > > real culprit is some other difference between mvebu and the > > > > > other drivers. > > > > > > > > It is possible. But similar patch (revert; linked below) was > > > > required for aardvark. I tested same approach on mvebu and it > > > > fixed affinity support. > > > > > > This feels like something we should understand better. If > > > irq_set_chained_handler_and_data() is a problem for affinity, we > > > should fix it across the board in all the drivers at once. > > > > > > If the real problem is something different, we should figure that > > > out and document it in the commit log. > > > > > > I cc'd Marc in case he has time to educate us. > > > > Thanks for roping me in. > > > > The problem of changing affinities for chained (or multiplexing) > > interrupts is, to make it short, that it breaks the existing > > userspace ABI that a change in affinity affects only the interrupt > > userspace acts upon, and no other. Which is why we don't expose any > > affinity setting for such an interrupt, as by definition changing > > its affinity affects all the interrupts that are muxed onto it. > > > > By turning a chained interrupt into a normal handler, people work > > around the above rule and break the contract the kernel has with > > userspace. > > > > Do I think this is acceptable? No. Can something be done about this? > > Maybe. > > > > Marek asked this exact question last month[1], and I gave a detailed > > explanation of what could be done to improve matters, allowing this > > to happen as long as userspace is made aware of the effects, which > > means creating a new userspace ABI. > > > > I would rather see people work on a proper solution than add bad > > hacks that only work in environments where the userspace ABI can be > > safely ignored, such as on an closed, embedded device. > > > > [1] https://lore.kernel.org/all/20220502102137.764606ee@thinkpad/ > > OK, this patch [2] has languished forever, and I don't know how to > move forward. > > The patch basically changes mvebu_pcie_irq_handler() from a chained > IRQ handler to a handler registered with devm_request_irq() so it can > be pinned to a CPU. > > How are we supposed to decide whether this is safe? What should we > look at in the patch? > > IIUC on mvebu, there's a single IRQ (port->intx_irq, described by a DT > "intx" in interrupt-names) that invokes mvebu_pcie_irq_handler(), > which loops through and handles INTA, INTB, INTC, and INTD. > > I think if port->intx_irq is pinned to CPU X, that means INTA, INTB, > INTC, and INTD are all pinned to that same CPU. > > I assume changing to devm_request_irq() means port->intx_irq will > appear in /proc/interrupts and can be pinned to a CPU. Is it a > problem if INTA, INTB, INTC, and INTD for that controller all > effectively share intx_irq and are pinned to the same CPU? > > AFAICT we currently have three PCI host controllers with INTx handlers > that are registered with devm_request_irq(), which is what [2] > proposes to do: > > advk_pcie_irq_handler() > xilinx_pl_dma_pcie_intx_flow() > xilinx_pcie_intr_handler() > > Do we assume that these are mistakes that shouldn't be emulated? > > [2] https://lore.kernel.org/r/20220524122817.7199-1-pali@xxxxxxxxxx >
