| 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: <08e12daa-3fcc-4ab2-ac55-dee8f69c429a@arm.com>
Date: Thu, 18 Apr 2024 23:07:31 +0100
From: Robin Murphy <robin.murphy@....com>
To: Tomasz Jeznach <tjeznach@...osinc.com>, Joerg Roedel <joro@...tes.org>,
Will Deacon <will@...nel.org>, Paul Walmsley <paul.walmsley@...ive.com>
Cc: Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
Anup Patel <apatel@...tanamicro.com>, Sunil V L <sunilvl@...tanamicro.com>,
Nick Kossifidis <mick@....forth.gr>, Sebastien Boeuf <seb@...osinc.com>,
Rob Herring <robh+dt@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, devicetree@...r.kernel.org,
iommu@...ts.linux.dev, linux-riscv@...ts.infradead.org,
linux-kernel@...r.kernel.org, linux@...osinc.com
Subject: Re: [PATCH v2 3/7] iommu/riscv: Add RISC-V IOMMU PCIe device driver
On 2024-04-18 5:32 pm, Tomasz Jeznach wrote:
> Introduce device driver for PCIe implementation
> of RISC-V IOMMU architected hardware.
>
> IOMMU hardware and system support for MSI or MSI-X is
> required by this implementation.
>
> Vendor and device identifiers used in this patch
> matches QEMU implementation of the RISC-V IOMMU PCIe
> device, from Rivos VID (0x1efd) range allocated by the PCI-SIG.
>
> Link: https://lore.kernel.org/qemu-devel/20240307160319.675044-1-dbarboza@ventanamicro.com/
> Co-developed-by: Nick Kossifidis <mick@....forth.gr>
> Signed-off-by: Nick Kossifidis <mick@....forth.gr>
> Signed-off-by: Tomasz Jeznach <tjeznach@...osinc.com>
> ---
> MAINTAINERS | 1 +
> drivers/iommu/riscv/Kconfig | 6 ++
> drivers/iommu/riscv/Makefile | 1 +
> drivers/iommu/riscv/iommu-pci.c | 154 ++++++++++++++++++++++++++++++++
> 4 files changed, 162 insertions(+)
> create mode 100644 drivers/iommu/riscv/iommu-pci.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 051599c76585..4da290d5e9db 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -18975,6 +18975,7 @@ F: Documentation/devicetree/bindings/iommu/riscv,iommu.yaml
> F: drivers/iommu/riscv/Kconfig
> F: drivers/iommu/riscv/Makefile
> F: drivers/iommu/riscv/iommu-bits.h
> +F: drivers/iommu/riscv/iommu-pci.c
> F: drivers/iommu/riscv/iommu-platform.c
> F: drivers/iommu/riscv/iommu.c
> F: drivers/iommu/riscv/iommu.h
> diff --git a/drivers/iommu/riscv/Kconfig b/drivers/iommu/riscv/Kconfig
> index d02326bddb4c..711326992585 100644
> --- a/drivers/iommu/riscv/Kconfig
> +++ b/drivers/iommu/riscv/Kconfig
> @@ -14,3 +14,9 @@ config RISCV_IOMMU
>
> Say Y here if your SoC includes an IOMMU device implementing
> the RISC-V IOMMU architecture.
> +
> +config RISCV_IOMMU_PCI
> + def_bool y if RISCV_IOMMU && PCI_MSI
> + depends on RISCV_IOMMU && PCI_MSI
> + help
> + Support for the PCI implementation of RISC-V IOMMU architecture.
Similar comments as before.
> diff --git a/drivers/iommu/riscv/Makefile b/drivers/iommu/riscv/Makefile
> index e4c189de58d3..f54c9ed17d41 100644
> --- a/drivers/iommu/riscv/Makefile
> +++ b/drivers/iommu/riscv/Makefile
> @@ -1,2 +1,3 @@
> # SPDX-License-Identifier: GPL-2.0-only
> obj-$(CONFIG_RISCV_IOMMU) += iommu.o iommu-platform.o
> +obj-$(CONFIG_RISCV_IOMMU_PCI) += iommu-pci.o
> diff --git a/drivers/iommu/riscv/iommu-pci.c b/drivers/iommu/riscv/iommu-pci.c
> new file mode 100644
> index 000000000000..9263c6e475be
> --- /dev/null
> +++ b/drivers/iommu/riscv/iommu-pci.c
> @@ -0,0 +1,154 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +/*
> + * Copyright © 2022-2024 Rivos Inc.
> + * Copyright © 2023 FORTH-ICS/CARV
> + *
> + * RISCV IOMMU as a PCIe device
> + *
> + * Authors
> + * Tomasz Jeznach <tjeznach@...osinc.com>
> + * Nick Kossifidis <mick@....forth.gr>
> + */
> +
> +#include <linux/compiler.h>
> +#include <linux/init.h>
> +#include <linux/iommu.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +
> +#include "iommu-bits.h"
> +#include "iommu.h"
> +
> +/* Rivos Inc. assigned PCI Vendor and Device IDs */
> +#ifndef PCI_VENDOR_ID_RIVOS
> +#define PCI_VENDOR_ID_RIVOS 0x1efd
> +#endif
> +
> +#ifndef PCI_DEVICE_ID_RIVOS_IOMMU
> +#define PCI_DEVICE_ID_RIVOS_IOMMU 0xedf1
> +#endif
> +
> +static int riscv_iommu_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> +{
> + struct device *dev = &pdev->dev;
> + struct riscv_iommu_device *iommu;
> + int rc, vec;
> +
> + rc = pci_enable_device_mem(pdev);
> + if (rc)
> + return rc;
> +
> + rc = pci_request_mem_regions(pdev, KBUILD_MODNAME);
> + if (rc)
> + goto fail;
> +
> + pci_set_master(pdev);
> +
> + if (!(pci_resource_flags(pdev, 0) & IORESOURCE_MEM))
> + goto fail;
> +
> + if (pci_resource_len(pdev, 0) < RISCV_IOMMU_REG_SIZE)
> + goto fail;
> +
> + iommu = devm_kzalloc(dev, sizeof(*iommu), GFP_KERNEL);
> + if (!iommu)
> + goto fail;
> +
> + iommu->dev = dev;
> + iommu->reg = pci_iomap(pdev, 0, RISCV_IOMMU_REG_SIZE);
Maybe consider some of the pcim_* devres helpers, to simplify
cleanup/remove?
> +
> + if (!iommu->reg)
> + goto fail;
> +
> + dev_set_drvdata(dev, iommu);
> +
> + /* Check device reported capabilities / features. */
> + iommu->caps = riscv_iommu_readq(iommu, RISCV_IOMMU_REG_CAP);
> + iommu->fctl = riscv_iommu_readl(iommu, RISCV_IOMMU_REG_FCTL);
> +
> + /* The PCI driver only uses MSIs, make sure the IOMMU supports this */
> + switch (FIELD_GET(RISCV_IOMMU_CAP_IGS, iommu->caps)) {
> + case RISCV_IOMMU_CAP_IGS_MSI:
> + case RISCV_IOMMU_CAP_IGS_BOTH:
> + break;
> + default:
> + dev_err(dev, "unable to use message-signaled interrupts\n");
> + rc = -ENODEV;
> + goto fail_unmap;
> + }
> +
> + /* Allocate and assign IRQ vectors for the various events */
> + rc = pci_alloc_irq_vectors(pdev, 1, RISCV_IOMMU_INTR_COUNT,
> + PCI_IRQ_MSIX | PCI_IRQ_MSI);
> + if (rc <= 0) {
> + dev_err(dev, "unable to allocate irq vectors\n");
> + goto fail_unmap;
> + }
> + for (vec = 0; vec < rc; vec++) {
> + iommu->irqs[vec] = msi_get_virq(dev, vec);
> + if (!iommu->irqs[vec])
Can that ever fail if the loop is already bounded to the number of
vectors successfully allocated?
> + break;
> + }
> + iommu->irqs_count = vec;
> +
> + /* Enable message-signaled interrupts, fctl.WSI */
> + if (iommu->fctl & RISCV_IOMMU_FCTL_WSI) {
> + iommu->fctl ^= RISCV_IOMMU_FCTL_WSI;
> + riscv_iommu_writel(iommu, RISCV_IOMMU_REG_FCTL, iommu->fctl);
> + }
> +
> + rc = riscv_iommu_init(iommu);
> + if (!rc)
> + return 0;
> +
> +fail_unmap:
> + iounmap(iommu->reg);
> + pci_free_irq_vectors(pdev);
> +fail:
> + pci_release_regions(pdev);
> + pci_clear_master(pdev);
> + pci_disable_device(pdev);
> + return rc;
> +}
> +
> +static void riscv_iommu_pci_remove(struct pci_dev *pdev)
> +{
> + struct riscv_iommu_device *iommu = dev_get_drvdata(&pdev->dev);
> +
> + riscv_iommu_remove(iommu);
> + iounmap(iommu->reg);
> + pci_free_irq_vectors(pdev);
> + pci_release_regions(pdev);
> + pci_clear_master(pdev);
> + pci_disable_device(pdev);
> +}
> +
> +static const struct pci_device_id riscv_iommu_pci_tbl[] = {
> + {PCI_VENDOR_ID_RIVOS, PCI_DEVICE_ID_RIVOS_IOMMU,
> + PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
> + {0,}
> +};
> +
> +MODULE_DEVICE_TABLE(pci, riscv_iommu_pci_tbl);
> +
> +static const struct of_device_id riscv_iommu_of_match[] = {
> + {.compatible = "riscv,pci-iommu",},
> + {},
> +};
> +
> +MODULE_DEVICE_TABLE(of, riscv_iommu_of_match);
> +
> +static struct pci_driver riscv_iommu_pci_driver = {
> + .name = KBUILD_MODNAME,
> + .id_table = riscv_iommu_pci_tbl,
> + .probe = riscv_iommu_pci_probe,
> + .remove = riscv_iommu_pci_remove,
> + .driver = {
> + .of_match_table = riscv_iommu_of_match,
Does an of_match_table serve any functional purpose for a PCI driver? I
can't find any other examples of this being done.
Thanks,
Robin.
> + .suppress_bind_attrs = true,
> + },
> +};
> +
> +module_pci_driver(riscv_iommu_pci_driver);
Powered by blists - more mailing lists