From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: Sander Eikelenboom <linux@eikelenboom.it>,
Andrew Cooper <andrew.cooper3@citrix.com>,
Keir Fraser <keir@xen.org>, xen-devel <xen-devel@lists.xen.org>
Subject: Re: [PATCH RFC 2/2] AMD IOMMU: allow command line overrides for broken IVRS tables
Date: Wed, 28 Aug 2013 09:59:23 -0500 [thread overview]
Message-ID: <521E104B.4060607@amd.com> (raw)
In-Reply-To: <521C79F502000078000EEA56@nat28.tlf.novell.com>
[-- Attachment #1: Type: text/plain, Size: 5995 bytes --]
Jan,
The current patch does not handle the case where the "handle" value is
mis-configured in the IVRS.
I have also included the patch which has additional check. Below is the
output from the patch.
(XEN) AMD-Vi: IVHD Device Entry: type 0x48 id 0 flags 0xd7
(XEN) AMD-Vi: IVHD Special: 0000:00:14.0 variety 0x1 handle 0xff used_id
0xa0
(XEN) AMD-Vi: IVHD Special: Usinging command override value for IO-APIC
0x8, bdf=0xa0
(XEN) AMD-Vi: IVHD Device Entry: type 0x48 id 0 flags 0xd7
(XEN) AMD-Vi: IVHD Special: 0000:00:14.0 variety 0x2 handle 0 used_id 0xa0
(XEN) AMD-Vi: IVHD Device Entry: type 0x48 id 0 flags 0
(XEN) AMD-Vi: IVHD Special: 0000:00:00.1 variety 0x1 handle 0xff used_id 0x1
(XEN) AMD-Vi: IVHD Special: Usinging command override value for IO-APIC
0x8, bdf=0xa0
(XEN) AMD-Vi: IOMMU 0 Enabled.
(XEN) I/O virtualisation enabled
Suravee
------- PATCH SNIPPET-------
From 86bf8c318a43e409d404377f4534fe586785a5c4 Mon Sep 17 00:00:00 2001
From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Date: Wed, 28 Aug 2013 09:49:32 -0500
Subject: [PATCH] Add additional check invalid special->handle
This patch add an additional logic to check for the case when the
speciail->handle
is invalid due to firmware bugs, and use the overide value instead.
Signed-off: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
xen/drivers/passthrough/amd/iommu_acpi.c | 53
++++++++++++++++++++----------
1 file changed, 35 insertions(+), 18 deletions(-)
diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c
b/xen/drivers/passthrough/amd/iommu_acpi.c
index 89b359c..8fb4c3f 100644
--- a/xen/drivers/passthrough/amd/iommu_acpi.c
+++ b/xen/drivers/passthrough/amd/iommu_acpi.c
@@ -698,14 +698,16 @@ static u16 __init parse_ivhd_device_special(
return 0;
}
- AMD_IOMMU_DEBUG("IVHD Special: %04x:%02x:%02x.%u variety %#x handle
%#x\n",
+ AMD_IOMMU_DEBUG("IVHD Special: %04x:%02x:%02x.%u variety %#x handle
%#x used_id %#x\n",
seg, PCI_BUS(bdf), PCI_SLOT(bdf), PCI_FUNC(bdf),
- special->variety, special->handle);
+ special->variety, special->handle, special->used_id);
add_ivrs_mapping_entry(bdf, bdf, special->header.data_setting, iommu);
switch ( special->variety )
{
case ACPI_IVHD_IOAPIC:
+ {
+ unsigned int apic_id = 0;
if ( !iommu_intremap )
break;
/*
@@ -715,29 +717,43 @@ static u16 __init parse_ivhd_device_special(
*/
for ( apic = 0; apic < nr_ioapics; apic++ )
{
- if ( IO_APIC_ID(apic) != special->handle )
- continue;
+ apic_id = special->handle;
- if ( special->handle >= ARRAY_SIZE(ioapic_sbdf) )
+ if ( IO_APIC_ID(apic) != apic_id )
+ {
+ /* Some BIOSes have invalid value in the special->handle.
+ * Here we check to see if the user is overiding them
from commandline
+ */
+ if ( test_bit(IO_APIC_ID(apic), ioapic_cmdline) )
+ {
+ apic_id = IO_APIC_ID(apic);
+ AMD_IOMMU_DEBUG ("IVHD Special: Usinging command
override "
+ "value for IO-APIC %#x, bdf=%#x\n",
+ apic_id, ioapic_sbdf[apic_id].bdf);
+ }
+ else
+ continue;
+ }
+
+ if ( apic_id >= ARRAY_SIZE(ioapic_sbdf) )
{
printk(XENLOG_ERR "IVHD Error: IO-APIC %#x entry
beyond bounds\n",
- special->handle);
+ apic_id);
return 0;
}
- if ( test_bit(special->handle, ioapic_cmdline) )
- AMD_IOMMU_DEBUG("IVHD: Command line override present
for IO-APIC %#x\n",
- special->handle);
- else if ( ioapic_sbdf[special->handle].pin_2_idx )
+ if ( test_bit(apic_id, ioapic_cmdline) )
+ break;
+ else if ( ioapic_sbdf[apic_id].pin_2_idx )
{
- if ( ioapic_sbdf[special->handle].bdf == bdf &&
- ioapic_sbdf[special->handle].seg == seg )
+ if ( ioapic_sbdf[apic_id].bdf == bdf &&
+ ioapic_sbdf[apic_id].seg == seg )
AMD_IOMMU_DEBUG("IVHD Warning: Duplicate IO-APIC
%#x entries\n",
- special->handle);
+ apic_id);
else
{
printk(XENLOG_ERR "IVHD Error: Conflicting IO-APIC
%#x entries\n",
- special->handle);
+ apic_id);
if ( amd_iommu_perdev_intremap )
return 0;
}
@@ -745,10 +761,10 @@ static u16 __init parse_ivhd_device_special(
else
{
/* set device id of ioapic */
- ioapic_sbdf[special->handle].bdf = bdf;
- ioapic_sbdf[special->handle].seg = seg;
+ ioapic_sbdf[apic_id].bdf = bdf;
+ ioapic_sbdf[apic_id].seg = seg;
- ioapic_sbdf[special->handle].pin_2_idx = xmalloc_array(
+ ioapic_sbdf[apic_id].pin_2_idx = xmalloc_array(
u16, nr_ioapic_entries[apic]);
if ( nr_ioapic_entries[apic] &&
!ioapic_sbdf[IO_APIC_ID(apic)].pin_2_idx )
@@ -765,10 +781,11 @@ static u16 __init parse_ivhd_device_special(
if ( apic == nr_ioapics )
{
printk(XENLOG_ERR "IVHD Error: Invalid IO-APIC %#x\n",
- special->handle);
+ apic_id);
return 0;
}
break;
+ }
case ACPI_IVHD_HPET:
/* set device id of hpet */
if ( hpet_sbdf.iommu ||
--
1.7.10.4
[-- Attachment #2: 0001-Add-additional-check-for-when-the-handle-is-mis-conf.patch --]
[-- Type: text/plain, Size: 5064 bytes --]
>From 86bf8c318a43e409d404377f4534fe586785a5c4 Mon Sep 17 00:00:00 2001
From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Date: Wed, 28 Aug 2013 09:49:32 -0500
Subject: [PATCH] Add additional check invalid special->handle
This patch add an additional logic to check for the case when the speciail->handle
is invalid due to firmware bugs, and use the overide value instead.
Signed-off: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
xen/drivers/passthrough/amd/iommu_acpi.c | 53 ++++++++++++++++++++----------
1 file changed, 35 insertions(+), 18 deletions(-)
diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c b/xen/drivers/passthrough/amd/iommu_acpi.c
index 89b359c..8fb4c3f 100644
--- a/xen/drivers/passthrough/amd/iommu_acpi.c
+++ b/xen/drivers/passthrough/amd/iommu_acpi.c
@@ -698,14 +698,16 @@ static u16 __init parse_ivhd_device_special(
return 0;
}
- AMD_IOMMU_DEBUG("IVHD Special: %04x:%02x:%02x.%u variety %#x handle %#x\n",
+ AMD_IOMMU_DEBUG("IVHD Special: %04x:%02x:%02x.%u variety %#x handle %#x used_id %#x\n",
seg, PCI_BUS(bdf), PCI_SLOT(bdf), PCI_FUNC(bdf),
- special->variety, special->handle);
+ special->variety, special->handle, special->used_id);
add_ivrs_mapping_entry(bdf, bdf, special->header.data_setting, iommu);
switch ( special->variety )
{
case ACPI_IVHD_IOAPIC:
+ {
+ unsigned int apic_id = 0;
if ( !iommu_intremap )
break;
/*
@@ -715,29 +717,43 @@ static u16 __init parse_ivhd_device_special(
*/
for ( apic = 0; apic < nr_ioapics; apic++ )
{
- if ( IO_APIC_ID(apic) != special->handle )
- continue;
+ apic_id = special->handle;
- if ( special->handle >= ARRAY_SIZE(ioapic_sbdf) )
+ if ( IO_APIC_ID(apic) != apic_id )
+ {
+ /* Some BIOSes have invalid value in the special->handle.
+ * Here we check to see if the user is overiding them from commandline
+ */
+ if ( test_bit(IO_APIC_ID(apic), ioapic_cmdline) )
+ {
+ apic_id = IO_APIC_ID(apic);
+ AMD_IOMMU_DEBUG ("IVHD Special: Usinging command override "
+ "value for IO-APIC %#x, bdf=%#x\n",
+ apic_id, ioapic_sbdf[apic_id].bdf);
+ }
+ else
+ continue;
+ }
+
+ if ( apic_id >= ARRAY_SIZE(ioapic_sbdf) )
{
printk(XENLOG_ERR "IVHD Error: IO-APIC %#x entry beyond bounds\n",
- special->handle);
+ apic_id);
return 0;
}
- if ( test_bit(special->handle, ioapic_cmdline) )
- AMD_IOMMU_DEBUG("IVHD: Command line override present for IO-APIC %#x\n",
- special->handle);
- else if ( ioapic_sbdf[special->handle].pin_2_idx )
+ if ( test_bit(apic_id, ioapic_cmdline) )
+ break;
+ else if ( ioapic_sbdf[apic_id].pin_2_idx )
{
- if ( ioapic_sbdf[special->handle].bdf == bdf &&
- ioapic_sbdf[special->handle].seg == seg )
+ if ( ioapic_sbdf[apic_id].bdf == bdf &&
+ ioapic_sbdf[apic_id].seg == seg )
AMD_IOMMU_DEBUG("IVHD Warning: Duplicate IO-APIC %#x entries\n",
- special->handle);
+ apic_id);
else
{
printk(XENLOG_ERR "IVHD Error: Conflicting IO-APIC %#x entries\n",
- special->handle);
+ apic_id);
if ( amd_iommu_perdev_intremap )
return 0;
}
@@ -745,10 +761,10 @@ static u16 __init parse_ivhd_device_special(
else
{
/* set device id of ioapic */
- ioapic_sbdf[special->handle].bdf = bdf;
- ioapic_sbdf[special->handle].seg = seg;
+ ioapic_sbdf[apic_id].bdf = bdf;
+ ioapic_sbdf[apic_id].seg = seg;
- ioapic_sbdf[special->handle].pin_2_idx = xmalloc_array(
+ ioapic_sbdf[apic_id].pin_2_idx = xmalloc_array(
u16, nr_ioapic_entries[apic]);
if ( nr_ioapic_entries[apic] &&
!ioapic_sbdf[IO_APIC_ID(apic)].pin_2_idx )
@@ -765,10 +781,11 @@ static u16 __init parse_ivhd_device_special(
if ( apic == nr_ioapics )
{
printk(XENLOG_ERR "IVHD Error: Invalid IO-APIC %#x\n",
- special->handle);
+ apic_id);
return 0;
}
break;
+ }
case ACPI_IVHD_HPET:
/* set device id of hpet */
if ( hpet_sbdf.iommu ||
--
1.7.10.4
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
next prev parent reply other threads:[~2013-08-28 14:59 UTC|newest]
Thread overview: 71+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-22 20:50 [xen-unstable] Commit 2ca9fbd739b8a72b16dd790d0fff7b75f5488fb8 AMD IOMMU: allocate IRTE entries instead of using a static mapping, makes dom0 boot process stall several times Sander Eikelenboom
2013-08-05 7:48 ` Jan Beulich
2013-08-06 15:47 ` Suravee Suthikulanit
2013-08-15 14:43 ` Sander Eikelenboom
2013-08-15 15:15 ` Jan Beulich
2013-08-15 23:22 ` Sander Eikelenboom
2013-08-15 23:41 ` Sander Eikelenboom
2013-08-16 7:21 ` Jan Beulich
2013-08-16 7:42 ` Sander Eikelenboom
2013-08-16 8:03 ` Jan Beulich
2013-08-16 8:40 ` Sander Eikelenboom
2013-08-16 9:18 ` Jan Beulich
2013-08-16 10:44 ` Sander Eikelenboom
2013-08-16 13:15 ` Jan Beulich
2013-08-16 13:22 ` Sander Eikelenboom
2013-08-16 14:43 ` Sander Eikelenboom
2013-08-16 15:51 ` Sander Eikelenboom
2013-08-22 22:51 ` Sander Eikelenboom
2013-08-22 23:42 ` Sander Eikelenboom
2013-08-23 14:28 ` Jan Beulich
2013-08-23 14:45 ` Sander Eikelenboom
2013-08-23 15:05 ` Sander Eikelenboom
2013-08-23 15:11 ` Jan Beulich
2013-08-23 15:21 ` Jan Beulich
2013-08-23 15:48 ` Sander Eikelenboom
2013-08-23 16:01 ` Jan Beulich
2013-08-23 16:06 ` Sander Eikelenboom
2013-08-26 15:10 ` Suravee Suthikulpanit
2013-08-26 15:33 ` Jan Beulich
2013-08-23 17:01 ` Sander Eikelenboom
2013-08-23 15:29 ` Sander Eikelenboom
2013-08-26 6:59 ` Jan Beulich
2013-08-26 9:51 ` Sander Eikelenboom
2013-08-26 10:33 ` Jan Beulich
2013-08-26 11:07 ` Sander Eikelenboom
2013-08-26 11:23 ` Jan Beulich
2013-08-26 12:34 ` Sander Eikelenboom
2013-08-26 14:15 ` Jan Beulich
2013-08-26 14:35 ` Sander Eikelenboom
2013-08-26 11:21 ` Sander Eikelenboom
2013-08-26 11:25 ` Jan Beulich
2013-08-26 11:29 ` Jan Beulich
2013-08-26 11:36 ` Sander Eikelenboom
2013-08-26 15:37 ` Suravee Suthikulpanit
2013-08-26 15:50 ` Suravee Suthikulpanit
2013-08-27 8:23 ` Jan Beulich
2013-08-26 15:50 ` Jan Beulich
2013-08-26 16:19 ` Sander Eikelenboom
2013-08-27 8:00 ` [PATCH RFC 0/2] AMD IOMMU: allow command line overrides for broken IVRS tables Jan Beulich
2013-08-27 8:05 ` [PATCH RFC 1/2] PCI: centralize parsing of device coordinates in command line options Jan Beulich
2013-08-27 8:05 ` [PATCH RFC 2/2] AMD IOMMU: allow command line overrides for broken IVRS tables Jan Beulich
2013-08-27 9:52 ` Sander Eikelenboom
2013-08-27 11:09 ` Sander Eikelenboom
2013-08-27 12:04 ` Jan Beulich
2013-08-27 13:32 ` Sander Eikelenboom
2013-08-27 13:55 ` Jan Beulich
2013-08-27 14:15 ` Andrew Cooper
2013-08-27 16:17 ` Jan Beulich
2013-08-27 16:19 ` Sander Eikelenboom
2013-08-27 14:11 ` Jan Beulich
2013-08-27 14:15 ` Sander Eikelenboom
2013-08-27 15:59 ` Jan Beulich
2013-08-27 16:14 ` Sander Eikelenboom
2013-08-27 16:21 ` Jan Beulich
2013-08-27 16:30 ` Sander Eikelenboom
2013-08-27 17:09 ` Sander Eikelenboom
2013-08-28 8:06 ` Jan Beulich
2013-08-28 14:59 ` Suravee Suthikulpanit [this message]
2013-08-28 15:18 ` Jan Beulich
2013-08-27 18:07 ` [PATCH RFC 0/2] " Keir Fraser
[not found] <521E36AF02000078000EF2FD@nat28.tlf.novell.com>
2013-08-28 17:33 ` [RESEND] Re: [PATCH RFC 2/2] " Suravee Suthikulpanit
2013-08-29 6:22 ` Jan Beulich
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=521E104B.4060607@amd.com \
--to=suravee.suthikulpanit@amd.com \
--cc=JBeulich@suse.com \
--cc=andrew.cooper3@citrix.com \
--cc=keir@xen.org \
--cc=linux@eikelenboom.it \
--cc=xen-devel@lists.xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.