diff mbox series

[v2,12/13] iommu/vt-d: Add an irq_chip for posted MSIs

Message ID 20240405223110.1609888-13-jacob.jun.pan@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series Coalesced Interrupt Delivery with posted MSI | expand

Commit Message

Jacob Pan April 5, 2024, 10:31 p.m. UTC
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(+)

Comments

Tian, Kevin April 12, 2024, 9:36 a.m. UTC | #1
> 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. 
Jacob Pan April 16, 2024, 10:15 p.m. UTC | #2
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 mbox series

Patch

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));