2022-05-06 09:43:08

by Ricardo Neri

[permalink] [raw]
Subject: [PATCH v6 03/29] x86/apic/msi: Set the delivery mode individually for each IRQ

There are no restrictions in hardware to set MSI messages with its
own delivery mode. Use the mode specified in the provided IRQ hardware
configuration data. Since most of the IRQs are configured to use the
delivery mode of the APIC driver in use (set in all of them to
APIC_DELIVERY_MODE_FIXED), the only functional changes are where
IRQs are configured to use a specific delivery mode.

Changing the utility function __irq_msi_compose_msg() takes care of
implementing the change in the in the local APIC, PCI-MSI, and DMAR-MSI
irq_chips.

The IO-APIC irq_chip configures the entries in the interrupt redirection
table using the delivery mode specified in the corresponding MSI message.
Since the MSI message is composed by a higher irq_chip in the hierarchy,
it does not need to be updated.

Cc: Andi Kleen <[email protected]>
Cc: "Ravi V. Shankar" <[email protected]>
Cc: Stephane Eranian <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Reviewed-by: Tony Luck <[email protected]>
Signed-off-by: Ricardo Neri <[email protected]>
---
Changes since v5:
* Introduced this patch

Changes since v4:
* N/A

Changes since v3:
* N/A

Changes since v2:
* N/A

Changes since v1:
* N/A
---
arch/x86/kernel/apic/apic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 189d3a5e471a..d1e12da1e9af 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2528,7 +2528,7 @@ void __irq_msi_compose_msg(struct irq_cfg *cfg, struct msi_msg *msg,
msg->arch_addr_lo.dest_mode_logical = apic->dest_mode_logical;
msg->arch_addr_lo.destid_0_7 = cfg->dest_apicid & 0xFF;

- msg->arch_data.delivery_mode = APIC_DELIVERY_MODE_FIXED;
+ msg->arch_data.delivery_mode = cfg->delivery_mode;
msg->arch_data.vector = cfg->vector;

msg->address_hi = X86_MSI_BASE_ADDRESS_HIGH;
--
2.17.1



2022-05-08 15:09:11

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH v6 03/29] x86/apic/msi: Set the delivery mode individually for each IRQ

On Thu, May 05 2022 at 16:59, Ricardo Neri wrote:
> There are no restrictions in hardware to set MSI messages with its
> own delivery mode.

"messages with its own" ? Plural/singular confusion.

> Use the mode specified in the provided IRQ hardware
> configuration data. Since most of the IRQs are configured to use the
> delivery mode of the APIC driver in use (set in all of them to
> APIC_DELIVERY_MODE_FIXED), the only functional changes are where
> IRQs are configured to use a specific delivery mode.

This does not parse. There are no functional changes due to this patch
and there is no point talking about functional changes in subsequent
patches here.

> Changing the utility function __irq_msi_compose_msg() takes care of
> implementing the change in the in the local APIC, PCI-MSI, and DMAR-MSI

in the in the

> irq_chips.
>
> The IO-APIC irq_chip configures the entries in the interrupt redirection
> table using the delivery mode specified in the corresponding MSI message.
> Since the MSI message is composed by a higher irq_chip in the hierarchy,
> it does not need to be updated.

The point is that updating __irq_msi_compose_msg() covers _all_ MSI
consumers including IO-APIC.

I had to read that changelog 3 times to make sense of it. Something like
this perhaps:

"x86/apic/msi: Use the delivery mode from irq_cfg for message composition

irq_cfg provides a delivery mode for each interrupt. Use it instead
of the hardcoded APIC_DELIVERY_MODE_FIXED. This allows to compose
messages for NMI delivery mode which is required to implement a HPET
based NMI watchdog.

No functional change as the default delivery mode is set to
APIC_DELIVERY_MODE_FIXED."

Thanks,

tglx

2022-05-12 20:28:13

by Ricardo Neri

[permalink] [raw]
Subject: Re: [PATCH v6 03/29] x86/apic/msi: Set the delivery mode individually for each IRQ

On Fri, May 06, 2022 at 10:05:34PM +0200, Thomas Gleixner wrote:
> On Thu, May 05 2022 at 16:59, Ricardo Neri wrote:
> > There are no restrictions in hardware to set MSI messages with its
> > own delivery mode.
>
> "messages with its own" ? Plural/singular confusion.

Yes, this is not correct. It should have read "messages with their own..."

>
> > Use the mode specified in the provided IRQ hardware
> > configuration data. Since most of the IRQs are configured to use the
> > delivery mode of the APIC driver in use (set in all of them to
> > APIC_DELIVERY_MODE_FIXED), the only functional changes are where
> > IRQs are configured to use a specific delivery mode.
>
> This does not parse. There are no functional changes due to this patch
> and there is no point talking about functional changes in subsequent
> patches here.

I will remove this.

>
> > Changing the utility function __irq_msi_compose_msg() takes care of
> > implementing the change in the in the local APIC, PCI-MSI, and DMAR-MSI
>
> in the in the

Sorry! This is not correct.

>
> > irq_chips.
> >
> > The IO-APIC irq_chip configures the entries in the interrupt redirection
> > table using the delivery mode specified in the corresponding MSI message.
> > Since the MSI message is composed by a higher irq_chip in the hierarchy,
> > it does not need to be updated.
>
> The point is that updating __irq_msi_compose_msg() covers _all_ MSI
> consumers including IO-APIC.
>
> I had to read that changelog 3 times to make sense of it. Something like
> this perhaps:
>
> "x86/apic/msi: Use the delivery mode from irq_cfg for message composition
>
> irq_cfg provides a delivery mode for each interrupt. Use it instead
> of the hardcoded APIC_DELIVERY_MODE_FIXED. This allows to compose
> messages for NMI delivery mode which is required to implement a HPET
> based NMI watchdog.
>
> No functional change as the default delivery mode is set to
> APIC_DELIVERY_MODE_FIXED."

Thank you for your help on the changelog! I will take your suggestion.

BR,
Ricardo
>
> Thanks,
>
> tglx