diff mbox

[V4,01/42] x86, irq: update high address field when updating affinity for MSI IRQ

Message ID 1402302011-23642-2-git-send-email-jiang.liu@linux.intel.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Jiang Liu June 9, 2014, 8:19 a.m. UTC
If x2apic is enabled, the MSI high address field should also be aslo
updated when setting affinity for MSI IRQ, otherwise the MSI IRQ may
target wrong APIC IDs.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
Cc: stable@vger.kernel.org
---
 arch/x86/kernel/apic/io_apic.c |    4 ++++
 1 file changed, 4 insertions(+)

Comments

Yinghai Lu June 9, 2014, 11:46 p.m. UTC | #1
On Mon, Jun 9, 2014 at 1:19 AM, Jiang Liu <jiang.liu@linux.intel.com> wrote:
> If x2apic is enabled, the MSI high address field should also be aslo
> updated when setting affinity for MSI IRQ, otherwise the MSI IRQ may
> target wrong APIC IDs.

Do you have any test case to reveal the problem?

>
> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> Cc: stable@vger.kernel.org
> ---
>  arch/x86/kernel/apic/io_apic.c |    4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index 9d0a9795a0f8..2de992501a1b 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -3007,6 +3007,10 @@ msi_set_affinity(struct irq_data *data, const struct cpumask *mask, bool force)
>
>         __get_cached_msi_msg(data->msi_desc, &msg);
>
> +       msg.address_hi = MSI_ADDR_BASE_HI;
> +       if (x2apic_enabled())
> +               msg.address_hi |= MSI_ADDR_EXT_DEST_ID(dest);
> +
>         msg.data &= ~MSI_DATA_VECTOR_MASK;
>         msg.data |= MSI_DATA_VECTOR(cfg->vector);
>         msg.address_lo &= ~MSI_ADDR_DEST_ID_MASK;

No. This change is not needed.

When x2apic is used, and apicid > 255, irq remapping is used.

msi_chip.irq_set_affinity will be changed from msi_set_affinity to
x86_io_apic_ops.set_affinity (aka intel_setup_ioapic_entry),  via
irq_remap_modify_chip_defaults().

Thanks

Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Rientjes June 10, 2014, 12:22 a.m. UTC | #2
On Mon, 9 Jun 2014, Jiang Liu wrote:

> If x2apic is enabled, the MSI high address field should also be aslo
> updated when setting affinity for MSI IRQ, otherwise the MSI IRQ may
> target wrong APIC IDs.
> 
> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> Cc: stable@vger.kernel.org

Acked-by: David Rientjes <rientjes@google.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jiang Liu June 10, 2014, 2:54 a.m. UTC | #3
On 2014/6/10 7:46, Yinghai Lu wrote:
> On Mon, Jun 9, 2014 at 1:19 AM, Jiang Liu <jiang.liu@linux.intel.com> wrote:
>> If x2apic is enabled, the MSI high address field should also be aslo
>> updated when setting affinity for MSI IRQ, otherwise the MSI IRQ may
>> target wrong APIC IDs.
> 
> Do you have any test case to reveal the problem?
Just by code inspection.

> 
>>
>> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
>> Cc: stable@vger.kernel.org
>> ---
>>  arch/x86/kernel/apic/io_apic.c |    4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
>> index 9d0a9795a0f8..2de992501a1b 100644
>> --- a/arch/x86/kernel/apic/io_apic.c
>> +++ b/arch/x86/kernel/apic/io_apic.c
>> @@ -3007,6 +3007,10 @@ msi_set_affinity(struct irq_data *data, const struct cpumask *mask, bool force)
>>
>>         __get_cached_msi_msg(data->msi_desc, &msg);
>>
>> +       msg.address_hi = MSI_ADDR_BASE_HI;
>> +       if (x2apic_enabled())
>> +               msg.address_hi |= MSI_ADDR_EXT_DEST_ID(dest);
>> +
>>         msg.data &= ~MSI_DATA_VECTOR_MASK;
>>         msg.data |= MSI_DATA_VECTOR(cfg->vector);
>>         msg.address_lo &= ~MSI_ADDR_DEST_ID_MASK;
> 
> No. This change is not needed.
> 
> When x2apic is used, and apicid > 255, irq remapping is used.
> 
> msi_chip.irq_set_affinity will be changed from msi_set_affinity to
> x86_io_apic_ops.set_affinity (aka intel_setup_ioapic_entry),  via
> irq_remap_modify_chip_defaults().
Thanks for explanation, will drop this patch.
Gerry
> 
> Thanks
> 
> Yinghai
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 9d0a9795a0f8..2de992501a1b 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -3007,6 +3007,10 @@  msi_set_affinity(struct irq_data *data, const struct cpumask *mask, bool force)
 
 	__get_cached_msi_msg(data->msi_desc, &msg);
 
+	msg.address_hi = MSI_ADDR_BASE_HI;
+	if (x2apic_enabled())
+		msg.address_hi |= MSI_ADDR_EXT_DEST_ID(dest);
+
 	msg.data &= ~MSI_DATA_VECTOR_MASK;
 	msg.data |= MSI_DATA_VECTOR(cfg->vector);
 	msg.address_lo &= ~MSI_ADDR_DEST_ID_MASK;