2023-07-10 10:03:18

by Anup Patel

[permalink] [raw]
Subject: [PATCH v5 0/9] Linux RISC-V AIA Support

The RISC-V AIA specification is now frozen as-per the RISC-V international
process. The latest frozen specifcation can be found at:
https://github.com/riscv/riscv-aia/releases/download/1.0/riscv-interrupts-1.0.pdf

At a high-level, the AIA specification adds three things:
1) AIA CSRs
- Improved local interrupt support
2) Incoming Message Signaled Interrupt Controller (IMSIC)
- Per-HART MSI controller
- Support MSI virtualization
- Support IPI along with virtualization
3) Advanced Platform-Level Interrupt Controller (APLIC)
- Wired interrupt controller
- In MSI-mode, converts wired interrupt into MSIs (i.e. MSI generator)
- In Direct-mode, injects external interrupts directly into HARTs

For an overview of the AIA specification, refer the AIA virtualization
talk at KVM Forum 2022:
https://static.sched.com/hosted_files/kvmforum2022/a1/AIA_Virtualization_in_KVM_RISCV_final.pdf
https://www.youtube.com/watch?v=r071dL8Z0yo

To test this series, use QEMU v7.2 (or higher) and OpenSBI v1.2 (or higher).

These patches can also be found in the riscv_aia_v5 branch at:
https://github.com/avpatel/linux.git

Changes since v4:
- Rebased on Linux-6.5-rc1
- Added "Dependencies" in the APLIC bindings (PATCH6 in v4)
- Dropped the PATCH6 which was changing the IOMMU DMA domain APIs
- Dropped use of IOMMU DMA APIs in the IMSIC driver (PATCH4)

Changes since v3:
- Rebased on Linux-6.4-rc6
- Droped PATCH2 of v3 series instead we now set FWNODE_FLAG_BEST_EFFORT via
IRQCHIP_DECLARE()
- Extend riscv_fw_parent_hartid() to support both DT and ACPI in PATCH1
- Extend iommu_dma_compose_msi_msg() instead of adding iommu_dma_select_msi()
in PATCH6
- Addressed Conor's comments in PATCH3
- Addressed Conor's and Rob's comments in PATCH7

Changes since v2:
- Rebased on Linux-6.4-rc1
- Addressed Rob's comments on DT bindings patches 4 and 8.
- Addessed Marc's comments on IMSIC driver PATCH5
- Replaced use of OF apis in APLIC and IMSIC drivers with FWNODE apis
this makes both drivers easily portable for ACPI support. This also
removes unnecessary indirection from the APLIC and IMSIC drivers.
- PATCH1 is a new patch for portability with ACPI support
- PATCH2 is a new patch to fix probing in APLIC drivers for APLIC-only systems.
- PATCH7 is a new patch which addresses the IOMMU DMA domain issues pointed
out by SiFive

Changes since v1:
- Rebased on Linux-6.2-rc2
- Addressed comments on IMSIC DT bindings for PATCH4
- Use raw_spin_lock_irqsave() on ids_lock for PATCH5
- Improved MMIO alignment checks in PATCH5 to allow MMIO regions
with holes.
- Addressed comments on APLIC DT bindings for PATCH6
- Fixed warning splat in aplic_msi_write_msg() caused by
zeroed MSI message in PATCH7
- Dropped DT property riscv,slow-ipi instead will have module
parameter in future.

Anup Patel (9):
RISC-V: Add riscv_fw_parent_hartid() function
irqchip/riscv-intc: Add support for RISC-V AIA
dt-bindings: interrupt-controller: Add RISC-V incoming MSI controller
irqchip: Add RISC-V incoming MSI controller driver
irqchip/riscv-imsic: Add support for PCI MSI irqdomain
dt-bindings: interrupt-controller: Add RISC-V advanced PLIC
irqchip: Add RISC-V advanced PLIC driver
RISC-V: Select APLIC and IMSIC drivers
MAINTAINERS: Add entry for RISC-V AIA drivers

.../interrupt-controller/riscv,aplic.yaml | 172 +++
.../interrupt-controller/riscv,imsics.yaml | 172 +++
MAINTAINERS | 12 +
arch/riscv/Kconfig | 2 +
arch/riscv/include/asm/processor.h | 3 +
arch/riscv/kernel/cpu.c | 16 +
drivers/irqchip/Kconfig | 20 +-
drivers/irqchip/Makefile | 2 +
drivers/irqchip/irq-riscv-aplic.c | 774 ++++++++++++
drivers/irqchip/irq-riscv-imsic.c | 1060 +++++++++++++++++
drivers/irqchip/irq-riscv-intc.c | 36 +-
include/linux/irqchip/riscv-aplic.h | 119 ++
include/linux/irqchip/riscv-imsic.h | 86 ++
13 files changed, 2467 insertions(+), 7 deletions(-)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml
create mode 100644 drivers/irqchip/irq-riscv-aplic.c
create mode 100644 drivers/irqchip/irq-riscv-imsic.c
create mode 100644 include/linux/irqchip/riscv-aplic.h
create mode 100644 include/linux/irqchip/riscv-imsic.h

--
2.34.1



2023-07-10 10:04:07

by Anup Patel

[permalink] [raw]
Subject: [PATCH v5 2/9] irqchip/riscv-intc: Add support for RISC-V AIA

The RISC-V advanced interrupt architecture (AIA) extends the per-HART
local interrupts in following ways:
1. Minimum 64 local interrupts for both RV32 and RV64
2. Ability to process multiple pending local interrupts in same
interrupt handler
3. Priority configuration for each local interrupts
4. Special CSRs to configure/access the per-HART MSI controller

This patch adds support for RISC-V AIA in the RISC-V intc driver.

Signed-off-by: Anup Patel <[email protected]>
---
drivers/irqchip/irq-riscv-intc.c | 36 ++++++++++++++++++++++++++------
1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
index 4adeee1bc391..e235bf1708a4 100644
--- a/drivers/irqchip/irq-riscv-intc.c
+++ b/drivers/irqchip/irq-riscv-intc.c
@@ -17,6 +17,7 @@
#include <linux/module.h>
#include <linux/of.h>
#include <linux/smp.h>
+#include <asm/hwcap.h>

static struct irq_domain *intc_domain;

@@ -30,6 +31,15 @@ static asmlinkage void riscv_intc_irq(struct pt_regs *regs)
generic_handle_domain_irq(intc_domain, cause);
}

+static asmlinkage void riscv_intc_aia_irq(struct pt_regs *regs)
+{
+ unsigned long topi;
+
+ while ((topi = csr_read(CSR_TOPI)))
+ generic_handle_domain_irq(intc_domain,
+ topi >> TOPI_IID_SHIFT);
+}
+
/*
* On RISC-V systems local interrupts are masked or unmasked by writing
* the SIE (Supervisor Interrupt Enable) CSR. As CSRs can only be written
@@ -39,12 +49,18 @@ static asmlinkage void riscv_intc_irq(struct pt_regs *regs)

static void riscv_intc_irq_mask(struct irq_data *d)
{
- csr_clear(CSR_IE, BIT(d->hwirq));
+ if (d->hwirq < BITS_PER_LONG)
+ csr_clear(CSR_IE, BIT(d->hwirq));
+ else
+ csr_clear(CSR_IEH, BIT(d->hwirq - BITS_PER_LONG));
}

static void riscv_intc_irq_unmask(struct irq_data *d)
{
- csr_set(CSR_IE, BIT(d->hwirq));
+ if (d->hwirq < BITS_PER_LONG)
+ csr_set(CSR_IE, BIT(d->hwirq));
+ else
+ csr_set(CSR_IEH, BIT(d->hwirq - BITS_PER_LONG));
}

static void riscv_intc_irq_eoi(struct irq_data *d)
@@ -115,16 +131,22 @@ static struct fwnode_handle *riscv_intc_hwnode(void)

static int __init riscv_intc_init_common(struct fwnode_handle *fn)
{
- int rc;
+ int rc, nr_irqs = BITS_PER_LONG;
+
+ if (riscv_isa_extension_available(NULL, SxAIA) && BITS_PER_LONG == 32)
+ nr_irqs = nr_irqs * 2;

- intc_domain = irq_domain_create_linear(fn, BITS_PER_LONG,
+ intc_domain = irq_domain_create_linear(fn, nr_irqs,
&riscv_intc_domain_ops, NULL);
if (!intc_domain) {
pr_err("unable to add IRQ domain\n");
return -ENXIO;
}

- rc = set_handle_irq(&riscv_intc_irq);
+ if (riscv_isa_extension_available(NULL, SxAIA))
+ rc = set_handle_irq(&riscv_intc_aia_irq);
+ else
+ rc = set_handle_irq(&riscv_intc_irq);
if (rc) {
pr_err("failed to set irq handler\n");
return rc;
@@ -132,7 +154,9 @@ static int __init riscv_intc_init_common(struct fwnode_handle *fn)

riscv_set_intc_hwnode_fn(riscv_intc_hwnode);

- pr_info("%d local interrupts mapped\n", BITS_PER_LONG);
+ pr_info("%d local interrupts mapped%s\n",
+ nr_irqs, (riscv_isa_extension_available(NULL, SxAIA)) ?
+ " using AIA" : "");

return 0;
}
--
2.34.1


2023-07-11 14:22:08

by Andrew Jones

[permalink] [raw]
Subject: Re: [PATCH v5 2/9] irqchip/riscv-intc: Add support for RISC-V AIA

On Mon, Jul 10, 2023 at 03:13:14PM +0530, Anup Patel wrote:
> The RISC-V advanced interrupt architecture (AIA) extends the per-HART
> local interrupts in following ways:
> 1. Minimum 64 local interrupts for both RV32 and RV64
> 2. Ability to process multiple pending local interrupts in same
> interrupt handler
> 3. Priority configuration for each local interrupts
> 4. Special CSRs to configure/access the per-HART MSI controller

afaict, we're only doing (1) and (2) from this list in this patch.

>
> This patch adds support for RISC-V AIA in the RISC-V intc driver.
>
> Signed-off-by: Anup Patel <[email protected]>
> ---
> drivers/irqchip/irq-riscv-intc.c | 36 ++++++++++++++++++++++++++------
> 1 file changed, 30 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
> index 4adeee1bc391..e235bf1708a4 100644
> --- a/drivers/irqchip/irq-riscv-intc.c
> +++ b/drivers/irqchip/irq-riscv-intc.c
> @@ -17,6 +17,7 @@
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/smp.h>
> +#include <asm/hwcap.h>
>
> static struct irq_domain *intc_domain;
>
> @@ -30,6 +31,15 @@ static asmlinkage void riscv_intc_irq(struct pt_regs *regs)
> generic_handle_domain_irq(intc_domain, cause);
> }
>
> +static asmlinkage void riscv_intc_aia_irq(struct pt_regs *regs)
> +{
> + unsigned long topi;
> +
> + while ((topi = csr_read(CSR_TOPI)))
> + generic_handle_domain_irq(intc_domain,
> + topi >> TOPI_IID_SHIFT);
> +}
> +
> /*
> * On RISC-V systems local interrupts are masked or unmasked by writing
> * the SIE (Supervisor Interrupt Enable) CSR. As CSRs can only be written
> @@ -39,12 +49,18 @@ static asmlinkage void riscv_intc_irq(struct pt_regs *regs)
>
> static void riscv_intc_irq_mask(struct irq_data *d)
> {
> - csr_clear(CSR_IE, BIT(d->hwirq));
> + if (d->hwirq < BITS_PER_LONG)
> + csr_clear(CSR_IE, BIT(d->hwirq));
> + else
> + csr_clear(CSR_IEH, BIT(d->hwirq - BITS_PER_LONG));

We can optimize rv64 by allowing the compiler to remove the branch

if (IS_ENABLED(CONFIG_32BIT) && d->hwirq >= 32)
csr_clear(CSR_IEH, BIT(d->hwirq - 32));
else
csr_clear(CSR_IE, BIT(d->hwirq));


> }
>
> static void riscv_intc_irq_unmask(struct irq_data *d)
> {
> - csr_set(CSR_IE, BIT(d->hwirq));
> + if (d->hwirq < BITS_PER_LONG)
> + csr_set(CSR_IE, BIT(d->hwirq));
> + else
> + csr_set(CSR_IEH, BIT(d->hwirq - BITS_PER_LONG));

Same comment as above.

> }
>
> static void riscv_intc_irq_eoi(struct irq_data *d)
> @@ -115,16 +131,22 @@ static struct fwnode_handle *riscv_intc_hwnode(void)
>
> static int __init riscv_intc_init_common(struct fwnode_handle *fn)
> {
> - int rc;
> + int rc, nr_irqs = BITS_PER_LONG;
> +
> + if (riscv_isa_extension_available(NULL, SxAIA) && BITS_PER_LONG == 32)
> + nr_irqs = nr_irqs * 2;

The AIA spec states sie and sip are explicitly 64, so how about writing
this as

int rc, nr_irqs = BITS_PER_LONG;

if (riscv_isa_extension_available(NULL, SxAIA))
nr_irqs = 64;

>
> - intc_domain = irq_domain_create_linear(fn, BITS_PER_LONG,
> + intc_domain = irq_domain_create_linear(fn, nr_irqs,
> &riscv_intc_domain_ops, NULL);
> if (!intc_domain) {
> pr_err("unable to add IRQ domain\n");
> return -ENXIO;
> }
>
> - rc = set_handle_irq(&riscv_intc_irq);
> + if (riscv_isa_extension_available(NULL, SxAIA))
> + rc = set_handle_irq(&riscv_intc_aia_irq);
> + else
> + rc = set_handle_irq(&riscv_intc_irq);

nit: blank line here

> if (rc) {
> pr_err("failed to set irq handler\n");
> return rc;
> @@ -132,7 +154,9 @@ static int __init riscv_intc_init_common(struct fwnode_handle *fn)
>
> riscv_set_intc_hwnode_fn(riscv_intc_hwnode);
>
> - pr_info("%d local interrupts mapped\n", BITS_PER_LONG);
> + pr_info("%d local interrupts mapped%s\n",
> + nr_irqs, (riscv_isa_extension_available(NULL, SxAIA)) ?

nit: unnecessary ()

> + " using AIA" : "");
>
> return 0;
> }
> --
> 2.34.1
>

Thanks,
drew

2023-07-17 06:51:14

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH v5 2/9] irqchip/riscv-intc: Add support for RISC-V AIA

On Tue, Jul 11, 2023 at 7:42 PM Andrew Jones <[email protected]> wrote:
>
> On Mon, Jul 10, 2023 at 03:13:14PM +0530, Anup Patel wrote:
> > The RISC-V advanced interrupt architecture (AIA) extends the per-HART
> > local interrupts in following ways:
> > 1. Minimum 64 local interrupts for both RV32 and RV64
> > 2. Ability to process multiple pending local interrupts in same
> > interrupt handler
> > 3. Priority configuration for each local interrupts
> > 4. Special CSRs to configure/access the per-HART MSI controller
>
> afaict, we're only doing (1) and (2) from this list in this patch.

Okay, I will update the commit description.

>
> >
> > This patch adds support for RISC-V AIA in the RISC-V intc driver.
> >
> > Signed-off-by: Anup Patel <[email protected]>
> > ---
> > drivers/irqchip/irq-riscv-intc.c | 36 ++++++++++++++++++++++++++------
> > 1 file changed, 30 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
> > index 4adeee1bc391..e235bf1708a4 100644
> > --- a/drivers/irqchip/irq-riscv-intc.c
> > +++ b/drivers/irqchip/irq-riscv-intc.c
> > @@ -17,6 +17,7 @@
> > #include <linux/module.h>
> > #include <linux/of.h>
> > #include <linux/smp.h>
> > +#include <asm/hwcap.h>
> >
> > static struct irq_domain *intc_domain;
> >
> > @@ -30,6 +31,15 @@ static asmlinkage void riscv_intc_irq(struct pt_regs *regs)
> > generic_handle_domain_irq(intc_domain, cause);
> > }
> >
> > +static asmlinkage void riscv_intc_aia_irq(struct pt_regs *regs)
> > +{
> > + unsigned long topi;
> > +
> > + while ((topi = csr_read(CSR_TOPI)))
> > + generic_handle_domain_irq(intc_domain,
> > + topi >> TOPI_IID_SHIFT);
> > +}
> > +
> > /*
> > * On RISC-V systems local interrupts are masked or unmasked by writing
> > * the SIE (Supervisor Interrupt Enable) CSR. As CSRs can only be written
> > @@ -39,12 +49,18 @@ static asmlinkage void riscv_intc_irq(struct pt_regs *regs)
> >
> > static void riscv_intc_irq_mask(struct irq_data *d)
> > {
> > - csr_clear(CSR_IE, BIT(d->hwirq));
> > + if (d->hwirq < BITS_PER_LONG)
> > + csr_clear(CSR_IE, BIT(d->hwirq));
> > + else
> > + csr_clear(CSR_IEH, BIT(d->hwirq - BITS_PER_LONG));
>
> We can optimize rv64 by allowing the compiler to remove the branch
>
> if (IS_ENABLED(CONFIG_32BIT) && d->hwirq >= 32)
> csr_clear(CSR_IEH, BIT(d->hwirq - 32));
> else
> csr_clear(CSR_IE, BIT(d->hwirq));
>

Makes sense, I will update.

>
> > }
> >
> > static void riscv_intc_irq_unmask(struct irq_data *d)
> > {
> > - csr_set(CSR_IE, BIT(d->hwirq));
> > + if (d->hwirq < BITS_PER_LONG)
> > + csr_set(CSR_IE, BIT(d->hwirq));
> > + else
> > + csr_set(CSR_IEH, BIT(d->hwirq - BITS_PER_LONG));
>
> Same comment as above.

Okay, I will update.

>
> > }
> >
> > static void riscv_intc_irq_eoi(struct irq_data *d)
> > @@ -115,16 +131,22 @@ static struct fwnode_handle *riscv_intc_hwnode(void)
> >
> > static int __init riscv_intc_init_common(struct fwnode_handle *fn)
> > {
> > - int rc;
> > + int rc, nr_irqs = BITS_PER_LONG;
> > +
> > + if (riscv_isa_extension_available(NULL, SxAIA) && BITS_PER_LONG == 32)
> > + nr_irqs = nr_irqs * 2;
>
> The AIA spec states sie and sip are explicitly 64, so how about writing
> this as
>
> int rc, nr_irqs = BITS_PER_LONG;
>
> if (riscv_isa_extension_available(NULL, SxAIA))
> nr_irqs = 64;

Okay, I will update.

>
> >
> > - intc_domain = irq_domain_create_linear(fn, BITS_PER_LONG,
> > + intc_domain = irq_domain_create_linear(fn, nr_irqs,
> > &riscv_intc_domain_ops, NULL);
> > if (!intc_domain) {
> > pr_err("unable to add IRQ domain\n");
> > return -ENXIO;
> > }
> >
> > - rc = set_handle_irq(&riscv_intc_irq);
> > + if (riscv_isa_extension_available(NULL, SxAIA))
> > + rc = set_handle_irq(&riscv_intc_aia_irq);
> > + else
> > + rc = set_handle_irq(&riscv_intc_irq);
>
> nit: blank line here

I prefer no blank line here because the "if (rc)" below
checks for errors for the above calls.

>
> > if (rc) {
> > pr_err("failed to set irq handler\n");
> > return rc;
> > @@ -132,7 +154,9 @@ static int __init riscv_intc_init_common(struct fwnode_handle *fn)
> >
> > riscv_set_intc_hwnode_fn(riscv_intc_hwnode);
> >
> > - pr_info("%d local interrupts mapped\n", BITS_PER_LONG);
> > + pr_info("%d local interrupts mapped%s\n",
> > + nr_irqs, (riscv_isa_extension_available(NULL, SxAIA)) ?
>
> nit: unnecessary ()

Okay, I will update.

>
> > + " using AIA" : "");
> >
> > return 0;
> > }
> > --
> > 2.34.1
> >
>
> Thanks,
> drew

Regards,
Anup