2012-05-30 04:48:49

by Alex Williamson

[permalink] [raw]
Subject: [PATCH 0/5] PCI: Prep for VFIO and IOMMU Groups

These are the PCI related patches from the previously submitted
v2 IOMMU Groups + VFIO series. I've updated them with Don's
comments and split them off on their own so that we can hopefully
make some progress getting these in. Patches are against 20120529
linux-next tree.

Patches 1 & 2 provide DMA quirking and ACS checking which will be
used by IOMMU drivers supporting IOMMU groups. Patch 3 enables the
set of pci user config access functions to be called from other
modules, which will be used by VFIO-pci. Patch 4 enables a common
translation of pcibios errors into errno for returning to userspace,
also to be used by VFIO-pci. Patch 5 adds additional PCI register
definitions so we can more easily parse config space for devices in
VFIO-pci.

These patches, as well as IOMMU group support and VFIO can be
found in git here:

git://github.com/awilliam/linux-vfio.git (iommu-group-vfio-next-20120529)

Please consider these for 3.5, but I'll settle for any kind of next
branch. Thanks,

Alex

---

Alex Williamson (5):
pci: Misc pci_reg additions
pci: Create common pcibios_err_to_errno
pci: export pci_user functions for use by other drivers
pci: Add ACS validation utility
pci: Add PCI DMA source ID quirk


drivers/pci/access.c | 6 +-
drivers/pci/pci.c | 68 ++++++++++++++++++++
drivers/pci/pci.h | 7 --
drivers/pci/quirks.c | 69 +++++++++++++++++++++
drivers/xen/xen-pciback/conf_space.c | 6 +-
include/linux/pci.h | 49 ++++++++++++++-
include/linux/pci_regs.h | 113 ++++++++++++++++++++++++++++++----
7 files changed, 293 insertions(+), 25 deletions(-)


2012-05-30 04:48:56

by Alex Williamson

[permalink] [raw]
Subject: [PATCH 2/5] pci: Add ACS validation utility

In a PCI environment, transactions aren't always required to reach
the root bus before being re-routed. Intermediate switches between
an endpoint and the root bus can redirect DMA back downstream before
things like IOMMUs have a chance to intervene. Legacy PCI is always
susceptible to this as it operates on a shared bus. PCIe added a
new capability to describe and control this behavior, Access Control
Services, or ACS. The utility function pci_acs_enabled() allows us
to test the ACS capabilities of an individual devices against a set
of flags while pci_acs_path_enabled() tests a complete path from
a given downstream device up to the specified upstream device. We
also include the ability to add device specific tests as it's
likely we'll see devices that do no implement ACS, but want to
indicate support for various capabilities in this space.

Signed-off-by: Alex Williamson <[email protected]>
---

drivers/pci/pci.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++
drivers/pci/quirks.c | 29 +++++++++++++++++++++
include/linux/pci.h | 10 +++++++
3 files changed, 106 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 7629256..5883624 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2370,6 +2370,74 @@ void pci_enable_acs(struct pci_dev *dev)
}

/**
+ * pci_acs_enabled - test ACS against required flags for a given device
+ * @pdev: device to test
+ * @acs_flags: required PCI ACS flags
+ *
+ * Return true if the device supports the provided flags. Automatically
+ * filters out flags that are not implemented on multifunction devices.
+ */
+bool pci_acs_enabled(struct pci_dev *pdev, u16 acs_flags)
+{
+ int pos;
+ u16 ctrl;
+
+ if (pci_dev_specific_acs_enabled(pdev, acs_flags))
+ return true;
+
+ if (!pci_is_pcie(pdev))
+ return false;
+
+ /* Filter out flags not applicable to multifunction */
+ if (pdev->multifunction)
+ acs_flags &= (PCI_ACS_RR | PCI_ACS_CR |
+ PCI_ACS_EC | PCI_ACS_DT);
+
+ if (pdev->pcie_type == PCI_EXP_TYPE_DOWNSTREAM ||
+ pdev->pcie_type == PCI_EXP_TYPE_ROOT_PORT ||
+ pdev->multifunction) {
+ pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_ACS);
+ if (!pos)
+ return false;
+
+ pci_read_config_word(pdev, pos + PCI_ACS_CTRL, &ctrl);
+ if ((ctrl & acs_flags) != acs_flags)
+ return false;
+ }
+
+ return true;
+}
+
+/**
+ * pci_acs_path_enable - test ACS flags from start to end in a hierarchy
+ * @start: starting downstream device
+ * @end: ending upstream device or NULL to search to the root bus
+ * @acs_flags: required flags
+ *
+ * Walk up a device tree from start to end testing PCI ACS support. If
+ * any step along the way does not support the required flags, return false.
+ */
+bool pci_acs_path_enabled(struct pci_dev *start,
+ struct pci_dev *end, u16 acs_flags)
+{
+ struct pci_dev *pdev, *parent = start;
+
+ do {
+ pdev = parent;
+
+ if (!pci_acs_enabled(pdev, acs_flags))
+ return false;
+
+ if (pci_is_root_bus(pdev->bus))
+ return (end == NULL);
+
+ parent = pdev->bus->self;
+ } while (pdev != end);
+
+ return true;
+}
+
+/**
* 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/quirks.c b/drivers/pci/quirks.c
index 004e167..0f2cf5c 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3219,3 +3219,32 @@ struct pci_dev *pci_dma_source(struct pci_dev *dev)

return dev;
}
+
+static const struct pci_dev_acs_enabled {
+ u16 vendor;
+ u16 device;
+ bool (*acs_enabled)(struct pci_dev *dev, u16 acs_flags);
+} pci_dev_acs_enabled[] = {
+ { 0 }
+};
+
+bool pci_dev_specific_acs_enabled(struct pci_dev *dev, u16 acs_flags)
+{
+ const struct pci_dev_acs_enabled *i;
+
+ /*
+ * Allow devices that do not expose standard PCIe ACS capabilities
+ * or control to indicate their support here. Multi-function express
+ * devices which do not allow internal peer-to-peer between functions,
+ * but do not implement PCIe ACS may wish to return true here.
+ */
+ for (i = pci_dev_acs_enabled; i->acs_enabled; i++) {
+ if ((i->vendor == dev->vendor ||
+ i->vendor == (u16)PCI_ANY_ID) &&
+ (i->device == dev->device ||
+ i->device == (u16)PCI_ANY_ID))
+ return i->acs_enabled(dev, acs_flags);
+ }
+
+ return false;
+}
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 5bc7502..8ceef38 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1487,6 +1487,7 @@ enum pci_fixup_pass {
#ifdef CONFIG_PCI_QUIRKS
void pci_fixup_device(enum pci_fixup_pass pass, struct pci_dev *dev);
struct pci_dev *pci_dma_source(struct pci_dev *dev);
+bool pci_dev_specific_acs_enabled(struct pci_dev *dev, u16 acs_flags);
#else
static inline void pci_fixup_device(enum pci_fixup_pass pass,
struct pci_dev *dev) {}
@@ -1494,6 +1495,11 @@ static inline struct pci_dev *pci_dma_source(struct pci_dev *dev)
{
return dev;
}
+static inline bool pci_dev_specific_acs_enabled(struct pci_dev *dev,
+ u16 acs_flags)
+{
+ return false;
+}
#endif

void __iomem *pcim_iomap(struct pci_dev *pdev, int bar, unsigned long maxlen);
@@ -1596,7 +1602,9 @@ static inline bool pci_is_pcie(struct pci_dev *dev)
}

void pci_request_acs(void);
-
+bool pci_acs_enabled(struct pci_dev *pdev, u16 acs_flags);
+bool pci_acs_path_enabled(struct pci_dev *start,
+ struct pci_dev *end, u16 acs_flags);

#define PCI_VPD_LRDT 0x80 /* Large Resource Data Type */
#define PCI_VPD_LRDT_ID(x) (x | PCI_VPD_LRDT)

2012-05-30 04:49:06

by Alex Williamson

[permalink] [raw]
Subject: [PATCH 3/5] pci: export pci_user functions for use by other drivers

VFIO PCI support will make use of these for user initiated
PCI config accesses.

Signed-off-by: Alex Williamson <[email protected]>
Acked-by: Bjorn Helgaas <[email protected]>
---

drivers/pci/access.c | 6 ++++--
drivers/pci/pci.h | 7 -------
include/linux/pci.h | 8 ++++++++
3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/access.c b/drivers/pci/access.c
index 2a58164..ba91a7e 100644
--- a/drivers/pci/access.c
+++ b/drivers/pci/access.c
@@ -162,7 +162,8 @@ int pci_user_read_config_##size \
if (ret > 0) \
ret = -EINVAL; \
return ret; \
-}
+} \
+EXPORT_SYMBOL_GPL(pci_user_read_config_##size);

/* Returns 0 on success, negative values indicate error. */
#define PCI_USER_WRITE_CONFIG(size,type) \
@@ -181,7 +182,8 @@ int pci_user_write_config_##size \
if (ret > 0) \
ret = -EINVAL; \
return ret; \
-}
+} \
+EXPORT_SYMBOL_GPL(pci_user_write_config_##size);

PCI_USER_READ_CONFIG(byte, u8)
PCI_USER_READ_CONFIG(word, u16)
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index e494347..f2dcc46 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -86,13 +86,6 @@ static inline bool pci_is_bridge(struct pci_dev *pci_dev)
return !!(pci_dev->subordinate);
}

-extern int pci_user_read_config_byte(struct pci_dev *dev, int where, u8 *val);
-extern int pci_user_read_config_word(struct pci_dev *dev, int where, u16 *val);
-extern int pci_user_read_config_dword(struct pci_dev *dev, int where, u32 *val);
-extern int pci_user_write_config_byte(struct pci_dev *dev, int where, u8 val);
-extern int pci_user_write_config_word(struct pci_dev *dev, int where, u16 val);
-extern int pci_user_write_config_dword(struct pci_dev *dev, int where, u32 val);
-
struct pci_vpd_ops {
ssize_t (*read)(struct pci_dev *dev, loff_t pos, size_t count, void *buf);
ssize_t (*write)(struct pci_dev *dev, loff_t pos, size_t count, const void *buf);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 8ceef38..1b6d1c5 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -777,6 +777,14 @@ static inline int pci_write_config_dword(const struct pci_dev *dev, int where,
return pci_bus_write_config_dword(dev->bus, dev->devfn, where, val);
}

+/* user-space driven config access */
+int pci_user_read_config_byte(struct pci_dev *dev, int where, u8 *val);
+int pci_user_read_config_word(struct pci_dev *dev, int where, u16 *val);
+int pci_user_read_config_dword(struct pci_dev *dev, int where, u32 *val);
+int pci_user_write_config_byte(struct pci_dev *dev, int where, u8 val);
+int pci_user_write_config_word(struct pci_dev *dev, int where, u16 val);
+int pci_user_write_config_dword(struct pci_dev *dev, int where, u32 val);
+
int __must_check pci_enable_device(struct pci_dev *dev);
int __must_check pci_enable_device_io(struct pci_dev *dev);
int __must_check pci_enable_device_mem(struct pci_dev *dev);

2012-05-30 04:49:18

by Alex Williamson

[permalink] [raw]
Subject: [PATCH 5/5] pci: Misc pci_reg additions

Fill in many missing definitions and add sizeof fields for many
sections allowing for more extensive config parsing.

Signed-off-by: Alex Williamson <[email protected]>
---

include/linux/pci_regs.h | 113 +++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 101 insertions(+), 12 deletions(-)

diff --git a/include/linux/pci_regs.h b/include/linux/pci_regs.h
index 4b608f5..526d2c4 100644
--- a/include/linux/pci_regs.h
+++ b/include/linux/pci_regs.h
@@ -26,6 +26,7 @@
* Under PCI, each device has 256 bytes of configuration address space,
* of which the first 64 bytes are standardized as follows:
*/
+#define PCI_STD_HEADER_SIZEOF 64
#define PCI_VENDOR_ID 0x00 /* 16 bits */
#define PCI_DEVICE_ID 0x02 /* 16 bits */
#define PCI_COMMAND 0x04 /* 16 bits */
@@ -209,9 +210,12 @@
#define PCI_CAP_ID_SHPC 0x0C /* PCI Standard Hot-Plug Controller */
#define PCI_CAP_ID_SSVID 0x0D /* Bridge subsystem vendor/device ID */
#define PCI_CAP_ID_AGP3 0x0E /* AGP Target PCI-PCI bridge */
+#define PCI_CAP_ID_SECDEV 0x0F /* Secure Device */
#define PCI_CAP_ID_EXP 0x10 /* PCI Express */
#define PCI_CAP_ID_MSIX 0x11 /* MSI-X */
+#define PCI_CAP_ID_SATA 0x12 /* SATA Data/Index Conf. */
#define PCI_CAP_ID_AF 0x13 /* PCI Advanced Features */
+#define PCI_CAP_ID_MAX PCI_CAP_ID_AF
#define PCI_CAP_LIST_NEXT 1 /* Next capability in the list */
#define PCI_CAP_FLAGS 2 /* Capability defined flags (16 bits) */
#define PCI_CAP_SIZEOF 4
@@ -276,6 +280,7 @@
#define PCI_VPD_ADDR_MASK 0x7fff /* Address mask */
#define PCI_VPD_ADDR_F 0x8000 /* Write 0, 1 indicates completion */
#define PCI_VPD_DATA 4 /* 32-bits of data returned here */
+#define PCI_CAP_VPD_SIZEOF 8

/* Slot Identification */

@@ -297,8 +302,10 @@
#define PCI_MSI_ADDRESS_HI 8 /* Upper 32 bits (if PCI_MSI_FLAGS_64BIT set) */
#define PCI_MSI_DATA_32 8 /* 16 bits of data for 32-bit devices */
#define PCI_MSI_MASK_32 12 /* Mask bits register for 32-bit devices */
+#define PCI_MSI_PENDING_32 16 /* Pending intrs for 32-bit devices */
#define PCI_MSI_DATA_64 12 /* 16 bits of data for 64-bit devices */
#define PCI_MSI_MASK_64 16 /* Mask bits register for 64-bit devices */
+#define PCI_MSI_PENDING_64 20 /* Pending intrs for 64-bit devices */

/* MSI-X registers */
#define PCI_MSIX_FLAGS 2
@@ -308,6 +315,7 @@
#define PCI_MSIX_TABLE 4
#define PCI_MSIX_PBA 8
#define PCI_MSIX_FLAGS_BIRMASK (7 << 0)
+#define PCI_CAP_MSIX_SIZEOF 12 /* size of MSIX registers */

/* MSI-X entry's format */
#define PCI_MSIX_ENTRY_SIZE 16
@@ -338,6 +346,7 @@
#define PCI_AF_CTRL_FLR 0x01
#define PCI_AF_STATUS 5
#define PCI_AF_STATUS_TP 0x01
+#define PCI_CAP_AF_SIZEOF 6 /* size of AF registers */

/* PCI-X registers */

@@ -374,6 +383,10 @@
#define PCI_X_STATUS_SPL_ERR 0x20000000 /* Rcvd Split Completion Error Msg */
#define PCI_X_STATUS_266MHZ 0x40000000 /* 266 MHz capable */
#define PCI_X_STATUS_533MHZ 0x80000000 /* 533 MHz capable */
+#define PCI_X_ECC_CSR 8 /* ECC control and status */
+#define PCI_CAP_PCIX_SIZEOF_V0 8 /* size of registers for Version 0 */
+#define PCI_CAP_PCIX_SIZEOF_V1 24 /* size for Version 1 */
+#define PCI_CAP_PCIX_SIZEOF_V2 PCI_CAP_PCIX_SIZEOF_V1 /* Same for v2 */

/* PCI Bridge Subsystem ID registers */

@@ -462,6 +475,7 @@
#define PCI_EXP_LNKSTA_DLLLA 0x2000 /* Data Link Layer Link Active */
#define PCI_EXP_LNKSTA_LBMS 0x4000 /* Link Bandwidth Management Status */
#define PCI_EXP_LNKSTA_LABS 0x8000 /* Link Autonomous Bandwidth Status */
+#define PCI_CAP_EXP_ENDPOINT_SIZEOF_V1 20 /* v1 endpoints end here */
#define PCI_EXP_SLTCAP 20 /* Slot Capabilities */
#define PCI_EXP_SLTCAP_ABP 0x00000001 /* Attention Button Present */
#define PCI_EXP_SLTCAP_PCP 0x00000002 /* Power Controller Present */
@@ -521,6 +535,7 @@
#define PCI_EXP_OBFF_MSGA_EN 0x2000 /* OBFF enable with Message type A */
#define PCI_EXP_OBFF_MSGB_EN 0x4000 /* OBFF enable with Message type B */
#define PCI_EXP_OBFF_WAKE_EN 0x6000 /* OBFF using WAKE# signaling */
+#define PCI_CAP_EXP_ENDPOINT_SIZEOF_V2 44 /* v2 endpoints end here */
#define PCI_EXP_LNKCTL2 48 /* Link Control 2 */
#define PCI_EXP_SLTCTL2 56 /* Slot Control 2 */

@@ -529,23 +544,43 @@
#define PCI_EXT_CAP_VER(header) ((header >> 16) & 0xf)
#define PCI_EXT_CAP_NEXT(header) ((header >> 20) & 0xffc)

-#define PCI_EXT_CAP_ID_ERR 1
-#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_VNDR 11
-#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
-#define PCI_EXT_CAP_ID_PRI 19
-#define PCI_EXT_CAP_ID_LTR 24
-#define PCI_EXT_CAP_ID_PASID 27
+#define PCI_EXT_CAP_ID_ERR 0x01 /* Advanced Error Reporting */
+#define PCI_EXT_CAP_ID_VC 0x02 /* Virtual Channel Capability */
+#define PCI_EXT_CAP_ID_DSN 0x03 /* Device Serial Number */
+#define PCI_EXT_CAP_ID_PWR 0x04 /* Power Budgeting */
+#define PCI_EXT_CAP_ID_RCLD 0x05 /* Root Complex Link Declaration */
+#define PCI_EXT_CAP_ID_RCILC 0x06 /* Root Complex Internal Link Control */
+#define PCI_EXT_CAP_ID_RCEC 0x07 /* Root Complex Event Collector */
+#define PCI_EXT_CAP_ID_MFVC 0x08 /* Multi-Function VC Capability */
+#define PCI_EXT_CAP_ID_VC9 0x09 /* same as _VC */
+#define PCI_EXT_CAP_ID_RCRB 0x0A /* Root Complex RB? */
+#define PCI_EXT_CAP_ID_VNDR 0x0B /* Vendor Specific */
+#define PCI_EXT_CAP_ID_CAC 0x0C /* Config Access - obsolete */
+#define PCI_EXT_CAP_ID_ACS 0x0D /* Access Control Services */
+#define PCI_EXT_CAP_ID_ARI 0x0E /* Alternate Routing ID */
+#define PCI_EXT_CAP_ID_ATS 0x0F /* Address Translation Services */
+#define PCI_EXT_CAP_ID_SRIOV 0x10 /* Single Root I/O Virtualization */
+#define PCI_EXT_CAP_ID_MRIOV 0x11 /* Multi Root I/O Virtualization */
+#define PCI_EXT_CAP_ID_MCAST 0x12 /* Multicast */
+#define PCI_EXT_CAP_ID_PRI 0x13 /* Page Request Interface */
+#define PCI_EXT_CAP_ID_AMD_XXX 0x14 /* reserved for AMD */
+#define PCI_EXT_CAP_ID_REBAR 0x15 /* resizable BAR */
+#define PCI_EXT_CAP_ID_DPA 0x16 /* dynamic power alloc */
+#define PCI_EXT_CAP_ID_TPH 0x17 /* TPH request */
+#define PCI_EXT_CAP_ID_LTR 0x18 /* latency tolerance reporting */
+#define PCI_EXT_CAP_ID_SECPCI 0x19 /* Secondary PCIe */
+#define PCI_EXT_CAP_ID_PMUX 0x1A /* Protocol Multiplexing */
+#define PCI_EXT_CAP_ID_PASID 0x1B /* Process Address Space ID */
+#define PCI_EXT_CAP_ID_MAX PCI_EXT_CAP_ID_PASID
+
+#define PCI_EXT_CAP_DSN_SIZEOF 12
+#define PCI_EXT_CAP_MCAST_ENDPOINT_SIZEOF 40

/* Advanced Error Reporting */
#define PCI_ERR_UNCOR_STATUS 4 /* Uncorrectable Error Status */
#define PCI_ERR_UNC_TRAIN 0x00000001 /* Training */
#define PCI_ERR_UNC_DLP 0x00000010 /* Data Link Protocol */
+#define PCI_ERR_UNC_SURPDN 0x00000020 /* Surprise Down */
#define PCI_ERR_UNC_POISON_TLP 0x00001000 /* Poisoned TLP */
#define PCI_ERR_UNC_FCP 0x00002000 /* Flow Control Protocol */
#define PCI_ERR_UNC_COMP_TIME 0x00004000 /* Completion Timeout */
@@ -555,6 +590,11 @@
#define PCI_ERR_UNC_MALF_TLP 0x00040000 /* Malformed TLP */
#define PCI_ERR_UNC_ECRC 0x00080000 /* ECRC Error Status */
#define PCI_ERR_UNC_UNSUP 0x00100000 /* Unsupported Request */
+#define PCI_ERR_UNC_ACSV 0x00200000 /* ACS Violation */
+#define PCI_ERR_UNC_INTN 0x00400000 /* internal error */
+#define PCI_ERR_UNC_MCBTLP 0x00800000 /* MC blocked TLP */
+#define PCI_ERR_UNC_ATOMEG 0x01000000 /* Atomic egress blocked */
+#define PCI_ERR_UNC_TLPPRE 0x02000000 /* TLP prefix blocked */
#define PCI_ERR_UNCOR_MASK 8 /* Uncorrectable Error Mask */
/* Same bits as above */
#define PCI_ERR_UNCOR_SEVER 12 /* Uncorrectable Error Severity */
@@ -565,6 +605,9 @@
#define PCI_ERR_COR_BAD_DLLP 0x00000080 /* Bad DLLP Status */
#define PCI_ERR_COR_REP_ROLL 0x00000100 /* REPLAY_NUM Rollover */
#define PCI_ERR_COR_REP_TIMER 0x00001000 /* Replay Timer Timeout */
+#define PCI_ERR_COR_ADV_NFAT 0x00002000 /* Advisory Non-Fatal */
+#define PCI_ERR_COR_INTERNAL 0x00004000 /* Corrected Internal */
+#define PCI_ERR_COR_LOG_OVER 0x00008000 /* Header Log Overflow */
#define PCI_ERR_COR_MASK 20 /* Correctable Error Mask */
/* Same bits as above */
#define PCI_ERR_CAP 24 /* Advanced Error Capabilities */
@@ -596,12 +639,18 @@

/* Virtual Channel */
#define PCI_VC_PORT_REG1 4
+#define PCI_VC_REG1_EVCC 0x7 /* extended vc count */
#define PCI_VC_PORT_REG2 8
+#define PCI_VC_REG2_32_PHASE 0x2
+#define PCI_VC_REG2_64_PHASE 0x4
+#define PCI_VC_REG2_128_PHASE 0x8
#define PCI_VC_PORT_CTRL 12
#define PCI_VC_PORT_STATUS 14
#define PCI_VC_RES_CAP 16
#define PCI_VC_RES_CTRL 20
#define PCI_VC_RES_STATUS 26
+#define PCI_CAP_VC_BASE_SIZEOF 0x10
+#define PCI_CAP_VC_PER_VC_SIZEOF 0x0C

/* Power Budgeting */
#define PCI_PWR_DSR 4 /* Data Select Register */
@@ -614,6 +663,7 @@
#define PCI_PWR_DATA_RAIL(x) (((x) >> 18) & 7) /* Power Rail */
#define PCI_PWR_CAP 12 /* Capability */
#define PCI_PWR_CAP_BUDGET(x) ((x) & 1) /* Included in system budget */
+#define PCI_EXT_CAP_PWR_SIZEOF 16

/*
* Hypertransport sub capability types
@@ -646,6 +696,8 @@
#define HT_CAPTYPE_ERROR_RETRY 0xC0 /* Retry on error configuration */
#define HT_CAPTYPE_GEN3 0xD0 /* Generation 3 hypertransport configuration */
#define HT_CAPTYPE_PM 0xE0 /* Hypertransport powermanagement configuration */
+#define HT_CAP_SIZEOF_LONG 28 /* slave & primary */
+#define HT_CAP_SIZEOF_SHORT 24 /* host & secondary */

/* Alternative Routing-ID Interpretation */
#define PCI_ARI_CAP 0x04 /* ARI Capability Register */
@@ -656,6 +708,7 @@
#define PCI_ARI_CTRL_MFVC 0x0001 /* MFVC Function Groups Enable */
#define PCI_ARI_CTRL_ACS 0x0002 /* ACS Function Groups Enable */
#define PCI_ARI_CTRL_FG(x) (((x) >> 4) & 7) /* Function Group */
+#define PCI_EXT_CAP_ARI_SIZEOF 8

/* Address Translation Service */
#define PCI_ATS_CAP 0x04 /* ATS Capability Register */
@@ -665,6 +718,7 @@
#define PCI_ATS_CTRL_ENABLE 0x8000 /* ATS Enable */
#define PCI_ATS_CTRL_STU(x) ((x) & 0x1f) /* Smallest Translation Unit */
#define PCI_ATS_MIN_STU 12 /* shift of minimum STU block */
+#define PCI_EXT_CAP_ATS_SIZEOF 8

/* Page Request Interface */
#define PCI_PRI_CTRL 0x04 /* PRI control register */
@@ -676,6 +730,7 @@
#define PCI_PRI_STATUS_STOPPED 0x100 /* PRI Stopped */
#define PCI_PRI_MAX_REQ 0x08 /* PRI max reqs supported */
#define PCI_PRI_ALLOC_REQ 0x0c /* PRI max reqs allowed */
+#define PCI_EXT_CAP_PRI_SIZEOF 16

/* PASID capability */
#define PCI_PASID_CAP 0x04 /* PASID feature register */
@@ -685,6 +740,7 @@
#define PCI_PASID_CTRL_ENABLE 0x01 /* Enable bit */
#define PCI_PASID_CTRL_EXEC 0x02 /* Exec permissions Enable */
#define PCI_PASID_CTRL_PRIV 0x04 /* Priviledge Mode Enable */
+#define PCI_EXT_CAP_PASID_SIZEOF 8

/* Single Root I/O Virtualization */
#define PCI_SRIOV_CAP 0x04 /* SR-IOV Capabilities */
@@ -716,12 +772,14 @@
#define PCI_SRIOV_VFM_MI 0x1 /* Dormant.MigrateIn */
#define PCI_SRIOV_VFM_MO 0x2 /* Active.MigrateOut */
#define PCI_SRIOV_VFM_AV 0x3 /* Active.Available */
+#define PCI_EXT_CAP_SRIOV_SIZEOF 64

#define PCI_LTR_MAX_SNOOP_LAT 0x4
#define PCI_LTR_MAX_NOSNOOP_LAT 0x6
#define PCI_LTR_VALUE_MASK 0x000003ff
#define PCI_LTR_SCALE_MASK 0x00001c00
#define PCI_LTR_SCALE_SHIFT 10
+#define PCI_EXT_CAP_LTR_SIZEOF 8

/* Access Control Service */
#define PCI_ACS_CAP 0x04 /* ACS Capability Register */
@@ -732,7 +790,38 @@
#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_EGRESS_BITS 0x05 /* ACS Egress Control Vector Size */
#define PCI_ACS_CTRL 0x06 /* ACS Control Register */
#define PCI_ACS_EGRESS_CTL_V 0x08 /* ACS Egress Control Vector */

+#define PCI_VSEC_HDR 4 /* extended cap - vendor specific */
+#define PCI_VSEC_HDR_LEN_SHIFT 20 /* shift for length field */
+
+/* sata capability */
+#define PCI_SATA_REGS 4 /* SATA REGs specifier */
+#define PCI_SATA_REGS_MASK 0xF /* location - BAR#/inline */
+#define PCI_SATA_REGS_INLINE 0xF /* REGS in config space */
+#define PCI_SATA_SIZEOF_SHORT 8
+#define PCI_SATA_SIZEOF_LONG 16
+
+/* resizable BARs */
+#define PCI_REBAR_CTRL 8 /* control register */
+#define PCI_REBAR_CTRL_NBAR_MASK (7 << 5) /* mask for # bars */
+#define PCI_REBAR_CTRL_NBAR_SHIFT 5 /* shift for # bars */
+
+/* dynamic power allocation */
+#define PCI_DPA_CAP 4 /* capability register */
+#define PCI_DPA_CAP_SUBSTATE_MASK 0x1F /* # substates - 1 */
+#define PCI_DPA_BASE_SIZEOF 16 /* size with 0 substates */
+
+/* TPH Requester */
+#define PCI_TPH_CAP 4 /* capability register */
+#define PCI_TPH_CAP_LOC_MASK 0x600 /* location mask */
+#define PCI_TPH_LOC_NONE 0x000 /* no location */
+#define PCI_TPH_LOC_CAP 0x200 /* in capability */
+#define PCI_TPH_LOC_MSIX 0x400 /* in MSI-X */
+#define PCI_TPH_CAP_ST_MASK 0x07FF0000 /* st table mask */
+#define PCI_TPH_CAP_ST_SHIFT 16 /* st table shift */
+#define PCI_TPH_BASE_SIZEOF 12 /* size with no st table */
+
#endif /* LINUX_PCI_REGS_H */

2012-05-30 04:49:48

by Alex Williamson

[permalink] [raw]
Subject: [PATCH 4/5] pci: Create common pcibios_err_to_errno

For returning errors out to non-PCI code. Re-name xen's version.

Signed-off-by: Alex Williamson <[email protected]>
Acked-by: Konrad Rzeszutek Wilk <[email protected]>
---

drivers/xen/xen-pciback/conf_space.c | 6 +++---
include/linux/pci.h | 26 ++++++++++++++++++++++++++
2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/drivers/xen/xen-pciback/conf_space.c b/drivers/xen/xen-pciback/conf_space.c
index 30d7be0..46ae0f9 100644
--- a/drivers/xen/xen-pciback/conf_space.c
+++ b/drivers/xen/xen-pciback/conf_space.c
@@ -124,7 +124,7 @@ static inline u32 merge_value(u32 val, u32 new_val, u32 new_val_mask,
return val;
}

-static int pcibios_err_to_errno(int err)
+static int xen_pcibios_err_to_errno(int err)
{
switch (err) {
case PCIBIOS_SUCCESSFUL:
@@ -202,7 +202,7 @@ out:
pci_name(dev), size, offset, value);

*ret_val = value;
- return pcibios_err_to_errno(err);
+ return xen_pcibios_err_to_errno(err);
}

int xen_pcibk_config_write(struct pci_dev *dev, int offset, int size, u32 value)
@@ -290,7 +290,7 @@ int xen_pcibk_config_write(struct pci_dev *dev, int offset, int size, u32 value)
}
}

- return pcibios_err_to_errno(err);
+ return xen_pcibios_err_to_errno(err);
}

void xen_pcibk_config_free_dyn_fields(struct pci_dev *dev)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 1b6d1c5..a8b5e8c 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -474,6 +474,32 @@ static inline bool pci_dev_msi_enabled(struct pci_dev *pci_dev) { return false;
#define PCIBIOS_SET_FAILED 0x88
#define PCIBIOS_BUFFER_TOO_SMALL 0x89

+/*
+ * Translate above to generic errno for passing back through non-pci.
+ */
+static inline int pcibios_err_to_errno(int err)
+{
+ if (err <= PCIBIOS_SUCCESSFUL)
+ return err; /* Assume already errno */
+
+ switch (err) {
+ case PCIBIOS_FUNC_NOT_SUPPORTED:
+ return -ENOENT;
+ case PCIBIOS_BAD_VENDOR_ID:
+ return -EINVAL;
+ case PCIBIOS_DEVICE_NOT_FOUND:
+ return -ENODEV;
+ case PCIBIOS_BAD_REGISTER_NUMBER:
+ return -EFAULT;
+ case PCIBIOS_SET_FAILED:
+ return -EIO;
+ case PCIBIOS_BUFFER_TOO_SMALL:
+ return -ENOSPC;
+ }
+
+ return -ENOTTY;
+}
+
/* Low-level architecture-dependent routines */

struct pci_ops {

2012-05-30 04:48:50

by Alex Williamson

[permalink] [raw]
Subject: [PATCH 1/5] pci: Add PCI DMA source ID quirk

DMA transactions are tagged with the source ID of the device making
the request. Occasionally hardware screws this up and uses the
source ID of a different device (often the wrong function number of
a multifunction device). A specific Ricoh multifunction device is
a prime example of this problem and included in this patch. The
purpose of this function is that given a pci_dev, return the pci_dev
to use as the source ID for DMA. When hardware works correctly,
this returns the input device. For the components of the Ricoh
multifunction device, return the pci_dev for function 0.

This will be used by IOMMU drivers for determining the boundaries
of IOMMU groups as multiple devices using the same source ID must
be contained within the same group. This can also be used by
existing streaming DMA paths for the same purpose.

Signed-off-by: Alex Williamson <[email protected]>
---

drivers/pci/quirks.c | 40 ++++++++++++++++++++++++++++++++++++++++
include/linux/pci.h | 5 +++++
2 files changed, 45 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 2a75216..004e167 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3179,3 +3179,43 @@ int pci_dev_specific_reset(struct pci_dev *dev, int probe)

return -ENOTTY;
}
+
+static struct pci_dev *pci_func_0_dma_source(struct pci_dev *dev)
+{
+ return pci_get_slot(dev->bus, PCI_DEVFN(PCI_SLOT(dev->devfn), 0));
+}
+
+static const struct pci_dev_dma_source {
+ u16 vendor;
+ u16 device;
+ struct pci_dev *(*dma_source)(struct pci_dev *dev);
+} pci_dev_dma_source[] = {
+ /*
+ * https://bugzilla.redhat.com/show_bug.cgi?id=605888
+ *
+ * Some Ricoh devices use the function 0 source ID for DMA on
+ * other functions of a multifunction device. The DMA devices
+ * is therefore function 0, which will have implications of the
+ * iommu grouping of these devices.
+ */
+ { PCI_VENDOR_ID_RICOH, 0xe822, pci_func_0_dma_source },
+ { PCI_VENDOR_ID_RICOH, 0xe230, pci_func_0_dma_source },
+ { PCI_VENDOR_ID_RICOH, 0xe832, pci_func_0_dma_source },
+ { PCI_VENDOR_ID_RICOH, 0xe832, pci_func_0_dma_source },
+ { 0 }
+};
+
+struct pci_dev *pci_dma_source(struct pci_dev *dev)
+{
+ const struct pci_dev_dma_source *i;
+
+ for (i = pci_dev_dma_source; i->dma_source; i++) {
+ if ((i->vendor == dev->vendor ||
+ i->vendor == (u16)PCI_ANY_ID) &&
+ (i->device == dev->device ||
+ i->device == (u16)PCI_ANY_ID))
+ return i->dma_source(dev);
+ }
+
+ return dev;
+}
diff --git a/include/linux/pci.h b/include/linux/pci.h
index d8c379d..5bc7502 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1486,9 +1486,14 @@ enum pci_fixup_pass {

#ifdef CONFIG_PCI_QUIRKS
void pci_fixup_device(enum pci_fixup_pass pass, struct pci_dev *dev);
+struct pci_dev *pci_dma_source(struct pci_dev *dev);
#else
static inline void pci_fixup_device(enum pci_fixup_pass pass,
struct pci_dev *dev) {}
+static inline struct pci_dev *pci_dma_source(struct pci_dev *dev)
+{
+ return dev;
+}
#endif

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

2012-05-30 08:43:28

by Gavin Shan

[permalink] [raw]
Subject: Re: [PATCH 1/5] pci: Add PCI DMA source ID quirk

Hi Alex,

>DMA transactions are tagged with the source ID of the device making
>the request. Occasionally hardware screws this up and uses the
>source ID of a different device (often the wrong function number of
>a multifunction device). A specific Ricoh multifunction device is
>a prime example of this problem and included in this patch. The
>purpose of this function is that given a pci_dev, return the pci_dev
>to use as the source ID for DMA. When hardware works correctly,
>this returns the input device. For the components of the Ricoh
>multifunction device, return the pci_dev for function 0.
>
>This will be used by IOMMU drivers for determining the boundaries
>of IOMMU groups as multiple devices using the same source ID must
>be contained within the same group. This can also be used by
>existing streaming DMA paths for the same purpose.
>
>Signed-off-by: Alex Williamson <[email protected]>
>---
>
> drivers/pci/quirks.c | 40 ++++++++++++++++++++++++++++++++++++++++
> include/linux/pci.h | 5 +++++
> 2 files changed, 45 insertions(+)
>
>diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>index 2a75216..004e167 100644
>--- a/drivers/pci/quirks.c
>+++ b/drivers/pci/quirks.c
>@@ -3179,3 +3179,43 @@ int pci_dev_specific_reset(struct pci_dev *dev, int probe)
>
> return -ENOTTY;
> }
>+
>+static struct pci_dev *pci_func_0_dma_source(struct pci_dev *dev)
>+{
>+ return pci_get_slot(dev->bus, PCI_DEVFN(PCI_SLOT(dev->devfn), 0));
>+}

I think we needn't retrieve the PCI device passed in again from the list
of PCI devices if its function number is zero.

if (!PCI_FUNC(dev->devfn))
return dev;

Thanks,
Gavin

>+
>+static const struct pci_dev_dma_source {
>+ u16 vendor;
>+ u16 device;
>+ struct pci_dev *(*dma_source)(struct pci_dev *dev);
>+} pci_dev_dma_source[] = {
>+ /*
>+ * https://bugzilla.redhat.com/show_bug.cgi?id=605888
>+ *
>+ * Some Ricoh devices use the function 0 source ID for DMA on
>+ * other functions of a multifunction device. The DMA devices
>+ * is therefore function 0, which will have implications of the
>+ * iommu grouping of these devices.
>+ */
>+ { PCI_VENDOR_ID_RICOH, 0xe822, pci_func_0_dma_source },
>+ { PCI_VENDOR_ID_RICOH, 0xe230, pci_func_0_dma_source },
>+ { PCI_VENDOR_ID_RICOH, 0xe832, pci_func_0_dma_source },
>+ { PCI_VENDOR_ID_RICOH, 0xe832, pci_func_0_dma_source },
>+ { 0 }
>+};
>+
>+struct pci_dev *pci_dma_source(struct pci_dev *dev)
>+{
>+ const struct pci_dev_dma_source *i;
>+
>+ for (i = pci_dev_dma_source; i->dma_source; i++) {
>+ if ((i->vendor == dev->vendor ||
>+ i->vendor == (u16)PCI_ANY_ID) &&
>+ (i->device == dev->device ||
>+ i->device == (u16)PCI_ANY_ID))
>+ return i->dma_source(dev);
>+ }
>+
>+ return dev;
>+}
>diff --git a/include/linux/pci.h b/include/linux/pci.h
>index d8c379d..5bc7502 100644
>--- a/include/linux/pci.h
>+++ b/include/linux/pci.h
>@@ -1486,9 +1486,14 @@ enum pci_fixup_pass {
>
> #ifdef CONFIG_PCI_QUIRKS
> void pci_fixup_device(enum pci_fixup_pass pass, struct pci_dev *dev);
>+struct pci_dev *pci_dma_source(struct pci_dev *dev);
> #else
> static inline void pci_fixup_device(enum pci_fixup_pass pass,
> struct pci_dev *dev) {}
>+static inline struct pci_dev *pci_dma_source(struct pci_dev *dev)
>+{
>+ return dev;
>+}
> #endif
>
> void __iomem *pcim_iomap(struct pci_dev *pdev, int bar, unsigned long maxlen);
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>the body of a message to [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2012-05-30 13:29:49

by Jiang Liu

[permalink] [raw]
Subject: Re: [PATCH 1/5] pci: Add PCI DMA source ID quirk

Hi Alex,
pci_get_slot() will hold a reference count on the returned
pci device, so need to release the reference count for balance.
According to patches "[PATCH 5/7] amd_iommu: Make use of DMA quirks
and ACS checks in IOMMU groups" and "[PATCH 6/7] intel-iommu: Make
use of DMA quirks and ACS checks in IOMMU groups", current patches
failed to release the reference count gained by pci_get_slot().

Thanks
Gerry

On 05/30/2012 12:48 PM, Alex Williamson wrote:
> DMA transactions are tagged with the source ID of the device making
> the request. Occasionally hardware screws this up and uses the
> source ID of a different device (often the wrong function number of
> a multifunction device). A specific Ricoh multifunction device is
> a prime example of this problem and included in this patch. The
> purpose of this function is that given a pci_dev, return the pci_dev
> to use as the source ID for DMA. When hardware works correctly,
> this returns the input device. For the components of the Ricoh
> multifunction device, return the pci_dev for function 0.
>
> This will be used by IOMMU drivers for determining the boundaries
> of IOMMU groups as multiple devices using the same source ID must
> be contained within the same group. This can also be used by
> existing streaming DMA paths for the same purpose.
>
> Signed-off-by: Alex Williamson <[email protected]>
> ---
>
> drivers/pci/quirks.c | 40 ++++++++++++++++++++++++++++++++++++++++
> include/linux/pci.h | 5 +++++
> 2 files changed, 45 insertions(+)
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 2a75216..004e167 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -3179,3 +3179,43 @@ int pci_dev_specific_reset(struct pci_dev *dev, int probe)
>
> return -ENOTTY;
> }
> +
> +static struct pci_dev *pci_func_0_dma_source(struct pci_dev *dev)
> +{
> + return pci_get_slot(dev->bus, PCI_DEVFN(PCI_SLOT(dev->devfn), 0));
> +}
> +
> +static const struct pci_dev_dma_source {
> + u16 vendor;
> + u16 device;
> + struct pci_dev *(*dma_source)(struct pci_dev *dev);
> +} pci_dev_dma_source[] = {
> + /*
> + * https://bugzilla.redhat.com/show_bug.cgi?id=605888
> + *
> + * Some Ricoh devices use the function 0 source ID for DMA on
> + * other functions of a multifunction device. The DMA devices
> + * is therefore function 0, which will have implications of the
> + * iommu grouping of these devices.
> + */
> + { PCI_VENDOR_ID_RICOH, 0xe822, pci_func_0_dma_source },
> + { PCI_VENDOR_ID_RICOH, 0xe230, pci_func_0_dma_source },
> + { PCI_VENDOR_ID_RICOH, 0xe832, pci_func_0_dma_source },
> + { PCI_VENDOR_ID_RICOH, 0xe832, pci_func_0_dma_source },
> + { 0 }
> +};
> +
> +struct pci_dev *pci_dma_source(struct pci_dev *dev)
> +{
> + const struct pci_dev_dma_source *i;
> +
> + for (i = pci_dev_dma_source; i->dma_source; i++) {
> + if ((i->vendor == dev->vendor ||
> + i->vendor == (u16)PCI_ANY_ID) &&
> + (i->device == dev->device ||
> + i->device == (u16)PCI_ANY_ID))
> + return i->dma_source(dev);
> + }
> +
> + return dev;
> +}
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index d8c379d..5bc7502 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1486,9 +1486,14 @@ enum pci_fixup_pass {
>
> #ifdef CONFIG_PCI_QUIRKS
> void pci_fixup_device(enum pci_fixup_pass pass, struct pci_dev *dev);
> +struct pci_dev *pci_dma_source(struct pci_dev *dev);
> #else
> static inline void pci_fixup_device(enum pci_fixup_pass pass,
> struct pci_dev *dev) {}
> +static inline struct pci_dev *pci_dma_source(struct pci_dev *dev)
> +{
> + return dev;
> +}
> #endif
>
> void __iomem *pcim_iomap(struct pci_dev *pdev, int bar, unsigned long maxlen);
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2012-05-30 15:37:38

by Alex Williamson

[permalink] [raw]
Subject: Re: [PATCH 1/5] pci: Add PCI DMA source ID quirk

On Wed, 2012-05-30 at 16:34 +0800, Gavin Shan wrote:
> Hi Alex,
>
> >DMA transactions are tagged with the source ID of the device making
> >the request. Occasionally hardware screws this up and uses the
> >source ID of a different device (often the wrong function number of
> >a multifunction device). A specific Ricoh multifunction device is
> >a prime example of this problem and included in this patch. The
> >purpose of this function is that given a pci_dev, return the pci_dev
> >to use as the source ID for DMA. When hardware works correctly,
> >this returns the input device. For the components of the Ricoh
> >multifunction device, return the pci_dev for function 0.
> >
> >This will be used by IOMMU drivers for determining the boundaries
> >of IOMMU groups as multiple devices using the same source ID must
> >be contained within the same group. This can also be used by
> >existing streaming DMA paths for the same purpose.
> >
> >Signed-off-by: Alex Williamson <[email protected]>
> >---
> >
> > drivers/pci/quirks.c | 40 ++++++++++++++++++++++++++++++++++++++++
> > include/linux/pci.h | 5 +++++
> > 2 files changed, 45 insertions(+)
> >
> >diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> >index 2a75216..004e167 100644
> >--- a/drivers/pci/quirks.c
> >+++ b/drivers/pci/quirks.c
> >@@ -3179,3 +3179,43 @@ int pci_dev_specific_reset(struct pci_dev *dev, int probe)
> >
> > return -ENOTTY;
> > }
> >+
> >+static struct pci_dev *pci_func_0_dma_source(struct pci_dev *dev)
> >+{
> >+ return pci_get_slot(dev->bus, PCI_DEVFN(PCI_SLOT(dev->devfn), 0));
> >+}
>
> I think we needn't retrieve the PCI device passed in again from the list
> of PCI devices if its function number is zero.
>
> if (!PCI_FUNC(dev->devfn))
> return dev;

I guess I was expecting we wouldn't quirk a function 0 device, but
looking again at the bug below I see that I am including the func 0
vendor/device. I've also got a bad cut-n-paste as I'm including the
same device twice. I'll take your suggestion. Thanks,

Alex

> >+
> >+static const struct pci_dev_dma_source {
> >+ u16 vendor;
> >+ u16 device;
> >+ struct pci_dev *(*dma_source)(struct pci_dev *dev);
> >+} pci_dev_dma_source[] = {
> >+ /*
> >+ * https://bugzilla.redhat.com/show_bug.cgi?id=605888
> >+ *
> >+ * Some Ricoh devices use the function 0 source ID for DMA on
> >+ * other functions of a multifunction device. The DMA devices
> >+ * is therefore function 0, which will have implications of the
> >+ * iommu grouping of these devices.
> >+ */
> >+ { PCI_VENDOR_ID_RICOH, 0xe822, pci_func_0_dma_source },
> >+ { PCI_VENDOR_ID_RICOH, 0xe230, pci_func_0_dma_source },
> >+ { PCI_VENDOR_ID_RICOH, 0xe832, pci_func_0_dma_source },
> >+ { PCI_VENDOR_ID_RICOH, 0xe832, pci_func_0_dma_source },
> >+ { 0 }
> >+};
> >+
> >+struct pci_dev *pci_dma_source(struct pci_dev *dev)
> >+{
> >+ const struct pci_dev_dma_source *i;
> >+
> >+ for (i = pci_dev_dma_source; i->dma_source; i++) {
> >+ if ((i->vendor == dev->vendor ||
> >+ i->vendor == (u16)PCI_ANY_ID) &&
> >+ (i->device == dev->device ||
> >+ i->device == (u16)PCI_ANY_ID))
> >+ return i->dma_source(dev);
> >+ }
> >+
> >+ return dev;
> >+}
> >diff --git a/include/linux/pci.h b/include/linux/pci.h
> >index d8c379d..5bc7502 100644
> >--- a/include/linux/pci.h
> >+++ b/include/linux/pci.h
> >@@ -1486,9 +1486,14 @@ enum pci_fixup_pass {
> >
> > #ifdef CONFIG_PCI_QUIRKS
> > void pci_fixup_device(enum pci_fixup_pass pass, struct pci_dev *dev);
> >+struct pci_dev *pci_dma_source(struct pci_dev *dev);
> > #else
> > static inline void pci_fixup_device(enum pci_fixup_pass pass,
> > struct pci_dev *dev) {}
> >+static inline struct pci_dev *pci_dma_source(struct pci_dev *dev)
> >+{
> >+ return dev;
> >+}
> > #endif
> >
> > void __iomem *pcim_iomap(struct pci_dev *pdev, int bar, unsigned long maxlen);
> >
> >--
> >To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> >the body of a message to [email protected]
> >More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>


2012-05-30 18:05:49

by Alex Williamson

[permalink] [raw]
Subject: Re: [PATCH 1/5] pci: Add PCI DMA source ID quirk

On Wed, 2012-05-30 at 21:27 +0800, Jiang Liu wrote:
> Hi Alex,
> pci_get_slot() will hold a reference count on the returned
> pci device, so need to release the reference count for balance.
> According to patches "[PATCH 5/7] amd_iommu: Make use of DMA quirks
> and ACS checks in IOMMU groups" and "[PATCH 6/7] intel-iommu: Make
> use of DMA quirks and ACS checks in IOMMU groups", current patches
> failed to release the reference count gained by pci_get_slot().

Thanks for pointing this out Gerry. I think I'll rename
pci_dma_source() to pci_get_dma_source() which will then increment the
reference of the device it returns, whether it's a quirk or the original
device. The caller will then be responsible for doing a pci_dev_put().
Thanks,

Alex

> On 05/30/2012 12:48 PM, Alex Williamson wrote:
> > DMA transactions are tagged with the source ID of the device making
> > the request. Occasionally hardware screws this up and uses the
> > source ID of a different device (often the wrong function number of
> > a multifunction device). A specific Ricoh multifunction device is
> > a prime example of this problem and included in this patch. The
> > purpose of this function is that given a pci_dev, return the pci_dev
> > to use as the source ID for DMA. When hardware works correctly,
> > this returns the input device. For the components of the Ricoh
> > multifunction device, return the pci_dev for function 0.
> >
> > This will be used by IOMMU drivers for determining the boundaries
> > of IOMMU groups as multiple devices using the same source ID must
> > be contained within the same group. This can also be used by
> > existing streaming DMA paths for the same purpose.
> >
> > Signed-off-by: Alex Williamson <[email protected]>
> > ---
> >
> > drivers/pci/quirks.c | 40 ++++++++++++++++++++++++++++++++++++++++
> > include/linux/pci.h | 5 +++++
> > 2 files changed, 45 insertions(+)
> >
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > index 2a75216..004e167 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -3179,3 +3179,43 @@ int pci_dev_specific_reset(struct pci_dev *dev, int probe)
> >
> > return -ENOTTY;
> > }
> > +
> > +static struct pci_dev *pci_func_0_dma_source(struct pci_dev *dev)
> > +{
> > + return pci_get_slot(dev->bus, PCI_DEVFN(PCI_SLOT(dev->devfn), 0));
> > +}
> > +
> > +static const struct pci_dev_dma_source {
> > + u16 vendor;
> > + u16 device;
> > + struct pci_dev *(*dma_source)(struct pci_dev *dev);
> > +} pci_dev_dma_source[] = {
> > + /*
> > + * https://bugzilla.redhat.com/show_bug.cgi?id=605888
> > + *
> > + * Some Ricoh devices use the function 0 source ID for DMA on
> > + * other functions of a multifunction device. The DMA devices
> > + * is therefore function 0, which will have implications of the
> > + * iommu grouping of these devices.
> > + */
> > + { PCI_VENDOR_ID_RICOH, 0xe822, pci_func_0_dma_source },
> > + { PCI_VENDOR_ID_RICOH, 0xe230, pci_func_0_dma_source },
> > + { PCI_VENDOR_ID_RICOH, 0xe832, pci_func_0_dma_source },
> > + { PCI_VENDOR_ID_RICOH, 0xe832, pci_func_0_dma_source },
> > + { 0 }
> > +};
> > +
> > +struct pci_dev *pci_dma_source(struct pci_dev *dev)
> > +{
> > + const struct pci_dev_dma_source *i;
> > +
> > + for (i = pci_dev_dma_source; i->dma_source; i++) {
> > + if ((i->vendor == dev->vendor ||
> > + i->vendor == (u16)PCI_ANY_ID) &&
> > + (i->device == dev->device ||
> > + i->device == (u16)PCI_ANY_ID))
> > + return i->dma_source(dev);
> > + }
> > +
> > + return dev;
> > +}
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index d8c379d..5bc7502 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -1486,9 +1486,14 @@ enum pci_fixup_pass {
> >
> > #ifdef CONFIG_PCI_QUIRKS
> > void pci_fixup_device(enum pci_fixup_pass pass, struct pci_dev *dev);
> > +struct pci_dev *pci_dma_source(struct pci_dev *dev);
> > #else
> > static inline void pci_fixup_device(enum pci_fixup_pass pass,
> > struct pci_dev *dev) {}
> > +static inline struct pci_dev *pci_dma_source(struct pci_dev *dev)
> > +{
> > + return dev;
> > +}
> > #endif
> >
> > void __iomem *pcim_iomap(struct pci_dev *pdev, int bar, unsigned long maxlen);
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>