PCI/iommu: Fix DMA alias problems
cover PCI/iommu: Fix DMA alias problems
This series attempts to fix a couple issues we've had outstanding in the PCI/IOMMU code for a while. The first issue is with devices that use the wrong requester ID for DMA transactions. We already have a sort of half-baked attempt to fix this for several Ricoh devices, but the fix only helps them be useful through IOMMU groups, not the general DMA case. There are also several Marvell devices which use use a different wrong requester ID and don't even fit into the DMA source idea. This series creates a DMA alias iterator that will step through each possible alias of a device, allowing IOMMUs to insert mappings for both the device and its aliases.
Hand-in-hand with this is our broken pci_find_upstream_pcie_bridge() function, which is known to blowup when it finds itself suddenly at a PCIe device without crossing a PCIe-to-PCI bridge (as identified by the PCIe capability). It also likes to make the invalid assumption that a PCIe device never has its requester ID masked by any usptream bus. We can fix this using the above new DMA alias iterator, since that's effectively what this function was meant to do.
Finally, with all these helpers, it makes sense to consolidate code for determining IOMMU groups. The first step in finding the root of a group is finding the final upstream DMA alias for the device, then applying additional ACS rules and incorporating device specific aliases. As this is all common to PCI, create a single implementation and remove piles of code from the individual IOMMU drivers.
This series allows devices like the Marvell 88SE9123 to finally work on Linux with either AMD-Vi or VT-d enabled on the box. I've collected device IDs from various bugs to support as many SKUs of these devices as possible, but I'm sure there are others that I've missed.
This should also enable motherboards with an onboard ASmedia ASM1083/1085 PCIe-to-PCI bridge to work with VT-d enabled. I've acquired an adapter board with this chip, but it actually exposes a PCIe capability, unlike most of the onboard controllers. Therefore I expect this series will fix the WARN_ON currently hit during boot, but there's a 50/50 chance whether the device behaves like a PCI bridge or a PCIe bridge with regard to the requester ID that it uses to take ownership of the transaction. If it turns out to use the PCIe bridge model, I expect we can quirk it using a dev_flags bit to identify a PCI bridge that takes ownership as if it was a PCIe bridge.
Please test and provide feedback. I expect IOMMU group topology should not change from this series, but if a case is found where it does, please share. Also, if there are additional quirks we need to add, please either file new or add to the existing bugs. Thanks,
Alex
---
Alex Williamson (13):
PCI: Add DMA alias iterator
PCI: quirk pci_for_each_dma_alias()
PCI: quirk dma_func_alias for Ricoh devices
PCI: quirk dma_func_alias for Marvell devices
PCI: Consolidate isolation domain code
iommu/amd: Use pci_find_dma_isolation_root() for IOMMU groups iommu/amd: Update to use PCI DMA aliases iommu/intel: Use pci_find_dma_isolation_root() for IOMMU groups iommu/intel: Update to use PCI DMA aliases iommu/fsl: Use pci_find_dma_isolation_root() for IOMMU groups
iommu: Remove pci.h
PCI: Remove pci_find_upstream_pcie_bridge()
PCI: Remove pci_get_dma_source()
drivers/iommu/amd_iommu.c | 185 ++++------------------ drivers/iommu/amd_iommu_types.h | 1 drivers/iommu/fsl_pamu_domain.c | 67 -------- drivers/iommu/intel-iommu.c | 293 +++++++++++++---------------------- drivers/iommu/intel_irq_remapping.c | 55 +++++-- drivers/iommu/pci.h | 29 --- drivers/pci/quirks.c | 73 ++++----- drivers/pci/search.c | 234 +++++++++++++++++++++++++--- include/linux/pci.h | 21 +-- 9 files changed, 430 insertions(+), 528 deletions(-) delete mode 100644 drivers/iommu/pci.h
PATCH 1/13 PCI: Add DMA alias iterator
In a mixed PCI/PCI-X/PCI-e topology, bridges can take ownership of transactions, replacing the original requester ID with their own. Sometimes we just want to know the resulting device or resulting alias, sometimes we want each step in the chain. This iterator allows either usage. When an endpoint is connected via an unbroken chain of PCIe switches and root ports, it has no alias and its requester ID is visible to the root bus. When PCI/X get in the way, we pick up aliases for bridges.
The reason why we potentially care about each step in the path is because of PCI-X. PCI-X has the concept of a requester ID, but bridges may or may not take ownership of various types of transactions. We therefore leave it to the consumer of this function to prune out what they don't care about rather than attempt to flatten the alias ourselves.
--- drivers/pci/search.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/pci.h | 4 +++ 2 files changed, 74 insertions(+)
drivers/pci/search.c +70 -0
@@ -18,6 +18,76 @@ DECLARE_RWSEM(pci_bus_sem);EXPORT_SYMBOL_GPL (pci_bus_sem);/* +* pci_for_each_dma_alias - Iterate over DMA aliases for a device +* @pdev: starting downstream device +* @fn: function to call for each alias +* @data: opaque data to pass to @fn +* +* Starting @pdev, walk up the bus calling @fn for each possible alias +* of @pdev at the root bus. +*/ +int pci_for_each_dma_alias (struct pci_dev *pdev, +int (*fn)(struct pci_dev *pdev, +u16 alias,void *data),void *data) +{ +struct pci_bus *bus; +int ret; + + ret =fn (pdev,PCI_DEVID (pdev->bus->number, pdev->devfn), data); +if (ret) +return ret; + +for (bus = pdev->bus; !pci_is_root_bus (bus); bus = bus->parent) { +struct pci_dev *tmp; + +/* Skip virtual buses */ +if (!bus->self) +continue ; + + tmp = bus->self; + +/* +* PCIe-to-PCI/X bridges alias transactions from downstream +* devices using the subordinate bus number (PCI Express to +* PCI/PCI-X Bridge Spec, rev 1.0, sec 2.3). For all cases +* where the upstream bus is PCI/X we alias to the bridge +* (there are various conditions in the previous reference +* where the bridge may take ownership of transactions, even +* when the secondary interface is PCI-X). +*/ +if (pci_is_pcie (tmp)) { +switch (pci_pcie_type (tmp)) { +case PCI_EXP_TYPE_ROOT_PORT : +case PCI_EXP_TYPE_UPSTREAM : +case PCI_EXP_TYPE_DOWNSTREAM : +continue ; +case PCI_EXP_TYPE_PCI_BRIDGE : + ret =fn (tmp, +PCI_DEVID (tmp->subordinate->number, +PCI_DEVFN (0 ,0 )), data); +if (ret) +return ret; +continue ; +case PCI_EXP_TYPE_PCIE_BRIDGE : + ret =fn (tmp, +PCI_DEVID (tmp->bus->number, + tmp->devfn), data); +if (ret) +return ret; +continue ; + } + }else { + ret =fn (tmp,PCI_DEVID (tmp->bus->number, tmp->devfn), + data); +if (ret) +return ret; + } + } + +return ret; +} + +/* * find the upstream PCIe-to-PCI bridge of a PCI device * if the device is PCIE, return NULL * if the device isn't connected to a PCIe bridge (that is its parent is a
include/linux/pci.h +4 -0
@@ -1795,6 +1795,10 @@ static inline struct eeh_dev *pci_dev_to_eeh_dev(struct pci_dev *pdev) }#endif +int pci_for_each_dma_alias (struct pci_dev *pdev, +int (*fn)(struct pci_dev *pdev, +u16 alias,void *data),void *data); +/** * pci_find_upstream_pcie_bridge - find upstream PCIe-to-PCI bridge of a device * @pdev: the PCI device
PATCH 2/13 PCI: quirk pci_for_each_dma_alias()
In reply to Alex WilliamsonThere are a few broken devices that use the requester ID of a different function in the slot for their DMA. To handle these, add a bitmap to struct pci_dev (using an alignment gap) that quirks can populate. As we iterate over the device and bus DMA aliases, also iterate over any bits in the map.
--- drivers/pci/search.c | 21 +++++++++++++++++++++ include/linux/pci.h | 2 ++ 2 files changed, 23 insertions(+)
drivers/pci/search.c +21 -0
@@ -37,6 +37,27 @@ int pci_for_each_dma_alias(struct pci_dev *pdev,if (ret)return ret; +/* +* dma_func_alias provides a bitmap of other function numbers on +* this same PCI slot to use as DMA aliases. +*/ +if (unlikely (pdev->dma_func_alias)) { +u8 map = pdev->dma_func_alias & ~(1 <<PCI_FUNC (pdev->devfn)); +int func; + +for (func =0 ; map && func <8 ; func++, map >>=1 ) { +if (!(map &1 )) +continue ; + + ret =fn (pdev, +PCI_DEVID (pdev->bus->number, +PCI_DEVFN (PCI_SLOT (pdev->devfn), + func)), data); +if (ret) +return ret; + } + } +for (bus = pdev->bus; !pci_is_root_bus (bus); bus = bus->parent) {struct pci_dev *tmp;
include/linux/pci.h +2 -0
@@ -268,6 +268,8 @@ struct pci_dev {u8 rom_base_reg;/* which config register controls the ROM */ u8 pin;/* which interrupt pin this device uses */ u16 pcie_flags_reg;/* cached PCIe Capabilities Register */ +u8 dma_func_alias;/* bitmap of functions used as DMA +aliases for this device */ struct pci_driver *driver;/* which driver has allocated this device */ u64 dma_mask;/* Mask of the bits of bus address this
PATCH 3/13 PCI: quirk dma_func_alias for Ricoh devices
In reply to Alex WilliamsonThe existing quirk for these devices doesn't really solve the problem, re-implement it using the DMA alias iterator. We'll come back later and remove the existing quirk and dma_source interface.
--- drivers/pci/quirks.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
drivers/pci/quirks.c +16 -0
@@ -3333,6 +3333,22 @@ int pci_dev_specific_reset(struct pci_dev *dev, int probe)return -ENOTTY ; } +static void quirk_dma_func0_alias (struct pci_dev *dev) +{ +if (PCI_SLOT (dev->devfn) !=0 ) + dev->dma_func_alias |= (1 <<0 ); +} + +/* +* https://bugzilla.redhat.com/show_bug.cgi?id=605888 +* +* Some Ricoh devices use function 0 as the PCIe requester ID for DMA. +*/ +DECLARE_PCI_FIXUP_HEADER (PCI_VENDOR_ID_RICOH ,0xe822 , quirk_dma_func0_alias); +DECLARE_PCI_FIXUP_HEADER (PCI_VENDOR_ID_RICOH ,0xe230 , quirk_dma_func0_alias); +DECLARE_PCI_FIXUP_HEADER (PCI_VENDOR_ID_RICOH ,0xe832 , quirk_dma_func0_alias); +DECLARE_PCI_FIXUP_HEADER (PCI_VENDOR_ID_RICOH ,0xe832 , quirk_dma_func0_alias); +static struct pci_dev *pci_func_0_dma_source (struct pci_dev *dev) {if (!PCI_FUNC (dev->devfn))
Re: [PATCH 03/13] PCI: quirk dma_func_alias for Ricoh devices
Hi Alex
Quoted 22 lines · Alex Williamson
The existing quirk for these devices doesn't really solve the problem, re-implement it using the DMA alias iterator. We'll come back later and remove the existing quirk and
dma_sourceinterface.Signed-off-by Alex Williamson <[email protected]>--- drivers/pci/quirks.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)drivers/pci/quirks.c +5
-0@@ -3333,6 +3333,22 @@ int pci_dev_specific_reset(struct pci_dev *dev, int probe)return -ENOTTY ; } +static void quirk_dma_func0_alias (struct pci_dev *dev) +{ +if (PCI_SLOT (dev->devfn) !=0 ) + dev->dma_func_alias |= (1 <<0 ); +}
drivers/pci/quirks.c int pci_dev_specific_reset(struct pci_dev *dev, int probe) · line 3350
Quoted 10 lines
drivers/pci/quirks.c +10
-0+ +/* +* https://bugzilla.redhat.com/show_bug.cgi?id=605888 +* +* Some Ricoh devices use function 0 as the PCIe requester ID for DMA. +*/ +DECLARE_PCI_FIXUP_HEADER (PCI_VENDOR_ID_RICOH ,0xe822 , quirk_dma_func0_alias); +DECLARE_PCI_FIXUP_HEADER (PCI_VENDOR_ID_RICOH ,0xe230 , quirk_dma_func0_alias); +DECLARE_PCI_FIXUP_HEADER (PCI_VENDOR_ID_RICOH ,0xe832 , quirk_dma_func0_alias); +DECLARE_PCI_FIXUP_HEADER (PCI_VENDOR_ID_RICOH ,0xe832 , quirk_dma_func0_alias);
0xe832 is listed twice.
Previously only 0xe832 needed the dma alias on my thinkpad T410, which has all three devices.
drivers/pci/quirks.c int pci_dev_specific_reset(struct pci_dev *dev, int probe) · line 3354
Quoted 5 lines
drivers/pci/quirks.c +1
-0+static struct pci_dev *pci_func_0_dma_source (struct pci_dev *dev) {if (!PCI_FUNC (dev->devfn))
Unfortunately, this quirk doesn't work for me. I tried it without modification, as well as with each alias individually. I get:
Set context mapping for 0d:00.3firewire_ohci 0000:0d:00.3: added OHCI v1.10 device as card 0, 4 IR + 4 IT contexts, quirks 0x11
dmar: DRHD: handling fault status reg 2
dmar: DMAR:[DMA Read] Request device [0d:00.0] fault addr fffff000 DMAR:[fault reason 02] Present bit in context entry is clear
I think I need to see
Set context mapping for 0d:00.0
before
Set context mapping for 0d:00.3
in the log, but it's not there. I'd love to look into this and understand it properly, but I don't have time for the next four weeks.
The devices are attached as follows:
BDF, device ID
0d:00.0, e822
0d:00.1, e230
0d:00.3, e832
The kernel log is attached.
Regards,
a.