2010-12-01 06:56:34

by Suresh Siddha

[permalink] [raw]
Subject: [patch 2/4] x86, vtd: fix the vt-d fault handling irq migration in the x2apic mode

From: Kenji Kaneshige <[email protected]>
Subject: x86, vtd: fix the vt-d fault handling irq migration in the x2apic mode

In x2apic mode, we need to set the upper address register of the fault
handling interrupt register of the vt-d hardware. Without this
irq migration of the vt-d fault handling interrupt is broken.

Signed-off-by: Kenji Kaneshige <[email protected]>
Signed-off-by: Suresh Siddha <[email protected]>
Cc: [email protected] [v2.6.32+]
---
arch/x86/kernel/apic/io_apic.c | 2 ++
1 file changed, 2 insertions(+)

Index: tip/arch/x86/kernel/apic/io_apic.c
===================================================================
--- tip.orig/arch/x86/kernel/apic/io_apic.c
+++ tip/arch/x86/kernel/apic/io_apic.c
@@ -3367,6 +3367,8 @@ dmar_msi_set_affinity(struct irq_data *d
msg.data |= MSI_DATA_VECTOR(cfg->vector);
msg.address_lo &= ~MSI_ADDR_DEST_ID_MASK;
msg.address_lo |= MSI_ADDR_DEST_ID(dest);
+ if (x2apic_mode)
+ msg.address_hi = MSI_ADDR_BASE_HI | MSI_ADDR_EXT_DEST_ID(dest);

dmar_msi_write(irq, &msg);



2010-12-01 08:53:16

by Chris Wright

[permalink] [raw]
Subject: Re: [patch 2/4] x86, vtd: fix the vt-d fault handling irq migration in the x2apic mode

* Suresh Siddha ([email protected]) wrote:
> From: Kenji Kaneshige <[email protected]>
> Subject: x86, vtd: fix the vt-d fault handling irq migration in the x2apic mode
>
> In x2apic mode, we need to set the upper address register of the fault
> handling interrupt register of the vt-d hardware. Without this
> irq migration of the vt-d fault handling interrupt is broken.
>
> Signed-off-by: Kenji Kaneshige <[email protected]>
> Signed-off-by: Suresh Siddha <[email protected]>

Looks correct, I didn't have a chance to test this patch.

Acked-by: Chris Wright <[email protected]>

2010-12-01 15:14:11

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [patch 2/4] x86, vtd: fix the vt-d fault handling irq migration in the x2apic mode

On Tue, Nov 30, 2010 at 10:22:27PM -0800, Suresh Siddha wrote:
> From: Kenji Kaneshige <[email protected]>
> Subject: x86, vtd: fix the vt-d fault handling irq migration in the x2apic mode
>
> In x2apic mode, we need to set the upper address register of the fault
> handling interrupt register of the vt-d hardware. Without this
> irq migration of the vt-d fault handling interrupt is broken.
>
> Signed-off-by: Kenji Kaneshige <[email protected]>
> Signed-off-by: Suresh Siddha <[email protected]>
> Cc: [email protected] [v2.6.32+]
> ---
> arch/x86/kernel/apic/io_apic.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> Index: tip/arch/x86/kernel/apic/io_apic.c
> ===================================================================
> --- tip.orig/arch/x86/kernel/apic/io_apic.c
> +++ tip/arch/x86/kernel/apic/io_apic.c
> @@ -3367,6 +3367,8 @@ dmar_msi_set_affinity(struct irq_data *d
> msg.data |= MSI_DATA_VECTOR(cfg->vector);
> msg.address_lo &= ~MSI_ADDR_DEST_ID_MASK;
> msg.address_lo |= MSI_ADDR_DEST_ID(dest);
> + if (x2apic_mode)
> + msg.address_hi = MSI_ADDR_BASE_HI | MSI_ADDR_EXT_DEST_ID(dest);

Is it necessary to test x2apic_mode here? It looks like
MSI_ADDR_EXT_DEST_ID() gives you everything above the low 8
bits of the APIC ID. If those bits are always zero except in
x2apic_mode, we might not need the test.

Does the ia64 dmar_msi_set_affinity() need the same fix?

Why do we have both x2apic_enabled() and x2apic_mode? They
seem sort of redundant. (Not related to this patch, of course.)

Bjorn

2010-12-01 17:40:29

by Suresh Siddha

[permalink] [raw]
Subject: Re: [patch 2/4] x86, vtd: fix the vt-d fault handling irq migration in the x2apic mode

On Wed, 2010-12-01 at 07:14 -0800, Bjorn Helgaas wrote:
> On Tue, Nov 30, 2010 at 10:22:27PM -0800, Suresh Siddha wrote:
> > + if (x2apic_mode)
> > + msg.address_hi = MSI_ADDR_BASE_HI | MSI_ADDR_EXT_DEST_ID(dest);
>
> Is it necessary to test x2apic_mode here? It looks like
> MSI_ADDR_EXT_DEST_ID() gives you everything above the low 8
> bits of the APIC ID. If those bits are always zero except in
> x2apic_mode, we might not need the test.

True. Appended the updated patch.

> Does the ia64 dmar_msi_set_affinity() need the same fix?

No.

>
> Why do we have both x2apic_enabled() and x2apic_mode? They
> seem sort of redundant. (Not related to this patch, of course.)

BIOS can handover to OS in x2apic mode in some cases. x2apic_enabled()
is used to check for that and it reads the MSR to check the status. Some
early portions of the kernel boot will use it.

For all others, we should be using x2apic_mode.

thanks,
suresh
---

From: Kenji Kaneshige <[email protected]>
Subject: x86, vtd: fix the vt-d fault handling irq migration in the x2apic mode

In x2apic mode, we need to set the upper address register of the fault
handling interrupt register of the vt-d hardware. Without this
irq migration of the vt-d fault handling interrupt is broken.

Signed-off-by: Kenji Kaneshige <[email protected]>
Signed-off-by: Suresh Siddha <[email protected]>
Cc: [email protected] [v2.6.32+]
---
arch/x86/kernel/apic/io_apic.c | 1 +
1 file changed, 1 insertion(+)

Index: tip/arch/x86/kernel/apic/io_apic.c
===================================================================
--- tip.orig/arch/x86/kernel/apic/io_apic.c
+++ tip/arch/x86/kernel/apic/io_apic.c
@@ -3367,6 +3367,7 @@ dmar_msi_set_affinity(struct irq_data *d
msg.data |= MSI_DATA_VECTOR(cfg->vector);
msg.address_lo &= ~MSI_ADDR_DEST_ID_MASK;
msg.address_lo |= MSI_ADDR_DEST_ID(dest);
+ msg.address_hi = MSI_ADDR_BASE_HI | MSI_ADDR_EXT_DEST_ID(dest);

dmar_msi_write(irq, &msg);


2010-12-07 17:41:42

by Takao Indoh

[permalink] [raw]
Subject: Re: [patch 2/4] x86, vtd: fix the vt-d fault handling irq migration in the x2apic mode

On Wed, 01 Dec 2010 09:40:32 -0800, Suresh Siddha wrote:

>On Wed, 2010-12-01 at 07:14 -0800, Bjorn Helgaas wrote:
>> On Tue, Nov 30, 2010 at 10:22:27PM -0800, Suresh Siddha wrote:
>> > + if (x2apic_mode)
>> > + msg.address_hi = MSI_ADDR_BASE_HI | MSI_ADDR_EXT_DEST_ID
>> > (dest);
>>
>> Is it necessary to test x2apic_mode here? It looks like
>> MSI_ADDR_EXT_DEST_ID() gives you everything above the low 8
>> bits of the APIC ID. If those bits are always zero except in
>> x2apic_mode, we might not need the test.
>
>True. Appended the updated patch.

I applied this patch against 2.6.36 and confirmed irq migration of
vt-d fault worked.

Tested-by: Takao Indoh <[email protected]>

Thanks,
Takao Indoh

>
>> Does the ia64 dmar_msi_set_affinity() need the same fix?
>
>No.
>
>>
>> Why do we have both x2apic_enabled() and x2apic_mode? They
>> seem sort of redundant. (Not related to this patch, of course.)
>
>BIOS can handover to OS in x2apic mode in some cases. x2apic_enabled()
>is used to check for that and it reads the MSR to check the status. Some
>early portions of the kernel boot will use it.
>
>For all others, we should be using x2apic_mode.
>
>thanks,
>suresh
>---
>
>From: Kenji Kaneshige <[email protected]>
>Subject: x86, vtd: fix the vt-d fault handling irq migration in the x2apic
>mode
>
>In x2apic mode, we need to set the upper address register of the fault
>handling interrupt register of the vt-d hardware. Without this
>irq migration of the vt-d fault handling interrupt is broken.
>
>Signed-off-by: Kenji Kaneshige <[email protected]>
>Signed-off-by: Suresh Siddha <[email protected]>
>Cc: [email protected] [v2.6.32+]
>---
> arch/x86/kernel/apic/io_apic.c | 1 +
> 1 file changed, 1 insertion(+)
>
>Index: tip/arch/x86/kernel/apic/io_apic.c
>===================================================================
>--- tip.orig/arch/x86/kernel/apic/io_apic.c
>+++ tip/arch/x86/kernel/apic/io_apic.c
>@@ -3367,6 +3367,7 @@ dmar_msi_set_affinity(struct irq_data *d
> msg.data |= MSI_DATA_VECTOR(cfg->vector);
> msg.address_lo &= ~MSI_ADDR_DEST_ID_MASK;
> msg.address_lo |= MSI_ADDR_DEST_ID(dest);
>+ msg.address_hi = MSI_ADDR_BASE_HI | MSI_ADDR_EXT_DEST_ID(dest);
>
> dmar_msi_write(irq, &msg);
>


---
印藤隆夫(INDOH Takao)
E-Mail : [email protected]

2010-12-14 01:16:42

by Kenji Kaneshige

[permalink] [raw]
Subject: [tip:x86/urgent] x86, vt-d: Fix the vt-d fault handling irq migration in the x2apic mode

Commit-ID: 086e8ced65d9bcc4a8e8f1cd39b09640f2883f90
Gitweb: http://git.kernel.org/tip/086e8ced65d9bcc4a8e8f1cd39b09640f2883f90
Author: Kenji Kaneshige <[email protected]>
AuthorDate: Wed, 1 Dec 2010 09:40:32 -0800
Committer: H. Peter Anvin <[email protected]>
CommitDate: Mon, 13 Dec 2010 16:52:52 -0800

x86, vt-d: Fix the vt-d fault handling irq migration in the x2apic mode

In x2apic mode, we need to set the upper address register of the fault
handling interrupt register of the vt-d hardware. Without this
irq migration of the vt-d fault handling interrupt is broken.

Signed-off-by: Kenji Kaneshige <[email protected]>
LKML-Reference: <1291225233.2648.39.camel@sbsiddha-MOBL3>
Signed-off-by: Suresh Siddha <[email protected]>
Cc: [email protected] [v2.6.32+]
Acked-by: Chris Wright <[email protected]>
Tested-by: Takao Indoh <[email protected]>
Signed-off-by: H. Peter Anvin <[email protected]>
---
arch/x86/kernel/apic/io_apic.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 226060e..fadcd74 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -3412,6 +3412,7 @@ dmar_msi_set_affinity(struct irq_data *data, const struct cpumask *mask,
msg.data |= MSI_DATA_VECTOR(cfg->vector);
msg.address_lo &= ~MSI_ADDR_DEST_ID_MASK;
msg.address_lo |= MSI_ADDR_DEST_ID(dest);
+ msg.address_hi = MSI_ADDR_BASE_HI | MSI_ADDR_EXT_DEST_ID(dest);

dmar_msi_write(irq, &msg);