| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <3d7abd75-68a2-4232-ad8c-e874c10df1ae@ti.com>
Date: Sun, 3 Nov 2024 11:45:58 +0530
From: Vignesh Raghavendra <vigneshr@...com>
To: Jan Kiszka <jan.kiszka@...mens.com>, Nishanth Menon <nm@...com>,
Santosh
Shilimkar <ssantosh@...nel.org>,
Tero Kristo <kristo@...nel.org>, Rob Herring
<robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley
<conor+dt@...nel.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>
CC: <linux-arm-kernel@...ts.infradead.org>, <linux-pci@...r.kernel.org>,
Siddharth Vadapalli <s-vadapalli@...com>,
Bao Cheng Su
<baocheng.su@...mens.com>,
Hua Qian Li <huaqian.li@...mens.com>,
Diogo Ivo
<diogo.ivo@...mens.com>,
Lorenzo Pieralisi <lpieralisi@...nel.org>,
Krzysztof Wilczyński <kw@...ux.com>,
Bjorn Helgaas
<bhelgaas@...gle.com>
Subject: Re: [PATCH v6 4/7] PCI: keystone: Add support for PVU-based DMA
isolation on AM654
On 09/09/24 22:33, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@...mens.com>
>
> The AM654 lacks an IOMMU, thus does not support isolating DMA requests
> from untrusted PCI devices to selected memory regions this way. Use
> static PVU-based protection instead. The PVU, when enabled, will only
> accept DMA requests that address previously configured regions.
>
> Use the availability of a restricted-dma-pool memory region as trigger
> and register it as valid DMA target with the PVU. In addition, enable
> the mapping of requester IDs to VirtIDs in the PCI RC. Use only a single
> VirtID so far, catching all devices. This may be extended later on.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@...mens.com>
> ---
> CC: Lorenzo Pieralisi <lpieralisi@...nel.org>
> CC: "Krzysztof Wilczyński" <kw@...ux.com>
> CC: Bjorn Helgaas <bhelgaas@...gle.com>
> CC: linux-pci@...r.kernel.org
> ---
> drivers/pci/controller/dwc/pci-keystone.c | 108 ++++++++++++++++++++++
> 1 file changed, 108 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> index 2219b1a866fa..a5954cae6d5d 100644
> --- a/drivers/pci/controller/dwc/pci-keystone.c
> +++ b/drivers/pci/controller/dwc/pci-keystone.c
> @@ -19,6 +19,7 @@
> #include <linux/mfd/syscon.h>
> #include <linux/msi.h>
> #include <linux/of.h>
> +#include <linux/of_address.h>
> #include <linux/of_irq.h>
> #include <linux/of_pci.h>
> #include <linux/phy/phy.h>
> @@ -26,6 +27,7 @@
> #include <linux/regmap.h>
> #include <linux/resource.h>
> #include <linux/signal.h>
> +#include <linux/ti-pvu.h>
>
> #include "../../pci.h"
> #include "pcie-designware.h"
> @@ -111,6 +113,16 @@
>
> #define PCI_DEVICE_ID_TI_AM654X 0xb00c
>
> +#define KS_PCI_VIRTID 0
> +
> +#define PCIE_VMAP_xP_CTRL 0x0
> +#define PCIE_VMAP_xP_REQID 0x4
> +#define PCIE_VMAP_xP_VIRTID 0x8
> +
> +#define PCIE_VMAP_xP_CTRL_EN BIT(0)
> +
> +#define PCIE_VMAP_xP_VIRTID_VID_MASK 0xfff
> +
> struct ks_pcie_of_data {
> enum dw_pcie_device_mode mode;
> const struct dw_pcie_host_ops *host_ops;
> @@ -1125,6 +1137,96 @@ static const struct of_device_id ks_pcie_of_match[] = {
> { },
> };
>
> +#ifdef CONFIG_TI_PVU
> +static int ks_init_vmap(struct platform_device *pdev, const char *vmap_name)
> +{
> + struct resource *res;
> + void __iomem *base;
> + u32 val;
> +
Nit:
if (!IS_ENABLED(CONFIG_TI_PVU))
return 0;
this looks cleaner than #ifdef.. #else..#endif .
Rest LGTM
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, vmap_name);
> + base = devm_pci_remap_cfg_resource(&pdev->dev, res);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + writel(0, base + PCIE_VMAP_xP_REQID);
> +
> + val = readl(base + PCIE_VMAP_xP_VIRTID);
> + val &= ~PCIE_VMAP_xP_VIRTID_VID_MASK;
> + val |= KS_PCI_VIRTID;
> + writel(val, base + PCIE_VMAP_xP_VIRTID);
> +
> + val = readl(base + PCIE_VMAP_xP_CTRL);
> + val |= PCIE_VMAP_xP_CTRL_EN;
> + writel(val, base + PCIE_VMAP_xP_CTRL);
> +
> + return 0;
> +}
> +
> +static int ks_init_restricted_dma(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct of_phandle_iterator it;
> + struct resource phys;
> + int err;
> +
> + /* Only process the first restricted dma pool, more are not allowed */
> + of_for_each_phandle(&it, err, dev->of_node, "memory-region",
> + NULL, 0) {
> + if (of_device_is_compatible(it.node, "restricted-dma-pool"))
> + break;
> + }
> + if (err)
> + return err == -ENOENT ? 0 : err;
> +
> + err = of_address_to_resource(it.node, 0, &phys);
> + if (err < 0) {
> + dev_err(dev, "failed to parse memory region %pOF: %d\n",
> + it.node, err);
> + return 0;
> + }
> +
> + /* Map all incoming requests on low and high prio port to virtID 0 */
> + err = ks_init_vmap(pdev, "vmap_lp");
> + if (err)
> + return err;
> + err = ks_init_vmap(pdev, "vmap_hp");
> + if (err)
> + return err;
> +
> + /*
> + * Enforce DMA pool usage with the help of the PVU.
> + * Any request outside will be dropped and raise an error at the PVU.
> + */
> + return ti_pvu_create_region(KS_PCI_VIRTID, &phys);
> +}
> +
> +static void ks_release_restricted_dma(struct platform_device *pdev)
> +{
> + struct of_phandle_iterator it;
> + struct resource phys;
> + int err;
> +
> + of_for_each_phandle(&it, err, pdev->dev.of_node, "memory-region",
> + NULL, 0) {
> + if (of_device_is_compatible(it.node, "restricted-dma-pool") &&
> + of_address_to_resource(it.node, 0, &phys) == 0) {
> + ti_pvu_remove_region(KS_PCI_VIRTID, &phys);
> + break;
> + }
> + }
> +}
> +#else
> +static inline int ks_init_restricted_dma(struct platform_device *pdev)
> +{
> + return 0;
> +}
> +
> +static inline void ks_release_restricted_dma(struct platform_device *pdev)
> +{
> +}
> +#endif
> +
> static int ks_pcie_probe(struct platform_device *pdev)
> {
> const struct dw_pcie_host_ops *host_ops;
> @@ -1273,6 +1375,10 @@ static int ks_pcie_probe(struct platform_device *pdev)
> if (ret < 0)
> goto err_get_sync;
>
> + ret = ks_init_restricted_dma(pdev);
> + if (ret < 0)
> + goto err_get_sync;
> +
> switch (mode) {
> case DW_PCIE_RC_TYPE:
> if (!IS_ENABLED(CONFIG_PCI_KEYSTONE_HOST)) {
> @@ -1354,6 +1460,8 @@ static void ks_pcie_remove(struct platform_device *pdev)
> int num_lanes = ks_pcie->num_lanes;
> struct device *dev = &pdev->dev;
>
> + ks_release_restricted_dma(pdev);
> +
> pm_runtime_put(dev);
> pm_runtime_disable(dev);
> ks_pcie_disable_phy(ks_pcie);
--
Regards
Vignesh
Powered by blists - more mailing lists