Messages in this thread | | | Date | Thu, 18 Apr 2024 23:07:31 +0100 | Subject | Re: [PATCH v2 3/7] iommu/riscv: Add RISC-V IOMMU PCIe device driver | From | Robin Murphy <> |
| |
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@ics.forth.gr> > Signed-off-by: Nick Kossifidis <mick@ics.forth.gr> > Signed-off-by: Tomasz Jeznach <tjeznach@rivosinc.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@rivosinc.com> > + * Nick Kossifidis <mick@ics.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);
| |