1. Patchew
  2. linux
  3. [PATCH v11 00/14] Linux RISC-V AIA Support
  4. View patch

[PATCH v11 12/14] irqchip/riscv-aplic: Add support for MSI-mode

Anup Patel posted 14 patches 2 years, 3 months ago
There is a newer version of this series
[PATCH v11 12/14] irqchip/riscv-aplic: Add support for MSI-mode
Posted by Anup Patel 2 years, 3 months ago 
The RISC-V advanced platform-level interrupt controller (APLIC) has
two modes of operation: 1) Direct mode and 2) MSI mode.
(For more details, refer https://github.com/riscv/riscv-aia)

In APLIC MSI-mode, wired interrupts are forwared as message signaled
interrupts (MSIs) to CPUs via IMSIC.

We extend the existing APLIC irqchip driver to support MSI-mode for
RISC-V platforms having both wired interrupts and MSIs.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 drivers/irqchip/Kconfig                |   6 +
 drivers/irqchip/Makefile               |   1 +
 drivers/irqchip/irq-riscv-aplic-main.c |   2 +-
 drivers/irqchip/irq-riscv-aplic-main.h |   8 +
 drivers/irqchip/irq-riscv-aplic-msi.c  | 285 +++++++++++++++++++++++++
 5 files changed, 301 insertions(+), 1 deletion(-)
 create mode 100644 drivers/irqchip/irq-riscv-aplic-msi.c

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 1996cc6f666a..7adc4dbe07ff 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -551,6 +551,12 @@ config RISCV_APLIC
 	depends on RISCV
 	select IRQ_DOMAIN_HIERARCHY
 
+config RISCV_APLIC_MSI
+	bool
+	depends on RISCV_APLIC
+	select GENERIC_MSI_IRQ
+	default RISCV_APLIC
+
 config RISCV_IMSIC
 	bool
 	depends on RISCV
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 7f8289790ed8..47995fdb2c60 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -96,6 +96,7 @@ obj-$(CONFIG_CSKY_MPINTC)		+= irq-csky-mpintc.o
 obj-$(CONFIG_CSKY_APB_INTC)		+= irq-csky-apb-intc.o
 obj-$(CONFIG_RISCV_INTC)		+= irq-riscv-intc.o
 obj-$(CONFIG_RISCV_APLIC)		+= irq-riscv-aplic-main.o irq-riscv-aplic-direct.o
+obj-$(CONFIG_RISCV_APLIC_MSI)		+= irq-riscv-aplic-msi.o
 obj-$(CONFIG_RISCV_IMSIC)		+= irq-riscv-imsic-state.o irq-riscv-imsic-early.o irq-riscv-imsic-platform.o
 obj-$(CONFIG_SIFIVE_PLIC)		+= irq-sifive-plic.o
 obj-$(CONFIG_IMX_IRQSTEER)		+= irq-imx-irqsteer.o
diff --git a/drivers/irqchip/irq-riscv-aplic-main.c b/drivers/irqchip/irq-riscv-aplic-main.c
index 87450708a733..d1b342b66551 100644
--- a/drivers/irqchip/irq-riscv-aplic-main.c
+++ b/drivers/irqchip/irq-riscv-aplic-main.c
@@ -205,7 +205,7 @@ static int aplic_probe(struct platform_device *pdev)
 		msi_mode = of_property_present(to_of_node(dev->fwnode),
 						"msi-parent");
 	if (msi_mode)
-		rc = -ENODEV;
+		rc = aplic_msi_setup(dev, regs);
 	else
 		rc = aplic_direct_setup(dev, regs);
 	if (rc) {
diff --git a/drivers/irqchip/irq-riscv-aplic-main.h b/drivers/irqchip/irq-riscv-aplic-main.h
index 474a04229334..78267ec58098 100644
--- a/drivers/irqchip/irq-riscv-aplic-main.h
+++ b/drivers/irqchip/irq-riscv-aplic-main.h
@@ -41,5 +41,13 @@ void aplic_init_hw_global(struct aplic_priv *priv, bool msi_mode);
 int aplic_setup_priv(struct aplic_priv *priv, struct device *dev,
 		     void __iomem *regs);
 int aplic_direct_setup(struct device *dev, void __iomem *regs);
+#ifdef CONFIG_RISCV_APLIC_MSI
+int aplic_msi_setup(struct device *dev, void __iomem *regs);
+#else
+static inline int aplic_msi_setup(struct device *dev, void __iomem *regs)
+{
+	return -ENODEV;
+}
+#endif
 
 #endif
diff --git a/drivers/irqchip/irq-riscv-aplic-msi.c b/drivers/irqchip/irq-riscv-aplic-msi.c
new file mode 100644
index 000000000000..086d00e0429e
--- /dev/null
+++ b/drivers/irqchip/irq-riscv-aplic-msi.c
@@ -0,0 +1,285 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2021 Western Digital Corporation or its affiliates.
+ * Copyright (C) 2022 Ventana Micro Systems Inc.
+ */
+
+#include <linux/bitops.h>
+#include <linux/cpu.h>
+#include <linux/interrupt.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/riscv-aplic.h>
+#include <linux/irqchip/riscv-imsic.h>
+#include <linux/module.h>
+#include <linux/msi.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/printk.h>
+#include <linux/smp.h>
+
+#include "irq-riscv-aplic-main.h"
+
+static void aplic_msi_irq_unmask(struct irq_data *d)
+{
+	aplic_irq_unmask(d);
+	irq_chip_unmask_parent(d);
+}
+
+static void aplic_msi_irq_mask(struct irq_data *d)
+{
+	aplic_irq_mask(d);
+	irq_chip_mask_parent(d);
+}
+
+static void aplic_msi_irq_eoi(struct irq_data *d)
+{
+	struct aplic_priv *priv = irq_data_get_irq_chip_data(d);
+	u32 reg_off, reg_mask;
+
+	/*
+	 * EOI handling only required only for level-triggered
+	 * interrupts in APLIC MSI mode.
+	 */
+
+	reg_off = APLIC_CLRIP_BASE + ((d->hwirq / APLIC_IRQBITS_PER_REG) * 4);
+	reg_mask = BIT(d->hwirq % APLIC_IRQBITS_PER_REG);
+	switch (irqd_get_trigger_type(d)) {
+	case IRQ_TYPE_LEVEL_LOW:
+		if (!(readl(priv->regs + reg_off) & reg_mask))
+			writel(d->hwirq, priv->regs + APLIC_SETIPNUM_LE);
+		break;
+	case IRQ_TYPE_LEVEL_HIGH:
+		if (readl(priv->regs + reg_off) & reg_mask)
+			writel(d->hwirq, priv->regs + APLIC_SETIPNUM_LE);
+		break;
+	}
+}
+
+static struct irq_chip aplic_msi_chip = {
+	.name		= "APLIC-MSI",
+	.irq_mask	= aplic_msi_irq_mask,
+	.irq_unmask	= aplic_msi_irq_unmask,
+	.irq_set_type	= aplic_irq_set_type,
+	.irq_eoi	= aplic_msi_irq_eoi,
+#ifdef CONFIG_SMP
+	.irq_set_affinity = irq_chip_set_affinity_parent,
+#endif
+	.flags		= IRQCHIP_SET_TYPE_MASKED |
+			  IRQCHIP_SKIP_SET_WAKE |
+			  IRQCHIP_MASK_ON_SUSPEND,
+};
+
+static int aplic_msi_irqdomain_translate(struct irq_domain *d,
+					 struct irq_fwspec *fwspec,
+					 unsigned long *hwirq,
+					 unsigned int *type)
+{
+	struct aplic_priv *priv = platform_msi_get_host_data(d);
+
+	return aplic_irqdomain_translate(fwspec, priv->gsi_base, hwirq, type);
+}
+
+static int aplic_msi_irqdomain_alloc(struct irq_domain *domain,
+				     unsigned int virq, unsigned int nr_irqs,
+				     void *arg)
+{
+	int i, ret;
+	unsigned int type;
+	irq_hw_number_t hwirq;
+	struct irq_fwspec *fwspec = arg;
+	struct aplic_priv *priv = platform_msi_get_host_data(domain);
+
+	ret = aplic_irqdomain_translate(fwspec, priv->gsi_base, &hwirq, &type);
+	if (ret)
+		return ret;
+
+	ret = platform_msi_device_domain_alloc(domain, virq, nr_irqs);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < nr_irqs; i++) {
+		irq_domain_set_info(domain, virq + i, hwirq + i,
+				    &aplic_msi_chip, priv, handle_fasteoi_irq,
+				    NULL, NULL);
+		/*
+		 * APLIC does not implement irq_disable() so Linux interrupt
+		 * subsystem will take a lazy approach for disabling an APLIC
+		 * interrupt. This means APLIC interrupts are left unmasked
+		 * upon system suspend and interrupts are not processed
+		 * immediately upon system wake up. To tackle this, we disable
+		 * the lazy approach for all APLIC interrupts.
+		 */
+		irq_set_status_flags(virq + i, IRQ_DISABLE_UNLAZY);
+	}
+
+	return 0;
+}
+
+static const struct irq_domain_ops aplic_msi_irqdomain_ops = {
+	.translate	= aplic_msi_irqdomain_translate,
+	.alloc		= aplic_msi_irqdomain_alloc,
+	.free		= platform_msi_device_domain_free,
+};
+
+static void aplic_msi_write_msg(struct msi_desc *desc, struct msi_msg *msg)
+{
+	unsigned int group_index, hart_index, guest_index, val;
+	struct irq_data *d = irq_get_irq_data(desc->irq);
+	struct aplic_priv *priv = irq_data_get_irq_chip_data(d);
+	struct aplic_msicfg *mc = &priv->msicfg;
+	phys_addr_t tppn, tbppn, msg_addr;
+	void __iomem *target;
+
+	/* For zeroed MSI, simply write zero into the target register */
+	if (!msg->address_hi && !msg->address_lo && !msg->data) {
+		target = priv->regs + APLIC_TARGET_BASE;
+		target += (d->hwirq - 1) * sizeof(u32);
+		writel(0, target);
+		return;
+	}
+
+	/* Sanity check on message data */
+	WARN_ON(msg->data > APLIC_TARGET_EIID_MASK);
+
+	/* Compute target MSI address */
+	msg_addr = (((u64)msg->address_hi) << 32) | msg->address_lo;
+	tppn = msg_addr >> APLIC_xMSICFGADDR_PPN_SHIFT;
+
+	/* Compute target HART Base PPN */
+	tbppn = tppn;
+	tbppn &= ~APLIC_xMSICFGADDR_PPN_HART(mc->lhxs);
+	tbppn &= ~APLIC_xMSICFGADDR_PPN_LHX(mc->lhxw, mc->lhxs);
+	tbppn &= ~APLIC_xMSICFGADDR_PPN_HHX(mc->hhxw, mc->hhxs);
+	WARN_ON(tbppn != mc->base_ppn);
+
+	/* Compute target group and hart indexes */
+	group_index = (tppn >> APLIC_xMSICFGADDR_PPN_HHX_SHIFT(mc->hhxs)) &
+		     APLIC_xMSICFGADDR_PPN_HHX_MASK(mc->hhxw);
+	hart_index = (tppn >> APLIC_xMSICFGADDR_PPN_LHX_SHIFT(mc->lhxs)) &
+		     APLIC_xMSICFGADDR_PPN_LHX_MASK(mc->lhxw);
+	hart_index |= (group_index << mc->lhxw);
+	WARN_ON(hart_index > APLIC_TARGET_HART_IDX_MASK);
+
+	/* Compute target guest index */
+	guest_index = tppn & APLIC_xMSICFGADDR_PPN_HART(mc->lhxs);
+	WARN_ON(guest_index > APLIC_TARGET_GUEST_IDX_MASK);
+
+	/* Update IRQ TARGET register */
+	target = priv->regs + APLIC_TARGET_BASE;
+	target += (d->hwirq - 1) * sizeof(u32);
+	val = (hart_index & APLIC_TARGET_HART_IDX_MASK)
+				<< APLIC_TARGET_HART_IDX_SHIFT;
+	val |= (guest_index & APLIC_TARGET_GUEST_IDX_MASK)
+				<< APLIC_TARGET_GUEST_IDX_SHIFT;
+	val |= (msg->data & APLIC_TARGET_EIID_MASK);
+	writel(val, target);
+}
+
+int aplic_msi_setup(struct device *dev, void __iomem *regs)
+{
+	const struct imsic_global_config *imsic_global;
+	struct irq_domain *irqdomain;
+	struct aplic_priv *priv;
+	struct aplic_msicfg *mc;
+	phys_addr_t pa;
+	int rc;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	rc = aplic_setup_priv(priv, dev, regs);
+	if (!priv) {
+		dev_err(dev, "failed to create APLIC context\n");
+		return rc;
+	}
+	mc = &priv->msicfg;
+
+	/*
+	 * The APLIC outgoing MSI config registers assume target MSI
+	 * controller to be RISC-V AIA IMSIC controller.
+	 */
+	imsic_global = imsic_get_global_config();
+	if (!imsic_global) {
+		dev_err(dev, "IMSIC global config not found\n");
+		return -ENODEV;
+	}
+
+	/* Find number of guest index bits (LHXS) */
+	mc->lhxs = imsic_global->guest_index_bits;
+	if (APLIC_xMSICFGADDRH_LHXS_MASK < mc->lhxs) {
+		dev_err(dev, "IMSIC guest index bits big for APLIC LHXS\n");
+		return -EINVAL;
+	}
+
+	/* Find number of HART index bits (LHXW) */
+	mc->lhxw = imsic_global->hart_index_bits;
+	if (APLIC_xMSICFGADDRH_LHXW_MASK < mc->lhxw) {
+		dev_err(dev, "IMSIC hart index bits big for APLIC LHXW\n");
+		return -EINVAL;
+	}
+
+	/* Find number of group index bits (HHXW) */
+	mc->hhxw = imsic_global->group_index_bits;
+	if (APLIC_xMSICFGADDRH_HHXW_MASK < mc->hhxw) {
+		dev_err(dev, "IMSIC group index bits big for APLIC HHXW\n");
+		return -EINVAL;
+	}
+
+	/* Find first bit position of group index (HHXS) */
+	mc->hhxs = imsic_global->group_index_shift;
+	if (mc->hhxs < (2 * APLIC_xMSICFGADDR_PPN_SHIFT)) {
+		dev_err(dev, "IMSIC group index shift should be >= %d\n",
+			(2 * APLIC_xMSICFGADDR_PPN_SHIFT));
+		return -EINVAL;
+	}
+	mc->hhxs -= (2 * APLIC_xMSICFGADDR_PPN_SHIFT);
+	if (APLIC_xMSICFGADDRH_HHXS_MASK < mc->hhxs) {
+		dev_err(dev, "IMSIC group index shift big for APLIC HHXS\n");
+		return -EINVAL;
+	}
+
+	/* Compute PPN base */
+	mc->base_ppn = imsic_global->base_addr >> APLIC_xMSICFGADDR_PPN_SHIFT;
+	mc->base_ppn &= ~APLIC_xMSICFGADDR_PPN_HART(mc->lhxs);
+	mc->base_ppn &= ~APLIC_xMSICFGADDR_PPN_LHX(mc->lhxw, mc->lhxs);
+	mc->base_ppn &= ~APLIC_xMSICFGADDR_PPN_HHX(mc->hhxw, mc->hhxs);
+
+	/* Setup global config and interrupt delivery */
+	aplic_init_hw_global(priv, true);
+
+	/* Set the APLIC device MSI domain if not available */
+	if (!dev_get_msi_domain(dev)) {
+		/*
+		 * The device MSI domain for OF devices is only set at the
+		 * time of populating/creating OF device. If the device MSI
+		 * domain is discovered later after the OF device is created
+		 * then we need to set it explicitly before using any platform
+		 * MSI functions.
+		 *
+		 * In case of APLIC device, the parent MSI domain is always
+		 * IMSIC and the IMSIC MSI domains are created later through
+		 * the platform driver probing so we set it explicitly here.
+		 */
+		if (is_of_node(dev->fwnode))
+			of_msi_configure(dev, to_of_node(dev->fwnode));
+	}
+
+	/* Create irq domain instance for the APLIC MSI-mode */
+	irqdomain = platform_msi_create_device_domain(
+						dev, priv->nr_irqs + 1,
+						aplic_msi_write_msg,
+						&aplic_msi_irqdomain_ops,
+						priv);
+	if (!irqdomain) {
+		dev_err(dev, "failed to create MSI irq domain\n");
+		return -ENOMEM;
+	}
+
+	/* Advertise the interrupt controller */
+	pa = priv->msicfg.base_ppn << APLIC_xMSICFGADDR_PPN_SHIFT;
+	dev_info(dev, "%d interrupts forwared to MSI base %pa\n",
+		 priv->nr_irqs, &pa);
+
+	return 0;
+}
-- 
2.34.1
Re:[PATCH v11 12/14] irqchip/riscv-aplic: Add support for MSI-mode
Posted by Ben 2 years, 3 months ago 
At 2023-10-24 01:27:58, "Anup Patel" <apatel@ventanamicro.com> wrote:
>The RISC-V advanced platform-level interrupt controller (APLIC) has
>two modes of operation: 1) Direct mode and 2) MSI mode.
>(For more details, refer https://github.com/riscv/riscv-aia)
>
>In APLIC MSI-mode, wired interrupts are forwared as message signaled
>interrupts (MSIs) to CPUs via IMSIC.
>
>We extend the existing APLIC irqchip driver to support MSI-mode for
>RISC-V platforms having both wired interrupts and MSIs.
>
>Signed-off-by: Anup Patel <apatel@ventanamicro.com>
>---
> drivers/irqchip/Kconfig                |   6 +
> drivers/irqchip/Makefile               |   1 +
> drivers/irqchip/irq-riscv-aplic-main.c |   2 +-
> drivers/irqchip/irq-riscv-aplic-main.h |   8 +
> drivers/irqchip/irq-riscv-aplic-msi.c  | 285 +++++++++++++++++++++++++
> 5 files changed, 301 insertions(+), 1 deletion(-)
> create mode 100644 drivers/irqchip/irq-riscv-aplic-msi.c
>
>diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
>index 1996cc6f666a..7adc4dbe07ff 100644
>--- a/drivers/irqchip/Kconfig
>+++ b/drivers/irqchip/Kconfig
>@@ -551,6 +551,12 @@ config RISCV_APLIC
> 	depends on RISCV
> 	select IRQ_DOMAIN_HIERARCHY
> 
>+config RISCV_APLIC_MSI
>+	bool
>+	depends on RISCV_APLIC
>+	select GENERIC_MSI_IRQ
>+	default RISCV_APLIC
>+
> config RISCV_IMSIC
> 	bool
> 	depends on RISCV
>diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
>index 7f8289790ed8..47995fdb2c60 100644
>--- a/drivers/irqchip/Makefile
>+++ b/drivers/irqchip/Makefile
>@@ -96,6 +96,7 @@ obj-$(CONFIG_CSKY_MPINTC)		+= irq-csky-mpintc.o
> obj-$(CONFIG_CSKY_APB_INTC)		+= irq-csky-apb-intc.o
> obj-$(CONFIG_RISCV_INTC)		+= irq-riscv-intc.o
> obj-$(CONFIG_RISCV_APLIC)		+= irq-riscv-aplic-main.o irq-riscv-aplic-direct.o
>+obj-$(CONFIG_RISCV_APLIC_MSI)		+= irq-riscv-aplic-msi.o
> obj-$(CONFIG_RISCV_IMSIC)		+= irq-riscv-imsic-state.o irq-riscv-imsic-early.o irq-riscv-imsic-platform.o
> obj-$(CONFIG_SIFIVE_PLIC)		+= irq-sifive-plic.o
> obj-$(CONFIG_IMX_IRQSTEER)		+= irq-imx-irqsteer.o
>diff --git a/drivers/irqchip/irq-riscv-aplic-main.c b/drivers/irqchip/irq-riscv-aplic-main.c
>index 87450708a733..d1b342b66551 100644
>--- a/drivers/irqchip/irq-riscv-aplic-main.c
>+++ b/drivers/irqchip/irq-riscv-aplic-main.c
>@@ -205,7 +205,7 @@ static int aplic_probe(struct platform_device *pdev)
> 		msi_mode = of_property_present(to_of_node(dev->fwnode),
> 						"msi-parent");
> 	if (msi_mode)
>-		rc = -ENODEV;
>+		rc = aplic_msi_setup(dev, regs);
> 	else
> 		rc = aplic_direct_setup(dev, regs);
> 	if (rc) {
>diff --git a/drivers/irqchip/irq-riscv-aplic-main.h b/drivers/irqchip/irq-riscv-aplic-main.h
>index 474a04229334..78267ec58098 100644
>--- a/drivers/irqchip/irq-riscv-aplic-main.h
>+++ b/drivers/irqchip/irq-riscv-aplic-main.h
>@@ -41,5 +41,13 @@ void aplic_init_hw_global(struct aplic_priv *priv, bool msi_mode);
> int aplic_setup_priv(struct aplic_priv *priv, struct device *dev,
> 		     void __iomem *regs);
> int aplic_direct_setup(struct device *dev, void __iomem *regs);
>+#ifdef CONFIG_RISCV_APLIC_MSI
>+int aplic_msi_setup(struct device *dev, void __iomem *regs);
>+#else
>+static inline int aplic_msi_setup(struct device *dev, void __iomem *regs)
>+{
>+	return -ENODEV;
>+}
>+#endif
> 
> #endif
>diff --git a/drivers/irqchip/irq-riscv-aplic-msi.c b/drivers/irqchip/irq-riscv-aplic-msi.c
>new file mode 100644
>index 000000000000..086d00e0429e
>--- /dev/null
>+++ b/drivers/irqchip/irq-riscv-aplic-msi.c
>@@ -0,0 +1,285 @@
>+// SPDX-License-Identifier: GPL-2.0
>+/*
>+ * Copyright (C) 2021 Western Digital Corporation or its affiliates.
>+ * Copyright (C) 2022 Ventana Micro Systems Inc.
>+ */
>+
>+#include <linux/bitops.h>
>+#include <linux/cpu.h>
>+#include <linux/interrupt.h>
>+#include <linux/irqchip.h>
>+#include <linux/irqchip/riscv-aplic.h>
>+#include <linux/irqchip/riscv-imsic.h>
>+#include <linux/module.h>
>+#include <linux/msi.h>
>+#include <linux/of_irq.h>
>+#include <linux/platform_device.h>
>+#include <linux/printk.h>
>+#include <linux/smp.h>
>+
>+#include "irq-riscv-aplic-main.h"
>+
>+static void aplic_msi_irq_unmask(struct irq_data *d)
>+{
>+	aplic_irq_unmask(d);
>+	irq_chip_unmask_parent(d);
>+}
>+
>+static void aplic_msi_irq_mask(struct irq_data *d)
>+{
>+	aplic_irq_mask(d);
>+	irq_chip_mask_parent(d);
>+}
>+
>+static void aplic_msi_irq_eoi(struct irq_data *d)
>+{
>+	struct aplic_priv *priv = irq_data_get_irq_chip_data(d);
>+	u32 reg_off, reg_mask;
>+
>+	/*
>+	 * EOI handling only required only for level-triggered
>+	 * interrupts in APLIC MSI mode.
>+	 */
>+
>+	reg_off = APLIC_CLRIP_BASE + ((d->hwirq / APLIC_IRQBITS_PER_REG) * 4);
>+	reg_mask = BIT(d->hwirq % APLIC_IRQBITS_PER_REG);
>+	switch (irqd_get_trigger_type(d)) {
>+	case IRQ_TYPE_LEVEL_LOW:
>+		if (!(readl(priv->regs + reg_off) & reg_mask))
>+			writel(d->hwirq, priv->regs + APLIC_SETIPNUM_LE);
>+		break;
>+	case IRQ_TYPE_LEVEL_HIGH:
>+		if (readl(priv->regs + reg_off) & reg_mask)
>+			writel(d->hwirq, priv->regs + APLIC_SETIPNUM_LE);
>+		break;
>+	}
>+}
>+
>+static struct irq_chip aplic_msi_chip = {
>+	.name		= "APLIC-MSI",
>+	.irq_mask	= aplic_msi_irq_mask,
>+	.irq_unmask	= aplic_msi_irq_unmask,
>+	.irq_set_type	= aplic_irq_set_type,
>+	.irq_eoi	= aplic_msi_irq_eoi,
>+#ifdef CONFIG_SMP
>+	.irq_set_affinity = irq_chip_set_affinity_parent,
>+#endif
>+	.flags		= IRQCHIP_SET_TYPE_MASKED |
>+			  IRQCHIP_SKIP_SET_WAKE |
>+			  IRQCHIP_MASK_ON_SUSPEND,
>+};
>+
>+static int aplic_msi_irqdomain_translate(struct irq_domain *d,
>+					 struct irq_fwspec *fwspec,
>+					 unsigned long *hwirq,
>+					 unsigned int *type)
>+{
>+	struct aplic_priv *priv = platform_msi_get_host_data(d);
>+
>+	return aplic_irqdomain_translate(fwspec, priv->gsi_base, hwirq, type);
>+}
>+
>+static int aplic_msi_irqdomain_alloc(struct irq_domain *domain,
>+				     unsigned int virq, unsigned int nr_irqs,
>+				     void *arg)
>+{
>+	int i, ret;
>+	unsigned int type;
>+	irq_hw_number_t hwirq;
>+	struct irq_fwspec *fwspec = arg;
>+	struct aplic_priv *priv = platform_msi_get_host_data(domain);
>+
>+	ret = aplic_irqdomain_translate(fwspec, priv->gsi_base, &hwirq, &type);
>+	if (ret)
>+		return ret;

In your patchset, the wired IRQ and IRQ of platform device will go into APLIC-MSI domain firstly.
Let me assume here is a MSI IRQ not wired IRQ on a device, and it is a platform device in system.
so in aplic_irqdomain_translate() function, it will parse the APLIC physical IRQ number by fwspec->param[0],
but this is not a wried IRQ, it is a MSI IRQ, it should not has a APLIC physical IRQ number, the hwirq number should be allocated by MSI bitmap,
what value will be parse by DTS? zero or negative? 

if this is a nonexistent physical IRQ number for APLIC, in aplic_msi_irq_unmask()->aplic_irq_unmask(), how it works?

writel(d->hwirq, priv->regs + APLIC_SETIENUM);
Re: [PATCH v11 12/14] irqchip/riscv-aplic: Add support for MSI-mode
Posted by Anup Patel 2 years, 3 months ago 
On Sat, Nov 4, 2023 at 6:30 AM Ben <figure1802@126.com> wrote:
>
> At 2023-10-24 01:27:58, "Anup Patel" <apatel@ventanamicro.com> wrote:
> >The RISC-V advanced platform-level interrupt controller (APLIC) has
> >two modes of operation: 1) Direct mode and 2) MSI mode.
> >(For more details, refer https://github.com/riscv/riscv-aia)
> >
> >In APLIC MSI-mode, wired interrupts are forwared as message signaled
> >interrupts (MSIs) to CPUs via IMSIC.
> >
> >We extend the existing APLIC irqchip driver to support MSI-mode for
> >RISC-V platforms having both wired interrupts and MSIs.
> >
> >Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> >---
> > drivers/irqchip/Kconfig                |   6 +
> > drivers/irqchip/Makefile               |   1 +
> > drivers/irqchip/irq-riscv-aplic-main.c |   2 +-
> > drivers/irqchip/irq-riscv-aplic-main.h |   8 +
> > drivers/irqchip/irq-riscv-aplic-msi.c  | 285 +++++++++++++++++++++++++
> > 5 files changed, 301 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/irqchip/irq-riscv-aplic-msi.c
> >
> >diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> >index 1996cc6f666a..7adc4dbe07ff 100644
> >--- a/drivers/irqchip/Kconfig
> >+++ b/drivers/irqchip/Kconfig
> >@@ -551,6 +551,12 @@ config RISCV_APLIC
> >       depends on RISCV
> >       select IRQ_DOMAIN_HIERARCHY
> >
> >+config RISCV_APLIC_MSI
> >+      bool
> >+      depends on RISCV_APLIC
> >+      select GENERIC_MSI_IRQ
> >+      default RISCV_APLIC
> >+
> > config RISCV_IMSIC
> >       bool
> >       depends on RISCV
> >diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> >index 7f8289790ed8..47995fdb2c60 100644
> >--- a/drivers/irqchip/Makefile
> >+++ b/drivers/irqchip/Makefile
> >@@ -96,6 +96,7 @@ obj-$(CONFIG_CSKY_MPINTC)            += irq-csky-mpintc.o
> > obj-$(CONFIG_CSKY_APB_INTC)           += irq-csky-apb-intc.o
> > obj-$(CONFIG_RISCV_INTC)              += irq-riscv-intc.o
> > obj-$(CONFIG_RISCV_APLIC)             += irq-riscv-aplic-main.o irq-riscv-aplic-direct.o
> >+obj-$(CONFIG_RISCV_APLIC_MSI)         += irq-riscv-aplic-msi.o
> > obj-$(CONFIG_RISCV_IMSIC)             += irq-riscv-imsic-state.o irq-riscv-imsic-early.o irq-riscv-imsic-platform.o
> > obj-$(CONFIG_SIFIVE_PLIC)             += irq-sifive-plic.o
> > obj-$(CONFIG_IMX_IRQSTEER)            += irq-imx-irqsteer.o
> >diff --git a/drivers/irqchip/irq-riscv-aplic-main.c b/drivers/irqchip/irq-riscv-aplic-main.c
> >index 87450708a733..d1b342b66551 100644
> >--- a/drivers/irqchip/irq-riscv-aplic-main.c
> >+++ b/drivers/irqchip/irq-riscv-aplic-main.c
> >@@ -205,7 +205,7 @@ static int aplic_probe(struct platform_device *pdev)
> >               msi_mode = of_property_present(to_of_node(dev->fwnode),
> >                                               "msi-parent");
> >       if (msi_mode)
> >-              rc = -ENODEV;
> >+              rc = aplic_msi_setup(dev, regs);
> >       else
> >               rc = aplic_direct_setup(dev, regs);
> >       if (rc) {
> >diff --git a/drivers/irqchip/irq-riscv-aplic-main.h b/drivers/irqchip/irq-riscv-aplic-main.h
> >index 474a04229334..78267ec58098 100644
> >--- a/drivers/irqchip/irq-riscv-aplic-main.h
> >+++ b/drivers/irqchip/irq-riscv-aplic-main.h
> >@@ -41,5 +41,13 @@ void aplic_init_hw_global(struct aplic_priv *priv, bool msi_mode);
> > int aplic_setup_priv(struct aplic_priv *priv, struct device *dev,
> >                    void __iomem *regs);
> > int aplic_direct_setup(struct device *dev, void __iomem *regs);
> >+#ifdef CONFIG_RISCV_APLIC_MSI
> >+int aplic_msi_setup(struct device *dev, void __iomem *regs);
> >+#else
> >+static inline int aplic_msi_setup(struct device *dev, void __iomem *regs)
> >+{
> >+      return -ENODEV;
> >+}
> >+#endif
> >
> > #endif
> >diff --git a/drivers/irqchip/irq-riscv-aplic-msi.c b/drivers/irqchip/irq-riscv-aplic-msi.c
> >new file mode 100644
> >index 000000000000..086d00e0429e
> >--- /dev/null
> >+++ b/drivers/irqchip/irq-riscv-aplic-msi.c
> >@@ -0,0 +1,285 @@
> >+// SPDX-License-Identifier: GPL-2.0
> >+/*
> >+ * Copyright (C) 2021 Western Digital Corporation or its affiliates.
> >+ * Copyright (C) 2022 Ventana Micro Systems Inc.
> >+ */
> >+
> >+#include <linux/bitops.h>
> >+#include <linux/cpu.h>
> >+#include <linux/interrupt.h>
> >+#include <linux/irqchip.h>
> >+#include <linux/irqchip/riscv-aplic.h>
> >+#include <linux/irqchip/riscv-imsic.h>
> >+#include <linux/module.h>
> >+#include <linux/msi.h>
> >+#include <linux/of_irq.h>
> >+#include <linux/platform_device.h>
> >+#include <linux/printk.h>
> >+#include <linux/smp.h>
> >+
> >+#include "irq-riscv-aplic-main.h"
> >+
> >+static void aplic_msi_irq_unmask(struct irq_data *d)
> >+{
> >+      aplic_irq_unmask(d);
> >+      irq_chip_unmask_parent(d);
> >+}
> >+
> >+static void aplic_msi_irq_mask(struct irq_data *d)
> >+{
> >+      aplic_irq_mask(d);
> >+      irq_chip_mask_parent(d);
> >+}
> >+
> >+static void aplic_msi_irq_eoi(struct irq_data *d)
> >+{
> >+      struct aplic_priv *priv = irq_data_get_irq_chip_data(d);
> >+      u32 reg_off, reg_mask;
> >+
> >+      /*
> >+       * EOI handling only required only for level-triggered
> >+       * interrupts in APLIC MSI mode.
> >+       */
> >+
> >+      reg_off = APLIC_CLRIP_BASE + ((d->hwirq / APLIC_IRQBITS_PER_REG) * 4);
> >+      reg_mask = BIT(d->hwirq % APLIC_IRQBITS_PER_REG);
> >+      switch (irqd_get_trigger_type(d)) {
> >+      case IRQ_TYPE_LEVEL_LOW:
> >+              if (!(readl(priv->regs + reg_off) & reg_mask))
> >+                      writel(d->hwirq, priv->regs + APLIC_SETIPNUM_LE);
> >+              break;
> >+      case IRQ_TYPE_LEVEL_HIGH:
> >+              if (readl(priv->regs + reg_off) & reg_mask)
> >+                      writel(d->hwirq, priv->regs + APLIC_SETIPNUM_LE);
> >+              break;
> >+      }
> >+}
> >+
> >+static struct irq_chip aplic_msi_chip = {
> >+      .name           = "APLIC-MSI",
> >+      .irq_mask       = aplic_msi_irq_mask,
> >+      .irq_unmask     = aplic_msi_irq_unmask,
> >+      .irq_set_type   = aplic_irq_set_type,
> >+      .irq_eoi        = aplic_msi_irq_eoi,
> >+#ifdef CONFIG_SMP
> >+      .irq_set_affinity = irq_chip_set_affinity_parent,
> >+#endif
> >+      .flags          = IRQCHIP_SET_TYPE_MASKED |
> >+                        IRQCHIP_SKIP_SET_WAKE |
> >+                        IRQCHIP_MASK_ON_SUSPEND,
> >+};
> >+
> >+static int aplic_msi_irqdomain_translate(struct irq_domain *d,
> >+                                       struct irq_fwspec *fwspec,
> >+                                       unsigned long *hwirq,
> >+                                       unsigned int *type)
> >+{
> >+      struct aplic_priv *priv = platform_msi_get_host_data(d);
> >+
> >+      return aplic_irqdomain_translate(fwspec, priv->gsi_base, hwirq, type);
> >+}
> >+
> >+static int aplic_msi_irqdomain_alloc(struct irq_domain *domain,
> >+                                   unsigned int virq, unsigned int nr_irqs,
> >+                                   void *arg)
> >+{
> >+      int i, ret;
> >+      unsigned int type;
> >+      irq_hw_number_t hwirq;
> >+      struct irq_fwspec *fwspec = arg;
> >+      struct aplic_priv *priv = platform_msi_get_host_data(domain);
> >+
> >+      ret = aplic_irqdomain_translate(fwspec, priv->gsi_base, &hwirq, &type);
> >+      if (ret)
> >+              return ret;
>
> In your patchset, the wired IRQ and IRQ of platform device will go into APLIC-MSI domain firstly.

Yes, that is correct. In general, this applies to AIA specification
and nothing to do with this patchset.

> Let me assume here is a MSI IRQ not wired IRQ on a device, and it is a platform device in system.
> so in aplic_irqdomain_translate() function, it will parse the APLIC physical IRQ number by fwspec->param[0],
> but this is not a wried IRQ, it is a MSI IRQ, it should not has a APLIC physical IRQ number, the hwirq number should be allocated by MSI bitmap,
> what value will be parse by DTS? zero or negative?

For platform devices with MSI support, the MSIs will directly target
the per-HART
IMSICs and the DT node of such devices will never point to APLIC as the parent
MSI controller.

The IMSIC driver implements the IMSIC-PLAT domain for platform MSIs.

>
> if this is a nonexistent physical IRQ number for APLIC, in aplic_msi_irq_unmask()->aplic_irq_unmask(), how it works?
>
> writel(d->hwirq, priv->regs + APLIC_SETIENUM);
>
>

The platform wired IRQs and platform MSIs are not handled/described
in the same way. The APLIC has NO ROLE in platform MSI handling.

Regards,
Anup
Re:Re: [PATCH v11 12/14] irqchip/riscv-aplic: Add support for MSI-mode
Posted by Ben 2 years, 3 months ago 
At 2023-11-08 22:43:25, "Anup Patel" <apatel@ventanamicro.com> wrote:
>On Sat, Nov 4, 2023 at 6:30 AM Ben <figure1802@126.com> wrote:
>>
>> At 2023-10-24 01:27:58, "Anup Patel" <apatel@ventanamicro.com> wrote:
>> >The RISC-V advanced platform-level interrupt controller (APLIC) has
>> >two modes of operation: 1) Direct mode and 2) MSI mode.
>> >(For more details, refer https://github.com/riscv/riscv-aia)
>> >
>> >In APLIC MSI-mode, wired interrupts are forwared as message signaled
>> >interrupts (MSIs) to CPUs via IMSIC.
>> >
>> >We extend the existing APLIC irqchip driver to support MSI-mode for
>> >RISC-V platforms having both wired interrupts and MSIs.
>> >
>> >Signed-off-by: Anup Patel <apatel@ventanamicro.com>
>> >---
>> > drivers/irqchip/Kconfig                |   6 +
>> > drivers/irqchip/Makefile               |   1 +
>> > drivers/irqchip/irq-riscv-aplic-main.c |   2 +-
>> > drivers/irqchip/irq-riscv-aplic-main.h |   8 +
>> > drivers/irqchip/irq-riscv-aplic-msi.c  | 285 +++++++++++++++++++++++++
>> > 5 files changed, 301 insertions(+), 1 deletion(-)
>> > create mode 100644 drivers/irqchip/irq-riscv-aplic-msi.c
>> >
>> >diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
>> >index 1996cc6f666a..7adc4dbe07ff 100644
>> >--- a/drivers/irqchip/Kconfig
>> >+++ b/drivers/irqchip/Kconfig
>> >@@ -551,6 +551,12 @@ config RISCV_APLIC
>> >       depends on RISCV
>> >       select IRQ_DOMAIN_HIERARCHY
>> >
>> >+config RISCV_APLIC_MSI
>> >+      bool
>> >+      depends on RISCV_APLIC
>> >+      select GENERIC_MSI_IRQ
>> >+      default RISCV_APLIC
>> >+
>> > config RISCV_IMSIC
>> >       bool
>> >       depends on RISCV
>> >diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
>> >index 7f8289790ed8..47995fdb2c60 100644
>> >--- a/drivers/irqchip/Makefile
>> >+++ b/drivers/irqchip/Makefile
>> >@@ -96,6 +96,7 @@ obj-$(CONFIG_CSKY_MPINTC)            += irq-csky-mpintc.o
>> > obj-$(CONFIG_CSKY_APB_INTC)           += irq-csky-apb-intc.o
>> > obj-$(CONFIG_RISCV_INTC)              += irq-riscv-intc.o
>> > obj-$(CONFIG_RISCV_APLIC)             += irq-riscv-aplic-main.o irq-riscv-aplic-direct.o
>> >+obj-$(CONFIG_RISCV_APLIC_MSI)         += irq-riscv-aplic-msi.o
>> > obj-$(CONFIG_RISCV_IMSIC)             += irq-riscv-imsic-state.o irq-riscv-imsic-early.o irq-riscv-imsic-platform.o
>> > obj-$(CONFIG_SIFIVE_PLIC)             += irq-sifive-plic.o
>> > obj-$(CONFIG_IMX_IRQSTEER)            += irq-imx-irqsteer.o
>> >diff --git a/drivers/irqchip/irq-riscv-aplic-main.c b/drivers/irqchip/irq-riscv-aplic-main.c
>> >index 87450708a733..d1b342b66551 100644
>> >--- a/drivers/irqchip/irq-riscv-aplic-main.c
>> >+++ b/drivers/irqchip/irq-riscv-aplic-main.c
>> >@@ -205,7 +205,7 @@ static int aplic_probe(struct platform_device *pdev)
>> >               msi_mode = of_property_present(to_of_node(dev->fwnode),
>> >                                               "msi-parent");
>> >       if (msi_mode)
>> >-              rc = -ENODEV;
>> >+              rc = aplic_msi_setup(dev, regs);
>> >       else
>> >               rc = aplic_direct_setup(dev, regs);
>> >       if (rc) {
>> >diff --git a/drivers/irqchip/irq-riscv-aplic-main.h b/drivers/irqchip/irq-riscv-aplic-main.h
>> >index 474a04229334..78267ec58098 100644
>> >--- a/drivers/irqchip/irq-riscv-aplic-main.h
>> >+++ b/drivers/irqchip/irq-riscv-aplic-main.h
>> >@@ -41,5 +41,13 @@ void aplic_init_hw_global(struct aplic_priv *priv, bool msi_mode);
>> > int aplic_setup_priv(struct aplic_priv *priv, struct device *dev,
>> >                    void __iomem *regs);
>> > int aplic_direct_setup(struct device *dev, void __iomem *regs);
>> >+#ifdef CONFIG_RISCV_APLIC_MSI
>> >+int aplic_msi_setup(struct device *dev, void __iomem *regs);
>> >+#else
>> >+static inline int aplic_msi_setup(struct device *dev, void __iomem *regs)
>> >+{
>> >+      return -ENODEV;
>> >+}
>> >+#endif
>> >
>> > #endif
>> >diff --git a/drivers/irqchip/irq-riscv-aplic-msi.c b/drivers/irqchip/irq-riscv-aplic-msi.c
>> >new file mode 100644
>> >index 000000000000..086d00e0429e
>> >--- /dev/null
>> >+++ b/drivers/irqchip/irq-riscv-aplic-msi.c
>> >@@ -0,0 +1,285 @@
>> >+// SPDX-License-Identifier: GPL-2.0
>> >+/*
>> >+ * Copyright (C) 2021 Western Digital Corporation or its affiliates.
>> >+ * Copyright (C) 2022 Ventana Micro Systems Inc.
>> >+ */
>> >+
>> >+#include <linux/bitops.h>
>> >+#include <linux/cpu.h>
>> >+#include <linux/interrupt.h>
>> >+#include <linux/irqchip.h>
>> >+#include <linux/irqchip/riscv-aplic.h>
>> >+#include <linux/irqchip/riscv-imsic.h>
>> >+#include <linux/module.h>
>> >+#include <linux/msi.h>
>> >+#include <linux/of_irq.h>
>> >+#include <linux/platform_device.h>
>> >+#include <linux/printk.h>
>> >+#include <linux/smp.h>
>> >+
>> >+#include "irq-riscv-aplic-main.h"
>> >+
>> >+static void aplic_msi_irq_unmask(struct irq_data *d)
>> >+{
>> >+      aplic_irq_unmask(d);
>> >+      irq_chip_unmask_parent(d);
>> >+}
>> >+
>> >+static void aplic_msi_irq_mask(struct irq_data *d)
>> >+{
>> >+      aplic_irq_mask(d);
>> >+      irq_chip_mask_parent(d);
>> >+}
>> >+
>> >+static void aplic_msi_irq_eoi(struct irq_data *d)
>> >+{
>> >+      struct aplic_priv *priv = irq_data_get_irq_chip_data(d);
>> >+      u32 reg_off, reg_mask;
>> >+
>> >+      /*
>> >+       * EOI handling only required only for level-triggered
>> >+       * interrupts in APLIC MSI mode.
>> >+       */
>> >+
>> >+      reg_off = APLIC_CLRIP_BASE + ((d->hwirq / APLIC_IRQBITS_PER_REG) * 4);
>> >+      reg_mask = BIT(d->hwirq % APLIC_IRQBITS_PER_REG);
>> >+      switch (irqd_get_trigger_type(d)) {
>> >+      case IRQ_TYPE_LEVEL_LOW:
>> >+              if (!(readl(priv->regs + reg_off) & reg_mask))
>> >+                      writel(d->hwirq, priv->regs + APLIC_SETIPNUM_LE);
>> >+              break;
>> >+      case IRQ_TYPE_LEVEL_HIGH:
>> >+              if (readl(priv->regs + reg_off) & reg_mask)
>> >+                      writel(d->hwirq, priv->regs + APLIC_SETIPNUM_LE);
>> >+              break;
>> >+      }
>> >+}
>> >+
>> >+static struct irq_chip aplic_msi_chip = {
>> >+      .name           = "APLIC-MSI",
>> >+      .irq_mask       = aplic_msi_irq_mask,
>> >+      .irq_unmask     = aplic_msi_irq_unmask,
>> >+      .irq_set_type   = aplic_irq_set_type,
>> >+      .irq_eoi        = aplic_msi_irq_eoi,
>> >+#ifdef CONFIG_SMP
>> >+      .irq_set_affinity = irq_chip_set_affinity_parent,
>> >+#endif
>> >+      .flags          = IRQCHIP_SET_TYPE_MASKED |
>> >+                        IRQCHIP_SKIP_SET_WAKE |
>> >+                        IRQCHIP_MASK_ON_SUSPEND,
>> >+};
>> >+
>> >+static int aplic_msi_irqdomain_translate(struct irq_domain *d,
>> >+                                       struct irq_fwspec *fwspec,
>> >+                                       unsigned long *hwirq,
>> >+                                       unsigned int *type)
>> >+{
>> >+      struct aplic_priv *priv = platform_msi_get_host_data(d);
>> >+
>> >+      return aplic_irqdomain_translate(fwspec, priv->gsi_base, hwirq, type);
>> >+}
>> >+
>> >+static int aplic_msi_irqdomain_alloc(struct irq_domain *domain,
>> >+                                   unsigned int virq, unsigned int nr_irqs,
>> >+                                   void *arg)
>> >+{
>> >+      int i, ret;
>> >+      unsigned int type;
>> >+      irq_hw_number_t hwirq;
>> >+      struct irq_fwspec *fwspec = arg;
>> >+      struct aplic_priv *priv = platform_msi_get_host_data(domain);
>> >+
>> >+      ret = aplic_irqdomain_translate(fwspec, priv->gsi_base, &hwirq, &type);
>> >+      if (ret)
>> >+              return ret;
>>
>> In your patchset, the wired IRQ and IRQ of platform device will go into APLIC-MSI domain firstly.
>
>Yes, that is correct. In general, this applies to AIA specification
>and nothing to do with this patchset.
>
>> Let me assume here is a MSI IRQ not wired IRQ on a device, and it is a platform device in system.
>> so in aplic_irqdomain_translate() function, it will parse the APLIC physical IRQ number by fwspec->param[0],
>> but this is not a wried IRQ, it is a MSI IRQ, it should not has a APLIC physical IRQ number, the hwirq number should be allocated by MSI bitmap,
>> what value will be parse by DTS? zero or negative?
>
>For platform devices with MSI support, the MSIs will directly target
>the per-HART
>IMSICs and the DT node of such devices will never point to APLIC as the parent
>MSI controller.
>

>The IMSIC driver implements the IMSIC-PLAT domain for platform MSIs.

Have you test this case on QEMU? would you like share the test steps?
Re: Re: [PATCH v11 12/14] irqchip/riscv-aplic: Add support for MSI-mode
Posted by Anup Patel 2 years, 3 months ago 
On Wed, Nov 8, 2023 at 8:23 PM Ben <figure1802@126.com> wrote:
>
> At 2023-11-08 22:43:25, "Anup Patel" <apatel@ventanamicro.com> wrote:
> >On Sat, Nov 4, 2023 at 6:30 AM Ben <figure1802@126.com> wrote:
> >>
> >> At 2023-10-24 01:27:58, "Anup Patel" <apatel@ventanamicro.com> wrote:
> >> >The RISC-V advanced platform-level interrupt controller (APLIC) has
> >> >two modes of operation: 1) Direct mode and 2) MSI mode.
> >> >(For more details, refer https://github.com/riscv/riscv-aia)
> >> >
> >> >In APLIC MSI-mode, wired interrupts are forwared as message signaled
> >> >interrupts (MSIs) to CPUs via IMSIC.
> >> >
> >> >We extend the existing APLIC irqchip driver to support MSI-mode for
> >> >RISC-V platforms having both wired interrupts and MSIs.
> >> >
> >> >Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> >> >---
> >> > drivers/irqchip/Kconfig                |   6 +
> >> > drivers/irqchip/Makefile               |   1 +
> >> > drivers/irqchip/irq-riscv-aplic-main.c |   2 +-
> >> > drivers/irqchip/irq-riscv-aplic-main.h |   8 +
> >> > drivers/irqchip/irq-riscv-aplic-msi.c  | 285 +++++++++++++++++++++++++
> >> > 5 files changed, 301 insertions(+), 1 deletion(-)
> >> > create mode 100644 drivers/irqchip/irq-riscv-aplic-msi.c
> >> >
> >> >diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> >> >index 1996cc6f666a..7adc4dbe07ff 100644
> >> >--- a/drivers/irqchip/Kconfig
> >> >+++ b/drivers/irqchip/Kconfig
> >> >@@ -551,6 +551,12 @@ config RISCV_APLIC
> >> >       depends on RISCV
> >> >       select IRQ_DOMAIN_HIERARCHY
> >> >
> >> >+config RISCV_APLIC_MSI
> >> >+      bool
> >> >+      depends on RISCV_APLIC
> >> >+      select GENERIC_MSI_IRQ
> >> >+      default RISCV_APLIC
> >> >+
> >> > config RISCV_IMSIC
> >> >       bool
> >> >       depends on RISCV
> >> >diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> >> >index 7f8289790ed8..47995fdb2c60 100644
> >> >--- a/drivers/irqchip/Makefile
> >> >+++ b/drivers/irqchip/Makefile
> >> >@@ -96,6 +96,7 @@ obj-$(CONFIG_CSKY_MPINTC)            += irq-csky-mpintc.o
> >> > obj-$(CONFIG_CSKY_APB_INTC)           += irq-csky-apb-intc.o
> >> > obj-$(CONFIG_RISCV_INTC)              += irq-riscv-intc.o
> >> > obj-$(CONFIG_RISCV_APLIC)             += irq-riscv-aplic-main.o irq-riscv-aplic-direct.o
> >> >+obj-$(CONFIG_RISCV_APLIC_MSI)         += irq-riscv-aplic-msi.o
> >> > obj-$(CONFIG_RISCV_IMSIC)             += irq-riscv-imsic-state.o irq-riscv-imsic-early.o irq-riscv-imsic-platform.o
> >> > obj-$(CONFIG_SIFIVE_PLIC)             += irq-sifive-plic.o
> >> > obj-$(CONFIG_IMX_IRQSTEER)            += irq-imx-irqsteer.o
> >> >diff --git a/drivers/irqchip/irq-riscv-aplic-main.c b/drivers/irqchip/irq-riscv-aplic-main.c
> >> >index 87450708a733..d1b342b66551 100644
> >> >--- a/drivers/irqchip/irq-riscv-aplic-main.c
> >> >+++ b/drivers/irqchip/irq-riscv-aplic-main.c
> >> >@@ -205,7 +205,7 @@ static int aplic_probe(struct platform_device *pdev)
> >> >               msi_mode = of_property_present(to_of_node(dev->fwnode),
> >> >                                               "msi-parent");
> >> >       if (msi_mode)
> >> >-              rc = -ENODEV;
> >> >+              rc = aplic_msi_setup(dev, regs);
> >> >       else
> >> >               rc = aplic_direct_setup(dev, regs);
> >> >       if (rc) {
> >> >diff --git a/drivers/irqchip/irq-riscv-aplic-main.h b/drivers/irqchip/irq-riscv-aplic-main.h
> >> >index 474a04229334..78267ec58098 100644
> >> >--- a/drivers/irqchip/irq-riscv-aplic-main.h
> >> >+++ b/drivers/irqchip/irq-riscv-aplic-main.h
> >> >@@ -41,5 +41,13 @@ void aplic_init_hw_global(struct aplic_priv *priv, bool msi_mode);
> >> > int aplic_setup_priv(struct aplic_priv *priv, struct device *dev,
> >> >                    void __iomem *regs);
> >> > int aplic_direct_setup(struct device *dev, void __iomem *regs);
> >> >+#ifdef CONFIG_RISCV_APLIC_MSI
> >> >+int aplic_msi_setup(struct device *dev, void __iomem *regs);
> >> >+#else
> >> >+static inline int aplic_msi_setup(struct device *dev, void __iomem *regs)
> >> >+{
> >> >+      return -ENODEV;
> >> >+}
> >> >+#endif
> >> >
> >> > #endif
> >> >diff --git a/drivers/irqchip/irq-riscv-aplic-msi.c b/drivers/irqchip/irq-riscv-aplic-msi.c
> >> >new file mode 100644
> >> >index 000000000000..086d00e0429e
> >> >--- /dev/null
> >> >+++ b/drivers/irqchip/irq-riscv-aplic-msi.c
> >> >@@ -0,0 +1,285 @@
> >> >+// SPDX-License-Identifier: GPL-2.0
> >> >+/*
> >> >+ * Copyright (C) 2021 Western Digital Corporation or its affiliates.
> >> >+ * Copyright (C) 2022 Ventana Micro Systems Inc.
> >> >+ */
> >> >+
> >> >+#include <linux/bitops.h>
> >> >+#include <linux/cpu.h>
> >> >+#include <linux/interrupt.h>
> >> >+#include <linux/irqchip.h>
> >> >+#include <linux/irqchip/riscv-aplic.h>
> >> >+#include <linux/irqchip/riscv-imsic.h>
> >> >+#include <linux/module.h>
> >> >+#include <linux/msi.h>
> >> >+#include <linux/of_irq.h>
> >> >+#include <linux/platform_device.h>
> >> >+#include <linux/printk.h>
> >> >+#include <linux/smp.h>
> >> >+
> >> >+#include "irq-riscv-aplic-main.h"
> >> >+
> >> >+static void aplic_msi_irq_unmask(struct irq_data *d)
> >> >+{
> >> >+      aplic_irq_unmask(d);
> >> >+      irq_chip_unmask_parent(d);
> >> >+}
> >> >+
> >> >+static void aplic_msi_irq_mask(struct irq_data *d)
> >> >+{
> >> >+      aplic_irq_mask(d);
> >> >+      irq_chip_mask_parent(d);
> >> >+}
> >> >+
> >> >+static void aplic_msi_irq_eoi(struct irq_data *d)
> >> >+{
> >> >+      struct aplic_priv *priv = irq_data_get_irq_chip_data(d);
> >> >+      u32 reg_off, reg_mask;
> >> >+
> >> >+      /*
> >> >+       * EOI handling only required only for level-triggered
> >> >+       * interrupts in APLIC MSI mode.
> >> >+       */
> >> >+
> >> >+      reg_off = APLIC_CLRIP_BASE + ((d->hwirq / APLIC_IRQBITS_PER_REG) * 4);
> >> >+      reg_mask = BIT(d->hwirq % APLIC_IRQBITS_PER_REG);
> >> >+      switch (irqd_get_trigger_type(d)) {
> >> >+      case IRQ_TYPE_LEVEL_LOW:
> >> >+              if (!(readl(priv->regs + reg_off) & reg_mask))
> >> >+                      writel(d->hwirq, priv->regs + APLIC_SETIPNUM_LE);
> >> >+              break;
> >> >+      case IRQ_TYPE_LEVEL_HIGH:
> >> >+              if (readl(priv->regs + reg_off) & reg_mask)
> >> >+                      writel(d->hwirq, priv->regs + APLIC_SETIPNUM_LE);
> >> >+              break;
> >> >+      }
> >> >+}
> >> >+
> >> >+static struct irq_chip aplic_msi_chip = {
> >> >+      .name           = "APLIC-MSI",
> >> >+      .irq_mask       = aplic_msi_irq_mask,
> >> >+      .irq_unmask     = aplic_msi_irq_unmask,
> >> >+      .irq_set_type   = aplic_irq_set_type,
> >> >+      .irq_eoi        = aplic_msi_irq_eoi,
> >> >+#ifdef CONFIG_SMP
> >> >+      .irq_set_affinity = irq_chip_set_affinity_parent,
> >> >+#endif
> >> >+      .flags          = IRQCHIP_SET_TYPE_MASKED |
> >> >+                        IRQCHIP_SKIP_SET_WAKE |
> >> >+                        IRQCHIP_MASK_ON_SUSPEND,
> >> >+};
> >> >+
> >> >+static int aplic_msi_irqdomain_translate(struct irq_domain *d,
> >> >+                                       struct irq_fwspec *fwspec,
> >> >+                                       unsigned long *hwirq,
> >> >+                                       unsigned int *type)
> >> >+{
> >> >+      struct aplic_priv *priv = platform_msi_get_host_data(d);
> >> >+
> >> >+      return aplic_irqdomain_translate(fwspec, priv->gsi_base, hwirq, type);
> >> >+}
> >> >+
> >> >+static int aplic_msi_irqdomain_alloc(struct irq_domain *domain,
> >> >+                                   unsigned int virq, unsigned int nr_irqs,
> >> >+                                   void *arg)
> >> >+{
> >> >+      int i, ret;
> >> >+      unsigned int type;
> >> >+      irq_hw_number_t hwirq;
> >> >+      struct irq_fwspec *fwspec = arg;
> >> >+      struct aplic_priv *priv = platform_msi_get_host_data(domain);
> >> >+
> >> >+      ret = aplic_irqdomain_translate(fwspec, priv->gsi_base, &hwirq, &type);
> >> >+      if (ret)
> >> >+              return ret;
> >>
> >> In your patchset, the wired IRQ and IRQ of platform device will go into APLIC-MSI domain firstly.
> >
> >Yes, that is correct. In general, this applies to AIA specification
> >and nothing to do with this patchset.
> >
> >> Let me assume here is a MSI IRQ not wired IRQ on a device, and it is a platform device in system.
> >> so in aplic_irqdomain_translate() function, it will parse the APLIC physical IRQ number by fwspec->param[0],
> >> but this is not a wried IRQ, it is a MSI IRQ, it should not has a APLIC physical IRQ number, the hwirq number should be allocated by MSI bitmap,
> >> what value will be parse by DTS? zero or negative?
> >
> >For platform devices with MSI support, the MSIs will directly target
> >the per-HART
> >IMSICs and the DT node of such devices will never point to APLIC as the parent
> >MSI controller.
> >
>
> >The IMSIC driver implements the IMSIC-PLAT domain for platform MSIs.
>
> Have you test this case on QEMU? would you like share the test steps?

The APLIC in MSI-mode acts like a platform device with MSIs so yes this
is tested on QEMU.

Regards,
Anup
Re:Re: Re: [PATCH v11 12/14] irqchip/riscv-aplic: Add support for MSI-mode
Posted by Ben 2 years, 3 months ago 
At 2023-11-08 22:56:59, "Anup Patel" <apatel@ventanamicro.com> wrote:
>On Wed, Nov 8, 2023 at 8:23 PM Ben <figure1802@126.com> wrote:
>>
>> At 2023-11-08 22:43:25, "Anup Patel" <apatel@ventanamicro.com> wrote:
>> >On Sat, Nov 4, 2023 at 6:30 AM Ben <figure1802@126.com> wrote:
>> >>
>> >> At 2023-10-24 01:27:58, "Anup Patel" <apatel@ventanamicro.com> wrote:
>> >> >The RISC-V advanced platform-level interrupt controller (APLIC) has
>> >> >two modes of operation: 1) Direct mode and 2) MSI mode.
>> >> >(For more details, refer https://github.com/riscv/riscv-aia)
>> >> >
>> >> >In APLIC MSI-mode, wired interrupts are forwared as message signaled
>> >> >interrupts (MSIs) to CPUs via IMSIC.
>> >> >
>> >> >We extend the existing APLIC irqchip driver to support MSI-mode for
>> >> >RISC-V platforms having both wired interrupts and MSIs.
>> >> >
>> >> >Signed-off-by: Anup Patel <apatel@ventanamicro.com>
>> >> >---
>> >> > drivers/irqchip/Kconfig                |   6 +
>> >> > drivers/irqchip/Makefile               |   1 +
>> >> > drivers/irqchip/irq-riscv-aplic-main.c |   2 +-
>> >> > drivers/irqchip/irq-riscv-aplic-main.h |   8 +
>> >> > drivers/irqchip/irq-riscv-aplic-msi.c  | 285 +++++++++++++++++++++++++
>> >> > 5 files changed, 301 insertions(+), 1 deletion(-)
>> >> > create mode 100644 drivers/irqchip/irq-riscv-aplic-msi.c
>> >> >
>> >> >diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
>> >> >index 1996cc6f666a..7adc4dbe07ff 100644
>> >> >--- a/drivers/irqchip/Kconfig
>> >> >+++ b/drivers/irqchip/Kconfig
>> >> >@@ -551,6 +551,12 @@ config RISCV_APLIC
>> >> >       depends on RISCV
>> >> >       select IRQ_DOMAIN_HIERARCHY
>> >> >
>> >> >+config RISCV_APLIC_MSI
>> >> >+      bool
>> >> >+      depends on RISCV_APLIC
>> >> >+      select GENERIC_MSI_IRQ
>> >> >+      default RISCV_APLIC
>> >> >+
>> >> > config RISCV_IMSIC
>> >> >       bool
>> >> >       depends on RISCV
>> >> >diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
>> >> >index 7f8289790ed8..47995fdb2c60 100644
>> >> >--- a/drivers/irqchip/Makefile
>> >> >+++ b/drivers/irqchip/Makefile
>> >> >@@ -96,6 +96,7 @@ obj-$(CONFIG_CSKY_MPINTC)            += irq-csky-mpintc.o
>> >> > obj-$(CONFIG_CSKY_APB_INTC)           += irq-csky-apb-intc.o
>> >> > obj-$(CONFIG_RISCV_INTC)              += irq-riscv-intc.o
>> >> > obj-$(CONFIG_RISCV_APLIC)             += irq-riscv-aplic-main.o irq-riscv-aplic-direct.o
>> >> >+obj-$(CONFIG_RISCV_APLIC_MSI)         += irq-riscv-aplic-msi.o
>> >> > obj-$(CONFIG_RISCV_IMSIC)             += irq-riscv-imsic-state.o irq-riscv-imsic-early.o irq-riscv-imsic-platform.o
>> >> > obj-$(CONFIG_SIFIVE_PLIC)             += irq-sifive-plic.o
>> >> > obj-$(CONFIG_IMX_IRQSTEER)            += irq-imx-irqsteer.o
>> >> >diff --git a/drivers/irqchip/irq-riscv-aplic-main.c b/drivers/irqchip/irq-riscv-aplic-main.c
>> >> >index 87450708a733..d1b342b66551 100644
>> >> >--- a/drivers/irqchip/irq-riscv-aplic-main.c
>> >> >+++ b/drivers/irqchip/irq-riscv-aplic-main.c
>> >> >@@ -205,7 +205,7 @@ static int aplic_probe(struct platform_device *pdev)
>> >> >               msi_mode = of_property_present(to_of_node(dev->fwnode),
>> >> >                                               "msi-parent");
>> >> >       if (msi_mode)
>> >> >-              rc = -ENODEV;
>> >> >+              rc = aplic_msi_setup(dev, regs);
>> >> >       else
>> >> >               rc = aplic_direct_setup(dev, regs);
>> >> >       if (rc) {
>> >> >diff --git a/drivers/irqchip/irq-riscv-aplic-main.h b/drivers/irqchip/irq-riscv-aplic-main.h
>> >> >index 474a04229334..78267ec58098 100644
>> >> >--- a/drivers/irqchip/irq-riscv-aplic-main.h
>> >> >+++ b/drivers/irqchip/irq-riscv-aplic-main.h
>> >> >@@ -41,5 +41,13 @@ void aplic_init_hw_global(struct aplic_priv *priv, bool msi_mode);
>> >> > int aplic_setup_priv(struct aplic_priv *priv, struct device *dev,
>> >> >                    void __iomem *regs);
>> >> > int aplic_direct_setup(struct device *dev, void __iomem *regs);
>> >> >+#ifdef CONFIG_RISCV_APLIC_MSI
>> >> >+int aplic_msi_setup(struct device *dev, void __iomem *regs);
>> >> >+#else
>> >> >+static inline int aplic_msi_setup(struct device *dev, void __iomem *regs)
>> >> >+{
>> >> >+      return -ENODEV;
>> >> >+}
>> >> >+#endif
>> >> >
>> >> > #endif
>> >> >diff --git a/drivers/irqchip/irq-riscv-aplic-msi.c b/drivers/irqchip/irq-riscv-aplic-msi.c
>> >> >new file mode 100644
>> >> >index 000000000000..086d00e0429e
>> >> >--- /dev/null
>> >> >+++ b/drivers/irqchip/irq-riscv-aplic-msi.c
>> >> >@@ -0,0 +1,285 @@
>> >> >+// SPDX-License-Identifier: GPL-2.0
>> >> >+/*
>> >> >+ * Copyright (C) 2021 Western Digital Corporation or its affiliates.
>> >> >+ * Copyright (C) 2022 Ventana Micro Systems Inc.
>> >> >+ */
>> >> >+
>> >> >+#include <linux/bitops.h>
>> >> >+#include <linux/cpu.h>
>> >> >+#include <linux/interrupt.h>
>> >> >+#include <linux/irqchip.h>
>> >> >+#include <linux/irqchip/riscv-aplic.h>
>> >> >+#include <linux/irqchip/riscv-imsic.h>
>> >> >+#include <linux/module.h>
>> >> >+#include <linux/msi.h>
>> >> >+#include <linux/of_irq.h>
>> >> >+#include <linux/platform_device.h>
>> >> >+#include <linux/printk.h>
>> >> >+#include <linux/smp.h>
>> >> >+
>> >> >+#include "irq-riscv-aplic-main.h"
>> >> >+
>> >> >+static void aplic_msi_irq_unmask(struct irq_data *d)
>> >> >+{
>> >> >+      aplic_irq_unmask(d);
>> >> >+      irq_chip_unmask_parent(d);
>> >> >+}
>> >> >+
>> >> >+static void aplic_msi_irq_mask(struct irq_data *d)
>> >> >+{
>> >> >+      aplic_irq_mask(d);
>> >> >+      irq_chip_mask_parent(d);
>> >> >+}
>> >> >+
>> >> >+static void aplic_msi_irq_eoi(struct irq_data *d)
>> >> >+{
>> >> >+      struct aplic_priv *priv = irq_data_get_irq_chip_data(d);
>> >> >+      u32 reg_off, reg_mask;
>> >> >+
>> >> >+      /*
>> >> >+       * EOI handling only required only for level-triggered
>> >> >+       * interrupts in APLIC MSI mode.
>> >> >+       */
>> >> >+
>> >> >+      reg_off = APLIC_CLRIP_BASE + ((d->hwirq / APLIC_IRQBITS_PER_REG) * 4);
>> >> >+      reg_mask = BIT(d->hwirq % APLIC_IRQBITS_PER_REG);
>> >> >+      switch (irqd_get_trigger_type(d)) {
>> >> >+      case IRQ_TYPE_LEVEL_LOW:
>> >> >+              if (!(readl(priv->regs + reg_off) & reg_mask))
>> >> >+                      writel(d->hwirq, priv->regs + APLIC_SETIPNUM_LE);
>> >> >+              break;
>> >> >+      case IRQ_TYPE_LEVEL_HIGH:
>> >> >+              if (readl(priv->regs + reg_off) & reg_mask)
>> >> >+                      writel(d->hwirq, priv->regs + APLIC_SETIPNUM_LE);
>> >> >+              break;
>> >> >+      }
>> >> >+}
>> >> >+
>> >> >+static struct irq_chip aplic_msi_chip = {
>> >> >+      .name           = "APLIC-MSI",
>> >> >+      .irq_mask       = aplic_msi_irq_mask,
>> >> >+      .irq_unmask     = aplic_msi_irq_unmask,
>> >> >+      .irq_set_type   = aplic_irq_set_type,
>> >> >+      .irq_eoi        = aplic_msi_irq_eoi,
>> >> >+#ifdef CONFIG_SMP
>> >> >+      .irq_set_affinity = irq_chip_set_affinity_parent,
>> >> >+#endif
>> >> >+      .flags          = IRQCHIP_SET_TYPE_MASKED |
>> >> >+                        IRQCHIP_SKIP_SET_WAKE |
>> >> >+                        IRQCHIP_MASK_ON_SUSPEND,
>> >> >+};
>> >> >+
>> >> >+static int aplic_msi_irqdomain_translate(struct irq_domain *d,
>> >> >+                                       struct irq_fwspec *fwspec,
>> >> >+                                       unsigned long *hwirq,
>> >> >+                                       unsigned int *type)
>> >> >+{
>> >> >+      struct aplic_priv *priv = platform_msi_get_host_data(d);
>> >> >+
>> >> >+      return aplic_irqdomain_translate(fwspec, priv->gsi_base, hwirq, type);
>> >> >+}
>> >> >+
>> >> >+static int aplic_msi_irqdomain_alloc(struct irq_domain *domain,
>> >> >+                                   unsigned int virq, unsigned int nr_irqs,
>> >> >+                                   void *arg)
>> >> >+{
>> >> >+      int i, ret;
>> >> >+      unsigned int type;
>> >> >+      irq_hw_number_t hwirq;
>> >> >+      struct irq_fwspec *fwspec = arg;
>> >> >+      struct aplic_priv *priv = platform_msi_get_host_data(domain);
>> >> >+
>> >> >+      ret = aplic_irqdomain_translate(fwspec, priv->gsi_base, &hwirq, &type);
>> >> >+      if (ret)
>> >> >+              return ret;
>> >>
>> >> In your patchset, the wired IRQ and IRQ of platform device will go into APLIC-MSI domain firstly.
>> >
>> >Yes, that is correct. In general, this applies to AIA specification
>> >and nothing to do with this patchset.
>> >
>> >> Let me assume here is a MSI IRQ not wired IRQ on a device, and it is a platform device in system.
>> >> so in aplic_irqdomain_translate() function, it will parse the APLIC physical IRQ number by fwspec->param[0],
>> >> but this is not a wried IRQ, it is a MSI IRQ, it should not has a APLIC physical IRQ number, the hwirq number should be allocated by MSI bitmap,
>> >> what value will be parse by DTS? zero or negative?
>> >
>> >For platform devices with MSI support, the MSIs will directly target
>> >the per-HART
>> >IMSICs and the DT node of such devices will never point to APLIC as the parent
>> >MSI controller.
>> >
>>
>> >The IMSIC driver implements the IMSIC-PLAT domain for platform MSIs.
>>
>> Have you test this case on QEMU? would you like share the test steps?
>
>The APLIC in MSI-mode acts like a platform device with MSIs so yes this
>is tested on QEMU.

yet, I know the wired interrupt with MSI-mode of APLIC has tested on QEMU by virtio devices, but i want to know how about the MSI interrupt with non-PCI device?
have you tested this case?
Re: Re: Re: [PATCH v11 12/14] irqchip/riscv-aplic: Add support for MSI-mode
Posted by Anup Patel 2 years, 2 months ago 
On Wed, Nov 8, 2023 at 9:02 PM Ben <figure1802@126.com> wrote:
>
>
> At 2023-11-08 22:56:59, "Anup Patel" <apatel@ventanamicro.com> wrote:
> >On Wed, Nov 8, 2023 at 8:23 PM Ben <figure1802@126.com> wrote:
> >>
> >> At 2023-11-08 22:43:25, "Anup Patel" <apatel@ventanamicro.com> wrote:
> >> >On Sat, Nov 4, 2023 at 6:30 AM Ben <figure1802@126.com> wrote:
> >> >>
> >> >> At 2023-10-24 01:27:58, "Anup Patel" <apatel@ventanamicro.com> wrote:
> >> >> >The RISC-V advanced platform-level interrupt controller (APLIC) has
> >> >> >two modes of operation: 1) Direct mode and 2) MSI mode.
> >> >> >(For more details, refer https://github.com/riscv/riscv-aia)
> >> >> >
> >> >> >In APLIC MSI-mode, wired interrupts are forwared as message signaled
> >> >> >interrupts (MSIs) to CPUs via IMSIC.
> >> >> >
> >> >> >We extend the existing APLIC irqchip driver to support MSI-mode for
> >> >> >RISC-V platforms having both wired interrupts and MSIs.
> >> >> >
> >> >> >Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> >> >> >---
> >> >> > drivers/irqchip/Kconfig                |   6 +
> >> >> > drivers/irqchip/Makefile               |   1 +
> >> >> > drivers/irqchip/irq-riscv-aplic-main.c |   2 +-
> >> >> > drivers/irqchip/irq-riscv-aplic-main.h |   8 +
> >> >> > drivers/irqchip/irq-riscv-aplic-msi.c  | 285 +++++++++++++++++++++++++
> >> >> > 5 files changed, 301 insertions(+), 1 deletion(-)
> >> >> > create mode 100644 drivers/irqchip/irq-riscv-aplic-msi.c
> >> >> >
> >> >> >diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> >> >> >index 1996cc6f666a..7adc4dbe07ff 100644
> >> >> >--- a/drivers/irqchip/Kconfig
> >> >> >+++ b/drivers/irqchip/Kconfig
> >> >> >@@ -551,6 +551,12 @@ config RISCV_APLIC
> >> >> >       depends on RISCV
> >> >> >       select IRQ_DOMAIN_HIERARCHY
> >> >> >
> >> >> >+config RISCV_APLIC_MSI
> >> >> >+      bool
> >> >> >+      depends on RISCV_APLIC
> >> >> >+      select GENERIC_MSI_IRQ
> >> >> >+      default RISCV_APLIC
> >> >> >+
> >> >> > config RISCV_IMSIC
> >> >> >       bool
> >> >> >       depends on RISCV
> >> >> >diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> >> >> >index 7f8289790ed8..47995fdb2c60 100644
> >> >> >--- a/drivers/irqchip/Makefile
> >> >> >+++ b/drivers/irqchip/Makefile
> >> >> >@@ -96,6 +96,7 @@ obj-$(CONFIG_CSKY_MPINTC)            += irq-csky-mpintc.o
> >> >> > obj-$(CONFIG_CSKY_APB_INTC)           += irq-csky-apb-intc.o
> >> >> > obj-$(CONFIG_RISCV_INTC)              += irq-riscv-intc.o
> >> >> > obj-$(CONFIG_RISCV_APLIC)             += irq-riscv-aplic-main.o irq-riscv-aplic-direct.o
> >> >> >+obj-$(CONFIG_RISCV_APLIC_MSI)         += irq-riscv-aplic-msi.o
> >> >> > obj-$(CONFIG_RISCV_IMSIC)             += irq-riscv-imsic-state.o irq-riscv-imsic-early.o irq-riscv-imsic-platform.o
> >> >> > obj-$(CONFIG_SIFIVE_PLIC)             += irq-sifive-plic.o
> >> >> > obj-$(CONFIG_IMX_IRQSTEER)            += irq-imx-irqsteer.o
> >> >> >diff --git a/drivers/irqchip/irq-riscv-aplic-main.c b/drivers/irqchip/irq-riscv-aplic-main.c
> >> >> >index 87450708a733..d1b342b66551 100644
> >> >> >--- a/drivers/irqchip/irq-riscv-aplic-main.c
> >> >> >+++ b/drivers/irqchip/irq-riscv-aplic-main.c
> >> >> >@@ -205,7 +205,7 @@ static int aplic_probe(struct platform_device *pdev)
> >> >> >               msi_mode = of_property_present(to_of_node(dev->fwnode),
> >> >> >                                               "msi-parent");
> >> >> >       if (msi_mode)
> >> >> >-              rc = -ENODEV;
> >> >> >+              rc = aplic_msi_setup(dev, regs);
> >> >> >       else
> >> >> >               rc = aplic_direct_setup(dev, regs);
> >> >> >       if (rc) {
> >> >> >diff --git a/drivers/irqchip/irq-riscv-aplic-main.h b/drivers/irqchip/irq-riscv-aplic-main.h
> >> >> >index 474a04229334..78267ec58098 100644
> >> >> >--- a/drivers/irqchip/irq-riscv-aplic-main.h
> >> >> >+++ b/drivers/irqchip/irq-riscv-aplic-main.h
> >> >> >@@ -41,5 +41,13 @@ void aplic_init_hw_global(struct aplic_priv *priv, bool msi_mode);
> >> >> > int aplic_setup_priv(struct aplic_priv *priv, struct device *dev,
> >> >> >                    void __iomem *regs);
> >> >> > int aplic_direct_setup(struct device *dev, void __iomem *regs);
> >> >> >+#ifdef CONFIG_RISCV_APLIC_MSI
> >> >> >+int aplic_msi_setup(struct device *dev, void __iomem *regs);
> >> >> >+#else
> >> >> >+static inline int aplic_msi_setup(struct device *dev, void __iomem *regs)
> >> >> >+{
> >> >> >+      return -ENODEV;
> >> >> >+}
> >> >> >+#endif
> >> >> >
> >> >> > #endif
> >> >> >diff --git a/drivers/irqchip/irq-riscv-aplic-msi.c b/drivers/irqchip/irq-riscv-aplic-msi.c
> >> >> >new file mode 100644
> >> >> >index 000000000000..086d00e0429e
> >> >> >--- /dev/null
> >> >> >+++ b/drivers/irqchip/irq-riscv-aplic-msi.c
> >> >> >@@ -0,0 +1,285 @@
> >> >> >+// SPDX-License-Identifier: GPL-2.0
> >> >> >+/*
> >> >> >+ * Copyright (C) 2021 Western Digital Corporation or its affiliates.
> >> >> >+ * Copyright (C) 2022 Ventana Micro Systems Inc.
> >> >> >+ */
> >> >> >+
> >> >> >+#include <linux/bitops.h>
> >> >> >+#include <linux/cpu.h>
> >> >> >+#include <linux/interrupt.h>
> >> >> >+#include <linux/irqchip.h>
> >> >> >+#include <linux/irqchip/riscv-aplic.h>
> >> >> >+#include <linux/irqchip/riscv-imsic.h>
> >> >> >+#include <linux/module.h>
> >> >> >+#include <linux/msi.h>
> >> >> >+#include <linux/of_irq.h>
> >> >> >+#include <linux/platform_device.h>
> >> >> >+#include <linux/printk.h>
> >> >> >+#include <linux/smp.h>
> >> >> >+
> >> >> >+#include "irq-riscv-aplic-main.h"
> >> >> >+
> >> >> >+static void aplic_msi_irq_unmask(struct irq_data *d)
> >> >> >+{
> >> >> >+      aplic_irq_unmask(d);
> >> >> >+      irq_chip_unmask_parent(d);
> >> >> >+}
> >> >> >+
> >> >> >+static void aplic_msi_irq_mask(struct irq_data *d)
> >> >> >+{
> >> >> >+      aplic_irq_mask(d);
> >> >> >+      irq_chip_mask_parent(d);
> >> >> >+}
> >> >> >+
> >> >> >+static void aplic_msi_irq_eoi(struct irq_data *d)
> >> >> >+{
> >> >> >+      struct aplic_priv *priv = irq_data_get_irq_chip_data(d);
> >> >> >+      u32 reg_off, reg_mask;
> >> >> >+
> >> >> >+      /*
> >> >> >+       * EOI handling only required only for level-triggered
> >> >> >+       * interrupts in APLIC MSI mode.
> >> >> >+       */
> >> >> >+
> >> >> >+      reg_off = APLIC_CLRIP_BASE + ((d->hwirq / APLIC_IRQBITS_PER_REG) * 4);
> >> >> >+      reg_mask = BIT(d->hwirq % APLIC_IRQBITS_PER_REG);
> >> >> >+      switch (irqd_get_trigger_type(d)) {
> >> >> >+      case IRQ_TYPE_LEVEL_LOW:
> >> >> >+              if (!(readl(priv->regs + reg_off) & reg_mask))
> >> >> >+                      writel(d->hwirq, priv->regs + APLIC_SETIPNUM_LE);
> >> >> >+              break;
> >> >> >+      case IRQ_TYPE_LEVEL_HIGH:
> >> >> >+              if (readl(priv->regs + reg_off) & reg_mask)
> >> >> >+                      writel(d->hwirq, priv->regs + APLIC_SETIPNUM_LE);
> >> >> >+              break;
> >> >> >+      }
> >> >> >+}
> >> >> >+
> >> >> >+static struct irq_chip aplic_msi_chip = {
> >> >> >+      .name           = "APLIC-MSI",
> >> >> >+      .irq_mask       = aplic_msi_irq_mask,
> >> >> >+      .irq_unmask     = aplic_msi_irq_unmask,
> >> >> >+      .irq_set_type   = aplic_irq_set_type,
> >> >> >+      .irq_eoi        = aplic_msi_irq_eoi,
> >> >> >+#ifdef CONFIG_SMP
> >> >> >+      .irq_set_affinity = irq_chip_set_affinity_parent,
> >> >> >+#endif
> >> >> >+      .flags          = IRQCHIP_SET_TYPE_MASKED |
> >> >> >+                        IRQCHIP_SKIP_SET_WAKE |
> >> >> >+                        IRQCHIP_MASK_ON_SUSPEND,
> >> >> >+};
> >> >> >+
> >> >> >+static int aplic_msi_irqdomain_translate(struct irq_domain *d,
> >> >> >+                                       struct irq_fwspec *fwspec,
> >> >> >+                                       unsigned long *hwirq,
> >> >> >+                                       unsigned int *type)
> >> >> >+{
> >> >> >+      struct aplic_priv *priv = platform_msi_get_host_data(d);
> >> >> >+
> >> >> >+      return aplic_irqdomain_translate(fwspec, priv->gsi_base, hwirq, type);
> >> >> >+}
> >> >> >+
> >> >> >+static int aplic_msi_irqdomain_alloc(struct irq_domain *domain,
> >> >> >+                                   unsigned int virq, unsigned int nr_irqs,
> >> >> >+                                   void *arg)
> >> >> >+{
> >> >> >+      int i, ret;
> >> >> >+      unsigned int type;
> >> >> >+      irq_hw_number_t hwirq;
> >> >> >+      struct irq_fwspec *fwspec = arg;
> >> >> >+      struct aplic_priv *priv = platform_msi_get_host_data(domain);
> >> >> >+
> >> >> >+      ret = aplic_irqdomain_translate(fwspec, priv->gsi_base, &hwirq, &type);
> >> >> >+      if (ret)
> >> >> >+              return ret;
> >> >>
> >> >> In your patchset, the wired IRQ and IRQ of platform device will go into APLIC-MSI domain firstly.
> >> >
> >> >Yes, that is correct. In general, this applies to AIA specification
> >> >and nothing to do with this patchset.
> >> >
> >> >> Let me assume here is a MSI IRQ not wired IRQ on a device, and it is a platform device in system.
> >> >> so in aplic_irqdomain_translate() function, it will parse the APLIC physical IRQ number by fwspec->param[0],
> >> >> but this is not a wried IRQ, it is a MSI IRQ, it should not has a APLIC physical IRQ number, the hwirq number should be allocated by MSI bitmap,
> >> >> what value will be parse by DTS? zero or negative?
> >> >
> >> >For platform devices with MSI support, the MSIs will directly target
> >> >the per-HART
> >> >IMSICs and the DT node of such devices will never point to APLIC as the parent
> >> >MSI controller.
> >> >
> >>
> >> >The IMSIC driver implements the IMSIC-PLAT domain for platform MSIs.
> >>
> >> Have you test this case on QEMU? would you like share the test steps?
> >
> >The APLIC in MSI-mode acts like a platform device with MSIs so yes this
> >is tested on QEMU.
>
> yet, I know the wired interrupt with MSI-mode of APLIC has tested on QEMU by virtio devices, but i want to know how about the MSI interrupt with non-PCI device?
> have you tested this case?

I don't see any difference in the way APLIC MSI-mode platform
device use MSI versus any other platform using MSI.

We also have the RISC-V IOMMU platform device which uses
MSIs provided by the IMSIC-PLAT domain.

Regards,
Anup
Re:Re:[PATCH v11 12/14] irqchip/riscv-aplic: Add support for MSI-mode
Posted by Ben 2 years, 3 months ago 


At 2023-11-04 08:58:10, "Ben" <figure1802@126.com> wrote:
>At 2023-10-24 01:27:58, "Anup Patel" <apatel@ventanamicro.com> wrote:
>>The RISC-V advanced platform-level interrupt controller (APLIC) has
>>two modes of operation: 1) Direct mode and 2) MSI mode.
>>(For more details, refer https://github.com/riscv/riscv-aia)
>>
>>In APLIC MSI-mode, wired interrupts are forwared as message signaled
>>interrupts (MSIs) to CPUs via IMSIC.
>>
>>We extend the existing APLIC irqchip driver to support MSI-mode for
>>RISC-V platforms having both wired interrupts and MSIs.
>>
>>Signed-off-by: Anup Patel <apatel@ventanamicro.com>
>>---
>> drivers/irqchip/Kconfig                |   6 +
>> drivers/irqchip/Makefile               |   1 +
>> drivers/irqchip/irq-riscv-aplic-main.c |   2 +-
>> drivers/irqchip/irq-riscv-aplic-main.h |   8 +
>> drivers/irqchip/irq-riscv-aplic-msi.c  | 285 +++++++++++++++++++++++++
>> 5 files changed, 301 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/irqchip/irq-riscv-aplic-msi.c
>>
>>diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
>>index 1996cc6f666a..7adc4dbe07ff 100644
>>--- a/drivers/irqchip/Kconfig
>>+++ b/drivers/irqchip/Kconfig
>>@@ -551,6 +551,12 @@ config RISCV_APLIC
>> 	depends on RISCV
>> 	select IRQ_DOMAIN_HIERARCHY
>> 
>>+config RISCV_APLIC_MSI
>>+	bool
>>+	depends on RISCV_APLIC
>>+	select GENERIC_MSI_IRQ
>>+	default RISCV_APLIC
>>+
>> config RISCV_IMSIC
>> 	bool
>> 	depends on RISCV
>>diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
>>index 7f8289790ed8..47995fdb2c60 100644
>>--- a/drivers/irqchip/Makefile
>>+++ b/drivers/irqchip/Makefile
>>@@ -96,6 +96,7 @@ obj-$(CONFIG_CSKY_MPINTC)		+= irq-csky-mpintc.o
>> obj-$(CONFIG_CSKY_APB_INTC)		+= irq-csky-apb-intc.o
>> obj-$(CONFIG_RISCV_INTC)		+= irq-riscv-intc.o
>> obj-$(CONFIG_RISCV_APLIC)		+= irq-riscv-aplic-main.o irq-riscv-aplic-direct.o
>>+obj-$(CONFIG_RISCV_APLIC_MSI)		+= irq-riscv-aplic-msi.o
>> obj-$(CONFIG_RISCV_IMSIC)		+= irq-riscv-imsic-state.o irq-riscv-imsic-early.o irq-riscv-imsic-platform.o
>> obj-$(CONFIG_SIFIVE_PLIC)		+= irq-sifive-plic.o
>> obj-$(CONFIG_IMX_IRQSTEER)		+= irq-imx-irqsteer.o
>>diff --git a/drivers/irqchip/irq-riscv-aplic-main.c b/drivers/irqchip/irq-riscv-aplic-main.c
>>index 87450708a733..d1b342b66551 100644
>>--- a/drivers/irqchip/irq-riscv-aplic-main.c
>>+++ b/drivers/irqchip/irq-riscv-aplic-main.c
>>@@ -205,7 +205,7 @@ static int aplic_probe(struct platform_device *pdev)
>> 		msi_mode = of_property_present(to_of_node(dev->fwnode),
>> 						"msi-parent");
>> 	if (msi_mode)
>>-		rc = -ENODEV;
>>+		rc = aplic_msi_setup(dev, regs);
>> 	else
>> 		rc = aplic_direct_setup(dev, regs);
>> 	if (rc) {
>>diff --git a/drivers/irqchip/irq-riscv-aplic-main.h b/drivers/irqchip/irq-riscv-aplic-main.h
>>index 474a04229334..78267ec58098 100644
>>--- a/drivers/irqchip/irq-riscv-aplic-main.h
>>+++ b/drivers/irqchip/irq-riscv-aplic-main.h
>>@@ -41,5 +41,13 @@ void aplic_init_hw_global(struct aplic_priv *priv, bool msi_mode);
>> int aplic_setup_priv(struct aplic_priv *priv, struct device *dev,
>> 		     void __iomem *regs);
>> int aplic_direct_setup(struct device *dev, void __iomem *regs);
>>+#ifdef CONFIG_RISCV_APLIC_MSI
>>+int aplic_msi_setup(struct device *dev, void __iomem *regs);
>>+#else
>>+static inline int aplic_msi_setup(struct device *dev, void __iomem *regs)
>>+{
>>+	return -ENODEV;
>>+}
>>+#endif
>> 
>> #endif
>>diff --git a/drivers/irqchip/irq-riscv-aplic-msi.c b/drivers/irqchip/irq-riscv-aplic-msi.c
>>new file mode 100644
>>index 000000000000..086d00e0429e
>>--- /dev/null
>>+++ b/drivers/irqchip/irq-riscv-aplic-msi.c
>>@@ -0,0 +1,285 @@
>>+// SPDX-License-Identifier: GPL-2.0
>>+/*
>>+ * Copyright (C) 2021 Western Digital Corporation or its affiliates.
>>+ * Copyright (C) 2022 Ventana Micro Systems Inc.
>>+ */
>>+
>>+#include <linux/bitops.h>
>>+#include <linux/cpu.h>
>>+#include <linux/interrupt.h>
>>+#include <linux/irqchip.h>
>>+#include <linux/irqchip/riscv-aplic.h>
>>+#include <linux/irqchip/riscv-imsic.h>
>>+#include <linux/module.h>
>>+#include <linux/msi.h>
>>+#include <linux/of_irq.h>
>>+#include <linux/platform_device.h>
>>+#include <linux/printk.h>
>>+#include <linux/smp.h>
>>+
>>+#include "irq-riscv-aplic-main.h"
>>+
>>+static void aplic_msi_irq_unmask(struct irq_data *d)
>>+{
>>+	aplic_irq_unmask(d);
>>+	irq_chip_unmask_parent(d);
>>+}
>>+
>>+static void aplic_msi_irq_mask(struct irq_data *d)
>>+{
>>+	aplic_irq_mask(d);
>>+	irq_chip_mask_parent(d);
>>+}
>>+
>>+static void aplic_msi_irq_eoi(struct irq_data *d)
>>+{
>>+	struct aplic_priv *priv = irq_data_get_irq_chip_data(d);
>>+	u32 reg_off, reg_mask;
>>+
>>+	/*
>>+	 * EOI handling only required only for level-triggered
>>+	 * interrupts in APLIC MSI mode.
>>+	 */
>>+
>>+	reg_off = APLIC_CLRIP_BASE + ((d->hwirq / APLIC_IRQBITS_PER_REG) * 4);
>>+	reg_mask = BIT(d->hwirq % APLIC_IRQBITS_PER_REG);
>>+	switch (irqd_get_trigger_type(d)) {
>>+	case IRQ_TYPE_LEVEL_LOW:
>>+		if (!(readl(priv->regs + reg_off) & reg_mask))
>>+			writel(d->hwirq, priv->regs + APLIC_SETIPNUM_LE);
>>+		break;
>>+	case IRQ_TYPE_LEVEL_HIGH:
>>+		if (readl(priv->regs + reg_off) & reg_mask)
>>+			writel(d->hwirq, priv->regs + APLIC_SETIPNUM_LE);
>>+		break;
>>+	}
>>+}
>>+
>>+static struct irq_chip aplic_msi_chip = {
>>+	.name		= "APLIC-MSI",
>>+	.irq_mask	= aplic_msi_irq_mask,
>>+	.irq_unmask	= aplic_msi_irq_unmask,
>>+	.irq_set_type	= aplic_irq_set_type,
>>+	.irq_eoi	= aplic_msi_irq_eoi,
>>+#ifdef CONFIG_SMP
>>+	.irq_set_affinity = irq_chip_set_affinity_parent,
>>+#endif
>>+	.flags		= IRQCHIP_SET_TYPE_MASKED |
>>+			  IRQCHIP_SKIP_SET_WAKE |
>>+			  IRQCHIP_MASK_ON_SUSPEND,
>>+};
>>+
>>+static int aplic_msi_irqdomain_translate(struct irq_domain *d,
>>+					 struct irq_fwspec *fwspec,
>>+					 unsigned long *hwirq,
>>+					 unsigned int *type)
>>+{
>>+	struct aplic_priv *priv = platform_msi_get_host_data(d);
>>+
>>+	return aplic_irqdomain_translate(fwspec, priv->gsi_base, hwirq, type);
>>+}
>>+
>>+static int aplic_msi_irqdomain_alloc(struct irq_domain *domain,
>>+				     unsigned int virq, unsigned int nr_irqs,
>>+				     void *arg)
>>+{
>>+	int i, ret;
>>+	unsigned int type;
>>+	irq_hw_number_t hwirq;
>>+	struct irq_fwspec *fwspec = arg;
>>+	struct aplic_priv *priv = platform_msi_get_host_data(domain);
>>+
>>+	ret = aplic_irqdomain_translate(fwspec, priv->gsi_base, &hwirq, &type);
>>+	if (ret)
>>+		return ret;
>
>In your patchset, the wired IRQ and IRQ of platform device will go into APLIC-MSI domain firstly.
>Let me assume here is a MSI IRQ not wired IRQ on a device, and it is a platform device in system.
>so in aplic_irqdomain_translate() function, it will parse the APLIC physical IRQ number by fwspec->param[0],
>but this is not a wried IRQ, it is a MSI IRQ, it should not has a APLIC physical IRQ number, the hwirq number should be allocated by MSI bitmap,
>what value will be parse by DTS? zero or negative? 
>
>if this is a nonexistent physical IRQ number for APLIC, in aplic_msi_irq_unmask()->aplic_irq_unmask(), how it works?
>
>writel(d->hwirq, priv->regs + APLIC_SETIENUM);

hi Anup,
Any comments about this question for an MSI interrupt (not wired interrupt) of non-PCI device?

Thanks,
Ben
Re: [PATCH v11 12/14] irqchip/riscv-aplic: Add support for MSI-mode
Posted by Sunil V L 2 years, 3 months ago 
Hi Anup,

On Mon, Oct 23, 2023 at 10:57:58PM +0530, Anup Patel wrote:
> The RISC-V advanced platform-level interrupt controller (APLIC) has
> two modes of operation: 1) Direct mode and 2) MSI mode.
> (For more details, refer https://github.com/riscv/riscv-aia)
> 
> In APLIC MSI-mode, wired interrupts are forwared as message signaled
> interrupts (MSIs) to CPUs via IMSIC.
> 
> We extend the existing APLIC irqchip driver to support MSI-mode for
> RISC-V platforms having both wired interrupts and MSIs.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
[...]
> +int aplic_msi_setup(struct device *dev, void __iomem *regs)
> +{
> +	const struct imsic_global_config *imsic_global;
> +	struct irq_domain *irqdomain;
> +	struct aplic_priv *priv;
> +	struct aplic_msicfg *mc;
> +	phys_addr_t pa;
> +	int rc;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	rc = aplic_setup_priv(priv, dev, regs);
> +	if (!priv) {
This should check rc instead of priv.

> +		dev_err(dev, "failed to create APLIC context\n");
> +		return rc;
> +	}
> +	mc = &priv->msicfg;
> +
> +	/*
> +	 * The APLIC outgoing MSI config registers assume target MSI
> +	 * controller to be RISC-V AIA IMSIC controller.
> +	 */
> +	imsic_global = imsic_get_global_config();
> +	if (!imsic_global) {
> +		dev_err(dev, "IMSIC global config not found\n");
> +		return -ENODEV;
For all error return paths, priv should be freed.

Thanks,
Sunil

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.