On 2014/10/29 17:19, Thomas Gleixner wrote: >> Hi Thomas, >> Thanks for your great suggestion and I have worked out a draft >> patch to achieve what you want:) >> I have made following changes to irq core to get rid of remapped >> irq logic from msi.c: >> 1) Add IRQ_SET_MASK_OK_DONE in addition to IRQ_SET_MASK_OK and >> IRQ_SET_MASK_OK_NOCOPY. IRQ_SET_MASK_OK_DONE is the same as >> IRQ_SET_MASK_OK for irq core and indicates to stacked irqchip that >> parent irqchips have done all work and skip any handling in descendant >> irqchips. >> 2) Add int (*irq_compose_msi_msg)(struct irq_data *data, struct msi_msg >> *msg) into struct irq_chip. I'm still hesitating to return void or int >> here. By returning void, irq_chip_compose_msi_msg() will be simpler, >> but it loses flexibility. > > void should be sufficient. If the chip advertises this function it > better can provide a proper msi msg :) > >> With above changes to core, we could remove all knowledge of irq >> remapping from msi.c and the irq remapping interfaces get simpler too. >> Please refer to following patch for details. The patch passes basic >> booting tests with irq remapping enabled. If it's OK, I will fold >> it into the patch set. > > Yes. That looks reasonable. > >> IOAPIC runs into the same situation, but I need some more time >> to find a solution:) > > I'm sure you will! Hi Thomas, I have reviewed IOAPIC related change again, but the conclusion may not be what you expect:( Currently IOAPIC driver detects IRQ remapping for following tasks: 1) Issue different EOI command to IOAPIC chip for unammped and remapped interrupts. It uses pin number instead of vector number for remapped interrupts. 2) Print different format for IOAPIC entries for unmapped and remapped interrupts. 3) ioapic_ack_level() uses different method for unmapped and remapped interrupts 4) create different IOAPIC entry content for unmapped and remapped interrupts 5) choose different flow handler for unmapped and remapped interrupts For MSI, it only needs to solve task 4) above. For IOAPIC, it needs to solve all five tasks above, which may cause big changes to irq_chip. And it even may cause IRQ remapping driver call back into IOAPIC driver, which breaks another rules that only the driver touches corresponding interrupt controller. On the other hand, MSI is almost platform independent, so it's reasonable to change public struct irq_chip to support MSI. But IOAPIC is a sort of platform dependent (x86 and IA64), so it doesn't sound good to make great change to struct irq_chip to support IOAPIC. So I prefer keeping IOAPIC driver sensing interrupt remapping and acting different for unmapped and remapped interrupts. This breaks the layered design, but it does make code simpler. What's your thoughts? Regards! Gerry > > Thanks, > > tglx > -- 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
