On Tue, Mar 26, 2013 at 10:16:54PM +0100, Thomas Petazzoni wrote: > > FWIW, MSI-X is not restricted to 16 bits, so if you can detect from > > the PCI layer if it is setting up MSI or MSI-X you could allocate low > > bits first to MSI-X and high bits first to MSI, increasing the number > > of available MSI/MSI-X vectors. > > This could be an improvement. There are also other, non-per-CPU, > doorbell interrupts that could potentially be used. Can we consider > this a possible improvement, and not something that is fundamentally > necessary? For now, I'm trying to get the current feature set merged, > and not necessarily to extend it to cover all possible features of the > hardware. A major point of MSI is to be able to direct interrupts on a CPU by CPU basis. Looks like XP has per-cpu and all-cpu doorbell bits? Combined with this: > It is the responsibility of the PCIe driver to prepare the 'struct > msi_msg', which contains the physical address at which the PCIe device > should write to trigger an MSI. But this physical address is part of Makes me think the split of responsibility created by moving the MSI ops into the PCI host structure is not correct. The PCIe host driver just seems to get in the way, it has no knowledge it is adding to the process. irqchip knows: - what the physical address of the doorbell is - how to construct an address that is per-cpu or all-cpu - which bits in the doorbell registers are allocated and which are free pci has none of that info. Looking at this some more, there is tonnes of stuff in linux that when a PCI MSI is allocated a special IRQ number is created for it that has special properties - eg set_affinity on that number actually goes into the MSI table and changes it. The cleanest would be to keep the doorbell driver purely related to the doorbell and when a request for a PCI MSI comes in allocate a new irq_chip (like arch/x86/kernel/apic/io_apic.c does) that has all the special PCI stuff and chain it to the proper bit in the doorbell. Optimizing to remove function calls from the interrupt stack could happen later. > > However, Marvell's doorbell can be controlled at the destination, so > > it is better to handle it that way, especially since it creates > > symmetry with the IPI usage. > > Hum, ok. But the MSI and IPI are handled quite differently: MSIs have > to call handle_IRQ(), while IPIs have to call handle_IPI(), so you'd > still have to distinguish between IPIs and MSIs. In the current driver, > IPIs don't have an associated irq_chip structure. Once you have a proper generic interrupt driver you can go ahead and use request_irq to grab N bits of the doorbell register and assign them to a handler that only calls handle_IPI(ipinr,get_irq_regs()). It is not necessary to keep IPI and the irqchip driver convoluted together. > Again, I don't see how it's possible to not care whether it's MSI or > IPI. IPIs have to call handle_IPI() which is a ARM-specific call, and I > don't understand how handle_fasteoi_irq() would end up calling > handle_IPI(). - The main IRQ vector is entered, it decodes the main cause register and calls the handler for the doorbell - The doorbell handler is setup as a chained handler and it uses handle_fasteoi_irq to enter into armada_370_xp_handle_irq - armada_370_xp_handle_irq then runs through all bits and calls their handlers - the handler for IPI bits is associated with the IPI handler that simply calls handle_IPI(...) handle_fasteoi_irq acks's and clears the handled bits in the doorbell register at the proper time. > I'll let Arnd answer this one, but I'm pretty sure that using IRQ > domains is the way to go. The fact that a number of drivers don't yet > use IRQ domains is maybe just because they haven't been converted yet. Maybe, but they have irq domain code as well.. I'm curious about the answer too :) Jason -- 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
