Note: dom0 checking in v4 has been separated out into 2/2.
This patch enables P2P upstream forwarding in ACS capable PCIe switches.
It solves two potential problems in virtualization environment where a PCIe
device is assigned to a guest domain using a HW iommu such as VT-d:
1) Unintentional failure caused by guest physical address programmed
into the device's DMA that happens to match the memory address range
of other downstream ports in the same PCIe switch. This causes the PCI
transaction to go to the matching downstream port instead of go to the
root complex to get translated by VT-d as it should be.
2) Malicious guest software intentionally attacks another downstream
PCIe device by programming the DMA address into the assigned device
that matches memory address range of the downstream PCIe port.
We are in process of implementing device filtering software in KVM/XEN
management software to allow device assignment of PCIe devices behind a PCIe
switch only if it has ACS capability and with the P2P upstream forwarding bits
enabled. This patch is intended to work for both KVM and Xen environments.
Changes from initial to v1:
- removed #define ACS_ENABLE and dev_info() call
- changed ctrl value setting without using if-condition
- fixed ACS #defines in pci_regs.h
Changes from v2 to v3:
- change #define indention to 2 for PCI reg and 1 for bit
position
Changes from v3 to v4:
- enable ACS only if iommu is enabled or if kernel is xen dom0
- changed xen_domain_type from enum to #define to allow it to be used
in pci/probe.c and to avoid excessive changes to code structure.
Changes from v4 to v5:
- separate out dom0 checking out of v4
- use iommu_found() for detecting HW iommu presence
Signed-off-by: Allen Kay <[email protected]>
Reviewed--by: Mathew Wilcox <[email protected]>
Reviewed--by: Chris Wright <[email protected]>
---
drivers/pci/pci.c | 35 +++++++++++++++++++++++++++++++++++
drivers/pci/pci.h | 2 ++
drivers/pci/probe.c | 5 +++++
include/linux/pci_regs.h | 13 +++++++++++++
4 files changed, 55 insertions(+), 0 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 6edecff..ec61c76 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1533,6 +1533,41 @@ void pci_enable_ari(struct pci_dev *dev)
}
/**
+ * pci_enable_acs - enable ACS if hardware support it
+ * @dev: the PCI device
+ */
+void pci_enable_acs(struct pci_dev *dev)
+{
+ int pos;
+ u16 cap;
+ u16 ctrl;
+
+ if (!dev->is_pcie)
+ return;
+
+ pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ACS);
+ if (!pos)
+ return;
+
+ pci_read_config_word(dev, pos + PCI_ACS_CAP, &cap);
+ pci_read_config_word(dev, pos + PCI_ACS_CTRL, &ctrl);
+
+ /* Source Validation */
+ ctrl |= (cap & PCI_ACS_SV);
+
+ /* P2P Request Redirect */
+ ctrl |= (cap & PCI_ACS_RR);
+
+ /* P2P Completion Redirect */
+ ctrl |= (cap & PCI_ACS_CR);
+
+ /* Upstream Forwarding */
+ ctrl |= (cap & PCI_ACS_UF);
+
+ pci_write_config_word(dev, pos + PCI_ACS_CTRL, ctrl);
+}
+
+/**
* pci_swizzle_interrupt_pin - swizzle INTx for device behind bridge
* @dev: the PCI device
* @pin: the INTx pin (1=INTA, 2=INTB, 3=INTD, 4=INTD)
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index d92d195..33ed8e0 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -311,4 +311,6 @@ static inline int pci_resource_alignment(struct pci_dev *dev,
return resource_alignment(res);
}
+extern void pci_enable_acs(struct pci_dev *dev);
+
#endif /* DRIVERS_PCI_H */
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 8105e32..aa98258 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -10,6 +10,7 @@
#include <linux/module.h>
#include <linux/cpumask.h>
#include <linux/pci-aspm.h>
+#include <linux/iommu.h>
#include "pci.h"
#define CARDBUS_LATENCY_TIMER 176 /* secondary latency timer */
@@ -1014,6 +1015,10 @@ static void pci_init_capabilities(struct pci_dev *dev)
/* Single Root I/O Virtualization */
pci_iov_init(dev);
+
+ /* Enable ACS P2P upstream forwarding */
+ if (iommu_found())
+ pci_enable_acs(dev);
}
void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
diff --git a/include/linux/pci_regs.h b/include/linux/pci_regs.h
index dd0bed4..d798770 100644
--- a/include/linux/pci_regs.h
+++ b/include/linux/pci_regs.h
@@ -502,6 +502,7 @@
#define PCI_EXT_CAP_ID_VC 2
#define PCI_EXT_CAP_ID_DSN 3
#define PCI_EXT_CAP_ID_PWR 4
+#define PCI_EXT_CAP_ID_ACS 13
#define PCI_EXT_CAP_ID_ARI 14
#define PCI_EXT_CAP_ID_ATS 15
#define PCI_EXT_CAP_ID_SRIOV 16
@@ -662,4 +663,16 @@
#define PCI_SRIOV_VFM_MO 0x2 /* Active.MigrateOut */
#define PCI_SRIOV_VFM_AV 0x3 /* Active.Available */
+/* Access Control Service */
+#define PCI_ACS_CAP 0x04 /* ACS Capability Register */
+#define PCI_ACS_SV 0x01 /* Source Validation */
+#define PCI_ACS_TB 0x02 /* Translation Blocking */
+#define PCI_ACS_RR 0x04 /* P2P Request Redirect */
+#define PCI_ACS_CR 0x08 /* P2P Completion Redirect */
+#define PCI_ACS_UF 0x10 /* Upstream Forwarding */
+#define PCI_ACS_EC 0x20 /* P2P Egress Control */
+#define PCI_ACS_DT 0x40 /* Direct Translated P2P */
+#define PCI_ACS_CTRL 0x06 /* ACS Control Register */
+#define PCI_ACS_EGRESS_CTL_V 0x08 /* ACS Egress Control Vector */
+
#endif /* LINUX_PCI_REGS_H */
--
1.6.0.6
* Allen Kay ([email protected]) wrote:
> Changes from v4 to v5:
> - separate out dom0 checking out of v4
> - use iommu_found() for detecting HW iommu presence
Erk, turns out iommu_found() doesn't work (becuase the iommu has been
detected, but not initialized yet when this code runs).
This could be fixed in a few ways:
1) Have an isolation capable iommu actively ask for ACS to be enabled
2) Have pci core switch to iommu_detected (as Allen had done earlier).
This would need some work to make iommu_detected actually defined on
all arches that make use of pci. iommu_detected could be more
descriptive than a single boolean, turned into an enum that says what
kind of hw iommu was detected.
3) Just always enable ACS and pay the price.
I'll send a patch for option 1) momentarily.
Thoughts?
thanks,
-chris
Commit ae21ee65e8bc228416bbcc8a1da01c56a847a60c "PCI: acs p2p upsteram
forwarding enabling" doesn't actually enable ACS.
Add a function to pci core to allow an IOMMU to request that ACS
be enabled. The existing mechanism of using iommu_found() in the pci
core to know when ACS should be enabled doesn't actually work due to
initialization order; iommu has only been detected not initialized.
Have Intel and AMD IOMMUs request ACS, and Xen does as well during early
init of dom0.
Cc: Allen Kay <[email protected]>
Cc: David Woodhouse <[email protected]>
Cc: Jeremy Fitzhardinge <[email protected]>
Cc: Joerg Roedel <[email protected]>
Signed-off-by: Chris Wright <[email protected]>
---
Applies against the linux-next branch of Jesse's pci tree.
arch/x86/kernel/amd_iommu_init.c | 2 ++
arch/x86/xen/enlighten.c | 5 +++++
drivers/pci/dmar.c | 5 ++++-
drivers/pci/pci.c | 13 +++++++++++++
drivers/pci/probe.c | 5 +----
include/linux/pci.h | 2 ++
6 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kernel/amd_iommu_init.c b/arch/x86/kernel/amd_iommu_init.c
index b4b61d4..e60530a 100644
--- a/arch/x86/kernel/amd_iommu_init.c
+++ b/arch/x86/kernel/amd_iommu_init.c
@@ -1330,6 +1330,8 @@ void __init amd_iommu_detect(void)
gart_iommu_aperture_disabled = 1;
gart_iommu_aperture = 0;
#endif
+ /* Make sure ACS will be enabled */
+ pci_request_acs();
}
}
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 5bccd70..e2511bc 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -27,6 +27,7 @@
#include <linux/page-flags.h>
#include <linux/highmem.h>
#include <linux/console.h>
+#include <linux/pci.h>
#include <xen/xen.h>
#include <xen/interface/xen.h>
@@ -1170,7 +1171,11 @@ asmlinkage void __init xen_start_kernel(void)
add_preferred_console("xenboot", 0, NULL);
add_preferred_console("tty", 0, NULL);
add_preferred_console("hvc", 0, NULL);
+ } else {
+ /* Make sure ACS will be enabled */
+ pci_request_acs();
}
+
xen_raw_console_write("about to get started...\n");
diff --git a/drivers/pci/dmar.c b/drivers/pci/dmar.c
index e01ca4d..0e98f6b 100644
--- a/drivers/pci/dmar.c
+++ b/drivers/pci/dmar.c
@@ -614,8 +614,11 @@ void __init detect_intel_iommu(void)
#endif
#ifdef CONFIG_DMAR
if (ret && !no_iommu && !iommu_detected && !swiotlb &&
- !dmar_disabled)
+ !dmar_disabled) {
iommu_detected = 1;
+ /* Make sure ACS will be enabled */
+ pci_request_acs();
+ }
#endif
}
early_acpi_os_unmap_memory(dmar_tbl, dmar_tbl_size);
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 453d6ba..fc8fa3f 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1550,6 +1550,16 @@ void pci_enable_ari(struct pci_dev *dev)
bridge->ari_enabled = 1;
}
+static int pci_acs_enable;
+
+/**
+ * pci_request_acs - ask for ACS to be enabled if supported
+ */
+void pci_request_acs(void)
+{
+ pci_acs_enable = 1;
+}
+
/**
* pci_enable_acs - enable ACS if hardware support it
* @dev: the PCI device
@@ -1560,6 +1570,9 @@ void pci_enable_acs(struct pci_dev *dev)
u16 cap;
u16 ctrl;
+ if (!pci_acs_enable)
+ return;
+
if (!pci_is_pcie(dev))
return;
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 2fdffc0..98ffb2d 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -10,9 +10,7 @@
#include <linux/module.h>
#include <linux/cpumask.h>
#include <linux/pci-aspm.h>
-#include <linux/iommu.h>
#include <acpi/acpi_hest.h>
-#include <xen/xen.h>
#include "pci.h"
#define CARDBUS_LATENCY_TIMER 176 /* secondary latency timer */
@@ -1029,8 +1027,7 @@ static void pci_init_capabilities(struct pci_dev *dev)
pci_iov_init(dev);
/* Enable ACS P2P upstream forwarding */
- if (iommu_found() || xen_initial_domain())
- pci_enable_acs(dev);
+ pci_enable_acs(dev);
}
void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 2891c3d..04771b9 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1328,5 +1328,7 @@ static inline bool pci_is_pcie(struct pci_dev *dev)
return !!pci_pcie_cap(dev);
}
+void pci_request_acs(void);
+
#endif /* __KERNEL__ */
#endif /* LINUX_PCI_H */
On Fri, 4 Dec 2009 12:15:21 -0800
Chris Wright <[email protected]> wrote:
> Commit ae21ee65e8bc228416bbcc8a1da01c56a847a60c "PCI: acs p2p upsteram
> forwarding enabling" doesn't actually enable ACS.
>
> Add a function to pci core to allow an IOMMU to request that ACS
> be enabled. The existing mechanism of using iommu_found() in the pci
> core to know when ACS should be enabled doesn't actually work due to
> initialization order; iommu has only been detected not initialized.
>
> Have Intel and AMD IOMMUs request ACS, and Xen does as well during
> early init of dom0.
>
> Cc: Allen Kay <[email protected]>
> Cc: David Woodhouse <[email protected]>
> Cc: Jeremy Fitzhardinge <[email protected]>
> Cc: Joerg Roedel <[email protected]>
> Signed-off-by: Chris Wright <[email protected]>
Applied this one, thanks.
--
Jesse Barnes, Intel Open Source Technology Center