| Message ID | 20240405223110.1609888-13-jacob.jun.pan@linux.intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Coalesced Interrupt Delivery with posted MSI | expand |
> From: Jacob Pan <jacob.jun.pan@linux.intel.com> > Sent: Saturday, April 6, 2024 6:31 AM > > + * > + * For the example below, 3 MSIs are coalesced into one CPU notification. > Only > + * one apic_eoi() is needed. > + * > + * __sysvec_posted_msi_notification() > + * irq_enter(); > + * handle_edge_irq() > + * irq_chip_ack_parent() > + * dummy(); // No EOI > + * handle_irq_event() > + * driver_handler() > + * irq_enter(); > + * handle_edge_irq() > + * irq_chip_ack_parent() > + * dummy(); // No EOI > + * handle_irq_event() > + * driver_handler() > + * irq_enter(); > + * handle_edge_irq() > + * irq_chip_ack_parent() > + * dummy(); // No EOI > + * handle_irq_event() > + * driver_handler() typo: you added three irq_enter()'s here > + * apic_eoi() > + * irq_exit() > + */ > +static struct irq_chip intel_ir_chip_post_msi = { > + .name = "INTEL-IR-POST", > + .irq_ack = dummy, > + .irq_set_affinity = intel_ir_set_affinity, > + .irq_compose_msi_msg = intel_ir_compose_msi_msg, > + .irq_set_vcpu_affinity = intel_ir_set_vcpu_affinity, > +}; What about putting this patch at end of the series (combining the change in intel_irq_remapping_alloc()) to finally enable this feature? It reads slightly better to me to first get those callbacks extended to deal with the new mechanism (i.e. most changes in patch13) before using them in the new irqchip.
Hi Kevin, On Fri, 12 Apr 2024 09:36:10 +0000, "Tian, Kevin" <kevin.tian@intel.com> wrote: > > From: Jacob Pan <jacob.jun.pan@linux.intel.com> > > Sent: Saturday, April 6, 2024 6:31 AM > > > > + * > > + * For the example below, 3 MSIs are coalesced into one CPU > > notification. Only > > + * one apic_eoi() is needed. > > + * > > + * __sysvec_posted_msi_notification() > > + * irq_enter(); > > + * handle_edge_irq() > > + * irq_chip_ack_parent() > > + * dummy(); // No EOI > > + * handle_irq_event() > > + * driver_handler() > > + * irq_enter(); > > + * handle_edge_irq() > > + * irq_chip_ack_parent() > > + * dummy(); // No EOI > > + * handle_irq_event() > > + * driver_handler() > > + * irq_enter(); > > + * handle_edge_irq() > > + * irq_chip_ack_parent() > > + * dummy(); // No EOI > > + * handle_irq_event() > > + * driver_handler() > > typo: you added three irq_enter()'s here right, will remove the middle two. > > > + * apic_eoi() > > + * irq_exit() > > + */ > > +static struct irq_chip intel_ir_chip_post_msi = { > > + .name = "INTEL-IR-POST", > > + .irq_ack = dummy, > > + .irq_set_affinity = intel_ir_set_affinity, > > + .irq_compose_msi_msg = intel_ir_compose_msi_msg, > > + .irq_set_vcpu_affinity = intel_ir_set_vcpu_affinity, > > +}; > > What about putting this patch at end of the series (combining the > change in intel_irq_remapping_alloc()) to finally enable this > feature? > > It reads slightly better to me to first get those callbacks extended > to deal with the new mechanism (i.e. most changes in patch13) > before using them in the new irqchip.
diff --git a/drivers/iommu/intel/irq_remapping.c b/drivers/iommu/intel/irq_remapping.c index 566297bc87dd..fa719936b44e 100644 --- a/drivers/iommu/intel/irq_remapping.c +++ b/drivers/iommu/intel/irq_remapping.c @@ -1233,6 +1233,52 @@ static struct irq_chip intel_ir_chip = { .irq_set_vcpu_affinity = intel_ir_set_vcpu_affinity, }; +static void dummy(struct irq_data *d) +{ +} + +/* + * With posted MSIs, all vectors are multiplexed into a single notification + * vector. Devices MSIs are then dispatched in a demux loop where + * EOIs can be coalesced as well. + * + * "INTEL-IR-POST" IRQ chip does not do EOI on ACK, thus the dummy irq_ack() + * function. Instead EOI is performed by the posted interrupt notification + * handler. + * + * For the example below, 3 MSIs are coalesced into one CPU notification. Only + * one apic_eoi() is needed. + * + * __sysvec_posted_msi_notification() + * irq_enter(); + * handle_edge_irq() + * irq_chip_ack_parent() + * dummy(); // No EOI + * handle_irq_event() + * driver_handler() + * irq_enter(); + * handle_edge_irq() + * irq_chip_ack_parent() + * dummy(); // No EOI + * handle_irq_event() + * driver_handler() + * irq_enter(); + * handle_edge_irq() + * irq_chip_ack_parent() + * dummy(); // No EOI + * handle_irq_event() + * driver_handler() + * apic_eoi() + * irq_exit() + */ +static struct irq_chip intel_ir_chip_post_msi = { + .name = "INTEL-IR-POST", + .irq_ack = dummy, + .irq_set_affinity = intel_ir_set_affinity, + .irq_compose_msi_msg = intel_ir_compose_msi_msg, + .irq_set_vcpu_affinity = intel_ir_set_vcpu_affinity, +}; + static void fill_msi_msg(struct msi_msg *msg, u32 index, u32 subhandle) { memset(msg, 0, sizeof(*msg));
Introduce a new irq_chip for posted MSIs, the key difference is in irq_ack where EOI is performed by the notification handler. When posted MSI is enabled, MSI domain/chip hierarchy will look like this example: domain: IR-PCI-MSIX-0000:50:00.0-12 hwirq: 0x29 chip: IR-PCI-MSIX-0000:50:00.0 flags: 0x430 IRQCHIP_SKIP_SET_WAKE IRQCHIP_ONESHOT_SAFE parent: domain: INTEL-IR-10-13 hwirq: 0x2d0000 chip: INTEL-IR-POST flags: 0x0 parent: domain: VECTOR hwirq: 0x77 chip: APIC Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com> --- drivers/iommu/intel/irq_remapping.c | 46 +++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+)