* [PATCH 1/1 V4] x86/AMD: Fix setup ssss:bb:dd:f for d0 failed
@ 2013-09-11 19:39 suravee.suthikulpanit
2013-09-12 8:17 ` [PATCH v5] AMD IOMMU: fix Dom0 device setup failure for host bridges Jan Beulich
0 siblings, 1 reply; 9+ messages in thread
From: suravee.suthikulpanit @ 2013-09-11 19:39 UTC (permalink / raw)
To: JBeulich, xiantao.zhang; +Cc: Suravee Suthikulpanit, stefan.bader, xen-devel
From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
The host bridge device (i.e. 0x18 for AMD) does not
require IOMMU, and therefore is not included in the IVRS.
The current logic tries to map all PCI devices to an IOMMU.
In this case, "xl dmesg" shows the following message on AMD sytem.
(XEN) setup 0000:00:18.0 for d0 failed (-19)
(XEN) setup 0000:00:18.1 for d0 failed (-19)
(XEN) setup 0000:00:18.2 for d0 failed (-19)
(XEN) setup 0000:00:18.3 for d0 failed (-19)
(XEN) setup 0000:00:18.4 for d0 failed (-19)
(XEN) setup 0000:00:18.5 for d0 failed (-19)
This patch add new device type (i.e. DEV_TYPE_PCI_HOST_BRIDGE) which
is corresponded to PCI class code 0x06 and sub-class 0x00. Then, it
use this new type to filter when trying to map device to IOMMU.
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Reported-by: Stefan Bader <stefan.bader@canonical.com>
---
Changes for V4:
- Minor clean up
xen/drivers/passthrough/amd/pci_amd_iommu.c | 13 +++++++++++--
xen/drivers/passthrough/pci.c | 9 +++++----
xen/drivers/passthrough/vtd/intremap.c | 1 +
xen/drivers/passthrough/vtd/iommu.c | 14 ++++++++++++++
xen/include/xen/pci.h | 1 +
5 files changed, 32 insertions(+), 6 deletions(-)
diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/amd/pci_amd_iommu.c
index 9684ae8..501b56c 100644
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -147,9 +147,10 @@ static void amd_iommu_setup_domain_device(
amd_iommu_flush_device(iommu, req_id);
- AMD_IOMMU_DEBUG("Setup I/O page table: device id = %#x, "
+ AMD_IOMMU_DEBUG("Setup I/O page table: device id = %#x, type = %#x, "
"root table = %#"PRIx64", "
- "domain = %d, paging mode = %d\n", req_id,
+ "domain = %d, paging mode = %d\n",
+ req_id, pdev->type,
page_to_maddr(hd->root_table),
hd->domain_id, hd->paging_mode);
}
@@ -175,6 +176,14 @@ static int __init amd_iommu_setup_dom0_device(u8 devfn, struct pci_dev *pdev)
if ( unlikely(!iommu) )
{
+ /* Filter the bridge devices */
+ if ( pdev->type == DEV_TYPE_PCI_HOST_BRIDGE )
+ {
+ AMD_IOMMU_DEBUG("Skipping host bridge %04x:%02x:%02x.%u\n",
+ pdev->seg, PCI_BUS(bdf), PCI_SLOT(bdf), PCI_FUNC(bdf));
+ return 0;
+ }
+
AMD_IOMMU_DEBUG("No iommu for device %04x:%02x:%02x.%u\n",
pdev->seg, pdev->bus,
PCI_SLOT(devfn), PCI_FUNC(devfn));
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index b488e2a..c151bc8 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -203,9 +203,6 @@ static struct pci_dev *alloc_pdev(struct pci_seg *pseg, u8 bus, u8 devfn)
u16 cap;
u8 sec_bus, sub_bus;
- case DEV_TYPE_PCIe_BRIDGE:
- break;
-
case DEV_TYPE_PCIe2PCI_BRIDGE:
case DEV_TYPE_LEGACY_PCI_BRIDGE:
sec_bus = pci_conf_read8(pseg->nr, bus, PCI_SLOT(devfn),
@@ -253,6 +250,8 @@ static struct pci_dev *alloc_pdev(struct pci_seg *pseg, u8 bus, u8 devfn)
break;
case DEV_TYPE_PCI:
+ case DEV_TYPE_PCIe_BRIDGE:
+ case DEV_TYPE_PCI_HOST_BRIDGE:
break;
default:
@@ -706,6 +705,7 @@ void pci_release_devices(struct domain *d)
spin_unlock(&pcidevs_lock);
}
+#define PCI_CLASS_BRIDGE_HOST 0x0600
#define PCI_CLASS_BRIDGE_PCI 0x0604
enum pdev_type pdev_type(u16 seg, u8 bus, u8 devfn)
@@ -729,7 +729,8 @@ enum pdev_type pdev_type(u16 seg, u8 bus, u8 devfn)
return DEV_TYPE_PCI2PCIe_BRIDGE;
}
return DEV_TYPE_PCIe_BRIDGE;
-
+ case PCI_CLASS_BRIDGE_HOST:
+ return DEV_TYPE_PCI_HOST_BRIDGE;
case 0x0000: case 0xffff:
return DEV_TYPE_PCI_UNKNOWN;
}
diff --git a/xen/drivers/passthrough/vtd/intremap.c b/xen/drivers/passthrough/vtd/intremap.c
index f3bb31b..2f8b9eb 100644
--- a/xen/drivers/passthrough/vtd/intremap.c
+++ b/xen/drivers/passthrough/vtd/intremap.c
@@ -442,6 +442,7 @@ static void set_msi_source_id(struct pci_dev *pdev, struct iremap_entry *ire)
case DEV_TYPE_PCIe_ENDPOINT:
case DEV_TYPE_PCIe_BRIDGE:
case DEV_TYPE_PCIe2PCI_BRIDGE:
+ case DEV_TYPE_PCI_HOST_BRIDGE:
switch ( pdev->phantom_stride )
{
case 1: sq = SQ_13_IGNORE_3; break;
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index fd3abcb..288ebaf 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1433,6 +1433,13 @@ static int domain_context_mapping(
switch ( pdev->type )
{
+ case DEV_TYPE_PCI_HOST_BRIDGE:
+ if (iommu_verbose)
+ dprintk(VTDPREFIX, "d%d:Hostbridge map skip %04x:%02x:%02x.%u\n",
+ domain->domain_id, seg, bus,
+ PCI_SLOT(devfn), PCI_FUNC(devfn));
+ break;
+
case DEV_TYPE_PCIe_BRIDGE:
case DEV_TYPE_PCIe2PCI_BRIDGE:
case DEV_TYPE_LEGACY_PCI_BRIDGE:
@@ -1563,6 +1570,13 @@ static int domain_context_unmap(
switch ( pdev->type )
{
+ case DEV_TYPE_PCI_HOST_BRIDGE:
+ if (iommu_verbose)
+ dprintk(VTDPREFIX, "d%d:Hostbridge unmap skip %04x:%02x:%02x.%u\n",
+ domain->domain_id, seg, bus,
+ PCI_SLOT(devfn), PCI_FUNC(devfn));
+ goto out;
+
case DEV_TYPE_PCIe_BRIDGE:
case DEV_TYPE_PCIe2PCI_BRIDGE:
case DEV_TYPE_LEGACY_PCI_BRIDGE:
diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index c367736..d33a15d 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -63,6 +63,7 @@ struct pci_dev {
DEV_TYPE_PCIe2PCI_BRIDGE, // PCIe-to-PCI/PCIx bridge
DEV_TYPE_PCI2PCIe_BRIDGE, // PCI/PCIx-to-PCIe bridge
DEV_TYPE_LEGACY_PCI_BRIDGE, // Legacy PCI bridge
+ DEV_TYPE_PCI_HOST_BRIDGE, // PCI Host bridge
DEV_TYPE_PCI,
} type;
--
1.8.1.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v5] AMD IOMMU: fix Dom0 device setup failure for host bridges
2013-09-11 19:39 [PATCH 1/1 V4] x86/AMD: Fix setup ssss:bb:dd:f for d0 failed suravee.suthikulpanit
@ 2013-09-12 8:17 ` Jan Beulich
2013-09-12 9:42 ` Stefan Bader
2013-09-18 12:51 ` Ping: " Jan Beulich
0 siblings, 2 replies; 9+ messages in thread
From: Jan Beulich @ 2013-09-12 8:17 UTC (permalink / raw)
To: suravee.suthikulpanit, xiantao.zhang; +Cc: stefan.bader, xen-devel
[-- Attachment #1: Type: text/plain, Size: 5535 bytes --]
The host bridge device (i.e. 0x18 for AMD) does not require IOMMU, and
therefore is not included in the IVRS. The current logic tries to map
all PCI devices to an IOMMU. In this case, "xl dmesg" shows the
following message on AMD sytem.
(XEN) setup 0000:00:18.0 for d0 failed (-19)
(XEN) setup 0000:00:18.1 for d0 failed (-19)
(XEN) setup 0000:00:18.2 for d0 failed (-19)
(XEN) setup 0000:00:18.3 for d0 failed (-19)
(XEN) setup 0000:00:18.4 for d0 failed (-19)
(XEN) setup 0000:00:18.5 for d0 failed (-19)
This patch adds a new device type (i.e. DEV_TYPE_PCI_HOST_BRIDGE) which
corresponds to PCI class code 0x06 and sub-class 0x00. Then, it uses
this new type to filter when trying to map device to IOMMU.
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Reported-by: Stefan Bader <stefan.bader@canonical.com>
On VT-d refuse (un)mapping host bridges for other than the hardware
domain.
Coding style cleanup.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -147,9 +147,10 @@ static void amd_iommu_setup_domain_devic
amd_iommu_flush_device(iommu, req_id);
- AMD_IOMMU_DEBUG("Setup I/O page table: device id = %#x, "
+ AMD_IOMMU_DEBUG("Setup I/O page table: device id = %#x, type = %#x, "
"root table = %#"PRIx64", "
- "domain = %d, paging mode = %d\n", req_id,
+ "domain = %d, paging mode = %d\n",
+ req_id, pdev->type,
page_to_maddr(hd->root_table),
hd->domain_id, hd->paging_mode);
}
@@ -175,6 +176,15 @@ static int __init amd_iommu_setup_dom0_d
if ( unlikely(!iommu) )
{
+ /* Filter the bridge devices */
+ if ( pdev->type == DEV_TYPE_PCI_HOST_BRIDGE )
+ {
+ AMD_IOMMU_DEBUG("Skipping host bridge %04x:%02x:%02x.%u\n",
+ pdev->seg, PCI_BUS(bdf), PCI_SLOT(bdf),
+ PCI_FUNC(bdf));
+ return 0;
+ }
+
AMD_IOMMU_DEBUG("No iommu for device %04x:%02x:%02x.%u\n",
pdev->seg, pdev->bus,
PCI_SLOT(devfn), PCI_FUNC(devfn));
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -194,9 +194,6 @@ static struct pci_dev *alloc_pdev(struct
u16 cap;
u8 sec_bus, sub_bus;
- case DEV_TYPE_PCIe_BRIDGE:
- break;
-
case DEV_TYPE_PCIe2PCI_BRIDGE:
case DEV_TYPE_LEGACY_PCI_BRIDGE:
sec_bus = pci_conf_read8(pseg->nr, bus, PCI_SLOT(devfn),
@@ -244,6 +241,8 @@ static struct pci_dev *alloc_pdev(struct
break;
case DEV_TYPE_PCI:
+ case DEV_TYPE_PCIe_BRIDGE:
+ case DEV_TYPE_PCI_HOST_BRIDGE:
break;
default:
@@ -697,6 +696,7 @@ void pci_release_devices(struct domain *
spin_unlock(&pcidevs_lock);
}
+#define PCI_CLASS_BRIDGE_HOST 0x0600
#define PCI_CLASS_BRIDGE_PCI 0x0604
enum pdev_type pdev_type(u16 seg, u8 bus, u8 devfn)
@@ -720,6 +720,8 @@ enum pdev_type pdev_type(u16 seg, u8 bus
return DEV_TYPE_PCI2PCIe_BRIDGE;
}
return DEV_TYPE_PCIe_BRIDGE;
+ case PCI_CLASS_BRIDGE_HOST:
+ return DEV_TYPE_PCI_HOST_BRIDGE;
case 0x0000: case 0xffff:
return DEV_TYPE_PCI_UNKNOWN;
--- a/xen/drivers/passthrough/vtd/intremap.c
+++ b/xen/drivers/passthrough/vtd/intremap.c
@@ -442,6 +442,7 @@ static void set_msi_source_id(struct pci
case DEV_TYPE_PCIe_ENDPOINT:
case DEV_TYPE_PCIe_BRIDGE:
case DEV_TYPE_PCIe2PCI_BRIDGE:
+ case DEV_TYPE_PCI_HOST_BRIDGE:
switch ( pdev->phantom_stride )
{
case 1: sq = SQ_13_IGNORE_3; break;
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1433,6 +1433,15 @@ static int domain_context_mapping(
switch ( pdev->type )
{
+ case DEV_TYPE_PCI_HOST_BRIDGE:
+ if ( iommu_verbose )
+ dprintk(VTDPREFIX, "d%d:Hostbridge: skip %04x:%02x:%02x.%u map\n",
+ domain->domain_id, seg, bus,
+ PCI_SLOT(devfn), PCI_FUNC(devfn));
+ if ( !is_hardware_domain(domain) )
+ return -EPERM;
+ break;
+
case DEV_TYPE_PCIe_BRIDGE:
case DEV_TYPE_PCIe2PCI_BRIDGE:
case DEV_TYPE_LEGACY_PCI_BRIDGE:
@@ -1563,6 +1572,15 @@ static int domain_context_unmap(
switch ( pdev->type )
{
+ case DEV_TYPE_PCI_HOST_BRIDGE:
+ if ( iommu_verbose )
+ dprintk(VTDPREFIX, "d%d:Hostbridge: skip %04x:%02x:%02x.%u unmap\n",
+ domain->domain_id, seg, bus,
+ PCI_SLOT(devfn), PCI_FUNC(devfn));
+ if ( !is_hardware_domain(domain) )
+ return -EPERM;
+ goto out;
+
case DEV_TYPE_PCIe_BRIDGE:
case DEV_TYPE_PCIe2PCI_BRIDGE:
case DEV_TYPE_LEGACY_PCI_BRIDGE:
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -63,6 +63,7 @@ struct pci_dev {
DEV_TYPE_PCIe2PCI_BRIDGE, // PCIe-to-PCI/PCIx bridge
DEV_TYPE_PCI2PCIe_BRIDGE, // PCI/PCIx-to-PCIe bridge
DEV_TYPE_LEGACY_PCI_BRIDGE, // Legacy PCI bridge
+ DEV_TYPE_PCI_HOST_BRIDGE, // PCI Host bridge
DEV_TYPE_PCI,
} type;
[-- Attachment #2: AMD-IOMMU-suppress-hostbridge-setup.patch --]
[-- Type: text/plain, Size: 5592 bytes --]
AMD IOMMU: fix Dom0 device setup failure for host bridges
The host bridge device (i.e. 0x18 for AMD) does not require IOMMU, and
therefore is not included in the IVRS. The current logic tries to map
all PCI devices to an IOMMU. In this case, "xl dmesg" shows the
following message on AMD sytem.
(XEN) setup 0000:00:18.0 for d0 failed (-19)
(XEN) setup 0000:00:18.1 for d0 failed (-19)
(XEN) setup 0000:00:18.2 for d0 failed (-19)
(XEN) setup 0000:00:18.3 for d0 failed (-19)
(XEN) setup 0000:00:18.4 for d0 failed (-19)
(XEN) setup 0000:00:18.5 for d0 failed (-19)
This patch adds a new device type (i.e. DEV_TYPE_PCI_HOST_BRIDGE) which
corresponds to PCI class code 0x06 and sub-class 0x00. Then, it uses
this new type to filter when trying to map device to IOMMU.
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Reported-by: Stefan Bader <stefan.bader@canonical.com>
On VT-d refuse (un)mapping host bridges for other than the hardware
domain.
Coding style cleanup.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -147,9 +147,10 @@ static void amd_iommu_setup_domain_devic
amd_iommu_flush_device(iommu, req_id);
- AMD_IOMMU_DEBUG("Setup I/O page table: device id = %#x, "
+ AMD_IOMMU_DEBUG("Setup I/O page table: device id = %#x, type = %#x, "
"root table = %#"PRIx64", "
- "domain = %d, paging mode = %d\n", req_id,
+ "domain = %d, paging mode = %d\n",
+ req_id, pdev->type,
page_to_maddr(hd->root_table),
hd->domain_id, hd->paging_mode);
}
@@ -175,6 +176,15 @@ static int __init amd_iommu_setup_dom0_d
if ( unlikely(!iommu) )
{
+ /* Filter the bridge devices */
+ if ( pdev->type == DEV_TYPE_PCI_HOST_BRIDGE )
+ {
+ AMD_IOMMU_DEBUG("Skipping host bridge %04x:%02x:%02x.%u\n",
+ pdev->seg, PCI_BUS(bdf), PCI_SLOT(bdf),
+ PCI_FUNC(bdf));
+ return 0;
+ }
+
AMD_IOMMU_DEBUG("No iommu for device %04x:%02x:%02x.%u\n",
pdev->seg, pdev->bus,
PCI_SLOT(devfn), PCI_FUNC(devfn));
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -194,9 +194,6 @@ static struct pci_dev *alloc_pdev(struct
u16 cap;
u8 sec_bus, sub_bus;
- case DEV_TYPE_PCIe_BRIDGE:
- break;
-
case DEV_TYPE_PCIe2PCI_BRIDGE:
case DEV_TYPE_LEGACY_PCI_BRIDGE:
sec_bus = pci_conf_read8(pseg->nr, bus, PCI_SLOT(devfn),
@@ -244,6 +241,8 @@ static struct pci_dev *alloc_pdev(struct
break;
case DEV_TYPE_PCI:
+ case DEV_TYPE_PCIe_BRIDGE:
+ case DEV_TYPE_PCI_HOST_BRIDGE:
break;
default:
@@ -697,6 +696,7 @@ void pci_release_devices(struct domain *
spin_unlock(&pcidevs_lock);
}
+#define PCI_CLASS_BRIDGE_HOST 0x0600
#define PCI_CLASS_BRIDGE_PCI 0x0604
enum pdev_type pdev_type(u16 seg, u8 bus, u8 devfn)
@@ -720,6 +720,8 @@ enum pdev_type pdev_type(u16 seg, u8 bus
return DEV_TYPE_PCI2PCIe_BRIDGE;
}
return DEV_TYPE_PCIe_BRIDGE;
+ case PCI_CLASS_BRIDGE_HOST:
+ return DEV_TYPE_PCI_HOST_BRIDGE;
case 0x0000: case 0xffff:
return DEV_TYPE_PCI_UNKNOWN;
--- a/xen/drivers/passthrough/vtd/intremap.c
+++ b/xen/drivers/passthrough/vtd/intremap.c
@@ -442,6 +442,7 @@ static void set_msi_source_id(struct pci
case DEV_TYPE_PCIe_ENDPOINT:
case DEV_TYPE_PCIe_BRIDGE:
case DEV_TYPE_PCIe2PCI_BRIDGE:
+ case DEV_TYPE_PCI_HOST_BRIDGE:
switch ( pdev->phantom_stride )
{
case 1: sq = SQ_13_IGNORE_3; break;
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1433,6 +1433,15 @@ static int domain_context_mapping(
switch ( pdev->type )
{
+ case DEV_TYPE_PCI_HOST_BRIDGE:
+ if ( iommu_verbose )
+ dprintk(VTDPREFIX, "d%d:Hostbridge: skip %04x:%02x:%02x.%u map\n",
+ domain->domain_id, seg, bus,
+ PCI_SLOT(devfn), PCI_FUNC(devfn));
+ if ( !is_hardware_domain(domain) )
+ return -EPERM;
+ break;
+
case DEV_TYPE_PCIe_BRIDGE:
case DEV_TYPE_PCIe2PCI_BRIDGE:
case DEV_TYPE_LEGACY_PCI_BRIDGE:
@@ -1563,6 +1572,15 @@ static int domain_context_unmap(
switch ( pdev->type )
{
+ case DEV_TYPE_PCI_HOST_BRIDGE:
+ if ( iommu_verbose )
+ dprintk(VTDPREFIX, "d%d:Hostbridge: skip %04x:%02x:%02x.%u unmap\n",
+ domain->domain_id, seg, bus,
+ PCI_SLOT(devfn), PCI_FUNC(devfn));
+ if ( !is_hardware_domain(domain) )
+ return -EPERM;
+ goto out;
+
case DEV_TYPE_PCIe_BRIDGE:
case DEV_TYPE_PCIe2PCI_BRIDGE:
case DEV_TYPE_LEGACY_PCI_BRIDGE:
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -63,6 +63,7 @@ struct pci_dev {
DEV_TYPE_PCIe2PCI_BRIDGE, // PCIe-to-PCI/PCIx bridge
DEV_TYPE_PCI2PCIe_BRIDGE, // PCI/PCIx-to-PCIe bridge
DEV_TYPE_LEGACY_PCI_BRIDGE, // Legacy PCI bridge
+ DEV_TYPE_PCI_HOST_BRIDGE, // PCI Host bridge
DEV_TYPE_PCI,
} type;
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5] AMD IOMMU: fix Dom0 device setup failure for host bridges
2013-09-12 8:17 ` [PATCH v5] AMD IOMMU: fix Dom0 device setup failure for host bridges Jan Beulich
@ 2013-09-12 9:42 ` Stefan Bader
2013-09-18 12:51 ` Ping: " Jan Beulich
1 sibling, 0 replies; 9+ messages in thread
From: Stefan Bader @ 2013-09-12 9:42 UTC (permalink / raw)
To: Jan Beulich; +Cc: xiantao.zhang, suravee.suthikulpanit, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 5974 bytes --]
On 12.09.2013 10:17, Jan Beulich wrote:
> The host bridge device (i.e. 0x18 for AMD) does not require IOMMU, and
> therefore is not included in the IVRS. The current logic tries to map
> all PCI devices to an IOMMU. In this case, "xl dmesg" shows the
> following message on AMD sytem.
>
> (XEN) setup 0000:00:18.0 for d0 failed (-19)
> (XEN) setup 0000:00:18.1 for d0 failed (-19)
> (XEN) setup 0000:00:18.2 for d0 failed (-19)
> (XEN) setup 0000:00:18.3 for d0 failed (-19)
> (XEN) setup 0000:00:18.4 for d0 failed (-19)
> (XEN) setup 0000:00:18.5 for d0 failed (-19)
>
> This patch adds a new device type (i.e. DEV_TYPE_PCI_HOST_BRIDGE) which
> corresponds to PCI class code 0x06 and sub-class 0x00. Then, it uses
> this new type to filter when trying to map device to IOMMU.
>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Reported-by: Stefan Bader <stefan.bader@canonical.com>
>
> On VT-d refuse (un)mapping host bridges for other than the hardware
> domain.
>
> Coding style cleanup.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
As this looks to be about final.
Tested-by: Stefan Bader <stefan.bader@canonical.com>
>
> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
> +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> @@ -147,9 +147,10 @@ static void amd_iommu_setup_domain_devic
>
> amd_iommu_flush_device(iommu, req_id);
>
> - AMD_IOMMU_DEBUG("Setup I/O page table: device id = %#x, "
> + AMD_IOMMU_DEBUG("Setup I/O page table: device id = %#x, type = %#x, "
> "root table = %#"PRIx64", "
> - "domain = %d, paging mode = %d\n", req_id,
> + "domain = %d, paging mode = %d\n",
> + req_id, pdev->type,
> page_to_maddr(hd->root_table),
> hd->domain_id, hd->paging_mode);
> }
> @@ -175,6 +176,15 @@ static int __init amd_iommu_setup_dom0_d
>
> if ( unlikely(!iommu) )
> {
> + /* Filter the bridge devices */
> + if ( pdev->type == DEV_TYPE_PCI_HOST_BRIDGE )
> + {
> + AMD_IOMMU_DEBUG("Skipping host bridge %04x:%02x:%02x.%u\n",
> + pdev->seg, PCI_BUS(bdf), PCI_SLOT(bdf),
> + PCI_FUNC(bdf));
> + return 0;
> + }
> +
> AMD_IOMMU_DEBUG("No iommu for device %04x:%02x:%02x.%u\n",
> pdev->seg, pdev->bus,
> PCI_SLOT(devfn), PCI_FUNC(devfn));
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -194,9 +194,6 @@ static struct pci_dev *alloc_pdev(struct
> u16 cap;
> u8 sec_bus, sub_bus;
>
> - case DEV_TYPE_PCIe_BRIDGE:
> - break;
> -
> case DEV_TYPE_PCIe2PCI_BRIDGE:
> case DEV_TYPE_LEGACY_PCI_BRIDGE:
> sec_bus = pci_conf_read8(pseg->nr, bus, PCI_SLOT(devfn),
> @@ -244,6 +241,8 @@ static struct pci_dev *alloc_pdev(struct
> break;
>
> case DEV_TYPE_PCI:
> + case DEV_TYPE_PCIe_BRIDGE:
> + case DEV_TYPE_PCI_HOST_BRIDGE:
> break;
>
> default:
> @@ -697,6 +696,7 @@ void pci_release_devices(struct domain *
> spin_unlock(&pcidevs_lock);
> }
>
> +#define PCI_CLASS_BRIDGE_HOST 0x0600
> #define PCI_CLASS_BRIDGE_PCI 0x0604
>
> enum pdev_type pdev_type(u16 seg, u8 bus, u8 devfn)
> @@ -720,6 +720,8 @@ enum pdev_type pdev_type(u16 seg, u8 bus
> return DEV_TYPE_PCI2PCIe_BRIDGE;
> }
> return DEV_TYPE_PCIe_BRIDGE;
> + case PCI_CLASS_BRIDGE_HOST:
> + return DEV_TYPE_PCI_HOST_BRIDGE;
>
> case 0x0000: case 0xffff:
> return DEV_TYPE_PCI_UNKNOWN;
> --- a/xen/drivers/passthrough/vtd/intremap.c
> +++ b/xen/drivers/passthrough/vtd/intremap.c
> @@ -442,6 +442,7 @@ static void set_msi_source_id(struct pci
> case DEV_TYPE_PCIe_ENDPOINT:
> case DEV_TYPE_PCIe_BRIDGE:
> case DEV_TYPE_PCIe2PCI_BRIDGE:
> + case DEV_TYPE_PCI_HOST_BRIDGE:
> switch ( pdev->phantom_stride )
> {
> case 1: sq = SQ_13_IGNORE_3; break;
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -1433,6 +1433,15 @@ static int domain_context_mapping(
>
> switch ( pdev->type )
> {
> + case DEV_TYPE_PCI_HOST_BRIDGE:
> + if ( iommu_verbose )
> + dprintk(VTDPREFIX, "d%d:Hostbridge: skip %04x:%02x:%02x.%u map\n",
> + domain->domain_id, seg, bus,
> + PCI_SLOT(devfn), PCI_FUNC(devfn));
> + if ( !is_hardware_domain(domain) )
> + return -EPERM;
> + break;
> +
> case DEV_TYPE_PCIe_BRIDGE:
> case DEV_TYPE_PCIe2PCI_BRIDGE:
> case DEV_TYPE_LEGACY_PCI_BRIDGE:
> @@ -1563,6 +1572,15 @@ static int domain_context_unmap(
>
> switch ( pdev->type )
> {
> + case DEV_TYPE_PCI_HOST_BRIDGE:
> + if ( iommu_verbose )
> + dprintk(VTDPREFIX, "d%d:Hostbridge: skip %04x:%02x:%02x.%u unmap\n",
> + domain->domain_id, seg, bus,
> + PCI_SLOT(devfn), PCI_FUNC(devfn));
> + if ( !is_hardware_domain(domain) )
> + return -EPERM;
> + goto out;
> +
> case DEV_TYPE_PCIe_BRIDGE:
> case DEV_TYPE_PCIe2PCI_BRIDGE:
> case DEV_TYPE_LEGACY_PCI_BRIDGE:
> --- a/xen/include/xen/pci.h
> +++ b/xen/include/xen/pci.h
> @@ -63,6 +63,7 @@ struct pci_dev {
> DEV_TYPE_PCIe2PCI_BRIDGE, // PCIe-to-PCI/PCIx bridge
> DEV_TYPE_PCI2PCIe_BRIDGE, // PCI/PCIx-to-PCIe bridge
> DEV_TYPE_LEGACY_PCI_BRIDGE, // Legacy PCI bridge
> + DEV_TYPE_PCI_HOST_BRIDGE, // PCI Host bridge
> DEV_TYPE_PCI,
> } type;
>
>
>
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Ping: [PATCH v5] AMD IOMMU: fix Dom0 device setup failure for host bridges
2013-09-12 8:17 ` [PATCH v5] AMD IOMMU: fix Dom0 device setup failure for host bridges Jan Beulich
2013-09-12 9:42 ` Stefan Bader
@ 2013-09-18 12:51 ` Jan Beulich
2013-09-18 22:27 ` Auld, Will
1 sibling, 1 reply; 9+ messages in thread
From: Jan Beulich @ 2013-09-18 12:51 UTC (permalink / raw)
To: xiantao.zhang; +Cc: xen-devel, Will Auld, stefan.bader, suravee.suthikulpanit
Xiantao,
despite the title this has implications on VT-d, and hence a review/
ack from you would be much appreciated.
Jan
>>> On 12.09.13 at 10:17, "Jan Beulich" <JBeulich@suse.com> wrote:
> The host bridge device (i.e. 0x18 for AMD) does not require IOMMU, and
> therefore is not included in the IVRS. The current logic tries to map
> all PCI devices to an IOMMU. In this case, "xl dmesg" shows the
> following message on AMD sytem.
>
> (XEN) setup 0000:00:18.0 for d0 failed (-19)
> (XEN) setup 0000:00:18.1 for d0 failed (-19)
> (XEN) setup 0000:00:18.2 for d0 failed (-19)
> (XEN) setup 0000:00:18.3 for d0 failed (-19)
> (XEN) setup 0000:00:18.4 for d0 failed (-19)
> (XEN) setup 0000:00:18.5 for d0 failed (-19)
>
> This patch adds a new device type (i.e. DEV_TYPE_PCI_HOST_BRIDGE) which
> corresponds to PCI class code 0x06 and sub-class 0x00. Then, it uses
> this new type to filter when trying to map device to IOMMU.
>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Reported-by: Stefan Bader <stefan.bader@canonical.com>
>
> On VT-d refuse (un)mapping host bridges for other than the hardware
> domain.
>
> Coding style cleanup.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
> +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> @@ -147,9 +147,10 @@ static void amd_iommu_setup_domain_devic
>
> amd_iommu_flush_device(iommu, req_id);
>
> - AMD_IOMMU_DEBUG("Setup I/O page table: device id = %#x, "
> + AMD_IOMMU_DEBUG("Setup I/O page table: device id = %#x, type = %#x, "
> "root table = %#"PRIx64", "
> - "domain = %d, paging mode = %d\n", req_id,
> + "domain = %d, paging mode = %d\n",
> + req_id, pdev->type,
> page_to_maddr(hd->root_table),
> hd->domain_id, hd->paging_mode);
> }
> @@ -175,6 +176,15 @@ static int __init amd_iommu_setup_dom0_d
>
> if ( unlikely(!iommu) )
> {
> + /* Filter the bridge devices */
> + if ( pdev->type == DEV_TYPE_PCI_HOST_BRIDGE )
> + {
> + AMD_IOMMU_DEBUG("Skipping host bridge %04x:%02x:%02x.%u\n",
> + pdev->seg, PCI_BUS(bdf), PCI_SLOT(bdf),
> + PCI_FUNC(bdf));
> + return 0;
> + }
> +
> AMD_IOMMU_DEBUG("No iommu for device %04x:%02x:%02x.%u\n",
> pdev->seg, pdev->bus,
> PCI_SLOT(devfn), PCI_FUNC(devfn));
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -194,9 +194,6 @@ static struct pci_dev *alloc_pdev(struct
> u16 cap;
> u8 sec_bus, sub_bus;
>
> - case DEV_TYPE_PCIe_BRIDGE:
> - break;
> -
> case DEV_TYPE_PCIe2PCI_BRIDGE:
> case DEV_TYPE_LEGACY_PCI_BRIDGE:
> sec_bus = pci_conf_read8(pseg->nr, bus, PCI_SLOT(devfn),
> @@ -244,6 +241,8 @@ static struct pci_dev *alloc_pdev(struct
> break;
>
> case DEV_TYPE_PCI:
> + case DEV_TYPE_PCIe_BRIDGE:
> + case DEV_TYPE_PCI_HOST_BRIDGE:
> break;
>
> default:
> @@ -697,6 +696,7 @@ void pci_release_devices(struct domain *
> spin_unlock(&pcidevs_lock);
> }
>
> +#define PCI_CLASS_BRIDGE_HOST 0x0600
> #define PCI_CLASS_BRIDGE_PCI 0x0604
>
> enum pdev_type pdev_type(u16 seg, u8 bus, u8 devfn)
> @@ -720,6 +720,8 @@ enum pdev_type pdev_type(u16 seg, u8 bus
> return DEV_TYPE_PCI2PCIe_BRIDGE;
> }
> return DEV_TYPE_PCIe_BRIDGE;
> + case PCI_CLASS_BRIDGE_HOST:
> + return DEV_TYPE_PCI_HOST_BRIDGE;
>
> case 0x0000: case 0xffff:
> return DEV_TYPE_PCI_UNKNOWN;
> --- a/xen/drivers/passthrough/vtd/intremap.c
> +++ b/xen/drivers/passthrough/vtd/intremap.c
> @@ -442,6 +442,7 @@ static void set_msi_source_id(struct pci
> case DEV_TYPE_PCIe_ENDPOINT:
> case DEV_TYPE_PCIe_BRIDGE:
> case DEV_TYPE_PCIe2PCI_BRIDGE:
> + case DEV_TYPE_PCI_HOST_BRIDGE:
> switch ( pdev->phantom_stride )
> {
> case 1: sq = SQ_13_IGNORE_3; break;
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -1433,6 +1433,15 @@ static int domain_context_mapping(
>
> switch ( pdev->type )
> {
> + case DEV_TYPE_PCI_HOST_BRIDGE:
> + if ( iommu_verbose )
> + dprintk(VTDPREFIX, "d%d:Hostbridge: skip %04x:%02x:%02x.%u map\n",
> + domain->domain_id, seg, bus,
> + PCI_SLOT(devfn), PCI_FUNC(devfn));
> + if ( !is_hardware_domain(domain) )
> + return -EPERM;
> + break;
> +
> case DEV_TYPE_PCIe_BRIDGE:
> case DEV_TYPE_PCIe2PCI_BRIDGE:
> case DEV_TYPE_LEGACY_PCI_BRIDGE:
> @@ -1563,6 +1572,15 @@ static int domain_context_unmap(
>
> switch ( pdev->type )
> {
> + case DEV_TYPE_PCI_HOST_BRIDGE:
> + if ( iommu_verbose )
> + dprintk(VTDPREFIX, "d%d:Hostbridge: skip %04x:%02x:%02x.%u unmap\n",
> + domain->domain_id, seg, bus,
> + PCI_SLOT(devfn), PCI_FUNC(devfn));
> + if ( !is_hardware_domain(domain) )
> + return -EPERM;
> + goto out;
> +
> case DEV_TYPE_PCIe_BRIDGE:
> case DEV_TYPE_PCIe2PCI_BRIDGE:
> case DEV_TYPE_LEGACY_PCI_BRIDGE:
> --- a/xen/include/xen/pci.h
> +++ b/xen/include/xen/pci.h
> @@ -63,6 +63,7 @@ struct pci_dev {
> DEV_TYPE_PCIe2PCI_BRIDGE, // PCIe-to-PCI/PCIx bridge
> DEV_TYPE_PCI2PCIe_BRIDGE, // PCI/PCIx-to-PCIe bridge
> DEV_TYPE_LEGACY_PCI_BRIDGE, // Legacy PCI bridge
> + DEV_TYPE_PCI_HOST_BRIDGE, // PCI Host bridge
> DEV_TYPE_PCI,
> } type;
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Ping: [PATCH v5] AMD IOMMU: fix Dom0 device setup failure for host bridges
2013-09-18 12:51 ` Ping: " Jan Beulich
@ 2013-09-18 22:27 ` Auld, Will
2013-09-26 8:09 ` Jan Beulich
0 siblings, 1 reply; 9+ messages in thread
From: Auld, Will @ 2013-09-18 22:27 UTC (permalink / raw)
To: Jan Beulich, Zhang, Xiantao
Cc: xen-devel, Auld, Will, stefan.bader, suravee.suthikulpanit
FYI: PRC is on holiday Thursday and Friday.
Will
> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Wednesday, September 18, 2013 5:51 AM
> To: Zhang, Xiantao
> Cc: suravee.suthikulpanit@amd.com; stefan.bader@canonical.com; Auld,
> Will; xen-devel
> Subject: Ping: [PATCH v5] AMD IOMMU: fix Dom0 device setup failure for
> host bridges
>
> Xiantao,
>
> despite the title this has implications on VT-d, and hence a review/
> ack from you would be much appreciated.
>
> Jan
>
> >>> On 12.09.13 at 10:17, "Jan Beulich" <JBeulich@suse.com> wrote:
> > The host bridge device (i.e. 0x18 for AMD) does not require IOMMU,
> and
> > therefore is not included in the IVRS. The current logic tries to map
> > all PCI devices to an IOMMU. In this case, "xl dmesg" shows the
> > following message on AMD sytem.
> >
> > (XEN) setup 0000:00:18.0 for d0 failed (-19)
> > (XEN) setup 0000:00:18.1 for d0 failed (-19)
> > (XEN) setup 0000:00:18.2 for d0 failed (-19)
> > (XEN) setup 0000:00:18.3 for d0 failed (-19)
> > (XEN) setup 0000:00:18.4 for d0 failed (-19)
> > (XEN) setup 0000:00:18.5 for d0 failed (-19)
> >
> > This patch adds a new device type (i.e. DEV_TYPE_PCI_HOST_BRIDGE)
> > which corresponds to PCI class code 0x06 and sub-class 0x00. Then, it
> > uses this new type to filter when trying to map device to IOMMU.
> >
> > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> > Reported-by: Stefan Bader <stefan.bader@canonical.com>
> >
> > On VT-d refuse (un)mapping host bridges for other than the hardware
> > domain.
> >
> > Coding style cleanup.
> >
> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >
> > --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
> > +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> > @@ -147,9 +147,10 @@ static void amd_iommu_setup_domain_devic
> >
> > amd_iommu_flush_device(iommu, req_id);
> >
> > - AMD_IOMMU_DEBUG("Setup I/O page table: device id = %#x, "
> > + AMD_IOMMU_DEBUG("Setup I/O page table: device id = %#x, type
> = %#x, "
> > "root table = %#"PRIx64", "
> > - "domain = %d, paging mode = %d\n", req_id,
> > + "domain = %d, paging mode = %d\n",
> > + req_id, pdev->type,
> > page_to_maddr(hd->root_table),
> > hd->domain_id, hd->paging_mode);
> > }
> > @@ -175,6 +176,15 @@ static int __init amd_iommu_setup_dom0_d
> >
> > if ( unlikely(!iommu) )
> > {
> > + /* Filter the bridge devices */
> > + if ( pdev->type == DEV_TYPE_PCI_HOST_BRIDGE )
> > + {
> > + AMD_IOMMU_DEBUG("Skipping host bridge
> %04x:%02x:%02x.%u\n",
> > + pdev->seg, PCI_BUS(bdf), PCI_SLOT(bdf),
> > + PCI_FUNC(bdf));
> > + return 0;
> > + }
> > +
> > AMD_IOMMU_DEBUG("No iommu for device %04x:%02x:%02x.%u\n",
> > pdev->seg, pdev->bus,
> > PCI_SLOT(devfn), PCI_FUNC(devfn));
> > --- a/xen/drivers/passthrough/pci.c
> > +++ b/xen/drivers/passthrough/pci.c
> > @@ -194,9 +194,6 @@ static struct pci_dev *alloc_pdev(struct
> > u16 cap;
> > u8 sec_bus, sub_bus;
> >
> > - case DEV_TYPE_PCIe_BRIDGE:
> > - break;
> > -
> > case DEV_TYPE_PCIe2PCI_BRIDGE:
> > case DEV_TYPE_LEGACY_PCI_BRIDGE:
> > sec_bus = pci_conf_read8(pseg->nr, bus, PCI_SLOT(devfn),
> > @@ -244,6 +241,8 @@ static struct pci_dev *alloc_pdev(struct
> > break;
> >
> > case DEV_TYPE_PCI:
> > + case DEV_TYPE_PCIe_BRIDGE:
> > + case DEV_TYPE_PCI_HOST_BRIDGE:
> > break;
> >
> > default:
> > @@ -697,6 +696,7 @@ void pci_release_devices(struct domain *
> > spin_unlock(&pcidevs_lock);
> > }
> >
> > +#define PCI_CLASS_BRIDGE_HOST 0x0600
> > #define PCI_CLASS_BRIDGE_PCI 0x0604
> >
> > enum pdev_type pdev_type(u16 seg, u8 bus, u8 devfn) @@ -720,6 +720,8
> > @@ enum pdev_type pdev_type(u16 seg, u8 bus
> > return DEV_TYPE_PCI2PCIe_BRIDGE;
> > }
> > return DEV_TYPE_PCIe_BRIDGE;
> > + case PCI_CLASS_BRIDGE_HOST:
> > + return DEV_TYPE_PCI_HOST_BRIDGE;
> >
> > case 0x0000: case 0xffff:
> > return DEV_TYPE_PCI_UNKNOWN;
> > --- a/xen/drivers/passthrough/vtd/intremap.c
> > +++ b/xen/drivers/passthrough/vtd/intremap.c
> > @@ -442,6 +442,7 @@ static void set_msi_source_id(struct pci
> > case DEV_TYPE_PCIe_ENDPOINT:
> > case DEV_TYPE_PCIe_BRIDGE:
> > case DEV_TYPE_PCIe2PCI_BRIDGE:
> > + case DEV_TYPE_PCI_HOST_BRIDGE:
> > switch ( pdev->phantom_stride )
> > {
> > case 1: sq = SQ_13_IGNORE_3; break;
> > --- a/xen/drivers/passthrough/vtd/iommu.c
> > +++ b/xen/drivers/passthrough/vtd/iommu.c
> > @@ -1433,6 +1433,15 @@ static int domain_context_mapping(
> >
> > switch ( pdev->type )
> > {
> > + case DEV_TYPE_PCI_HOST_BRIDGE:
> > + if ( iommu_verbose )
> > + dprintk(VTDPREFIX, "d%d:Hostbridge: skip
> %04x:%02x:%02x.%u map\n",
> > + domain->domain_id, seg, bus,
> > + PCI_SLOT(devfn), PCI_FUNC(devfn));
> > + if ( !is_hardware_domain(domain) )
> > + return -EPERM;
> > + break;
> > +
> > case DEV_TYPE_PCIe_BRIDGE:
> > case DEV_TYPE_PCIe2PCI_BRIDGE:
> > case DEV_TYPE_LEGACY_PCI_BRIDGE:
> > @@ -1563,6 +1572,15 @@ static int domain_context_unmap(
> >
> > switch ( pdev->type )
> > {
> > + case DEV_TYPE_PCI_HOST_BRIDGE:
> > + if ( iommu_verbose )
> > + dprintk(VTDPREFIX, "d%d:Hostbridge: skip
> %04x:%02x:%02x.%u unmap\n",
> > + domain->domain_id, seg, bus,
> > + PCI_SLOT(devfn), PCI_FUNC(devfn));
> > + if ( !is_hardware_domain(domain) )
> > + return -EPERM;
> > + goto out;
> > +
> > case DEV_TYPE_PCIe_BRIDGE:
> > case DEV_TYPE_PCIe2PCI_BRIDGE:
> > case DEV_TYPE_LEGACY_PCI_BRIDGE:
> > --- a/xen/include/xen/pci.h
> > +++ b/xen/include/xen/pci.h
> > @@ -63,6 +63,7 @@ struct pci_dev {
> > DEV_TYPE_PCIe2PCI_BRIDGE, // PCIe-to-PCI/PCIx bridge
> > DEV_TYPE_PCI2PCIe_BRIDGE, // PCI/PCIx-to-PCIe bridge
> > DEV_TYPE_LEGACY_PCI_BRIDGE, // Legacy PCI bridge
> > + DEV_TYPE_PCI_HOST_BRIDGE, // PCI Host bridge
> > DEV_TYPE_PCI,
> > } type;
> >
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Ping: [PATCH v5] AMD IOMMU: fix Dom0 device setup failure for host bridges
2013-09-18 22:27 ` Auld, Will
@ 2013-09-26 8:09 ` Jan Beulich
2013-09-27 7:10 ` Zhang, Xiantao
0 siblings, 1 reply; 9+ messages in thread
From: Jan Beulich @ 2013-09-26 8:09 UTC (permalink / raw)
To: Will Auld, Xiantao Zhang; +Cc: xen-devel, stefan.bader, suravee.suthikulpanit
>>> On 19.09.13 at 00:27, "Auld, Will" <will.auld@intel.com> wrote:
> FYI: PRC is on holiday Thursday and Friday.
Understood, but it's now Thursday the next week, and there was
_still_ no response.
Jan
>> -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@suse.com]
>> Sent: Wednesday, September 18, 2013 5:51 AM
>> To: Zhang, Xiantao
>> Cc: suravee.suthikulpanit@amd.com; stefan.bader@canonical.com; Auld,
>> Will; xen-devel
>> Subject: Ping: [PATCH v5] AMD IOMMU: fix Dom0 device setup failure for
>> host bridges
>>
>> Xiantao,
>>
>> despite the title this has implications on VT-d, and hence a review/
>> ack from you would be much appreciated.
>>
>> Jan
>>
>> >>> On 12.09.13 at 10:17, "Jan Beulich" <JBeulich@suse.com> wrote:
>> > The host bridge device (i.e. 0x18 for AMD) does not require IOMMU,
>> and
>> > therefore is not included in the IVRS. The current logic tries to map
>> > all PCI devices to an IOMMU. In this case, "xl dmesg" shows the
>> > following message on AMD sytem.
>> >
>> > (XEN) setup 0000:00:18.0 for d0 failed (-19)
>> > (XEN) setup 0000:00:18.1 for d0 failed (-19)
>> > (XEN) setup 0000:00:18.2 for d0 failed (-19)
>> > (XEN) setup 0000:00:18.3 for d0 failed (-19)
>> > (XEN) setup 0000:00:18.4 for d0 failed (-19)
>> > (XEN) setup 0000:00:18.5 for d0 failed (-19)
>> >
>> > This patch adds a new device type (i.e. DEV_TYPE_PCI_HOST_BRIDGE)
>> > which corresponds to PCI class code 0x06 and sub-class 0x00. Then, it
>> > uses this new type to filter when trying to map device to IOMMU.
>> >
>> > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>> > Reported-by: Stefan Bader <stefan.bader@canonical.com>
>> >
>> > On VT-d refuse (un)mapping host bridges for other than the hardware
>> > domain.
>> >
>> > Coding style cleanup.
>> >
>> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> >
>> > --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
>> > +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
>> > @@ -147,9 +147,10 @@ static void amd_iommu_setup_domain_devic
>> >
>> > amd_iommu_flush_device(iommu, req_id);
>> >
>> > - AMD_IOMMU_DEBUG("Setup I/O page table: device id = %#x, "
>> > + AMD_IOMMU_DEBUG("Setup I/O page table: device id = %#x, type
>> = %#x, "
>> > "root table = %#"PRIx64", "
>> > - "domain = %d, paging mode = %d\n", req_id,
>> > + "domain = %d, paging mode = %d\n",
>> > + req_id, pdev->type,
>> > page_to_maddr(hd->root_table),
>> > hd->domain_id, hd->paging_mode);
>> > }
>> > @@ -175,6 +176,15 @@ static int __init amd_iommu_setup_dom0_d
>> >
>> > if ( unlikely(!iommu) )
>> > {
>> > + /* Filter the bridge devices */
>> > + if ( pdev->type == DEV_TYPE_PCI_HOST_BRIDGE )
>> > + {
>> > + AMD_IOMMU_DEBUG("Skipping host bridge
>> %04x:%02x:%02x.%u\n",
>> > + pdev->seg, PCI_BUS(bdf), PCI_SLOT(bdf),
>> > + PCI_FUNC(bdf));
>> > + return 0;
>> > + }
>> > +
>> > AMD_IOMMU_DEBUG("No iommu for device %04x:%02x:%02x.%u\n",
>> > pdev->seg, pdev->bus,
>> > PCI_SLOT(devfn), PCI_FUNC(devfn));
>> > --- a/xen/drivers/passthrough/pci.c
>> > +++ b/xen/drivers/passthrough/pci.c
>> > @@ -194,9 +194,6 @@ static struct pci_dev *alloc_pdev(struct
>> > u16 cap;
>> > u8 sec_bus, sub_bus;
>> >
>> > - case DEV_TYPE_PCIe_BRIDGE:
>> > - break;
>> > -
>> > case DEV_TYPE_PCIe2PCI_BRIDGE:
>> > case DEV_TYPE_LEGACY_PCI_BRIDGE:
>> > sec_bus = pci_conf_read8(pseg->nr, bus, PCI_SLOT(devfn),
>> > @@ -244,6 +241,8 @@ static struct pci_dev *alloc_pdev(struct
>> > break;
>> >
>> > case DEV_TYPE_PCI:
>> > + case DEV_TYPE_PCIe_BRIDGE:
>> > + case DEV_TYPE_PCI_HOST_BRIDGE:
>> > break;
>> >
>> > default:
>> > @@ -697,6 +696,7 @@ void pci_release_devices(struct domain *
>> > spin_unlock(&pcidevs_lock);
>> > }
>> >
>> > +#define PCI_CLASS_BRIDGE_HOST 0x0600
>> > #define PCI_CLASS_BRIDGE_PCI 0x0604
>> >
>> > enum pdev_type pdev_type(u16 seg, u8 bus, u8 devfn) @@ -720,6 +720,8
>> > @@ enum pdev_type pdev_type(u16 seg, u8 bus
>> > return DEV_TYPE_PCI2PCIe_BRIDGE;
>> > }
>> > return DEV_TYPE_PCIe_BRIDGE;
>> > + case PCI_CLASS_BRIDGE_HOST:
>> > + return DEV_TYPE_PCI_HOST_BRIDGE;
>> >
>> > case 0x0000: case 0xffff:
>> > return DEV_TYPE_PCI_UNKNOWN;
>> > --- a/xen/drivers/passthrough/vtd/intremap.c
>> > +++ b/xen/drivers/passthrough/vtd/intremap.c
>> > @@ -442,6 +442,7 @@ static void set_msi_source_id(struct pci
>> > case DEV_TYPE_PCIe_ENDPOINT:
>> > case DEV_TYPE_PCIe_BRIDGE:
>> > case DEV_TYPE_PCIe2PCI_BRIDGE:
>> > + case DEV_TYPE_PCI_HOST_BRIDGE:
>> > switch ( pdev->phantom_stride )
>> > {
>> > case 1: sq = SQ_13_IGNORE_3; break;
>> > --- a/xen/drivers/passthrough/vtd/iommu.c
>> > +++ b/xen/drivers/passthrough/vtd/iommu.c
>> > @@ -1433,6 +1433,15 @@ static int domain_context_mapping(
>> >
>> > switch ( pdev->type )
>> > {
>> > + case DEV_TYPE_PCI_HOST_BRIDGE:
>> > + if ( iommu_verbose )
>> > + dprintk(VTDPREFIX, "d%d:Hostbridge: skip
>> %04x:%02x:%02x.%u map\n",
>> > + domain->domain_id, seg, bus,
>> > + PCI_SLOT(devfn), PCI_FUNC(devfn));
>> > + if ( !is_hardware_domain(domain) )
>> > + return -EPERM;
>> > + break;
>> > +
>> > case DEV_TYPE_PCIe_BRIDGE:
>> > case DEV_TYPE_PCIe2PCI_BRIDGE:
>> > case DEV_TYPE_LEGACY_PCI_BRIDGE:
>> > @@ -1563,6 +1572,15 @@ static int domain_context_unmap(
>> >
>> > switch ( pdev->type )
>> > {
>> > + case DEV_TYPE_PCI_HOST_BRIDGE:
>> > + if ( iommu_verbose )
>> > + dprintk(VTDPREFIX, "d%d:Hostbridge: skip
>> %04x:%02x:%02x.%u unmap\n",
>> > + domain->domain_id, seg, bus,
>> > + PCI_SLOT(devfn), PCI_FUNC(devfn));
>> > + if ( !is_hardware_domain(domain) )
>> > + return -EPERM;
>> > + goto out;
>> > +
>> > case DEV_TYPE_PCIe_BRIDGE:
>> > case DEV_TYPE_PCIe2PCI_BRIDGE:
>> > case DEV_TYPE_LEGACY_PCI_BRIDGE:
>> > --- a/xen/include/xen/pci.h
>> > +++ b/xen/include/xen/pci.h
>> > @@ -63,6 +63,7 @@ struct pci_dev {
>> > DEV_TYPE_PCIe2PCI_BRIDGE, // PCIe-to-PCI/PCIx bridge
>> > DEV_TYPE_PCI2PCIe_BRIDGE, // PCI/PCIx-to-PCIe bridge
>> > DEV_TYPE_LEGACY_PCI_BRIDGE, // Legacy PCI bridge
>> > + DEV_TYPE_PCI_HOST_BRIDGE, // PCI Host bridge
>> > DEV_TYPE_PCI,
>> > } type;
>> >
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Ping: [PATCH v5] AMD IOMMU: fix Dom0 device setup failure for host bridges
2013-09-26 8:09 ` Jan Beulich
@ 2013-09-27 7:10 ` Zhang, Xiantao
2013-09-27 7:19 ` Jan Beulich
0 siblings, 1 reply; 9+ messages in thread
From: Zhang, Xiantao @ 2013-09-27 7:10 UTC (permalink / raw)
To: Jan Beulich, Auld, Will; +Cc: xen-devel, stefan.bader, suravee.suthikulpanit
Hi, Jan
Sorry for the late. For VT-d, have you seen any issue ? I don't think host bridge is covered by any DRHD. If it is not covered by one DRHD, once it is assigned to dom0, domain_context_mapping does nothing for it.
Xiantao
-----Original Message-----
From: Jan Beulich [mailto:JBeulich@suse.com]
Sent: Thursday, September 26, 2013 4:10 PM
To: Auld, Will; Zhang, Xiantao
Cc: suravee.suthikulpanit@amd.com; stefan.bader@canonical.com; xen-devel
Subject: RE: Ping: [PATCH v5] AMD IOMMU: fix Dom0 device setup failure for host bridges
>>> On 19.09.13 at 00:27, "Auld, Will" <will.auld@intel.com> wrote:
> FYI: PRC is on holiday Thursday and Friday.
Understood, but it's now Thursday the next week, and there was _still_ no response.
Jan
>> -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@suse.com]
>> Sent: Wednesday, September 18, 2013 5:51 AM
>> To: Zhang, Xiantao
>> Cc: suravee.suthikulpanit@amd.com; stefan.bader@canonical.com; Auld,
>> Will; xen-devel
>> Subject: Ping: [PATCH v5] AMD IOMMU: fix Dom0 device setup failure
>> for host bridges
>>
>> Xiantao,
>>
>> despite the title this has implications on VT-d, and hence a review/
>> ack from you would be much appreciated.
>>
>> Jan
>>
>> >>> On 12.09.13 at 10:17, "Jan Beulich" <JBeulich@suse.com> wrote:
>> > The host bridge device (i.e. 0x18 for AMD) does not require IOMMU,
>> and
>> > therefore is not included in the IVRS. The current logic tries to
>> > map all PCI devices to an IOMMU. In this case, "xl dmesg" shows the
>> > following message on AMD sytem.
>> >
>> > (XEN) setup 0000:00:18.0 for d0 failed (-19)
>> > (XEN) setup 0000:00:18.1 for d0 failed (-19)
>> > (XEN) setup 0000:00:18.2 for d0 failed (-19)
>> > (XEN) setup 0000:00:18.3 for d0 failed (-19)
>> > (XEN) setup 0000:00:18.4 for d0 failed (-19)
>> > (XEN) setup 0000:00:18.5 for d0 failed (-19)
>> >
>> > This patch adds a new device type (i.e. DEV_TYPE_PCI_HOST_BRIDGE)
>> > which corresponds to PCI class code 0x06 and sub-class 0x00. Then,
>> > it uses this new type to filter when trying to map device to IOMMU.
>> >
>> > Signed-off-by: Suravee Suthikulpanit
>> > <suravee.suthikulpanit@amd.com>
>> > Reported-by: Stefan Bader <stefan.bader@canonical.com>
>> >
>> > On VT-d refuse (un)mapping host bridges for other than the hardware
>> > domain.
>> >
>> > Coding style cleanup.
>> >
>> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> >
>> > --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
>> > +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
>> > @@ -147,9 +147,10 @@ static void amd_iommu_setup_domain_devic
>> >
>> > amd_iommu_flush_device(iommu, req_id);
>> >
>> > - AMD_IOMMU_DEBUG("Setup I/O page table: device id = %#x, "
>> > + AMD_IOMMU_DEBUG("Setup I/O page table: device id = %#x,
>> > + type
>> = %#x, "
>> > "root table = %#"PRIx64", "
>> > - "domain = %d, paging mode = %d\n", req_id,
>> > + "domain = %d, paging mode = %d\n",
>> > + req_id, pdev->type,
>> > page_to_maddr(hd->root_table),
>> > hd->domain_id, hd->paging_mode);
>> > }
>> > @@ -175,6 +176,15 @@ static int __init amd_iommu_setup_dom0_d
>> >
>> > if ( unlikely(!iommu) )
>> > {
>> > + /* Filter the bridge devices */
>> > + if ( pdev->type == DEV_TYPE_PCI_HOST_BRIDGE )
>> > + {
>> > + AMD_IOMMU_DEBUG("Skipping host bridge
>> %04x:%02x:%02x.%u\n",
>> > + pdev->seg, PCI_BUS(bdf), PCI_SLOT(bdf),
>> > + PCI_FUNC(bdf));
>> > + return 0;
>> > + }
>> > +
>> > AMD_IOMMU_DEBUG("No iommu for device %04x:%02x:%02x.%u\n",
>> > pdev->seg, pdev->bus,
>> > PCI_SLOT(devfn), PCI_FUNC(devfn));
>> > --- a/xen/drivers/passthrough/pci.c
>> > +++ b/xen/drivers/passthrough/pci.c
>> > @@ -194,9 +194,6 @@ static struct pci_dev *alloc_pdev(struct
>> > u16 cap;
>> > u8 sec_bus, sub_bus;
>> >
>> > - case DEV_TYPE_PCIe_BRIDGE:
>> > - break;
>> > -
>> > case DEV_TYPE_PCIe2PCI_BRIDGE:
>> > case DEV_TYPE_LEGACY_PCI_BRIDGE:
>> > sec_bus = pci_conf_read8(pseg->nr, bus,
>> > PCI_SLOT(devfn), @@ -244,6 +241,8 @@ static struct pci_dev *alloc_pdev(struct
>> > break;
>> >
>> > case DEV_TYPE_PCI:
>> > + case DEV_TYPE_PCIe_BRIDGE:
>> > + case DEV_TYPE_PCI_HOST_BRIDGE:
>> > break;
>> >
>> > default:
>> > @@ -697,6 +696,7 @@ void pci_release_devices(struct domain *
>> > spin_unlock(&pcidevs_lock);
>> > }
>> >
>> > +#define PCI_CLASS_BRIDGE_HOST 0x0600
>> > #define PCI_CLASS_BRIDGE_PCI 0x0604
>> >
>> > enum pdev_type pdev_type(u16 seg, u8 bus, u8 devfn) @@ -720,6
>> > +720,8 @@ enum pdev_type pdev_type(u16 seg, u8 bus
>> > return DEV_TYPE_PCI2PCIe_BRIDGE;
>> > }
>> > return DEV_TYPE_PCIe_BRIDGE;
>> > + case PCI_CLASS_BRIDGE_HOST:
>> > + return DEV_TYPE_PCI_HOST_BRIDGE;
>> >
>> > case 0x0000: case 0xffff:
>> > return DEV_TYPE_PCI_UNKNOWN;
>> > --- a/xen/drivers/passthrough/vtd/intremap.c
>> > +++ b/xen/drivers/passthrough/vtd/intremap.c
>> > @@ -442,6 +442,7 @@ static void set_msi_source_id(struct pci
>> > case DEV_TYPE_PCIe_ENDPOINT:
>> > case DEV_TYPE_PCIe_BRIDGE:
>> > case DEV_TYPE_PCIe2PCI_BRIDGE:
>> > + case DEV_TYPE_PCI_HOST_BRIDGE:
>> > switch ( pdev->phantom_stride )
>> > {
>> > case 1: sq = SQ_13_IGNORE_3; break;
>> > --- a/xen/drivers/passthrough/vtd/iommu.c
>> > +++ b/xen/drivers/passthrough/vtd/iommu.c
>> > @@ -1433,6 +1433,15 @@ static int domain_context_mapping(
>> >
>> > switch ( pdev->type )
>> > {
>> > + case DEV_TYPE_PCI_HOST_BRIDGE:
>> > + if ( iommu_verbose )
>> > + dprintk(VTDPREFIX, "d%d:Hostbridge: skip
>> %04x:%02x:%02x.%u map\n",
>> > + domain->domain_id, seg, bus,
>> > + PCI_SLOT(devfn), PCI_FUNC(devfn));
>> > + if ( !is_hardware_domain(domain) )
>> > + return -EPERM;
>> > + break;
>> > +
>> > case DEV_TYPE_PCIe_BRIDGE:
>> > case DEV_TYPE_PCIe2PCI_BRIDGE:
>> > case DEV_TYPE_LEGACY_PCI_BRIDGE:
>> > @@ -1563,6 +1572,15 @@ static int domain_context_unmap(
>> >
>> > switch ( pdev->type )
>> > {
>> > + case DEV_TYPE_PCI_HOST_BRIDGE:
>> > + if ( iommu_verbose )
>> > + dprintk(VTDPREFIX, "d%d:Hostbridge: skip
>> %04x:%02x:%02x.%u unmap\n",
>> > + domain->domain_id, seg, bus,
>> > + PCI_SLOT(devfn), PCI_FUNC(devfn));
>> > + if ( !is_hardware_domain(domain) )
>> > + return -EPERM;
>> > + goto out;
>> > +
>> > case DEV_TYPE_PCIe_BRIDGE:
>> > case DEV_TYPE_PCIe2PCI_BRIDGE:
>> > case DEV_TYPE_LEGACY_PCI_BRIDGE:
>> > --- a/xen/include/xen/pci.h
>> > +++ b/xen/include/xen/pci.h
>> > @@ -63,6 +63,7 @@ struct pci_dev {
>> > DEV_TYPE_PCIe2PCI_BRIDGE, // PCIe-to-PCI/PCIx bridge
>> > DEV_TYPE_PCI2PCIe_BRIDGE, // PCI/PCIx-to-PCIe bridge
>> > DEV_TYPE_LEGACY_PCI_BRIDGE, // Legacy PCI bridge
>> > + DEV_TYPE_PCI_HOST_BRIDGE, // PCI Host bridge
>> > DEV_TYPE_PCI,
>> > } type;
>> >
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Ping: [PATCH v5] AMD IOMMU: fix Dom0 device setup failure for host bridges
2013-09-27 7:10 ` Zhang, Xiantao
@ 2013-09-27 7:19 ` Jan Beulich
2013-09-27 7:28 ` Zhang, Xiantao
0 siblings, 1 reply; 9+ messages in thread
From: Jan Beulich @ 2013-09-27 7:19 UTC (permalink / raw)
To: Will Auld, Xiantao Zhang; +Cc: xen-devel, stefan.bader, suravee.suthikulpanit
>>> On 27.09.13 at 09:10, "Zhang, Xiantao" <xiantao.zhang@intel.com> wrote:
> Sorry for the late. For VT-d, have you seen any issue ? I don't think host
> bridge is covered by any DRHD. If it is not covered by one DRHD, once it is
> assigned to dom0, domain_context_mapping does nothing for it.
Right, but that's not the question. The issue is that for the AMD
side we need the new device type, and the question is whether
the handling done by Suravee's patch for VT-d is correct. I.e.
we're asking for your review of the (unavoidable) VT-d changes
in that patch.
Jan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Ping: [PATCH v5] AMD IOMMU: fix Dom0 device setup failure for host bridges
2013-09-27 7:19 ` Jan Beulich
@ 2013-09-27 7:28 ` Zhang, Xiantao
0 siblings, 0 replies; 9+ messages in thread
From: Zhang, Xiantao @ 2013-09-27 7:28 UTC (permalink / raw)
To: Jan Beulich, Auld, Will; +Cc: xen-devel, stefan.bader, suravee.suthikulpanit
Okay, understand. The change doesn't impact VT-d side, although some lines are actually added. Looks good to me, thanks!
Acked-by: Xiantao Zhang <xiantao.zhang@intel.com>
-----Original Message-----
From: Jan Beulich [mailto:JBeulich@suse.com]
Sent: Friday, September 27, 2013 3:19 PM
To: Auld, Will; Zhang, Xiantao
Cc: suravee.suthikulpanit@amd.com; stefan.bader@canonical.com; xen-devel
Subject: RE: Ping: [PATCH v5] AMD IOMMU: fix Dom0 device setup failure for host bridges
>>> On 27.09.13 at 09:10, "Zhang, Xiantao" <xiantao.zhang@intel.com> wrote:
> Sorry for the late. For VT-d, have you seen any issue ? I don't think
> host bridge is covered by any DRHD. If it is not covered by one
> DRHD, once it is assigned to dom0, domain_context_mapping does nothing for it.
Right, but that's not the question. The issue is that for the AMD side we need the new device type, and the question is whether the handling done by Suravee's patch for VT-d is correct. I.e.
we're asking for your review of the (unavoidable) VT-d changes in that patch.
Jan
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-09-27 7:28 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-11 19:39 [PATCH 1/1 V4] x86/AMD: Fix setup ssss:bb:dd:f for d0 failed suravee.suthikulpanit
2013-09-12 8:17 ` [PATCH v5] AMD IOMMU: fix Dom0 device setup failure for host bridges Jan Beulich
2013-09-12 9:42 ` Stefan Bader
2013-09-18 12:51 ` Ping: " Jan Beulich
2013-09-18 22:27 ` Auld, Will
2013-09-26 8:09 ` Jan Beulich
2013-09-27 7:10 ` Zhang, Xiantao
2013-09-27 7:19 ` Jan Beulich
2013-09-27 7:28 ` Zhang, Xiantao
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.