Messages in this thread | ![/](/images/icornerl.gif) | | From | Thomas Gleixner <> | Subject | Re: [PATCH RFC 11/13] iommu/vt-d: Add an irq_chip for posted MSIs | Date | Wed, 06 Dec 2023 21:15:24 +0100 |
| |
On Sat, Nov 11 2023 at 20:16, Jacob Pan wrote: > With posted MSIs, end of interrupt is handled by the notification > handler. Each MSI handler does not go through local APIC IRR, ISR > processing. There's no need to do apic_eoi() in those handlers. > > Add a new acpi_ack_irq_no_eoi() for the posted MSI IR chip. At runtime > the call trace looks like: > > __sysvec_posted_msi_notification() { > irq_chip_ack_parent() { > apic_ack_irq_no_eoi(); > }
Huch? There is something missing here to make sense.
> handle_irq_event() { > handle_irq_event_percpu() { > driver_handler() > } > } > > IO-APIC IR is excluded the from posted MSI, we need to make sure it > still performs EOI.
We need to make the code correct and write changelogs which make sense. This sentence makes no sense whatsoever.
What has the IO-APIC to do with posted MSIs?
It's a different interrupt chip hierarchy, no?
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c > index 00da6cf6b07d..ca398ee9075b 100644 > --- a/arch/x86/kernel/apic/io_apic.c > +++ b/arch/x86/kernel/apic/io_apic.c > @@ -1993,7 +1993,7 @@ static struct irq_chip ioapic_ir_chip __read_mostly = { > .irq_startup = startup_ioapic_irq, > .irq_mask = mask_ioapic_irq, > .irq_unmask = unmask_ioapic_irq, > - .irq_ack = irq_chip_ack_parent, > + .irq_ack = apic_ack_irq,
Why?
> .irq_eoi = ioapic_ir_ack_level, > .irq_set_affinity = ioapic_set_affinity, > .irq_retrigger = irq_chip_retrigger_hierarchy, > diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c > index 14fc33cfdb37..01223ac4f57a 100644 > --- a/arch/x86/kernel/apic/vector.c > +++ b/arch/x86/kernel/apic/vector.c > @@ -911,6 +911,11 @@ void apic_ack_irq(struct irq_data *irqd) > apic_eoi(); > } > > +void apic_ack_irq_no_eoi(struct irq_data *irqd) > +{ > + irq_move_irq(irqd); > +} > +
The exact purpose of that function is to invoke irq_move_irq() which is a completely pointless exercise for interrupts which are remapped.
| ![\](/images/icornerr.gif) |