From a3751212a8eeece59d2018c455000f30ed7e5bb7 Mon Sep 17 00:00:00 2001 From: Frank Li Date: Tue, 14 Jan 2025 15:37:08 -0500 Subject: [PATCH 1/4] PCI: Add enable_device() and disable_device() callbacks for bridges MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some PCI host bridges require special handling when enabling or disabling PCI devices. For example, the i.MX95 platform has a lookup table to map Requester IDs to StreamIDs, which the SMMU and MSI controller use to identify the source of DMA accesses. Without this mapping, DMA accesses may target unintended memory, which would corrupt memory or read the wrong data. Add a host bridge enable_device() hook the imx6 driver can use to configure the Requester ID to StreamID mapping. The hardware table isn't big enough to map all possible Requester IDs, so this hook may fail if no table space is available. In that case, return failure from pci_enable_device(). It might make more sense to make pci_set_master() decline to enable bus mastering and return failure, but it currently doesn't have a way to return failure. Link: https://lore.kernel.org/r/20250114-imx95_lut-v9-1-39f58dbed03a@nxp.com Tested-by: Marc Zyngier Signed-off-by: Frank Li Signed-off-by: Bjorn Helgaas [kwilczynski: commit log] Signed-off-by: Krzysztof Wilczyński Reviewed-by: Marc Zyngier Acked-by: Manivannan Sadhasivam --- drivers/pci/pci.c | 36 +++++++++++++++++++++++++++++++++++- include/linux/pci.h | 2 ++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 0b29ec6e8e5e..773ca3cbd322 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -2059,6 +2059,28 @@ int __weak pcibios_enable_device(struct pci_dev *dev, int bars) return pci_enable_resources(dev, bars); } +static int pci_host_bridge_enable_device(struct pci_dev *dev) +{ + struct pci_host_bridge *host_bridge = pci_find_host_bridge(dev->bus); + int err; + + if (host_bridge && host_bridge->enable_device) { + err = host_bridge->enable_device(host_bridge, dev); + if (err) + return err; + } + + return 0; +} + +static void pci_host_bridge_disable_device(struct pci_dev *dev) +{ + struct pci_host_bridge *host_bridge = pci_find_host_bridge(dev->bus); + + if (host_bridge && host_bridge->disable_device) + host_bridge->disable_device(host_bridge, dev); +} + static int do_pci_enable_device(struct pci_dev *dev, int bars) { int err; @@ -2074,9 +2096,13 @@ static int do_pci_enable_device(struct pci_dev *dev, int bars) if (bridge) pcie_aspm_powersave_config_link(bridge); + err = pci_host_bridge_enable_device(dev); + if (err) + return err; + err = pcibios_enable_device(dev, bars); if (err < 0) - return err; + goto err_enable; pci_fixup_device(pci_fixup_enable, dev); if (dev->msi_enabled || dev->msix_enabled) @@ -2091,6 +2117,12 @@ static int do_pci_enable_device(struct pci_dev *dev, int bars) } return 0; + +err_enable: + pci_host_bridge_disable_device(dev); + + return err; + } /** @@ -2274,6 +2306,8 @@ void pci_disable_device(struct pci_dev *dev) if (atomic_dec_return(&dev->enable_cnt) != 0) return; + pci_host_bridge_disable_device(dev); + do_pci_disable_device(dev); dev->is_busmaster = 0; diff --git a/include/linux/pci.h b/include/linux/pci.h index db9b47ce3eef..bcbef004dd56 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -595,6 +595,8 @@ struct pci_host_bridge { u8 (*swizzle_irq)(struct pci_dev *, u8 *); /* Platform IRQ swizzler */ int (*map_irq)(const struct pci_dev *, u8, u8); void (*release_fn)(struct pci_host_bridge *); + int (*enable_device)(struct pci_host_bridge *bridge, struct pci_dev *dev); + void (*disable_device)(struct pci_host_bridge *bridge, struct pci_dev *dev); void *release_data; unsigned int ignore_reset_delay:1; /* For entire hierarchy */ unsigned int no_ext_tags:1; /* No Extended Tags */ From ce4c4301728541db7e5f571a5688a3a236d9e488 Mon Sep 17 00:00:00 2001 From: Frank Li Date: Tue, 14 Jan 2025 15:37:09 -0500 Subject: [PATCH 2/4] PCI: imx6: Add IOMMU and ITS MSI support for i.MX95 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For the i.MX95, the configuration of a LUT is necessary to convert PCIe Requester IDs (RIDs) to StreamIDs, which are used by both IOMMU and ITS. This involves checking msi-map and iommu-map device tree properties to ensure consistent mapping of Requester IDs to the same StreamIDs. Subsequently, LUT-related registers are configured. If a msi-map isn't detected, the platform relies on DWC built-in controller for MSIs that do not need StreamIDs. Implement PCI bus callback function to handle enable_device() and disable_device() operations, setting up the LUT whenever a new PCI device is enabled. Known limitations: - If iommu-map exists in the device tree but the IOMMU controller is disabled, StreamIDs are programmed into the LUT. However, if a RID is out of range of the iommu-map, enabling the PCI device would result in a failure, although the PCI device can work without the IOMMU. - If msi-map exists in the device tree but the MSI controller is disabled, MSIs will not work. The DWC driver skips initializing the built-in MSI controller, falling back to legacy PCI INTx only. Link: https://lore.kernel.org/r/20250114-imx95_lut-v9-2-39f58dbed03a@nxp.com Signed-off-by: Frank Li [kwilczynski: commit log] Signed-off-by: Krzysztof Wilczyński [bhelgaas: fix uninitialized "sid" in imx_pcie_enable_device()] Signed-off-by: Bjorn Helgaas Acked-by: Richard Zhu --- drivers/pci/controller/dwc/pci-imx6.c | 208 +++++++++++++++++++++++++- 1 file changed, 207 insertions(+), 1 deletion(-) diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c index c8d5c90aa4d4..6ed56ff390d9 100644 --- a/drivers/pci/controller/dwc/pci-imx6.c +++ b/drivers/pci/controller/dwc/pci-imx6.c @@ -55,6 +55,22 @@ #define IMX95_PE0_GEN_CTRL_3 0x1058 #define IMX95_PCIE_LTSSM_EN BIT(0) +#define IMX95_PE0_LUT_ACSCTRL 0x1008 +#define IMX95_PEO_LUT_RWA BIT(16) +#define IMX95_PE0_LUT_ENLOC GENMASK(4, 0) + +#define IMX95_PE0_LUT_DATA1 0x100c +#define IMX95_PE0_LUT_VLD BIT(31) +#define IMX95_PE0_LUT_DAC_ID GENMASK(10, 8) +#define IMX95_PE0_LUT_STREAM_ID GENMASK(5, 0) + +#define IMX95_PE0_LUT_DATA2 0x1010 +#define IMX95_PE0_LUT_REQID GENMASK(31, 16) +#define IMX95_PE0_LUT_MASK GENMASK(15, 0) + +#define IMX95_SID_MASK GENMASK(5, 0) +#define IMX95_MAX_LUT 32 + #define to_imx_pcie(x) dev_get_drvdata((x)->dev) enum imx_pcie_variants { @@ -87,6 +103,7 @@ enum imx_pcie_variants { * workaround suspend resume on some devices which are affected by this errata. */ #define IMX_PCIE_FLAG_BROKEN_SUSPEND BIT(9) +#define IMX_PCIE_FLAG_HAS_LUT BIT(10) #define imx_check_flag(pci, val) (pci->drvdata->flags & val) @@ -139,6 +156,9 @@ struct imx_pcie { struct device *pd_pcie_phy; struct phy *phy; const struct imx_pcie_drvdata *drvdata; + + /* Ensure that only one device's LUT is configured at any given time */ + struct mutex lock; }; /* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */ @@ -930,6 +950,184 @@ static void imx_pcie_stop_link(struct dw_pcie *pci) imx_pcie_ltssm_disable(dev); } +static int imx_pcie_add_lut(struct imx_pcie *imx_pcie, u16 rid, u8 sid) +{ + struct dw_pcie *pci = imx_pcie->pci; + struct device *dev = pci->dev; + u32 data1, data2; + int free = -1; + int i; + + if (sid >= 64) { + dev_err(dev, "Invalid SID for index %d\n", sid); + return -EINVAL; + } + + guard(mutex)(&imx_pcie->lock); + + /* + * Iterate through all LUT entries to check for duplicate RID and + * identify the first available entry. Configure this available entry + * immediately after verification to avoid rescanning it. + */ + for (i = 0; i < IMX95_MAX_LUT; i++) { + regmap_write(imx_pcie->iomuxc_gpr, + IMX95_PE0_LUT_ACSCTRL, IMX95_PEO_LUT_RWA | i); + regmap_read(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA1, &data1); + + if (!(data1 & IMX95_PE0_LUT_VLD)) { + if (free < 0) + free = i; + continue; + } + + regmap_read(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA2, &data2); + + /* Do not add duplicate RID */ + if (rid == FIELD_GET(IMX95_PE0_LUT_REQID, data2)) { + dev_warn(dev, "Existing LUT entry available for RID (%d)", rid); + return 0; + } + } + + if (free < 0) { + dev_err(dev, "LUT entry is not available\n"); + return -ENOSPC; + } + + data1 = FIELD_PREP(IMX95_PE0_LUT_DAC_ID, 0); + data1 |= FIELD_PREP(IMX95_PE0_LUT_STREAM_ID, sid); + data1 |= IMX95_PE0_LUT_VLD; + regmap_write(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA1, data1); + + data2 = IMX95_PE0_LUT_MASK; /* Match all bits of RID */ + data2 |= FIELD_PREP(IMX95_PE0_LUT_REQID, rid); + regmap_write(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA2, data2); + + regmap_write(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_ACSCTRL, free); + + return 0; +} + +static void imx_pcie_remove_lut(struct imx_pcie *imx_pcie, u16 rid) +{ + u32 data2; + int i; + + guard(mutex)(&imx_pcie->lock); + + for (i = 0; i < IMX95_MAX_LUT; i++) { + regmap_write(imx_pcie->iomuxc_gpr, + IMX95_PE0_LUT_ACSCTRL, IMX95_PEO_LUT_RWA | i); + regmap_read(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA2, &data2); + if (FIELD_GET(IMX95_PE0_LUT_REQID, data2) == rid) { + regmap_write(imx_pcie->iomuxc_gpr, + IMX95_PE0_LUT_DATA1, 0); + regmap_write(imx_pcie->iomuxc_gpr, + IMX95_PE0_LUT_DATA2, 0); + regmap_write(imx_pcie->iomuxc_gpr, + IMX95_PE0_LUT_ACSCTRL, i); + + break; + } + } +} + +static int imx_pcie_enable_device(struct pci_host_bridge *bridge, + struct pci_dev *pdev) +{ + struct imx_pcie *imx_pcie = to_imx_pcie(to_dw_pcie_from_pp(bridge->sysdata)); + u32 sid_i, sid_m, rid = pci_dev_id(pdev); + struct device_node *target; + struct device *dev; + int err_i, err_m; + u32 sid = 0; + + dev = imx_pcie->pci->dev; + + target = NULL; + err_i = of_map_id(dev->of_node, rid, "iommu-map", "iommu-map-mask", + &target, &sid_i); + if (target) { + of_node_put(target); + } else { + /* + * "target == NULL && err_i == 0" means RID out of map range. + * Use 1:1 map RID to streamID. Hardware can't support this + * because the streamID is only 6 bits + */ + err_i = -EINVAL; + } + + target = NULL; + err_m = of_map_id(dev->of_node, rid, "msi-map", "msi-map-mask", + &target, &sid_m); + + /* + * err_m target + * 0 NULL RID out of range. Use 1:1 map RID to + * streamID, Current hardware can't + * support it, so return -EINVAL. + * != 0 NULL msi-map does not exist, use built-in MSI + * 0 != NULL Get correct streamID from RID + * != 0 != NULL Invalid combination + */ + if (!err_m && !target) + return -EINVAL; + else if (target) + of_node_put(target); /* Find streamID map entry for RID in msi-map */ + + /* + * msi-map iommu-map + * N N DWC MSI Ctrl + * Y Y ITS + SMMU, require the same SID + * Y N ITS + * N Y DWC MSI Ctrl + SMMU + */ + if (err_i && err_m) + return 0; + + if (!err_i && !err_m) { + /* + * Glue Layer + * <==========> + * ┌─────┐ ┌──────────┐ + * │ LUT │ 6-bit streamID │ │ + * │ │─────────────────►│ MSI │ + * └─────┘ 2-bit ctrl ID │ │ + * ┌───────────►│ │ + * (i.MX95) │ │ │ + * 00 PCIe0 │ │ │ + * 01 ENETC │ │ │ + * 10 PCIe1 │ │ │ + * │ └──────────┘ + * The MSI glue layer auto adds 2 bits controller ID ahead of + * streamID, so mask these 2 bits to get streamID. The + * IOMMU glue layer doesn't do that. + */ + if (sid_i != (sid_m & IMX95_SID_MASK)) { + dev_err(dev, "iommu-map and msi-map entries mismatch!\n"); + return -EINVAL; + } + } + + if (!err_i) + sid = sid_i; + else if (!err_m) + sid = sid_m & IMX95_SID_MASK; + + return imx_pcie_add_lut(imx_pcie, rid, sid); +} + +static void imx_pcie_disable_device(struct pci_host_bridge *bridge, + struct pci_dev *pdev) +{ + struct imx_pcie *imx_pcie; + + imx_pcie = to_imx_pcie(to_dw_pcie_from_pp(bridge->sysdata)); + imx_pcie_remove_lut(imx_pcie, pci_dev_id(pdev)); +} + static int imx_pcie_host_init(struct dw_pcie_rp *pp) { struct dw_pcie *pci = to_dw_pcie_from_pp(pp); @@ -946,6 +1144,11 @@ static int imx_pcie_host_init(struct dw_pcie_rp *pp) } } + if (pp->bridge && imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_LUT)) { + pp->bridge->enable_device = imx_pcie_enable_device; + pp->bridge->disable_device = imx_pcie_disable_device; + } + imx_pcie_assert_core_reset(imx_pcie); if (imx_pcie->drvdata->init_phy) @@ -1330,6 +1533,8 @@ static int imx_pcie_probe(struct platform_device *pdev) imx_pcie->pci = pci; imx_pcie->drvdata = of_device_get_match_data(dev); + mutex_init(&imx_pcie->lock); + /* Find the PHY if one is defined, only imx7d uses it */ np = of_parse_phandle(node, "fsl,imx7d-pcie-phy", 0); if (np) { @@ -1627,7 +1832,8 @@ static const struct imx_pcie_drvdata drvdata[] = { }, [IMX95] = { .variant = IMX95, - .flags = IMX_PCIE_FLAG_HAS_SERDES, + .flags = IMX_PCIE_FLAG_HAS_SERDES | + IMX_PCIE_FLAG_HAS_LUT, .clk_names = imx8mq_clks, .clks_cnt = ARRAY_SIZE(imx8mq_clks), .ltssm_off = IMX95_PE0_GEN_CTRL_3, From 477ac7b0a7984e12e8db07fe54ad64443c5f8928 Mon Sep 17 00:00:00 2001 From: Marc Zyngier Date: Wed, 4 Dec 2024 15:01:44 +0000 Subject: [PATCH 3/4] PCI: host-generic: Allow {en,dis}able_device() to be provided via pci_ecam_ops In order to let host controller drivers using the host-generic infrastructure use the {en,dis}able_device() callbacks that can be used to configure sideband RID mapping hardware, provide these two callbacks as part of the pci_ecam_ops structure. Link: https://lore.kernel.org/r/20241204150145.800408-2-maz@kernel.org Signed-off-by: Marc Zyngier Signed-off-by: Bjorn Helgaas Reviewed-by: Frank Li Reviewed-by: Manivannan Sadhasivam --- drivers/pci/controller/pci-host-common.c | 2 ++ include/linux/pci-ecam.h | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c index cf5f59a745b3..f441bfd6f96a 100644 --- a/drivers/pci/controller/pci-host-common.c +++ b/drivers/pci/controller/pci-host-common.c @@ -75,6 +75,8 @@ int pci_host_common_probe(struct platform_device *pdev) bridge->sysdata = cfg; bridge->ops = (struct pci_ops *)&ops->pci_ops; + bridge->enable_device = ops->enable_device; + bridge->disable_device = ops->disable_device; bridge->msi_domain = true; return pci_host_probe(bridge); diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h index 3a4860bd2758..3a10f8cfc3ad 100644 --- a/include/linux/pci-ecam.h +++ b/include/linux/pci-ecam.h @@ -45,6 +45,10 @@ struct pci_ecam_ops { unsigned int bus_shift; struct pci_ops pci_ops; int (*init)(struct pci_config_window *); + int (*enable_device)(struct pci_host_bridge *, + struct pci_dev *); + void (*disable_device)(struct pci_host_bridge *, + struct pci_dev *); }; /* From d9f6642ab7ca007ab899b2a89a6b754dcabcad47 Mon Sep 17 00:00:00 2001 From: Marc Zyngier Date: Wed, 4 Dec 2024 15:01:45 +0000 Subject: [PATCH 4/4] PCI: apple: Convert to {en,dis}able_device() callbacks Now that the core host-bridge infrastructure is able to give us a callback on each device being added or removed, convert the bus-notifier hack to it. Link: https://lore.kernel.org/r/20241204150145.800408-3-maz@kernel.org Signed-off-by: Marc Zyngier Signed-off-by: Bjorn Helgaas Reviewed-by: Frank Li Reviewed-by: Manivannan Sadhasivam --- drivers/pci/controller/pcie-apple.c | 75 ++++++----------------------- 1 file changed, 15 insertions(+), 60 deletions(-) diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c index fefab2758a06..a7e51bc1c2fe 100644 --- a/drivers/pci/controller/pcie-apple.c +++ b/drivers/pci/controller/pcie-apple.c @@ -26,7 +26,6 @@ #include #include #include -#include #include #include @@ -667,12 +666,16 @@ static struct apple_pcie_port *apple_pcie_get_port(struct pci_dev *pdev) return NULL; } -static int apple_pcie_add_device(struct apple_pcie_port *port, - struct pci_dev *pdev) +static int apple_pcie_enable_device(struct pci_host_bridge *bridge, struct pci_dev *pdev) { u32 sid, rid = pci_dev_id(pdev); + struct apple_pcie_port *port; int idx, err; + port = apple_pcie_get_port(pdev); + if (!port) + return 0; + dev_dbg(&pdev->dev, "added to bus %s, index %d\n", pci_name(pdev->bus->self), port->idx); @@ -698,12 +701,16 @@ static int apple_pcie_add_device(struct apple_pcie_port *port, return idx >= 0 ? 0 : -ENOSPC; } -static void apple_pcie_release_device(struct apple_pcie_port *port, - struct pci_dev *pdev) +static void apple_pcie_disable_device(struct pci_host_bridge *bridge, struct pci_dev *pdev) { + struct apple_pcie_port *port; u32 rid = pci_dev_id(pdev); int idx; + port = apple_pcie_get_port(pdev); + if (!port) + return; + mutex_lock(&port->pcie->lock); for_each_set_bit(idx, port->sid_map, port->sid_map_sz) { @@ -721,45 +728,6 @@ static void apple_pcie_release_device(struct apple_pcie_port *port, mutex_unlock(&port->pcie->lock); } -static int apple_pcie_bus_notifier(struct notifier_block *nb, - unsigned long action, - void *data) -{ - struct device *dev = data; - struct pci_dev *pdev = to_pci_dev(dev); - struct apple_pcie_port *port; - int err; - - /* - * This is a bit ugly. We assume that if we get notified for - * any PCI device, we must be in charge of it, and that there - * is no other PCI controller in the whole system. It probably - * holds for now, but who knows for how long? - */ - port = apple_pcie_get_port(pdev); - if (!port) - return NOTIFY_DONE; - - switch (action) { - case BUS_NOTIFY_ADD_DEVICE: - err = apple_pcie_add_device(port, pdev); - if (err) - return notifier_from_errno(err); - break; - case BUS_NOTIFY_DEL_DEVICE: - apple_pcie_release_device(port, pdev); - break; - default: - return NOTIFY_DONE; - } - - return NOTIFY_OK; -} - -static struct notifier_block apple_pcie_nb = { - .notifier_call = apple_pcie_bus_notifier, -}; - static int apple_pcie_init(struct pci_config_window *cfg) { struct device *dev = cfg->parent; @@ -799,23 +767,10 @@ static int apple_pcie_init(struct pci_config_window *cfg) return 0; } -static int apple_pcie_probe(struct platform_device *pdev) -{ - int ret; - - ret = bus_register_notifier(&pci_bus_type, &apple_pcie_nb); - if (ret) - return ret; - - ret = pci_host_common_probe(pdev); - if (ret) - bus_unregister_notifier(&pci_bus_type, &apple_pcie_nb); - - return ret; -} - static const struct pci_ecam_ops apple_pcie_cfg_ecam_ops = { .init = apple_pcie_init, + .enable_device = apple_pcie_enable_device, + .disable_device = apple_pcie_disable_device, .pci_ops = { .map_bus = pci_ecam_map_bus, .read = pci_generic_config_read, @@ -830,7 +785,7 @@ static const struct of_device_id apple_pcie_of_match[] = { MODULE_DEVICE_TABLE(of, apple_pcie_of_match); static struct platform_driver apple_pcie_driver = { - .probe = apple_pcie_probe, + .probe = pci_host_common_probe, .driver = { .name = "pcie-apple", .of_match_table = apple_pcie_of_match,