On Thu, 30 Oct 2014, Jiang Liu wrote: > On 2014/10/29 17:19, Thomas Gleixner wrote: > >> 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 So todays code does 1/2/3 via irq_remap_modify_chip_defaults() and via x86_io_apic_ops.eoi_ioapic_pin with convoluted back and forth calls from remap to ioapic code. 4) is solved via x86_io_apic_ops.setup_entry 5) via setup_remapped_irq() Now with the stacked irq domains we end up with the following two scenarios: ioapic_domain -> vector_domain ioapic_domain -> remap_domain -> vector_domain So if you look at the various issues you want to solve: #1 Is simple to solve via: static void ioapic_eoi(struct irq_data *data) { if (data->parent->chip->irq_eoi) data->parent->chip->irq_eoi(data->parent); else plain_ioapic_eoi(data); } #2/3 Ditto #4/5 Should be solvable via the irq_startup callback in a similar way static int ioapic_startup(struct irq_data *data) { if (data->parent->chip->irq_startup) return data->parent->chip->irq_startup(data->parent); else return plain_ioapic_startup(data); } I.e. you set the entry and the handler from the startup function of ioapic or remap. It's probably not that simple as the above, but I'm pretty confident, that we can map it without adding a gazillion of new callbacks to irqchip. > 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. If the remap driver calls ioapic functions which are provided for that purpose then I think that's unavoidable and ok. But I really want to avoid the intermingled mess in the other code pathes which call back and forth. > On the other hand, MSI is almost platform independent, so it's > reasonable to change public struct irq_chip to support MSI. Right, but I think we can get away without adding new functions to irqchip for the ioapic/remap thing. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html