2024-04-05 22:30:02

by Jacob Pan

[permalink] [raw]
Subject: [PATCH v2 12/13] iommu/vt-d: Add an irq_chip for posted MSIs

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 <[email protected]>
---
drivers/iommu/intel/irq_remapping.c | 46 +++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)

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



2024-04-12 09:57:52

by Tian, Kevin

[permalink] [raw]
Subject: RE: [PATCH v2 12/13] iommu/vt-d: Add an irq_chip for posted MSIs

> From: Jacob Pan <[email protected]>
> 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. ????

2024-04-17 00:12:04

by Jacob Pan

[permalink] [raw]
Subject: Re: [PATCH v2 12/13] iommu/vt-d: Add an irq_chip for posted MSIs

Hi Kevin,

On Fri, 12 Apr 2024 09:36:10 +0000, "Tian, Kevin" <[email protected]>
wrote:

> > From: Jacob Pan <[email protected]>
> > 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. ????

makes sense, will do.

Thanks,

Jacob