Re: [RFC PATCH v2 04/18] iommu/riscv: Add IRQ domain for interrupt remapping

Nutty.Liu
2025-09-28T09:30:43Z

On 9/21/2025 4:38 AM, Andrew Jones wrote:
> This is just a skeleton. Until irq-set-affinity functions are
> implemented the IRQ domain doesn't serve any purpose.
>
> Signed-off-by: Andrew Jones<ajones@ventanamicro.com>
> ---
> drivers/iommu/riscv/Makefile | 2 +-
> drivers/iommu/riscv/iommu-ir.c | 114 +++++++++++++++++++++++++++++++++
> drivers/iommu/riscv/iommu.c | 36 +++++++++++
> drivers/iommu/riscv/iommu.h | 12 ++++
> 4 files changed, 163 insertions(+), 1 deletion(-)
> create mode 100644 drivers/iommu/riscv/iommu-ir.c
>
> diff --git a/drivers/iommu/riscv/Makefile b/drivers/iommu/riscv/Makefile
> index b5929f9f23e6..9c83f877d50f 100644
> --- a/drivers/iommu/riscv/Makefile
> +++ b/drivers/iommu/riscv/Makefile
> @@ -1,3 +1,3 @@
> # SPDX-License-Identifier: GPL-2.0-only
> -obj-y += iommu.o iommu-platform.o
> +obj-y += iommu.o iommu-ir.o iommu-platform.o
> obj-$(CONFIG_RISCV_IOMMU_PCI) += iommu-pci.o
> diff --git a/drivers/iommu/riscv/iommu-ir.c b/drivers/iommu/riscv/iommu-ir.c
> new file mode 100644
> index 000000000000..08cf159b587d
> --- /dev/null
> +++ b/drivers/iommu/riscv/iommu-ir.c
> @@ -0,0 +1,114 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * IOMMU Interrupt Remapping
> + *
> + * Copyright © 2025 Ventana Micro Systems Inc.
> + */
> +#include <linux/irqdomain.h>
> +#include <linux/msi.h>
> +
> +#include "iommu.h"
> +
> +static struct irq_chip riscv_iommu_ir_irq_chip = {
> + .name = "IOMMU-IR",
> + .irq_ack = irq_chip_ack_parent,
> + .irq_mask = irq_chip_mask_parent,
> + .irq_unmask = irq_chip_unmask_parent,
> + .irq_set_affinity = irq_chip_set_affinity_parent,
> +};
> +
> +static int riscv_iommu_ir_irq_domain_alloc_irqs(struct irq_domain *irqdomain,
> + unsigned int irq_base, unsigned int nr_irqs,
> + void *arg)
> +{
> + struct irq_data *data;
> + int i, ret;
> +
> + ret = irq_domain_alloc_irqs_parent(irqdomain, irq_base, nr_irqs, arg);
> + if (ret)
> + return ret;
> +
> + for (i = 0; i < nr_irqs; i++) {
> + data = irq_domain_get_irq_data(irqdomain, irq_base + i);
Nitpick:  Would it be better to check if 'data' is NULL ?

> + data->chip = &riscv_iommu_ir_irq_chip;
> + }
> +
> + return 0;
> +}
> +
> +static const struct irq_domain_ops riscv_iommu_ir_irq_domain_ops = {
> + .alloc = riscv_iommu_ir_irq_domain_alloc_irqs,
> + .free = irq_domain_free_irqs_parent,
> +};
> +
> +static const struct msi_parent_ops riscv_iommu_ir_msi_parent_ops = {
> + .prefix = "IR-",
> + .supported_flags = MSI_GENERIC_FLAGS_MASK |
> + MSI_FLAG_PCI_MSIX,
> + .required_flags = MSI_FLAG_USE_DEF_DOM_OPS |
> + MSI_FLAG_USE_DEF_CHIP_OPS |
> + MSI_FLAG_PCI_MSI_MASK_PARENT,
> + .chip_flags = MSI_CHIP_FLAG_SET_ACK,
> + .init_dev_msi_info = msi_parent_init_dev_msi_info,
> +};
> +
> +struct irq_domain *riscv_iommu_ir_irq_domain_create(struct riscv_iommu_device *iommu,
> + struct device *dev,
> + struct riscv_iommu_info *info)
> +{
> + struct irq_domain *irqparent = dev_get_msi_domain(dev);
> + struct irq_domain *irqdomain;
> + struct fwnode_handle *fn;
> + char *fwname;
> +
> + fwname = kasprintf(GFP_KERNEL, "IOMMU-IR-%s", dev_name(dev));
> + if (!fwname)
> + return NULL;
> +
> + fn = irq_domain_alloc_named_fwnode(fwname);
> + kfree(fwname);
> + if (!fn) {
> + dev_err(iommu->dev, "Couldn't allocate fwnode\n");
> + return NULL;
> + }
> +
> + irqdomain = irq_domain_create_hierarchy(irqparent, 0, 0, fn,
> + &riscv_iommu_ir_irq_domain_ops,
> + info);
> + if (!irqdomain) {
> + dev_err(iommu->dev, "Failed to create IOMMU irq domain\n");
> + irq_domain_free_fwnode(fn);
> + return NULL;
> + }
> +
> + irqdomain->flags |= IRQ_DOMAIN_FLAG_MSI_PARENT;
> + irqdomain->msi_parent_ops = &riscv_iommu_ir_msi_parent_ops;
> + irq_domain_update_bus_token(irqdomain, DOMAIN_BUS_MSI_REMAP);
> +
> + dev_set_msi_domain(dev, irqdomain);
> +
> + return irqdomain;
> +}
> +
> +void riscv_iommu_ir_irq_domain_remove(struct riscv_iommu_info *info)
> +{
> + struct fwnode_handle *fn;
> +
> + if (!info->irqdomain)
> + return;
> +
> + fn = info->irqdomain->fwnode;
> + irq_domain_remove(info->irqdomain);
> + info->irqdomain = NULL;
> + irq_domain_free_fwnode(fn);
> +}
> +
> +int riscv_iommu_ir_attach_paging_domain(struct riscv_iommu_domain *domain,
> + struct device *dev)
> +{
> + return 0;
> +}
> +
> +void riscv_iommu_ir_free_paging_domain(struct riscv_iommu_domain *domain)
> +{
> +}
> diff --git a/drivers/iommu/riscv/iommu.c b/drivers/iommu/riscv/iommu.c
> index a44c67a848fa..db2acd9dc64b 100644
> --- a/drivers/iommu/riscv/iommu.c
> +++ b/drivers/iommu/riscv/iommu.c
> @@ -17,6 +17,8 @@
> #include <linux/init.h>
> #include <linux/iommu.h>
> #include <linux/iopoll.h>
> +#include <linux/irqchip/riscv-imsic.h>
> +#include <linux/irqdomain.h>
> #include <linux/kernel.h>
> #include <linux/pci.h>
>
> @@ -1026,6 +1028,9 @@ static void riscv_iommu_iodir_update(struct riscv_iommu_device *iommu,
>
> WRITE_ONCE(dc->fsc, new_dc->fsc);
> WRITE_ONCE(dc->ta, new_dc->ta & RISCV_IOMMU_PC_TA_PSCID);
> + WRITE_ONCE(dc->msiptp, new_dc->msiptp);
> + WRITE_ONCE(dc->msi_addr_mask, new_dc->msi_addr_mask);
> + WRITE_ONCE(dc->msi_addr_pattern, new_dc->msi_addr_pattern);
Since the MSI page table pointer (msiptp) has been changed,
should all cache entries from this MSI page table need to be invalidated ?

Otherwise,
Reviewed-by: Nutty Liu <nutty.liu@hotmail.com>

Thanks,
Nutty