2014-02-03 21:27:32

by Alex Williamson

[permalink] [raw]
Subject: [PATCH v2 0/3] Quirk Intel PCH root ports for ACS-like features

v2:
- Remove bus #0 bug in filtering matching
- Add 2/3 introducing PCI_DEV_FLAGS_ACS_ENABLED_QUIRK, this gives
is better tracking and addresses the theoretical hotplug issue
- Update 3/3 for PCI_DEV_FLAGS_ACS_ENABLED_QUIRK
- Add dev_info to print regardless of whether we changes bits
- Add Intel cc

As described in 3/3 many Intel root ports lack PCIe ACS capabilities
which results in excessively large IOMMU groups. Many of these root
ports do provide isolation capabilities, we just need to use device
specific mechanisms to enable and verify. Long term, I hope we can
round out this list (particularly to include X79 root ports) and
more importantly, encourage proper PCIe ACS support in future
products. I'm really hoping we can get this in during the 3.14 cycle.
Thanks,

Alex
---

Alex Williamson (3):
pci: Add device specific PCI ACS enable
pci: Add pci_dev_flag for ACS enable quirks
pci/quirks: Enable quirks for PCIe ACS on Intel PCH root ports


drivers/pci/pci.c | 26 +++++--
drivers/pci/quirks.c | 185 ++++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/pci.h | 4 +
3 files changed, 209 insertions(+), 6 deletions(-)


2014-02-03 21:27:39

by Alex Williamson

[permalink] [raw]
Subject: [PATCH v2 1/3] pci: Add device specific PCI ACS enable

Some devices support PCI ACS-like features, but don't report it using
the standard PCIe capabilities. We already provide hooks for device
specific testing of ACS, but not for device specific enabling of ACS.
This provides that setup hook.

Signed-off-by: Alex Williamson <[email protected]>
---
drivers/pci/pci.c | 26 ++++++++++++++++++++------
drivers/pci/quirks.c | 25 +++++++++++++++++++++++++
include/linux/pci.h | 2 ++
3 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 1febe90..b89502f 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2180,21 +2180,18 @@ void pci_request_acs(void)
}

/**
- * pci_enable_acs - enable ACS if hardware support it
+ * pci_std_enable_acs - enable ACS on devices using standard ACS capabilites
* @dev: the PCI device
*/
-void pci_enable_acs(struct pci_dev *dev)
+static int pci_std_enable_acs(struct pci_dev *dev)
{
int pos;
u16 cap;
u16 ctrl;

- if (!pci_acs_enable)
- return;
-
pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ACS);
if (!pos)
- return;
+ return -ENODEV;

pci_read_config_word(dev, pos + PCI_ACS_CAP, &cap);
pci_read_config_word(dev, pos + PCI_ACS_CTRL, &ctrl);
@@ -2212,6 +2209,23 @@ void pci_enable_acs(struct pci_dev *dev)
ctrl |= (cap & PCI_ACS_UF);

pci_write_config_word(dev, pos + PCI_ACS_CTRL, ctrl);
+
+ return 0;
+}
+
+/**
+ * pci_enable_acs - enable ACS if hardware support it
+ * @dev: the PCI device
+ */
+void pci_enable_acs(struct pci_dev *dev)
+{
+ if (!pci_acs_enable)
+ return;
+
+ if (!pci_std_enable_acs(dev))
+ return;
+
+ pci_dev_specific_enable_acs(dev);
}

static bool pci_acs_flags_enabled(struct pci_dev *pdev, u16 acs_flags)
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 5cb726c..f681fb0 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3461,3 +3461,28 @@ int pci_dev_specific_acs_enabled(struct pci_dev *dev, u16 acs_flags)

return -ENOTTY;
}
+
+static const struct pci_dev_enable_acs {
+ u16 vendor;
+ u16 device;
+ int (*enable_acs)(struct pci_dev *dev);
+} pci_dev_enable_acs[] = {
+ { 0 }
+};
+
+void pci_dev_specific_enable_acs(struct pci_dev *dev)
+{
+ const struct pci_dev_enable_acs *i;
+ int ret;
+
+ for (i = pci_dev_enable_acs; i->enable_acs; i++) {
+ if ((i->vendor == dev->vendor ||
+ i->vendor == (u16)PCI_ANY_ID) &&
+ (i->device == dev->device ||
+ i->device == (u16)PCI_ANY_ID)) {
+ ret = i->enable_acs(dev);
+ if (ret >= 0)
+ return;
+ }
+ }
+}
diff --git a/include/linux/pci.h b/include/linux/pci.h
index fb57c89..f9a47dd 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1510,6 +1510,7 @@ enum pci_fixup_pass {
void pci_fixup_device(enum pci_fixup_pass pass, struct pci_dev *dev);
struct pci_dev *pci_get_dma_source(struct pci_dev *dev);
int pci_dev_specific_acs_enabled(struct pci_dev *dev, u16 acs_flags);
+void pci_dev_specific_enable_acs(struct pci_dev *dev);
#else
static inline void pci_fixup_device(enum pci_fixup_pass pass,
struct pci_dev *dev) { }
@@ -1522,6 +1523,7 @@ static inline int pci_dev_specific_acs_enabled(struct pci_dev *dev,
{
return -ENOTTY;
}
+static inline void pci_dev_specific_enable_acs(struct pci_dev *dev) {}
#endif

void __iomem *pcim_iomap(struct pci_dev *pdev, int bar, unsigned long maxlen);

2014-02-03 21:27:44

by Alex Williamson

[permalink] [raw]
Subject: [PATCH v2 2/3] pci: Add pci_dev_flag for ACS enable quirks

Quirks that enable ACS compatible functionality on a device need some
way to track whether a given device has been enabled. Rather than
create new data structures for this, allocate one of the pci_dev_flags
to indicate this setup.

Signed-off-by: Alex Williamson <[email protected]>
---
include/linux/pci.h | 2 ++
1 file changed, 2 insertions(+)

diff --git a/include/linux/pci.h b/include/linux/pci.h
index f9a47dd..cb9a109 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -170,6 +170,8 @@ enum pci_dev_flags {
PCI_DEV_FLAGS_NO_D3 = (__force pci_dev_flags_t) 2,
/* Provide indication device is assigned by a Virtual Machine Manager */
PCI_DEV_FLAGS_ASSIGNED = (__force pci_dev_flags_t) 4,
+ /* Flag for quirk use to store if quirk specific ACS is enabled */
+ PCI_DEV_FLAGS_ACS_ENABLED_QUIRK = (__force pci_dev_flags_t) 8,
};

enum pci_irq_reroute_variant {

2014-02-03 21:27:53

by Alex Williamson

[permalink] [raw]
Subject: [PATCH v2 3/3] pci/quirks: Enable quirks for PCIe ACS on Intel PCH root ports

Many of the currently available Intel PCH-based root ports do not
provide PCIe ACS capabilities. Without this, we must assume that
peer-to-peer traffic between multifunction root ports and between
devices behind root ports is possible. This lack of isolation is
exposed by grouping the devices together in the same IOMMU group.
If we want to expose these devices to userspace, vfio uses IOMMU
groups as the unit of ownership, thus making it very difficult to
assign individual devices to separate users.

The good news is that the chipset does provide ACS-like isolation
capabilities, but we do need to verify and enable those capabilities
if the BIOS has not done so. This patch implements the device
specific enabling and testing of equivalent ACS function for these
devices.

Signed-off-by: Alex Williamson <[email protected]>
Cc: Don Dugger <[email protected]>
---
drivers/pci/quirks.c | 160 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 160 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index f681fb0..ed2ed86 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3423,6 +3423,61 @@ static int pci_quirk_amd_sb_acs(struct pci_dev *dev, u16 acs_flags)
#endif
}

+/*
+ * Many Intel PCH root ports do provide ACS-like features to disable peer
+ * transactions and validate bus numbers in requests, but do not provide an
+ * actual PCIe ACS capability. This is the list of device IDs known to fall
+ * into that category as provided by Intel in Red Hat bugzilla 1037684.
+ */
+static const u16 pci_quirk_intel_pch_acs_ids[] = {
+ /* Ibexpeak PCH */
+ 0x3b42, 0x3b43, 0x3b44, 0x3b45, 0x3b46, 0x3b47, 0x3b48, 0x3b49,
+ 0x3b4a, 0x3b4b, 0x3b4c, 0x3b4d, 0x3b4e, 0x3b4f, 0x3b50, 0x3b51,
+ /* Cougarpoint PCH */
+ 0x1c10, 0x1c11, 0x1c12, 0x1c13, 0x1c14, 0x1c15, 0x1c16, 0x1c17,
+ 0x1c18, 0x1c19, 0x1c1a, 0x1c1b, 0x1c1c, 0x1c1d, 0x1c1e, 0x1c1f,
+ /* Pantherpoint PCH */
+ 0x1e10, 0x1e11, 0x1e12, 0x1e13, 0x1e14, 0x1e15, 0x1e16, 0x1e17,
+ 0x1e18, 0x1e19, 0x1e1a, 0x1e1b, 0x1e1c, 0x1e1d, 0x1e1e, 0x1e1f,
+ /* Lynxpoint-H PCH */
+ 0x8c10, 0x8c11, 0x8c12, 0x8c13, 0x8c14, 0x8c15, 0x8c16, 0x8c17,
+ 0x8c18, 0x8c19, 0x8c1a, 0x8c1b, 0x8c1c, 0x8c1d, 0x8c1e, 0x8c1f,
+ /* Lynxpoint-LP PCH */
+ 0x9c10, 0x9c11, 0x9c12, 0x9c13, 0x9c14, 0x9c15, 0x9c16, 0x9c17,
+ 0x9c18, 0x9c19, 0x9c1a, 0x9c1b,
+ /* Wildcat PCH */
+ 0x9c90, 0x9c91, 0x9c92, 0x9c93, 0x9c94, 0x9c95, 0x9c96, 0x9c97,
+ 0x9c98, 0x9c99, 0x9c9a, 0x9c9b,
+};
+
+static bool pci_quirk_intel_pch_acs_match(struct pci_dev *dev)
+{
+ int i;
+
+ /* Filter out a few obvious non-matches first */
+ if (!pci_is_pcie(dev) || pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT)
+ return false;
+
+ for (i = 0; i < ARRAY_SIZE(pci_quirk_intel_pch_acs_ids); i++)
+ if (pci_quirk_intel_pch_acs_ids[i] == dev->device)
+ return true;
+
+ return false;
+}
+
+#define INTEL_PCH_ACS_FLAGS (PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF | PCI_ACS_SV)
+
+static int pci_quirk_intel_pch_acs(struct pci_dev *dev, u16 acs_flags)
+{
+ u16 flags = dev->dev_flags & PCI_DEV_FLAGS_ACS_ENABLED_QUIRK ?
+ INTEL_PCH_ACS_FLAGS : 0;
+
+ if (!pci_quirk_intel_pch_acs_match(dev))
+ return -ENOTTY;
+
+ return acs_flags & ~flags ? 0 : 1;
+}
+
static const struct pci_dev_acs_enabled {
u16 vendor;
u16 device;
@@ -3434,6 +3489,7 @@ static const struct pci_dev_acs_enabled {
{ PCI_VENDOR_ID_ATI, 0x439d, pci_quirk_amd_sb_acs },
{ PCI_VENDOR_ID_ATI, 0x4384, pci_quirk_amd_sb_acs },
{ PCI_VENDOR_ID_ATI, 0x4399, pci_quirk_amd_sb_acs },
+ { PCI_VENDOR_ID_INTEL, PCI_ANY_ID, pci_quirk_intel_pch_acs },
{ 0 }
};

@@ -3462,11 +3518,115 @@ int pci_dev_specific_acs_enabled(struct pci_dev *dev, u16 acs_flags)
return -ENOTTY;
}

+/* Config space offset of Root Complex Base Address register */
+#define INTEL_LPC_RCBA_REG 0xf0
+/* 31:14 RCBA address */
+#define INTEL_LPC_RCBA_MASK 0xffffc000
+/* RCBA Enable */
+#define INTEL_LPC_RCBA_ENABLE (1 << 0)
+
+/* Backbone Scratch Pad Register */
+#define INTEL_BSPR_REG 0x1104
+/* Backbone Peer Non-Posted Disable */
+#define INTEL_BSPR_REG_BPNPD (1 << 8)
+/* Backbone Peer Posted Disable */
+#define INTEL_BSPR_REG_BPPD (1 << 9)
+
+/* Upstream Peer Decode Configuration Register */
+#define INTEL_UPDCR_REG 0x1114
+/* 5:0 Peer Decode Enable bits */
+#define INTEL_UPDCR_REG_MASK 0x3f
+
+static int pci_quirk_enable_intel_lpc_acs(struct pci_dev *dev)
+{
+ u32 rcba, bspr, updcr;
+ void __iomem *rcba_mem;
+
+ /*
+ * Read the RCBA register from the LPC (D31:F0). PCH root ports
+ * are D28:F* and therefore get probed before LPC, thus we can't
+ * use pci_get_slot/pci_read_config_dword here.
+ */
+ pci_bus_read_config_dword(dev->bus, PCI_DEVFN(31, 0),
+ INTEL_LPC_RCBA_REG, &rcba);
+ if (!(rcba & INTEL_LPC_RCBA_ENABLE))
+ return -EINVAL;
+
+ rcba_mem = ioremap_nocache(rcba & INTEL_LPC_RCBA_MASK,
+ PAGE_ALIGN(INTEL_UPDCR_REG));
+ if (!rcba_mem)
+ return -ENOMEM;
+
+ /*
+ * The BSPR can disallow peer cycles, but it's set by soft strap and
+ * therefore read-only. If both posted and non-posted peer cycles are
+ * disallowed, we're ok. If either are allowed, then we need to use
+ * the UPDCR to disable peer decodes for each port. This provides the
+ * PCIe ACS equivalent of PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF
+ */
+ bspr = readl(rcba_mem + INTEL_BSPR_REG);
+ bspr &= INTEL_BSPR_REG_BPNPD | INTEL_BSPR_REG_BPPD;
+ if (bspr != (INTEL_BSPR_REG_BPNPD | INTEL_BSPR_REG_BPPD)) {
+ updcr = readl(rcba_mem + INTEL_UPDCR_REG);
+ if (updcr & INTEL_UPDCR_REG_MASK) {
+ dev_info(&dev->dev, "Disabling UPDCR peer decodes\n");
+ updcr &= ~INTEL_UPDCR_REG_MASK;
+ writel(updcr, rcba_mem + INTEL_UPDCR_REG);
+ }
+ }
+
+ iounmap(rcba_mem);
+ return 0;
+}
+
+/* Miscellaneous Port Configuration register */
+#define INTEL_MPC_REG 0xd8
+/* MPC: Invalid Receive Bus Number Check Enable */
+#define INTEL_MPC_REG_IRBNCE (1 << 26)
+
+static void pci_quirk_enable_intel_rp_mpc_acs(struct pci_dev *dev)
+{
+ u32 mpc;
+
+ /*
+ * When enabled, the IRBNCE bit of the MPC register enables the
+ * equivalent of PCI ACS Source Validation (PCI_ACS_SV), which
+ * ensures that requester IDs fall within the bus number range
+ * of the bridge. Enable if not already.
+ */
+ pci_read_config_dword(dev, INTEL_MPC_REG, &mpc);
+ if (!(mpc & INTEL_MPC_REG_IRBNCE)) {
+ dev_info(&dev->dev, "Enabling MPC IRBNCE\n");
+ mpc |= INTEL_MPC_REG_IRBNCE;
+ pci_write_config_word(dev, INTEL_MPC_REG, mpc);
+ }
+}
+
+static int pci_quirk_enable_intel_pch_acs(struct pci_dev *dev)
+{
+ if (!pci_quirk_intel_pch_acs_match(dev))
+ return -ENOTTY;
+
+ if (pci_quirk_enable_intel_lpc_acs(dev)) {
+ dev_warn(&dev->dev, "Failed to enable Intel PCH ACS quirk\n");
+ return 0;
+ }
+
+ pci_quirk_enable_intel_rp_mpc_acs(dev);
+
+ dev->dev_flags |= PCI_DEV_FLAGS_ACS_ENABLED_QUIRK;
+
+ dev_info(&dev->dev, "Intel PCH root port ACS workaround enabled\n");
+
+ return 0;
+}
+
static const struct pci_dev_enable_acs {
u16 vendor;
u16 device;
int (*enable_acs)(struct pci_dev *dev);
} pci_dev_enable_acs[] = {
+ { PCI_VENDOR_ID_INTEL, PCI_ANY_ID, pci_quirk_enable_intel_pch_acs },
{ 0 }
};

2014-02-10 18:43:27

by Dugger, Donald D

[permalink] [raw]
Subject: RE: [PATCH v2 3/3] pci/quirks: Enable quirks for PCIe ACS on Intel PCH root ports

ACK - this is the best way to deal with this issue that we’ve found.

--
Don Dugger
"Censeo Toto nos in Kansa esse decisse." - D. Gale
Ph: 303/443-3786

-----Original Message-----
From: Alex Williamson [mailto:[email protected]]
Sent: Monday, February 3, 2014 2:28 PM
To: [email protected]; [email protected]
Cc: [email protected]; Dugger, Donald D
Subject: [PATCH v2 3/3] pci/quirks: Enable quirks for PCIe ACS on Intel PCH root ports

Many of the currently available Intel PCH-based root ports do not provide PCIe ACS capabilities. Without this, we must assume that peer-to-peer traffic between multifunction root ports and between devices behind root ports is possible. This lack of isolation is exposed by grouping the devices together in the same IOMMU group.
If we want to expose these devices to userspace, vfio uses IOMMU groups as the unit of ownership, thus making it very difficult to assign individual devices to separate users.

The good news is that the chipset does provide ACS-like isolation capabilities, but we do need to verify and enable those capabilities if the BIOS has not done so. This patch implements the device specific enabling and testing of equivalent ACS function for these devices.

Signed-off-by: Alex Williamson <[email protected]>
Cc: Don Dugger <[email protected]>
---
drivers/pci/quirks.c | 160 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 160 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index f681fb0..ed2ed86 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3423,6 +3423,61 @@ static int pci_quirk_amd_sb_acs(struct pci_dev *dev, u16 acs_flags) #endif }

+/*
+ * Many Intel PCH root ports do provide ACS-like features to disable
+peer
+ * transactions and validate bus numbers in requests, but do not
+provide an
+ * actual PCIe ACS capability. This is the list of device IDs known to
+fall
+ * into that category as provided by Intel in Red Hat bugzilla 1037684.
+ */
+static const u16 pci_quirk_intel_pch_acs_ids[] = {
+ /* Ibexpeak PCH */
+ 0x3b42, 0x3b43, 0x3b44, 0x3b45, 0x3b46, 0x3b47, 0x3b48, 0x3b49,
+ 0x3b4a, 0x3b4b, 0x3b4c, 0x3b4d, 0x3b4e, 0x3b4f, 0x3b50, 0x3b51,
+ /* Cougarpoint PCH */
+ 0x1c10, 0x1c11, 0x1c12, 0x1c13, 0x1c14, 0x1c15, 0x1c16, 0x1c17,
+ 0x1c18, 0x1c19, 0x1c1a, 0x1c1b, 0x1c1c, 0x1c1d, 0x1c1e, 0x1c1f,
+ /* Pantherpoint PCH */
+ 0x1e10, 0x1e11, 0x1e12, 0x1e13, 0x1e14, 0x1e15, 0x1e16, 0x1e17,
+ 0x1e18, 0x1e19, 0x1e1a, 0x1e1b, 0x1e1c, 0x1e1d, 0x1e1e, 0x1e1f,
+ /* Lynxpoint-H PCH */
+ 0x8c10, 0x8c11, 0x8c12, 0x8c13, 0x8c14, 0x8c15, 0x8c16, 0x8c17,
+ 0x8c18, 0x8c19, 0x8c1a, 0x8c1b, 0x8c1c, 0x8c1d, 0x8c1e, 0x8c1f,
+ /* Lynxpoint-LP PCH */
+ 0x9c10, 0x9c11, 0x9c12, 0x9c13, 0x9c14, 0x9c15, 0x9c16, 0x9c17,
+ 0x9c18, 0x9c19, 0x9c1a, 0x9c1b,
+ /* Wildcat PCH */
+ 0x9c90, 0x9c91, 0x9c92, 0x9c93, 0x9c94, 0x9c95, 0x9c96, 0x9c97,
+ 0x9c98, 0x9c99, 0x9c9a, 0x9c9b,
+};
+
+static bool pci_quirk_intel_pch_acs_match(struct pci_dev *dev) {
+ int i;
+
+ /* Filter out a few obvious non-matches first */
+ if (!pci_is_pcie(dev) || pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT)
+ return false;
+
+ for (i = 0; i < ARRAY_SIZE(pci_quirk_intel_pch_acs_ids); i++)
+ if (pci_quirk_intel_pch_acs_ids[i] == dev->device)
+ return true;
+
+ return false;
+}
+
+#define INTEL_PCH_ACS_FLAGS (PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF |
+PCI_ACS_SV)
+
+static int pci_quirk_intel_pch_acs(struct pci_dev *dev, u16 acs_flags)
+{
+ u16 flags = dev->dev_flags & PCI_DEV_FLAGS_ACS_ENABLED_QUIRK ?
+ INTEL_PCH_ACS_FLAGS : 0;
+
+ if (!pci_quirk_intel_pch_acs_match(dev))
+ return -ENOTTY;
+
+ return acs_flags & ~flags ? 0 : 1;
+}
+
static const struct pci_dev_acs_enabled {
u16 vendor;
u16 device;
@@ -3434,6 +3489,7 @@ static const struct pci_dev_acs_enabled {
{ PCI_VENDOR_ID_ATI, 0x439d, pci_quirk_amd_sb_acs },
{ PCI_VENDOR_ID_ATI, 0x4384, pci_quirk_amd_sb_acs },
{ PCI_VENDOR_ID_ATI, 0x4399, pci_quirk_amd_sb_acs },
+ { PCI_VENDOR_ID_INTEL, PCI_ANY_ID, pci_quirk_intel_pch_acs },
{ 0 }
};

@@ -3462,11 +3518,115 @@ int pci_dev_specific_acs_enabled(struct pci_dev *dev, u16 acs_flags)
return -ENOTTY;
}

+/* Config space offset of Root Complex Base Address register */ #define
+INTEL_LPC_RCBA_REG 0xf0
+/* 31:14 RCBA address */
+#define INTEL_LPC_RCBA_MASK 0xffffc000
+/* RCBA Enable */
+#define INTEL_LPC_RCBA_ENABLE (1 << 0)
+
+/* Backbone Scratch Pad Register */
+#define INTEL_BSPR_REG 0x1104
+/* Backbone Peer Non-Posted Disable */
+#define INTEL_BSPR_REG_BPNPD (1 << 8)
+/* Backbone Peer Posted Disable */
+#define INTEL_BSPR_REG_BPPD (1 << 9)
+
+/* Upstream Peer Decode Configuration Register */ #define
+INTEL_UPDCR_REG 0x1114
+/* 5:0 Peer Decode Enable bits */
+#define INTEL_UPDCR_REG_MASK 0x3f
+
+static int pci_quirk_enable_intel_lpc_acs(struct pci_dev *dev) {
+ u32 rcba, bspr, updcr;
+ void __iomem *rcba_mem;
+
+ /*
+ * Read the RCBA register from the LPC (D31:F0). PCH root ports
+ * are D28:F* and therefore get probed before LPC, thus we can't
+ * use pci_get_slot/pci_read_config_dword here.
+ */
+ pci_bus_read_config_dword(dev->bus, PCI_DEVFN(31, 0),
+ INTEL_LPC_RCBA_REG, &rcba);
+ if (!(rcba & INTEL_LPC_RCBA_ENABLE))
+ return -EINVAL;
+
+ rcba_mem = ioremap_nocache(rcba & INTEL_LPC_RCBA_MASK,
+ PAGE_ALIGN(INTEL_UPDCR_REG));
+ if (!rcba_mem)
+ return -ENOMEM;
+
+ /*
+ * The BSPR can disallow peer cycles, but it's set by soft strap and
+ * therefore read-only. If both posted and non-posted peer cycles are
+ * disallowed, we're ok. If either are allowed, then we need to use
+ * the UPDCR to disable peer decodes for each port. This provides the
+ * PCIe ACS equivalent of PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF
+ */
+ bspr = readl(rcba_mem + INTEL_BSPR_REG);
+ bspr &= INTEL_BSPR_REG_BPNPD | INTEL_BSPR_REG_BPPD;
+ if (bspr != (INTEL_BSPR_REG_BPNPD | INTEL_BSPR_REG_BPPD)) {
+ updcr = readl(rcba_mem + INTEL_UPDCR_REG);
+ if (updcr & INTEL_UPDCR_REG_MASK) {
+ dev_info(&dev->dev, "Disabling UPDCR peer decodes\n");
+ updcr &= ~INTEL_UPDCR_REG_MASK;
+ writel(updcr, rcba_mem + INTEL_UPDCR_REG);
+ }
+ }
+
+ iounmap(rcba_mem);
+ return 0;
+}
+
+/* Miscellaneous Port Configuration register */ #define INTEL_MPC_REG
+0xd8
+/* MPC: Invalid Receive Bus Number Check Enable */ #define
+INTEL_MPC_REG_IRBNCE (1 << 26)
+
+static void pci_quirk_enable_intel_rp_mpc_acs(struct pci_dev *dev) {
+ u32 mpc;
+
+ /*
+ * When enabled, the IRBNCE bit of the MPC register enables the
+ * equivalent of PCI ACS Source Validation (PCI_ACS_SV), which
+ * ensures that requester IDs fall within the bus number range
+ * of the bridge. Enable if not already.
+ */
+ pci_read_config_dword(dev, INTEL_MPC_REG, &mpc);
+ if (!(mpc & INTEL_MPC_REG_IRBNCE)) {
+ dev_info(&dev->dev, "Enabling MPC IRBNCE\n");
+ mpc |= INTEL_MPC_REG_IRBNCE;
+ pci_write_config_word(dev, INTEL_MPC_REG, mpc);
+ }
+}
+
+static int pci_quirk_enable_intel_pch_acs(struct pci_dev *dev) {
+ if (!pci_quirk_intel_pch_acs_match(dev))
+ return -ENOTTY;
+
+ if (pci_quirk_enable_intel_lpc_acs(dev)) {
+ dev_warn(&dev->dev, "Failed to enable Intel PCH ACS quirk\n");
+ return 0;
+ }
+
+ pci_quirk_enable_intel_rp_mpc_acs(dev);
+
+ dev->dev_flags |= PCI_DEV_FLAGS_ACS_ENABLED_QUIRK;
+
+ dev_info(&dev->dev, "Intel PCH root port ACS workaround enabled\n");
+
+ return 0;
+}
+
static const struct pci_dev_enable_acs {
u16 vendor;
u16 device;
int (*enable_acs)(struct pci_dev *dev); } pci_dev_enable_acs[] = {
+ { PCI_VENDOR_ID_INTEL, PCI_ANY_ID, pci_quirk_enable_intel_pch_acs },
{ 0 }
};


????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-02-14 21:43:44

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] Quirk Intel PCH root ports for ACS-like features

[+cc Don]

On Mon, Feb 03, 2014 at 02:27:27PM -0700, Alex Williamson wrote:
> v2:
> - Remove bus #0 bug in filtering matching
> - Add 2/3 introducing PCI_DEV_FLAGS_ACS_ENABLED_QUIRK, this gives
> is better tracking and addresses the theoretical hotplug issue
> - Update 3/3 for PCI_DEV_FLAGS_ACS_ENABLED_QUIRK
> - Add dev_info to print regardless of whether we changes bits
> - Add Intel cc
>
> As described in 3/3 many Intel root ports lack PCIe ACS capabilities
> which results in excessively large IOMMU groups. Many of these root
> ports do provide isolation capabilities, we just need to use device
> specific mechanisms to enable and verify. Long term, I hope we can
> round out this list (particularly to include X79 root ports) and
> more importantly, encourage proper PCIe ACS support in future
> products. I'm really hoping we can get this in during the 3.14 cycle.
> Thanks,
>
> Alex
> ---
>
> Alex Williamson (3):
> pci: Add device specific PCI ACS enable
> pci: Add pci_dev_flag for ACS enable quirks
> pci/quirks: Enable quirks for PCIe ACS on Intel PCH root ports

I applied these (with Don's ack for 3/3) to pci/virtualization.

I tried to figure out how to handle post-merge window patches. Per
Documentation/development-process/2.Process, "[after -rc1], only patches
which fix problems should be submitted to the mainline," so one might
conclude that a fix for any sort of problem is allowed. However,
Documentation/networking/netdev-FAQ.txt says "No new features get mainlined
after this [-rc1] -- only fixes to the rc1 content are expected," which is
how I've been operating.

In any case, these patches look more like new functionality (enabling a
non-standard ACS feature) than a bug fix to me, so my preference is to
merge them during the v3.15 merge window.

I understand the desire for v3.14, namely, "lots of Intel devices don't
support ACS, and that makes it hard for users to expose devices to
userspace with fine granularity, and waiting for v3.15 will mean another
two months." But this problem is really of Intel's own making: if they'd
used standard ACS, or if they'd documented their non-standard mechanism,
this wouldn't be an issue.

Bjorn

2014-02-14 22:17:31

by Dugger, Donald D

[permalink] [raw]
Subject: RE: [PATCH v2 0/3] Quirk Intel PCH root ports for ACS-like features

In re: Feature vs. bug fix

At the risk of trying to arguing about the meaning of `is' I would argue that this is more a bug fix than a feature. We're not adding a new capability, we are trying to work around deficient HW (no argument that we should have implemented ACS on all platforms).

Having said that, although I would obviously prefer to see this in v3.14 we can live with this hitting v3.15.

--
Don Dugger
"Censeo Toto nos in Kansa esse decisse." - D. Gale
Ph: 303/443-3786

-----Original Message-----
From: Bjorn Helgaas [mailto:[email protected]]
Sent: Friday, February 14, 2014 2:43 PM
To: Alex Williamson
Cc: [email protected]; [email protected]; Dugger, Donald D
Subject: Re: [PATCH v2 0/3] Quirk Intel PCH root ports for ACS-like features

[+cc Don]

On Mon, Feb 03, 2014 at 02:27:27PM -0700, Alex Williamson wrote:
> v2:
> - Remove bus #0 bug in filtering matching
> - Add 2/3 introducing PCI_DEV_FLAGS_ACS_ENABLED_QUIRK, this gives
> is better tracking and addresses the theoretical hotplug issue
> - Update 3/3 for PCI_DEV_FLAGS_ACS_ENABLED_QUIRK
> - Add dev_info to print regardless of whether we changes bits
> - Add Intel cc
>
> As described in 3/3 many Intel root ports lack PCIe ACS capabilities
> which results in excessively large IOMMU groups. Many of these root
> ports do provide isolation capabilities, we just need to use device
> specific mechanisms to enable and verify. Long term, I hope we can
> round out this list (particularly to include X79 root ports) and more
> importantly, encourage proper PCIe ACS support in future products.
> I'm really hoping we can get this in during the 3.14 cycle.
> Thanks,
>
> Alex
> ---
>
> Alex Williamson (3):
> pci: Add device specific PCI ACS enable
> pci: Add pci_dev_flag for ACS enable quirks
> pci/quirks: Enable quirks for PCIe ACS on Intel PCH root ports

I applied these (with Don's ack for 3/3) to pci/virtualization.

I tried to figure out how to handle post-merge window patches. Per Documentation/development-process/2.Process, "[after -rc1], only patches which fix problems should be submitted to the mainline," so one might conclude that a fix for any sort of problem is allowed. However, Documentation/networking/netdev-FAQ.txt says "No new features get mainlined after this [-rc1] -- only fixes to the rc1 content are expected," which is how I've been operating.

In any case, these patches look more like new functionality (enabling a non-standard ACS feature) than a bug fix to me, so my preference is to merge them during the v3.15 merge window.

I understand the desire for v3.14, namely, "lots of Intel devices don't support ACS, and that makes it hard for users to expose devices to userspace with fine granularity, and waiting for v3.15 will mean another two months." But this problem is really of Intel's own making: if they'd used standard ACS, or if they'd documented their non-standard mechanism, this wouldn't be an issue.

Bjorn
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-02-14 22:22:39

by Alex Williamson

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] Quirk Intel PCH root ports for ACS-like features

On Fri, 2014-02-14 at 14:43 -0700, Bjorn Helgaas wrote:
> [+cc Don]
>
> On Mon, Feb 03, 2014 at 02:27:27PM -0700, Alex Williamson wrote:
> > v2:
> > - Remove bus #0 bug in filtering matching
> > - Add 2/3 introducing PCI_DEV_FLAGS_ACS_ENABLED_QUIRK, this gives
> > is better tracking and addresses the theoretical hotplug issue
> > - Update 3/3 for PCI_DEV_FLAGS_ACS_ENABLED_QUIRK
> > - Add dev_info to print regardless of whether we changes bits
> > - Add Intel cc
> >
> > As described in 3/3 many Intel root ports lack PCIe ACS capabilities
> > which results in excessively large IOMMU groups. Many of these root
> > ports do provide isolation capabilities, we just need to use device
> > specific mechanisms to enable and verify. Long term, I hope we can
> > round out this list (particularly to include X79 root ports) and
> > more importantly, encourage proper PCIe ACS support in future
> > products. I'm really hoping we can get this in during the 3.14 cycle.
> > Thanks,
> >
> > Alex
> > ---
> >
> > Alex Williamson (3):
> > pci: Add device specific PCI ACS enable
> > pci: Add pci_dev_flag for ACS enable quirks
> > pci/quirks: Enable quirks for PCIe ACS on Intel PCH root ports
>
> I applied these (with Don's ack for 3/3) to pci/virtualization.
>
> I tried to figure out how to handle post-merge window patches. Per
> Documentation/development-process/2.Process, "[after -rc1], only patches
> which fix problems should be submitted to the mainline," so one might
> conclude that a fix for any sort of problem is allowed. However,
> Documentation/networking/netdev-FAQ.txt says "No new features get mainlined
> after this [-rc1] -- only fixes to the rc1 content are expected," which is
> how I've been operating.
>
> In any case, these patches look more like new functionality (enabling a
> non-standard ACS feature) than a bug fix to me, so my preference is to
> merge them during the v3.15 merge window.
>
> I understand the desire for v3.14, namely, "lots of Intel devices don't
> support ACS, and that makes it hard for users to expose devices to
> userspace with fine granularity, and waiting for v3.15 will mean another
> two months." But this problem is really of Intel's own making: if they'd
> used standard ACS, or if they'd documented their non-standard mechanism,
> this wouldn't be an issue.

I appreciate you taking the time to consider it for 3.14. I'll look
forward to it appearing in linux-next for 3.15. Thanks for review and
applying,

Alex