diff mbox series

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

Message ID	20231023172800.315343-13-apatel@ventanamicro.com
State	New
Headers	Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7; From: Anup Patel <apatel@ventanamicro.com> To: Palmer Dabbelt <palmer@dabbelt.com>, Paul Walmsley <paul.walmsley@sifive.com>, Thomas Gleixner <tglx@linutronix.de>, Rob Herring <robh+dt@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Frank Rowand <frowand.list@gmail.com>, Conor Dooley <conor+dt@kernel.org> Cc: Marc Zyngier <maz@kernel.org>, =?utf-8?b?QmrDtnJuIFTDtnBlbA==?= <bjorn@kernel.org>, Atish Patra <atishp@atishpatra.org>, Andrew Jones <ajones@ventanamicro.com>, Sunil V L <sunilvl@ventanamicro.com>, Saravana Kannan <saravanak@google.com>, Anup Patel <anup@brainfault.org>, linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Anup Patel <apatel@ventanamicro.com> Subject: [PATCH v11 12/14] irqchip/riscv-aplic: Add support for MSI-mode Date: Mon, 23 Oct 2023 22:57:58 +0530 Message-Id: <20231023172800.315343-13-apatel@ventanamicro.com> In-Reply-To: <20231023172800.315343-1-apatel@ventanamicro.com> References: <20231023172800.315343-1-apatel@ventanamicro.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk
Series	Linux RISC-V AIA Support \| [v11,00/14] Linux RISC-V AIA Support [v11,01/14] RISC-V: Don't fail in riscv_of_parent_hartid() for disabled HARTs [v11,02/14] of: property: Add fw_devlink support for msi-parent [v11,03/14] irqchip/sifive-plic: Fix syscore registration for multi-socket systems [v11,04/14] irqchip/sifive-plic: Convert PLIC driver into a platform driver [v11,05/14] irqchip/riscv-intc: Add support for RISC-V AIA [v11,06/14] dt-bindings: interrupt-controller: Add RISC-V incoming MSI controller [v11,07/14] irqchip: Add RISC-V incoming MSI controller early driver [v11,08/14] irqchip/riscv-imsic: Add support for platform MSI irqdomain [v11,09/14] irqchip/riscv-imsic: Add support for PCI MSI irqdomain [v11,10/14] dt-bindings: interrupt-controller: Add RISC-V advanced PLIC [v11,11/14] irqchip: Add RISC-V advanced PLIC driver for direct-mode [v11,12/14] irqchip/riscv-aplic: Add support for MSI-mode [v11,13/14] RISC-V: Select APLIC and IMSIC drivers [v11,14/14] MAINTAINERS: Add entry for RISC-V AIA drivers

Commit Message

Anup Patel Oct. 23, 2023, 5:27 p.m. UTC

  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

Comments

Sunil V L Oct. 24, 2023, 5:31 a.m. UTC | #1

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

Ben Nov. 2, 2023, 6:38 a.m. UTC | #2

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;
>+
>+	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);

>+	}


For platfrom MSI irq, it will call irq_domain_set_info() and irq_set_status_flags() twice, the first one is here:
platform_msi_device_domain_alloc->msi_domain_populate_irqs->irq_domain_alloc_irqs_hierarchy->imsic_irq_domain_alloc->irq_domain_set_info


so  i think here this for(...) is not necessary, can be removed.

Anup Patel Nov. 2, 2023, 12:37 p.m. UTC | #3

On Thu, Nov 2, 2023 at 11:55 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;
> >+
> >+ 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);
> >+ }
>
> For platfrom MSI irq, it will call irq_domain_set_info() and irq_set_status_flags() twice, the first one is here:
> platform_msi_device_domain_alloc->msi_domain_populate_irqs->irq_domain_alloc_irqs_hierarchy->imsic_irq_domain_alloc->irq_domain_set_info
>
> so  i think here this for(...) is not necessary, can be removed.

If we remove then it breaks APLIC MSI-mode because we have
hierarchical irq domains where the APLIC-MSI domain is a child
of the IMSIC-PLAT domain.

The irq_domain_set_info() called by IMSIC driver only sets irqchip
for IMSIC irq whereas irq_domain_set_info() called by APLIC driver
sets irqchip for APLIC irq. We use a different APLIC irqchip for the
APLIC domain to mask, unmask, and eoi irqs in an APLIC specific
way.

Regards,
Anup


>
>
> >+
> >+ 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
> >
> >
> >_______________________________________________
> >linux-riscv mailing list
> >linux-riscv@lists.infradead.org
> >http://lists.infradead.org/mailman/listinfo/linux-riscv

Ben Nov. 3, 2023, 9:39 a.m. UTC | #4

在 2023-11-02 20:37:42，"Anup Patel" <apatel@ventanamicro.com> 写道：
>On Thu, Nov 2, 2023 at 11:55 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;
>> >+
>> >+ 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);
>> >+ }
>>
>> For platfrom MSI irq, it will call irq_domain_set_info() and irq_set_status_flags() twice, the first one is here:
>> platform_msi_device_domain_alloc->msi_domain_populate_irqs->irq_domain_alloc_irqs_hierarchy->imsic_irq_domain_alloc->irq_domain_set_info
>>
>> so  i think here this for(...) is not necessary, can be removed.
>
>If we remove then it breaks APLIC MSI-mode because we have
>hierarchical irq domains where the APLIC-MSI domain is a child
>of the IMSIC-PLAT domain.
>
>The irq_domain_set_info() called by IMSIC driver only sets irqchip
>for IMSIC irq whereas irq_domain_set_info() called by APLIC driver
>sets irqchip for APLIC irq. We use a different APLIC irqchip for the
>APLIC domain to mask, unmask, and eoi irqs in an APLIC specific
>way.
>

As your said APLIC-MSI domain is a child of the IMSIC-PLAT domain, so all of platform IRQ or wired IRQ will go to APLIC-MSI domain firstly.
how about the pure MSI interrupt? for example the MSI of PCIe device or device driver call platform_msi_domain_alloc_irqs() to allocate a MSI ?
 in this scenario, it also go into  APLIC-MSI domain firstly?

would you like provide the steps how to test the PCI MSI for your patchset on QEMU? i run a QEMU system, but i cannot found any PCI devices using MSI, especially the virtio devices which using the platform IRQ.

~# cat /proc/interrupts 
           CPU0       CPU1       CPU2       CPU3       
 10:      38972      38946      38882      38924  RISC-V INTC   5 Edge      riscv-timer
 11:          0       1149          0          0  APLIC-MSI   8 Level     virtio0
 12:          0          0         21          0  APLIC-MSI   7 Level     virtio1
 13:        149          0          0        218  APLIC-MSI  10 Level     ttyS0
IPI0:        40         53         43         50  Rescheduling interrupts
IPI1:      7518       8899       6679       7959  Function call interrupts
IPI2:         0          0          0          0  CPU stop interrupts
IPI3:         0          0          0          0  CPU stop (for crash dump) interrupts
IPI4:         0          0          0          0  IRQ work interrupts
IPI5:         0          0          0          0  Timer broadcast interrupts

Anup Patel Nov. 3, 2023, 11:04 a.m. UTC | #5

On Fri, Nov 3, 2023 at 3:14 PM Ben <figure1802@126.com> wrote:
>
>
>
> 在 2023-11-02 20:37:42，"Anup Patel" <apatel@ventanamicro.com> 写道：
> >On Thu, Nov 2, 2023 at 11:55 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;
> >> >+
> >> >+ 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);
> >> >+ }
> >>
> >> For platfrom MSI irq, it will call irq_domain_set_info() and irq_set_status_flags() twice, the first one is here:
> >> platform_msi_device_domain_alloc->msi_domain_populate_irqs->irq_domain_alloc_irqs_hierarchy->imsic_irq_domain_alloc->irq_domain_set_info
> >>
> >> so  i think here this for(...) is not necessary, can be removed.
> >
> >If we remove then it breaks APLIC MSI-mode because we have
> >hierarchical irq domains where the APLIC-MSI domain is a child
> >of the IMSIC-PLAT domain.
> >
> >The irq_domain_set_info() called by IMSIC driver only sets irqchip
> >for IMSIC irq whereas irq_domain_set_info() called by APLIC driver
> >sets irqchip for APLIC irq. We use a different APLIC irqchip for the
> >APLIC domain to mask, unmask, and eoi irqs in an APLIC specific
> >way.
> >
>
> As your said APLIC-MSI domain is a child of the IMSIC-PLAT domain, so all of platform IRQ or wired IRQ will go to APLIC-MSI domain firstly.
> how about the pure MSI interrupt? for example the MSI of PCIe device or device driver call platform_msi_domain_alloc_irqs() to allocate a MSI ?

MSIs from PCIe device will directly go to IMSIC-PCI domain.

>  in this scenario, it also go into  APLIC-MSI domain firstly?

No

>
> would you like provide the steps how to test the PCI MSI for your patchset on QEMU? i run a QEMU system, but i cannot found any PCI devices using MSI, especially the virtio devices which using the platform IRQ.

Just add virtio-blk-pci OR some other PCI device in your QEMU
command line but ensure that you have corresponding device
driver enabled in your kernel.

>
> ~# cat /proc/interrupts
>            CPU0       CPU1       CPU2       CPU3
>  10:      38972      38946      38882      38924  RISC-V INTC   5 Edge      riscv-timer
>  11:          0       1149          0          0  APLIC-MSI   8 Level     virtio0
>  12:          0          0         21          0  APLIC-MSI   7 Level     virtio1
>  13:        149          0          0        218  APLIC-MSI  10 Level     ttyS0
> IPI0:        40         53         43         50  Rescheduling interrupts
> IPI1:      7518       8899       6679       7959  Function call interrupts
> IPI2:         0          0          0          0  CPU stop interrupts
> IPI3:         0          0          0          0  CPU stop (for crash dump) interrupts
> IPI4:         0          0          0          0  IRQ work interrupts
> IPI5:         0          0          0          0  Timer broadcast interrupts
>
>

Regards,
Anup

Ben Nov. 4, 2023, 12:58 a.m. UTC | #6

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);

Ben Nov. 8, 2023, 2:20 p.m. UTC | #7

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

Anup Patel Nov. 8, 2023, 2:43 p.m. UTC | #8

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

Ben Nov. 8, 2023, 2:51 p.m. UTC | #9

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?

Anup Patel Nov. 8, 2023, 2:56 p.m. UTC | #10

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

Ben Nov. 8, 2023, 3:32 p.m. UTC | #11

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?

Anup Patel Nov. 14, 2023, 9:21 a.m. UTC | #12

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

diff mbox series

Patch

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;
+}