| 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: <4fa5b5f5-6e76-4d3f-b3b4-1e977b7d3c4a@siemens.com>
Date: Sun, 3 Nov 2024 10:50:01 +0100
From: Jan Kiszka <jan.kiszka@...mens.com>
To: Vignesh Raghavendra <vigneshr@...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 03.11.24 07:15, Vignesh Raghavendra wrote:
>
>
> 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 .
>
Can be done, but it would move the stubbing to include/linux/ti-pvu.h
and ti_pvu_create_region and ti_pvu_remove_region.
Jan
>
> 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);
>
--
Siemens AG, Technology
Linux Expert Center
Powered by blists - more mailing lists