2021-06-18 13:02:51

by Anup Patel

[permalink] [raw]
Subject: [RFC PATCH v2 00/11] Linux RISC-V ACLINT Support

Most of the existing RISC-V platforms use SiFive CLINT to provide M-level
timer and IPI support whereas S-level uses SBI calls for timer and IPI
support. Also, the SiFive CLINT device is a single device providing both
timer and IPI functionality so RISC-V platforms can't partially implement
SiFive CLINT device and provide alternate mechanism for timer and IPI.

The RISC-V Advacned Core Local Interruptor (ACLINT) tries to address the
limitations of SiFive CLINT by:
1) Taking modular approach and defining timer and IPI functionality as
separate devices so that RISC-V platforms can include only required
devices
2) Providing dedicated MMIO device for S-level IPIs so that SBI calls
can be avoided for IPIs in Linux RISC-V
3) Allowing multiple instances of timer and IPI devices for a
multi-socket (or multi-die) NUMA systems
4) Being backward compatible to SiFive CLINT so that existing RISC-V
platforms stay compliant with RISC-V ACLINT specification

Latest RISC-V ACLINT specification (will be frozen in a month) can be
found at:
https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc

This series adds RISC-V ACLINT support and can be found in riscv_aclint_v2
branch at:
https://github.com/avpatel/linux

To test this series, the RISC-V ACLINT support for QEMU and OpenSBI
can be found in the riscv_aclint_v1 branch at:
https://github.com/avpatel/qemu
https://github.com/avpatel/opensbi

Changes since v1:
- Added a new PATCH3 to treat IPIs as normal Linux IRQs for RISC-V kernel
- New SBI IPI call based irqchip driver in PATCH3 which is only initialized
by riscv_ipi_setup() when no Linux IRQ numbers are available for IPIs
- Moved DT bindings patches before corresponding driver patches
- Implemented ACLINT SWI driver as a irqchip driver in PATCH7
- Minor nit fixes pointed by Bin Meng

Anup Patel (11):
RISC-V: Clear SIP bit only when using SBI IPI operations
RISC-V: Use common print prefix in smp.c
RISC-V: Treat IPIs as normal Linux IRQs
RISC-V: Allow marking IPIs as suitable for remote FENCEs
RISC-V: Use IPIs for remote TLB flush when possible
dt-bindings: interrupt-controller: Add ACLINT MSWI and SSWI bindings
irqchip: Add ACLINT software interrupt driver
RISC-V: Select ACLINT SWI driver for virt machine
dt-bindings: timer: Add ACLINT MTIMER bindings
clocksource: clint: Add support for ACLINT MTIMER device
MAINTAINERS: Add entry for RISC-V ACLINT drivers

.../riscv,aclint-swi.yaml | 82 ++++++
.../bindings/timer/riscv,aclint-mtimer.yaml | 55 ++++
MAINTAINERS | 9 +
arch/riscv/Kconfig | 1 +
arch/riscv/Kconfig.socs | 1 +
arch/riscv/include/asm/sbi.h | 2 +
arch/riscv/include/asm/smp.h | 48 +++-
arch/riscv/kernel/Makefile | 1 +
arch/riscv/kernel/cpu-hotplug.c | 2 +
arch/riscv/kernel/irq.c | 1 +
arch/riscv/kernel/sbi-ipi.c | 223 ++++++++++++++
arch/riscv/kernel/sbi.c | 15 -
arch/riscv/kernel/smp.c | 171 +++++------
arch/riscv/kernel/smpboot.c | 4 +-
arch/riscv/mm/tlbflush.c | 62 +++-
drivers/clocksource/timer-clint.c | 58 ++--
drivers/irqchip/Kconfig | 11 +
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-aclint-swi.c | 271 ++++++++++++++++++
drivers/irqchip/irq-riscv-intc.c | 55 ++--
20 files changed, 879 insertions(+), 194 deletions(-)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,aclint-swi.yaml
create mode 100644 Documentation/devicetree/bindings/timer/riscv,aclint-mtimer.yaml
create mode 100644 arch/riscv/kernel/sbi-ipi.c
create mode 100644 drivers/irqchip/irq-aclint-swi.c

--
2.25.1


2021-06-18 13:04:12

by Anup Patel

[permalink] [raw]
Subject: [RFC PATCH v2 01/11] RISC-V: Clear SIP bit only when using SBI IPI operations

The software interrupt pending (i.e. [M|S]SIP) bit is writeable for
S-mode but read-only for M-mode so we clear this bit only when using
SBI IPI operations.

Signed-off-by: Anup Patel <[email protected]>
Reviewed-by: Bin Meng <[email protected]>
---
arch/riscv/kernel/sbi.c | 8 +++++++-
arch/riscv/kernel/smp.c | 2 --
2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c
index 9a84f0cb5175..8aeca26198f2 100644
--- a/arch/riscv/kernel/sbi.c
+++ b/arch/riscv/kernel/sbi.c
@@ -598,8 +598,14 @@ static void sbi_send_cpumask_ipi(const struct cpumask *target)
sbi_send_ipi(cpumask_bits(&hartid_mask));
}

+static void sbi_ipi_clear(void)
+{
+ csr_clear(CSR_IP, IE_SIE);
+}
+
static const struct riscv_ipi_ops sbi_ipi_ops = {
- .ipi_inject = sbi_send_cpumask_ipi
+ .ipi_inject = sbi_send_cpumask_ipi,
+ .ipi_clear = sbi_ipi_clear
};

void __init sbi_init(void)
diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
index 921d9d7df400..547dc508f7d1 100644
--- a/arch/riscv/kernel/smp.c
+++ b/arch/riscv/kernel/smp.c
@@ -99,8 +99,6 @@ void riscv_clear_ipi(void)
{
if (ipi_ops && ipi_ops->ipi_clear)
ipi_ops->ipi_clear();
-
- csr_clear(CSR_IP, IE_SIE);
}
EXPORT_SYMBOL_GPL(riscv_clear_ipi);

--
2.25.1

2021-06-18 13:04:12

by Anup Patel

[permalink] [raw]
Subject: [RFC PATCH v2 03/11] RISC-V: Treat IPIs as normal Linux IRQs

Currently, the RISC-V kernel provides arch specific hooks (i.e.
struct riscv_ipi_ops) to register IPI handling methods. The stats
gathering of IPIs is also arch specific in the RISC-V kernel.

Other architectures (such as ARM, ARM64, and MIPS) have moved away
from custom arch specific IPI handling methods. Currently, these
architectures have Linux irqchip drivers providing a range of Linux
IRQ numbers to be used as IPIs and IPI triggering is done using
generic IPI APIs. This approach allows architectures to treat IPIs
as normal Linux IRQs and IPI stats gathering is done by the generic
Linux IRQ subsystem.

We extend the RISC-V IPI handling as-per above approach so that arch
specific IPI handling methods (struct riscv_ipi_ops) can be removed
and the IPI handling is totally contained Linux within irqchip drivers.

Signed-off-by: Anup Patel <[email protected]>
---
arch/riscv/Kconfig | 1 +
arch/riscv/include/asm/sbi.h | 2 +
arch/riscv/include/asm/smp.h | 34 +++--
arch/riscv/kernel/Makefile | 1 +
arch/riscv/kernel/cpu-hotplug.c | 2 +
arch/riscv/kernel/irq.c | 1 +
arch/riscv/kernel/sbi-ipi.c | 223 ++++++++++++++++++++++++++++++
arch/riscv/kernel/sbi.c | 21 ---
arch/riscv/kernel/smp.c | 157 ++++++++++-----------
arch/riscv/kernel/smpboot.c | 4 +-
drivers/clocksource/timer-clint.c | 23 ---
drivers/irqchip/irq-riscv-intc.c | 55 ++++----
12 files changed, 352 insertions(+), 172 deletions(-)
create mode 100644 arch/riscv/kernel/sbi-ipi.c

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index e7137d93119a..2afc9df7176f 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -50,6 +50,7 @@ config RISCV
select GENERIC_EARLY_IOREMAP
select GENERIC_GETTIMEOFDAY if HAVE_GENERIC_VDSO
select GENERIC_IOREMAP
+ select GENERIC_IRQ_IPI
select GENERIC_IRQ_MULTI_HANDLER
select GENERIC_IRQ_SHOW
select GENERIC_LIB_DEVMEM_IS_ALLOWED
diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
index 289621da4a2a..a992faeded7e 100644
--- a/arch/riscv/include/asm/sbi.h
+++ b/arch/riscv/include/asm/sbi.h
@@ -106,6 +106,7 @@ struct sbiret {
};

void sbi_init(void);
+void sbi_ipi_init(void);
struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0,
unsigned long arg1, unsigned long arg2,
unsigned long arg3, unsigned long arg4,
@@ -175,6 +176,7 @@ static inline unsigned long sbi_mk_version(unsigned long major,
int sbi_err_map_linux_errno(int err);
#else /* CONFIG_RISCV_SBI */
static inline int sbi_remote_fence_i(const unsigned long *hart_mask) { return -1; }
+static inline void sbi_ipi_init(void) { }
static inline void sbi_init(void) {}
#endif /* CONFIG_RISCV_SBI */
#endif /* _ASM_RISCV_SBI_H */
diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h
index a7d2811f3536..6bdaab122ffa 100644
--- a/arch/riscv/include/asm/smp.h
+++ b/arch/riscv/include/asm/smp.h
@@ -15,11 +15,6 @@
struct seq_file;
extern unsigned long boot_cpu_hartid;

-struct riscv_ipi_ops {
- void (*ipi_inject)(const struct cpumask *target);
- void (*ipi_clear)(void);
-};
-
#ifdef CONFIG_SMP
/*
* Mapping between linux logical cpu index and hartid.
@@ -33,9 +28,6 @@ void show_ipi_stats(struct seq_file *p, int prec);
/* SMP initialization hook for setup_arch */
void __init setup_smp(void);

-/* Called from C code, this handles an IPI. */
-void handle_IPI(struct pt_regs *regs);
-
/* Hook for the generic smp_call_function_many() routine. */
void arch_send_call_function_ipi_mask(struct cpumask *mask);

@@ -45,11 +37,17 @@ void arch_send_call_function_single_ipi(int cpu);
int riscv_hartid_to_cpuid(int hartid);
void riscv_cpuid_to_hartid_mask(const struct cpumask *in, struct cpumask *out);

-/* Set custom IPI operations */
-void riscv_set_ipi_ops(const struct riscv_ipi_ops *ops);
+/* Enable IPI for CPU hotplug */
+void riscv_ipi_enable(void);
+
+/* Disable IPI for CPU hotplug */
+void riscv_ipi_disable(void);

-/* Clear IPI for current CPU */
-void riscv_clear_ipi(void);
+/* Setup IPI for boot CPU */
+void riscv_ipi_setup(void);
+
+/* Set the IPI interrupt numbers for arch (called by irqchip drivers) */
+void riscv_ipi_set_virq_range(int virq, int nr_irqs);

/* Secondary hart entry */
asmlinkage void smp_callin(void);
@@ -92,11 +90,19 @@ static inline void riscv_cpuid_to_hartid_mask(const struct cpumask *in,
cpumask_set_cpu(boot_cpu_hartid, out);
}

-static inline void riscv_set_ipi_ops(const struct riscv_ipi_ops *ops)
+static inline void riscv_ipi_enable(void)
+{
+}
+
+static inline void riscv_ipi_disable(void)
+{
+}
+
+static inline void riscv_ipi_setup(void)
{
}

-static inline void riscv_clear_ipi(void)
+static inline void riscv_ipi_set_virq_range(int virq, int nr)
{
}

diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index d3081e4d9600..210c783c8136 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -54,6 +54,7 @@ obj-$(CONFIG_RISCV_BASE_PMU) += perf_event.o
obj-$(CONFIG_PERF_EVENTS) += perf_callchain.o
obj-$(CONFIG_HAVE_PERF_REGS) += perf_regs.o
obj-$(CONFIG_RISCV_SBI) += sbi.o
+obj-$(CONFIG_RISCV_SBI) += sbi-ipi.o
ifeq ($(CONFIG_RISCV_SBI), y)
obj-$(CONFIG_SMP) += cpu_ops_sbi.o
endif
diff --git a/arch/riscv/kernel/cpu-hotplug.c b/arch/riscv/kernel/cpu-hotplug.c
index df84e0c13db1..0f662b0113f3 100644
--- a/arch/riscv/kernel/cpu-hotplug.c
+++ b/arch/riscv/kernel/cpu-hotplug.c
@@ -13,6 +13,7 @@
#include <asm/irq.h>
#include <asm/cpu_ops.h>
#include <asm/sbi.h>
+#include <asm/smp.h>

void cpu_stop(void);
void arch_cpu_idle_dead(void)
@@ -47,6 +48,7 @@ int __cpu_disable(void)

remove_cpu_topology(cpu);
set_cpu_online(cpu, false);
+ riscv_ipi_disable();
irq_migrate_all_off_this_cpu();

return ret;
diff --git a/arch/riscv/kernel/irq.c b/arch/riscv/kernel/irq.c
index 7207fa08d78f..2817900a63e8 100644
--- a/arch/riscv/kernel/irq.c
+++ b/arch/riscv/kernel/irq.c
@@ -21,4 +21,5 @@ void __init init_IRQ(void)
irqchip_init();
if (!handle_arch_irq)
panic("No interrupt controller found.");
+ riscv_ipi_setup();
}
diff --git a/arch/riscv/kernel/sbi-ipi.c b/arch/riscv/kernel/sbi-ipi.c
new file mode 100644
index 000000000000..6cc0ea95e1af
--- /dev/null
+++ b/arch/riscv/kernel/sbi-ipi.c
@@ -0,0 +1,223 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * SBI based IPI support.
+ *
+ * Copyright (c) 2021 Western Digital Corporation or its affiliates.
+ */
+
+#define pr_fmt(fmt) "riscv-sbi-ipi: " fmt
+#include <linux/cpu.h>
+#include <linux/cpumask.h>
+#include <linux/init.h>
+#include <linux/irq.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/of.h>
+#include <linux/smp.h>
+#include <asm/sbi.h>
+
+static int intc_parent_irq __ro_after_init;
+static struct irq_domain *sbi_ipi_domain __ro_after_init;
+static DEFINE_PER_CPU(unsigned long, sbi_ipi_bits);
+
+static void sbi_ipi_dummy_mask_unmask(struct irq_data *d)
+{
+}
+
+static void sbi_ipi_send_mask(struct irq_data *d, const struct cpumask *mask)
+{
+ int cpu;
+ struct cpumask hartid_mask;
+
+ /* Barrier before doing atomic bit update to IPI bits */
+ smp_mb__before_atomic();
+ for_each_cpu(cpu, mask)
+ set_bit(d->hwirq, per_cpu_ptr(&sbi_ipi_bits, cpu));
+ /* Barrier after doing atomic bit update to IPI bits */
+ smp_mb__after_atomic();
+
+ riscv_cpuid_to_hartid_mask(mask, &hartid_mask);
+
+ sbi_send_ipi(cpumask_bits(&hartid_mask));
+}
+
+static struct irq_chip sbi_ipi_chip = {
+ .name = "RISC-V SBI IPI",
+ .irq_mask = sbi_ipi_dummy_mask_unmask,
+ .irq_unmask = sbi_ipi_dummy_mask_unmask,
+ .ipi_send_mask = sbi_ipi_send_mask,
+};
+
+static int sbi_ipi_domain_map(struct irq_domain *d, unsigned int irq,
+ irq_hw_number_t hwirq)
+{
+ irq_set_percpu_devid(irq);
+ irq_domain_set_info(d, irq, hwirq, &sbi_ipi_chip, d->host_data,
+ handle_percpu_devid_irq, NULL, NULL);
+
+ return 0;
+}
+
+static int sbi_ipi_domain_alloc(struct irq_domain *d, unsigned int virq,
+ unsigned int nr_irqs, void *arg)
+{
+ int i, ret;
+ irq_hw_number_t hwirq;
+ unsigned int type = IRQ_TYPE_NONE;
+ struct irq_fwspec *fwspec = arg;
+
+ ret = irq_domain_translate_onecell(d, fwspec, &hwirq, &type);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < nr_irqs; i++) {
+ ret = sbi_ipi_domain_map(d, virq + i, hwirq + i);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct irq_domain_ops sbi_ipi_domain_ops = {
+ .translate = irq_domain_translate_onecell,
+ .alloc = sbi_ipi_domain_alloc,
+ .free = irq_domain_free_irqs_top,
+};
+
+static void sbi_ipi_handle_irq(struct irq_desc *desc)
+{
+ int irq;
+ struct irq_chip *chip = irq_desc_get_chip(desc);
+ unsigned long irqs, *bits = this_cpu_ptr(&sbi_ipi_bits);
+ irq_hw_number_t hwirq;
+
+ chained_irq_enter(chip, desc);
+
+ csr_clear(CSR_IP, IE_SIE);
+
+ while (true) {
+ /* Order bit clearing and data access. */
+ mb();
+
+ irqs = xchg(bits, 0);
+ if (!irqs)
+ goto done;
+
+ for (hwirq = 0; hwirq < BITS_PER_LONG; hwirq++) {
+ if (!(BIT(hwirq) & irqs))
+ continue;
+
+ irq = irq_find_mapping(sbi_ipi_domain, hwirq);
+ if (unlikely(irq <= 0))
+ pr_warn_ratelimited(
+ "can't find mapping for hwirq %lu\n",
+ hwirq);
+ else
+ generic_handle_irq(irq);
+ }
+ }
+
+done:
+ chained_irq_exit(chip, desc);
+}
+
+static int sbi_ipi_dying_cpu(unsigned int cpu)
+{
+ disable_percpu_irq(intc_parent_irq);
+ return 0;
+}
+
+static int sbi_ipi_starting_cpu(unsigned int cpu)
+{
+ enable_percpu_irq(intc_parent_irq,
+ irq_get_trigger_type(intc_parent_irq));
+ return 0;
+}
+
+static int __init sbi_ipi_set_virq(void)
+{
+ int virq;
+ struct irq_fwspec ipi = {
+ .fwnode = sbi_ipi_domain->fwnode,
+ .param_count = 1,
+ .param[0] = 0,
+ };
+
+ virq = __irq_domain_alloc_irqs(sbi_ipi_domain, -1, BITS_PER_LONG,
+ NUMA_NO_NODE, &ipi,
+ false, NULL);
+ if (virq <= 0) {
+ pr_err("unable to alloc IRQs from SBI IPI IRQ domain\n");
+ return -ENOMEM;
+ }
+
+ riscv_ipi_set_virq_range(virq, BITS_PER_LONG);
+
+ return 0;
+}
+
+static int __init sbi_ipi_domain_init(struct device_node *node,
+ struct irq_domain *domain)
+{
+ struct irq_fwspec swi = {
+ .fwnode = domain->fwnode,
+ .param_count = 1,
+ .param[0] = RV_IRQ_SOFT,
+ };
+
+ intc_parent_irq = __irq_domain_alloc_irqs(domain, -1, 1,
+ NUMA_NO_NODE, &swi,
+ false, NULL);
+ if (intc_parent_irq <= 0) {
+ pr_err("unable to alloc IRQ from INTC IRQ domain\n");
+ return -ENOMEM;
+ }
+
+ irq_set_chained_handler(intc_parent_irq, sbi_ipi_handle_irq);
+
+ sbi_ipi_domain = irq_domain_add_linear(node, BITS_PER_LONG,
+ &sbi_ipi_domain_ops, NULL);
+ if (!sbi_ipi_domain) {
+ pr_err("unable to add SBI IPI IRQ domain\n");
+ return -ENOMEM;
+ }
+
+ cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
+ "irqchip/riscv/sbi-ipi:starting",
+ sbi_ipi_starting_cpu, sbi_ipi_dying_cpu);
+
+ return sbi_ipi_set_virq();
+}
+
+void __init sbi_ipi_init(void)
+{
+ struct irq_domain *domain = NULL;
+ struct device_node *cpu, *child, *node = NULL;
+
+ for_each_of_cpu_node(cpu) {
+ child = of_get_compatible_child(cpu, "riscv,cpu-intc");
+ if (!child) {
+ pr_err("failed to find INTC node [%pOF]\n", cpu);
+ return;
+ }
+
+ domain = irq_find_host(child);
+ if (domain) {
+ node = cpu;
+ break;
+ }
+
+ of_node_put(child);
+ }
+ if (!domain || !node) {
+ pr_err("can't find INTC IRQ domain\n");
+ return;
+ }
+
+ if (sbi_ipi_domain_init(node, domain))
+ pr_err("failed to register IPI domain\n");
+ else
+ pr_info("registered IPI domain\n");
+}
diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c
index 8aeca26198f2..372aa7e181d5 100644
--- a/arch/riscv/kernel/sbi.c
+++ b/arch/riscv/kernel/sbi.c
@@ -589,25 +589,6 @@ long sbi_get_mimpid(void)
return __sbi_base_ecall(SBI_EXT_BASE_GET_MIMPID);
}

-static void sbi_send_cpumask_ipi(const struct cpumask *target)
-{
- struct cpumask hartid_mask;
-
- riscv_cpuid_to_hartid_mask(target, &hartid_mask);
-
- sbi_send_ipi(cpumask_bits(&hartid_mask));
-}
-
-static void sbi_ipi_clear(void)
-{
- csr_clear(CSR_IP, IE_SIE);
-}
-
-static const struct riscv_ipi_ops sbi_ipi_ops = {
- .ipi_inject = sbi_send_cpumask_ipi,
- .ipi_clear = sbi_ipi_clear
-};
-
void __init sbi_init(void)
{
int ret;
@@ -654,6 +635,4 @@ void __init sbi_init(void)
__sbi_send_ipi = __sbi_send_ipi_v01;
__sbi_rfence = __sbi_rfence_v01;
}
-
- riscv_set_ipi_ops(&sbi_ipi_ops);
}
diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
index eea0c9d11d9f..26d563615f53 100644
--- a/arch/riscv/kernel/smp.c
+++ b/arch/riscv/kernel/smp.c
@@ -18,6 +18,7 @@
#include <linux/sched.h>
#include <linux/seq_file.h>
#include <linux/delay.h>
+#include <linux/irq.h>
#include <linux/irq_work.h>

#include <asm/sbi.h>
@@ -42,11 +43,9 @@ void __init smp_setup_processor_id(void)
cpuid_to_hartid_map(0) = boot_cpu_hartid;
}

-/* A collection of single bit ipi messages. */
-static struct {
- unsigned long stats[IPI_MAX] ____cacheline_aligned;
- unsigned long bits ____cacheline_aligned;
-} ipi_data[NR_CPUS] __cacheline_aligned;
+static int ipi_virq_base __ro_after_init;
+static int nr_ipi __ro_after_init = IPI_MAX;
+static struct irq_desc *ipi_desc[IPI_MAX] __read_mostly;

int riscv_hartid_to_cpuid(int hartid)
{
@@ -88,46 +87,14 @@ static void ipi_stop(void)
wait_for_interrupt();
}

-static const struct riscv_ipi_ops *ipi_ops __ro_after_init;
-
-void riscv_set_ipi_ops(const struct riscv_ipi_ops *ops)
-{
- ipi_ops = ops;
-}
-EXPORT_SYMBOL_GPL(riscv_set_ipi_ops);
-
-void riscv_clear_ipi(void)
-{
- if (ipi_ops && ipi_ops->ipi_clear)
- ipi_ops->ipi_clear();
-}
-EXPORT_SYMBOL_GPL(riscv_clear_ipi);
-
static void send_ipi_mask(const struct cpumask *mask, enum ipi_message_type op)
{
- int cpu;
-
- smp_mb__before_atomic();
- for_each_cpu(cpu, mask)
- set_bit(op, &ipi_data[cpu].bits);
- smp_mb__after_atomic();
-
- if (ipi_ops && ipi_ops->ipi_inject)
- ipi_ops->ipi_inject(mask);
- else
- pr_warn("IPI inject method not available\n");
+ __ipi_send_mask(ipi_desc[op], mask);
}

static void send_ipi_single(int cpu, enum ipi_message_type op)
{
- smp_mb__before_atomic();
- set_bit(op, &ipi_data[cpu].bits);
- smp_mb__after_atomic();
-
- if (ipi_ops && ipi_ops->ipi_inject)
- ipi_ops->ipi_inject(cpumask_of(cpu));
- else
- pr_warn("IPI inject method not available\n");
+ __ipi_send_mask(ipi_desc[op], cpumask_of(cpu));
}

#ifdef CONFIG_IRQ_WORK
@@ -137,62 +104,90 @@ void arch_irq_work_raise(void)
}
#endif

-void handle_IPI(struct pt_regs *regs)
+static irqreturn_t handle_IPI(int irq, void *data)
{
- struct pt_regs *old_regs = set_irq_regs(regs);
- unsigned long *pending_ipis = &ipi_data[smp_processor_id()].bits;
- unsigned long *stats = ipi_data[smp_processor_id()].stats;
+ int ipi = irq - ipi_virq_base;
+
+ switch (ipi) {
+ case IPI_RESCHEDULE:
+ scheduler_ipi();
+ break;
+ case IPI_CALL_FUNC:
+ generic_smp_call_function_interrupt();
+ break;
+ case IPI_CPU_STOP:
+ ipi_stop();
+ break;
+ case IPI_IRQ_WORK:
+ irq_work_run();
+ break;
+#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
+ case IPI_TIMER:
+ tick_receive_broadcast();
+ break;
+#endif
+ default:
+ pr_warn("CPU%d: unhandled IPI%d\n", smp_processor_id(), ipi);
+ break;
+ };

- irq_enter();
+ return IRQ_HANDLED;
+}

- riscv_clear_ipi();
+void riscv_ipi_enable(void)
+{
+ int i;

- while (true) {
- unsigned long ops;
+ if (WARN_ON_ONCE(!ipi_virq_base))
+ return;

- /* Order bit clearing and data access. */
- mb();
+ for (i = 0; i < nr_ipi; i++)
+ enable_percpu_irq(ipi_virq_base + i, 0);
+}

- ops = xchg(pending_ipis, 0);
- if (ops == 0)
- goto done;
+void riscv_ipi_disable(void)
+{
+ int i;

- if (ops & (1 << IPI_RESCHEDULE)) {
- stats[IPI_RESCHEDULE]++;
- scheduler_ipi();
- }
+ if (WARN_ON_ONCE(!ipi_virq_base))
+ return;

- if (ops & (1 << IPI_CALL_FUNC)) {
- stats[IPI_CALL_FUNC]++;
- generic_smp_call_function_interrupt();
- }
+ for (i = 0; i < nr_ipi; i++)
+ disable_percpu_irq(ipi_virq_base + i);
+}

- if (ops & (1 << IPI_CPU_STOP)) {
- stats[IPI_CPU_STOP]++;
- ipi_stop();
- }
+void riscv_ipi_setup(void)
+{
+ int i, err;

- if (ops & (1 << IPI_IRQ_WORK)) {
- stats[IPI_IRQ_WORK]++;
- irq_work_run();
- }
+ /* SBI based IPIs is our last option */
+ if (!ipi_virq_base)
+ sbi_ipi_init();

-#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
- if (ops & (1 << IPI_TIMER)) {
- stats[IPI_TIMER]++;
- tick_receive_broadcast();
- }
-#endif
- BUG_ON((ops >> IPI_MAX) != 0);
+ /* Request IPIs */
+ for (i = 0; i < nr_ipi; i++) {
+ err = request_percpu_irq(ipi_virq_base + i, handle_IPI,
+ "IPI", &ipi_virq_base);
+ WARN_ON(err);

- /* Order data access and bit testing. */
- mb();
+ ipi_desc[i] = irq_to_desc(ipi_virq_base + i);
+ irq_set_status_flags(ipi_virq_base + i, IRQ_HIDDEN);
}

-done:
- irq_exit();
- set_irq_regs(old_regs);
+ /* Enabled IPIs for boot CPU immediately */
+ riscv_ipi_enable();
+}
+
+void riscv_ipi_set_virq_range(int virq, int nr)
+{
+ if (WARN_ON(ipi_virq_base))
+ return;
+
+ WARN_ON(nr < IPI_MAX);
+ nr_ipi = min(nr, IPI_MAX);
+ ipi_virq_base = virq;
}
+EXPORT_SYMBOL_GPL(riscv_ipi_set_virq_range);

static const char * const ipi_names[] = {
[IPI_RESCHEDULE] = "Rescheduling interrupts",
@@ -210,7 +205,7 @@ void show_ipi_stats(struct seq_file *p, int prec)
seq_printf(p, "%*s%u:%s", prec - 1, "IPI", i,
prec >= 4 ? " " : "");
for_each_online_cpu(cpu)
- seq_printf(p, "%10lu ", ipi_data[cpu].stats[i]);
+ seq_printf(p, "%10u ", irq_desc_kstat_cpu(ipi_desc[i], cpu));
seq_printf(p, " %s\n", ipi_names[i]);
}
}
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 9a408e2942ac..b181d981a960 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -159,12 +159,12 @@ asmlinkage __visible void smp_callin(void)
struct mm_struct *mm = &init_mm;
unsigned int curr_cpuid = smp_processor_id();

- riscv_clear_ipi();
-
/* All kernel threads share the same mm context. */
mmgrab(mm);
current->active_mm = mm;

+ riscv_ipi_enable();
+
notify_cpu_starting(curr_cpuid);
numa_add_cpu(curr_cpuid);
update_siblings_masks(curr_cpuid);
diff --git a/drivers/clocksource/timer-clint.c b/drivers/clocksource/timer-clint.c
index 6cfe2ab73eb0..3b68ed53fe4a 100644
--- a/drivers/clocksource/timer-clint.c
+++ b/drivers/clocksource/timer-clint.c
@@ -30,7 +30,6 @@
#define CLINT_TIMER_VAL_OFF 0xbff8

/* CLINT manages IPI and Timer for RISC-V M-mode */
-static u32 __iomem *clint_ipi_base;
static u64 __iomem *clint_timer_cmp;
static u64 __iomem *clint_timer_val;
static unsigned long clint_timer_freq;
@@ -41,24 +40,6 @@ u64 __iomem *clint_time_val;
EXPORT_SYMBOL(clint_time_val);
#endif

-static void clint_send_ipi(const struct cpumask *target)
-{
- unsigned int cpu;
-
- for_each_cpu(cpu, target)
- writel(1, clint_ipi_base + cpuid_to_hartid_map(cpu));
-}
-
-static void clint_clear_ipi(void)
-{
- writel(0, clint_ipi_base + cpuid_to_hartid_map(smp_processor_id()));
-}
-
-static struct riscv_ipi_ops clint_ipi_ops = {
- .ipi_inject = clint_send_ipi,
- .ipi_clear = clint_clear_ipi,
-};
-
#ifdef CONFIG_64BIT
#define clint_get_cycles() readq_relaxed(clint_timer_val)
#else
@@ -189,7 +170,6 @@ static int __init clint_timer_init_dt(struct device_node *np)
return -ENODEV;
}

- clint_ipi_base = base + CLINT_IPI_OFF;
clint_timer_cmp = base + CLINT_TIMER_CMP_OFF;
clint_timer_val = base + CLINT_TIMER_VAL_OFF;
clint_timer_freq = riscv_timebase;
@@ -228,9 +208,6 @@ static int __init clint_timer_init_dt(struct device_node *np)
goto fail_free_irq;
}

- riscv_set_ipi_ops(&clint_ipi_ops);
- clint_clear_ipi();
-
return 0;

fail_free_irq:
diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
index 8017f6d32d52..65d9c5b0ddb8 100644
--- a/drivers/irqchip/irq-riscv-intc.c
+++ b/drivers/irqchip/irq-riscv-intc.c
@@ -26,20 +26,7 @@ static asmlinkage void riscv_intc_irq(struct pt_regs *regs)
if (unlikely(cause >= BITS_PER_LONG))
panic("unexpected interrupt cause");

- switch (cause) {
-#ifdef CONFIG_SMP
- case RV_IRQ_SOFT:
- /*
- * We only use software interrupts to pass IPIs, so if a
- * non-SMP system gets one, then we don't know what to do.
- */
- handle_IPI(regs);
- break;
-#endif
- default:
- handle_domain_irq(intc_domain, cause, regs);
- break;
- }
+ handle_domain_irq(intc_domain, cause, regs);
}

/*
@@ -59,18 +46,6 @@ static void riscv_intc_irq_unmask(struct irq_data *d)
csr_set(CSR_IE, BIT(d->hwirq));
}

-static int riscv_intc_cpu_starting(unsigned int cpu)
-{
- csr_set(CSR_IE, BIT(RV_IRQ_SOFT));
- return 0;
-}
-
-static int riscv_intc_cpu_dying(unsigned int cpu)
-{
- csr_clear(CSR_IE, BIT(RV_IRQ_SOFT));
- return 0;
-}
-
static struct irq_chip riscv_intc_chip = {
.name = "RISC-V INTC",
.irq_mask = riscv_intc_irq_mask,
@@ -87,9 +62,32 @@ static int riscv_intc_domain_map(struct irq_domain *d, unsigned int irq,
return 0;
}

+static int riscv_intc_domain_alloc(struct irq_domain *domain,
+ unsigned int virq, unsigned int nr_irqs,
+ void *arg)
+{
+ int i, ret;
+ irq_hw_number_t hwirq;
+ unsigned int type = IRQ_TYPE_NONE;
+ struct irq_fwspec *fwspec = arg;
+
+ ret = irq_domain_translate_onecell(domain, fwspec, &hwirq, &type);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < nr_irqs; i++) {
+ ret = riscv_intc_domain_map(domain, virq + i, hwirq + i);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
static const struct irq_domain_ops riscv_intc_domain_ops = {
.map = riscv_intc_domain_map,
.xlate = irq_domain_xlate_onecell,
+ .alloc = riscv_intc_domain_alloc
};

static int __init riscv_intc_init(struct device_node *node,
@@ -125,11 +123,6 @@ static int __init riscv_intc_init(struct device_node *node,
return rc;
}

- cpuhp_setup_state(CPUHP_AP_IRQ_RISCV_STARTING,
- "irqchip/riscv/intc:starting",
- riscv_intc_cpu_starting,
- riscv_intc_cpu_dying);
-
pr_info("%d local interrupts mapped\n", BITS_PER_LONG);

return 0;
--
2.25.1

2021-06-18 13:04:25

by Anup Patel

[permalink] [raw]
Subject: [RFC PATCH v2 04/11] RISC-V: Allow marking IPIs as suitable for remote FENCEs

To do remote FENCEs (i.e. remote TLB flushes) using IPI calls on
the RISC-V kernel, we need hardware mechanism to directly inject
IPI from the RISC-V kernel instead of using SBI calls.

The upcoming ACLINT [M|S]SWI devices and AIA IMSIC devices allow
direct IPI injection from the RISC-V kernel. To support this, we
extend the riscv_ipi_set_virq_range() function so that irqchip
drivers can mark IPIs as suitable for remote FENCEs.

Signed-off-by: Anup Patel <[email protected]>
---
arch/riscv/include/asm/smp.h | 18 ++++++++++++++++--
arch/riscv/kernel/sbi-ipi.c | 2 +-
arch/riscv/kernel/smp.c | 9 ++++++++-
3 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h
index 6bdaab122ffa..f4856c911335 100644
--- a/arch/riscv/include/asm/smp.h
+++ b/arch/riscv/include/asm/smp.h
@@ -16,6 +16,9 @@ struct seq_file;
extern unsigned long boot_cpu_hartid;

#ifdef CONFIG_SMP
+
+#include <linux/jump_label.h>
+
/*
* Mapping between linux logical cpu index and hartid.
*/
@@ -47,7 +50,12 @@ void riscv_ipi_disable(void);
void riscv_ipi_setup(void);

/* Set the IPI interrupt numbers for arch (called by irqchip drivers) */
-void riscv_ipi_set_virq_range(int virq, int nr_irqs);
+void riscv_ipi_set_virq_range(int virq, int nr_irqs, bool use_for_rfence);
+
+/* Check if we can use IPIs for remote FENCEs */
+DECLARE_STATIC_KEY_FALSE(riscv_ipi_for_rfence);
+#define riscv_use_ipi_for_rfence() \
+ static_branch_unlikely(&riscv_ipi_for_rfence)

/* Secondary hart entry */
asmlinkage void smp_callin(void);
@@ -102,10 +110,16 @@ static inline void riscv_ipi_setup(void)
{
}

-static inline void riscv_ipi_set_virq_range(int virq, int nr)
+static inline void riscv_ipi_set_virq_range(int virq, int nr,
+ bool use_for_rfence)
{
}

+static inline bool riscv_use_ipi_for_rfence(void)
+{
+ return false;
+}
+
#endif /* CONFIG_SMP */

#if defined(CONFIG_HOTPLUG_CPU) && (CONFIG_SMP)
diff --git a/arch/riscv/kernel/sbi-ipi.c b/arch/riscv/kernel/sbi-ipi.c
index 6cc0ea95e1af..0a06542d2b74 100644
--- a/arch/riscv/kernel/sbi-ipi.c
+++ b/arch/riscv/kernel/sbi-ipi.c
@@ -153,7 +153,7 @@ static int __init sbi_ipi_set_virq(void)
return -ENOMEM;
}

- riscv_ipi_set_virq_range(virq, BITS_PER_LONG);
+ riscv_ipi_set_virq_range(virq, BITS_PER_LONG, false);

return 0;
}
diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
index 26d563615f53..3905177b6748 100644
--- a/arch/riscv/kernel/smp.c
+++ b/arch/riscv/kernel/smp.c
@@ -178,7 +178,10 @@ void riscv_ipi_setup(void)
riscv_ipi_enable();
}

-void riscv_ipi_set_virq_range(int virq, int nr)
+DEFINE_STATIC_KEY_FALSE(riscv_ipi_for_rfence);
+EXPORT_SYMBOL_GPL(riscv_ipi_for_rfence);
+
+void riscv_ipi_set_virq_range(int virq, int nr, bool use_for_rfence)
{
if (WARN_ON(ipi_virq_base))
return;
@@ -186,6 +189,10 @@ void riscv_ipi_set_virq_range(int virq, int nr)
WARN_ON(nr < IPI_MAX);
nr_ipi = min(nr, IPI_MAX);
ipi_virq_base = virq;
+ if (use_for_rfence)
+ static_branch_enable(&riscv_ipi_for_rfence);
+ else
+ static_branch_disable(&riscv_ipi_for_rfence);
}
EXPORT_SYMBOL_GPL(riscv_ipi_set_virq_range);

--
2.25.1

2021-06-18 13:04:26

by Anup Patel

[permalink] [raw]
Subject: [RFC PATCH v2 06/11] dt-bindings: interrupt-controller: Add ACLINT MSWI and SSWI bindings

We add DT bindings documentation for the ACLINT MSWI and SSWI
devices found on RISC-V SOCs.

Signed-off-by: Anup Patel <[email protected]>
Reviewed-by: Bin Meng <[email protected]>
---
.../riscv,aclint-swi.yaml | 82 +++++++++++++++++++
1 file changed, 82 insertions(+)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,aclint-swi.yaml

diff --git a/Documentation/devicetree/bindings/interrupt-controller/riscv,aclint-swi.yaml b/Documentation/devicetree/bindings/interrupt-controller/riscv,aclint-swi.yaml
new file mode 100644
index 000000000000..b74025542866
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,aclint-swi.yaml
@@ -0,0 +1,82 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/riscv,aclint-swi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: RISC-V ACLINT Software Interrupt Devices
+
+maintainers:
+ - Anup Patel <[email protected]>
+
+description:
+ RISC-V SOCs include an implementation of the M-level software interrupt
+ (MSWI) device and the S-level software interrupt (SSWI) device defined
+ in the RISC-V Advanced Core Local Interruptor (ACLINT) specification.
+
+ The ACLINT MSWI and SSWI devices are documented in the RISC-V ACLINT
+ specification located at
+ https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc.
+
+ The ACLINT MSWI and SSWI devices directly connect to the M-level and
+ S-level software interrupt lines of various HARTs (or CPUs) respectively
+ so the RISC-V per-HART (or per-CPU) local interrupt controller is the
+ parent interrupt controller for the ACLINT MSWI and SSWI devices.
+
+allOf:
+ - $ref: /schemas/interrupt-controller.yaml#
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - riscv,aclint-mswi
+ - riscv,aclint-sswi
+
+ description:
+ Should be "<vendor>,<chip>-aclint-mswi" and "riscv,aclint-mswi" OR
+ "<vendor>,<chip>-aclint-sswi" and "riscv,aclint-sswi".
+
+ reg:
+ maxItems: 1
+
+ "#interrupt-cells":
+ const: 0
+
+ interrupts-extended:
+ minItems: 1
+
+ interrupt-controller: true
+
+additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - interrupts-extended
+ - interrupt-controller
+ - "#interrupt-cells"
+
+examples:
+ - |
+ // Example 1 (RISC-V MSWI device used by Linux RISC-V NoMMU kernel):
+
+ interrupt-controller@2000000 {
+ compatible = "riscv,aclint-mswi";
+ interrupts-extended = <&cpu1intc 3 &cpu2intc 3 &cpu3intc 3 &cpu4intc 3>;
+ reg = <0x2000000 0x4000>;
+ interrupt-controller;
+ #interrupt-cells = <0>;
+ };
+
+ - |
+ // Example 2 (RISC-V SSWI device used by Linux RISC-V MMU kernel):
+
+ interrupt-controller@2100000 {
+ compatible = "riscv,aclint-sswi";
+ interrupts-extended = <&cpu1intc 1 &cpu2intc 1 &cpu3intc 1 &cpu4intc 1>;
+ reg = <0x2100000 0x4000>;
+ interrupt-controller;
+ #interrupt-cells = <0>;
+ };
+...
--
2.25.1

2021-06-18 13:04:27

by Anup Patel

[permalink] [raw]
Subject: [RFC PATCH v2 05/11] RISC-V: Use IPIs for remote TLB flush when possible

If IPI calls are injected using SBI IPI calls then remote TLB flush
using SBI RFENCE calls is much faster because using IPIs for remote
TLB flush would still endup as SBI IPI calls with extra processing
on kernel side.

It is now possible to have specialized hardware (such as RISC-V AIA)
which allows S-mode software to directly inject IPIs without any
assistance from M-mode runtime firmware.

This patch extends remote TLB flush functions to use IPIs whenever
underlying IPI operations are suitable for remote FENCEs.

Signed-off-by: Anup Patel <[email protected]>
---
arch/riscv/mm/tlbflush.c | 62 +++++++++++++++++++++++++++++++---------
1 file changed, 48 insertions(+), 14 deletions(-)

diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
index 720b443c4528..009c56fa102d 100644
--- a/arch/riscv/mm/tlbflush.c
+++ b/arch/riscv/mm/tlbflush.c
@@ -1,39 +1,73 @@
// SPDX-License-Identifier: GPL-2.0
+/*
+ * TLB flush implementation.
+ *
+ * Copyright (c) 2021 Western Digital Corporation or its affiliates.
+ */

#include <linux/mm.h>
#include <linux/smp.h>
#include <linux/sched.h>
#include <asm/sbi.h>

+static void ipi_flush_tlb_all(void *info)
+{
+ local_flush_tlb_all();
+}
+
void flush_tlb_all(void)
{
- sbi_remote_sfence_vma(NULL, 0, -1);
+ if (!riscv_use_ipi_for_rfence())
+ sbi_remote_sfence_vma(NULL, 0, -1);
+ else
+ on_each_cpu(ipi_flush_tlb_all, NULL, 1);
+}
+
+struct flush_range_data {
+ unsigned long start;
+ unsigned long size;
+};
+
+static void ipi_flush_range(void *info)
+{
+ struct flush_range_data *data = info;
+
+ /* local cpu is the only cpu present in cpumask */
+ if (data->size <= PAGE_SIZE)
+ local_flush_tlb_page(data->start);
+ else
+ local_flush_tlb_all();
}

/*
- * This function must not be called with cmask being null.
+ * This function must not be called with NULL cpumask.
* Kernel may panic if cmask is NULL.
*/
-static void __sbi_tlb_flush_range(struct cpumask *cmask, unsigned long start,
- unsigned long size)
+static void flush_range(struct cpumask *cmask, unsigned long start,
+ unsigned long size)
{
+ struct flush_range_data info;
struct cpumask hmask;
unsigned int cpuid;

if (cpumask_empty(cmask))
return;

+ info.start = start;
+ info.size = size;
+
cpuid = get_cpu();

if (cpumask_any_but(cmask, cpuid) >= nr_cpu_ids) {
- /* local cpu is the only cpu present in cpumask */
- if (size <= PAGE_SIZE)
- local_flush_tlb_page(start);
- else
- local_flush_tlb_all();
+ ipi_flush_range(&info);
} else {
- riscv_cpuid_to_hartid_mask(cmask, &hmask);
- sbi_remote_sfence_vma(cpumask_bits(&hmask), start, size);
+ if (!riscv_use_ipi_for_rfence()) {
+ riscv_cpuid_to_hartid_mask(cmask, &hmask);
+ sbi_remote_sfence_vma(cpumask_bits(&hmask),
+ start, size);
+ } else {
+ on_each_cpu_mask(cmask, ipi_flush_range, &info, 1);
+ }
}

put_cpu();
@@ -41,16 +75,16 @@ static void __sbi_tlb_flush_range(struct cpumask *cmask, unsigned long start,

void flush_tlb_mm(struct mm_struct *mm)
{
- __sbi_tlb_flush_range(mm_cpumask(mm), 0, -1);
+ flush_range(mm_cpumask(mm), 0, -1);
}

void flush_tlb_page(struct vm_area_struct *vma, unsigned long addr)
{
- __sbi_tlb_flush_range(mm_cpumask(vma->vm_mm), addr, PAGE_SIZE);
+ flush_range(mm_cpumask(vma->vm_mm), addr, PAGE_SIZE);
}

void flush_tlb_range(struct vm_area_struct *vma, unsigned long start,
unsigned long end)
{
- __sbi_tlb_flush_range(mm_cpumask(vma->vm_mm), start, end - start);
+ flush_range(mm_cpumask(vma->vm_mm), start, end - start);
}
--
2.25.1

2021-06-18 13:07:05

by Anup Patel

[permalink] [raw]
Subject: [RFC PATCH v2 07/11] irqchip: Add ACLINT software interrupt driver

The RISC-V ACLINT provides MSWI and SSWI devices for M-mode and
S-mode software interrupts respectively. We add irqchip driver
which provide IPI operations based on ACLINT [M|S]SWI devices
to the Linux RISC-V kernel.

Signed-off-by: Anup Patel <[email protected]>
---
drivers/irqchip/Kconfig | 11 ++
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-aclint-swi.c | 271 +++++++++++++++++++++++++++++++
3 files changed, 283 insertions(+)
create mode 100644 drivers/irqchip/irq-aclint-swi.c

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 62543a4eccc0..2010d493b03b 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -508,6 +508,17 @@ config RISCV_INTC

If you don't know what to do here, say Y.

+config RISCV_ACLINT_SWI
+ bool "RISC-V Advanced Core Local Interruptor Software Interrupts"
+ depends on RISCV
+ help
+ This enables support for software interrupts using the Advanced
+ Core Local Interruptor (ACLINT) found in RISC-V systems. The
+ RISC-V ACLINT provides devices for inter-process interrupt and
+ timer functionality.
+
+ If you don't know what to do here, say Y.
+
config SIFIVE_PLIC
bool "SiFive Platform-Level Interrupt Controller"
depends on RISCV
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index f88cbf36a9d2..a6edf6733c1d 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -97,6 +97,7 @@ obj-$(CONFIG_QCOM_PDC) += qcom-pdc.o
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_ACLINT_SWI) += irq-aclint-swi.o
obj-$(CONFIG_SIFIVE_PLIC) += irq-sifive-plic.o
obj-$(CONFIG_IMX_IRQSTEER) += irq-imx-irqsteer.o
obj-$(CONFIG_IMX_INTMUX) += irq-imx-intmux.o
diff --git a/drivers/irqchip/irq-aclint-swi.c b/drivers/irqchip/irq-aclint-swi.c
new file mode 100644
index 000000000000..a31a7fc504d1
--- /dev/null
+++ b/drivers/irqchip/irq-aclint-swi.c
@@ -0,0 +1,271 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2021 Western Digital Corporation or its affiliates.
+ */
+
+#define pr_fmt(fmt) "aclint-swi: " fmt
+#include <linux/cpu.h>
+#include <linux/cpumask.h>
+#include <linux/io.h>
+#include <linux/init.h>
+#include <linux/irq.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/smp.h>
+
+struct aclint_swi {
+ void __iomem *sip_reg;
+ unsigned long bits;
+};
+
+static int aclint_swi_parent_irq __ro_after_init;
+static struct irq_domain *aclint_swi_domain __ro_after_init;
+static DEFINE_PER_CPU(struct aclint_swi, aclint_swis);
+
+static void aclint_swi_dummy_mask_unmask(struct irq_data *d)
+{
+}
+
+static void aclint_swi_send_mask(struct irq_data *d,
+ const struct cpumask *mask)
+{
+ int cpu;
+ struct aclint_swi *swi;
+
+ /* Barrier before doing atomic bit update to IPI bits */
+ smp_mb__before_atomic();
+
+ for_each_cpu(cpu, mask) {
+ swi = per_cpu_ptr(&aclint_swis, cpu);
+ set_bit(d->hwirq, &swi->bits);
+ writel(1, swi->sip_reg);
+ }
+
+ /* Barrier after doing atomic bit update to IPI bits */
+ smp_mb__after_atomic();
+}
+
+static struct irq_chip aclint_swi_chip = {
+ .name = "RISC-V ACLINT SWI",
+ .irq_mask = aclint_swi_dummy_mask_unmask,
+ .irq_unmask = aclint_swi_dummy_mask_unmask,
+ .ipi_send_mask = aclint_swi_send_mask,
+};
+
+static int aclint_swi_domain_map(struct irq_domain *d, unsigned int irq,
+ irq_hw_number_t hwirq)
+{
+ irq_set_percpu_devid(irq);
+ irq_domain_set_info(d, irq, hwirq, &aclint_swi_chip, d->host_data,
+ handle_percpu_devid_irq, NULL, NULL);
+
+ return 0;
+}
+
+static int aclint_swi_domain_alloc(struct irq_domain *d, unsigned int virq,
+ unsigned int nr_irqs, void *arg)
+{
+ int i, ret;
+ irq_hw_number_t hwirq;
+ unsigned int type = IRQ_TYPE_NONE;
+ struct irq_fwspec *fwspec = arg;
+
+ ret = irq_domain_translate_onecell(d, fwspec, &hwirq, &type);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < nr_irqs; i++) {
+ ret = aclint_swi_domain_map(d, virq + i, hwirq + i);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct irq_domain_ops aclint_swi_domain_ops = {
+ .translate = irq_domain_translate_onecell,
+ .alloc = aclint_swi_domain_alloc,
+ .free = irq_domain_free_irqs_top,
+};
+
+static void aclint_swi_handle_irq(struct irq_desc *desc)
+{
+ int irq;
+ unsigned long irqs;
+ irq_hw_number_t hwirq;
+ struct irq_chip *chip = irq_desc_get_chip(desc);
+ struct aclint_swi *swi = this_cpu_ptr(&aclint_swis);
+
+ chained_irq_enter(chip, desc);
+
+ writel(0, swi->sip_reg);
+
+ while (true) {
+ /* Order bit clearing and data access. */
+ mb();
+
+ irqs = xchg(&swi->bits, 0);
+ if (!irqs)
+ goto done;
+
+ for (hwirq = 0; hwirq < BITS_PER_LONG; hwirq++) {
+ if (!(BIT(hwirq) & irqs))
+ continue;
+
+ irq = irq_find_mapping(aclint_swi_domain, hwirq);
+ if (unlikely(irq <= 0))
+ pr_warn_ratelimited(
+ "can't find mapping for hwirq %lu\n",
+ hwirq);
+ else
+ generic_handle_irq(irq);
+ }
+ }
+
+done:
+ chained_irq_exit(chip, desc);
+}
+
+static int aclint_swi_dying_cpu(unsigned int cpu)
+{
+ disable_percpu_irq(aclint_swi_parent_irq);
+ return 0;
+}
+
+static int aclint_swi_starting_cpu(unsigned int cpu)
+{
+ enable_percpu_irq(aclint_swi_parent_irq,
+ irq_get_trigger_type(aclint_swi_parent_irq));
+ return 0;
+}
+
+static int __init aclint_swi_set_virq(void)
+{
+ int virq;
+ struct irq_fwspec ipi = {
+ .fwnode = aclint_swi_domain->fwnode,
+ .param_count = 1,
+ .param[0] = 0,
+ };
+
+ virq = __irq_domain_alloc_irqs(aclint_swi_domain, -1, BITS_PER_LONG,
+ NUMA_NO_NODE, &ipi,
+ false, NULL);
+ if (virq <= 0) {
+ pr_err("unable to alloc IRQs from SBI IPI IRQ domain\n");
+ return -ENOMEM;
+ }
+
+ riscv_ipi_set_virq_range(virq, BITS_PER_LONG, true);
+
+ return 0;
+}
+
+static int __init aclint_swi_domain_init(struct device_node *node)
+{
+ /*
+ * We can have multiple ACLINT SWI devices but we only need
+ * one IRQ domain for providing per-HART (or per-CPU) IPIs.
+ */
+ if (aclint_swi_domain)
+ return 0;
+
+ aclint_swi_domain = irq_domain_add_linear(node, BITS_PER_LONG,
+ &aclint_swi_domain_ops, NULL);
+ if (!aclint_swi_domain) {
+ pr_err("unable to add ACLINT SWI IRQ domain\n");
+ return -ENOMEM;
+ }
+
+ return aclint_swi_set_virq();
+}
+
+static int __init aclint_swi_init(struct device_node *node,
+ struct device_node *parent)
+{
+ int rc;
+ void __iomem *base;
+ struct aclint_swi *swi;
+ u32 i, nr_irqs, nr_cpus = 0;
+
+ /* Map the registers */
+ base = of_iomap(node, 0);
+ if (!base) {
+ pr_err("%pOFP: could not map registers\n", node);
+ return -ENODEV;
+ }
+
+ /* Iterarte over each target CPU connected with this ACLINT */
+ nr_irqs = of_irq_count(node);
+ for (i = 0; i < nr_irqs; i++) {
+ struct of_phandle_args parent;
+ int cpu, hartid;
+
+ if (of_irq_parse_one(node, i, &parent)) {
+ pr_err("%pOFP: failed to parse irq %d.\n",
+ node, i);
+ continue;
+ }
+
+ if (parent.args[0] != RV_IRQ_SOFT) {
+ pr_err("%pOFP: invalid irq %d (hwirq %d)\n",
+ node, i, parent.args[0]);
+ continue;
+ }
+
+ hartid = riscv_of_parent_hartid(parent.np);
+ if (hartid < 0) {
+ pr_warn("failed to parse hart ID for irq %d.\n", i);
+ continue;
+ }
+
+ cpu = riscv_hartid_to_cpuid(hartid);
+ if (cpu < 0) {
+ pr_warn("Invalid cpuid for irq %d\n", i);
+ continue;
+ }
+
+ /* Find parent domain and register chained handler */
+ if (!aclint_swi_parent_irq && irq_find_host(parent.np)) {
+ aclint_swi_parent_irq = irq_of_parse_and_map(node, i);
+ if (aclint_swi_parent_irq) {
+ irq_set_chained_handler(aclint_swi_parent_irq,
+ aclint_swi_handle_irq);
+ cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
+ "irqchip/riscv/aclint-swi:starting",
+ aclint_swi_starting_cpu,
+ aclint_swi_dying_cpu);
+ }
+ }
+
+ swi = per_cpu_ptr(&aclint_swis, cpu);
+ swi->sip_reg = base + i * sizeof(u32);
+ writel(0, swi->sip_reg);
+
+ nr_cpus++;
+ }
+
+ /* Create the IPI domain for ACLINT SWI device */
+ rc = aclint_swi_domain_init(node);
+ if (rc)
+ return rc;
+
+ /* Announce the ACLINT SWI device */
+ pr_info("%pOFP: providing IPIs for %d CPUs\n", node, nr_cpus);
+
+ return 0;
+}
+
+#ifdef CONFIG_RISCV_M_MODE
+IRQCHIP_DECLARE(riscv_aclint_swi, "riscv,clint0", aclint_swi_init);
+IRQCHIP_DECLARE(riscv_aclint_swi1, "sifive,clint0", aclint_swi_init);
+IRQCHIP_DECLARE(riscv_aclint_swi2, "riscv,aclint-mswi", aclint_swi_init);
+#else
+IRQCHIP_DECLARE(riscv_aclint_swi, "riscv,aclint-sswi", aclint_swi_init);
+#endif
--
2.25.1

2021-06-18 13:07:13

by Anup Patel

[permalink] [raw]
Subject: [RFC PATCH v2 08/11] RISC-V: Select ACLINT SWI driver for virt machine

The QEMU virt machine has provision to emulate ACLINT SWI device
for supervisor-mode so let's select corresponding driver from
SOC_VIRT kconfig option.

Signed-off-by: Anup Patel <[email protected]>
Reviewed-by: Bin Meng <[email protected]>
---
arch/riscv/Kconfig.socs | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
index ed963761fbd2..2687a0902ec4 100644
--- a/arch/riscv/Kconfig.socs
+++ b/arch/riscv/Kconfig.socs
@@ -27,6 +27,7 @@ config SOC_VIRT
select GOLDFISH
select RTC_DRV_GOLDFISH if RTC_CLASS
select SIFIVE_PLIC
+ select RISCV_ACLINT_SWI
help
This enables support for QEMU Virt Machine.

--
2.25.1

2021-06-18 13:07:24

by Anup Patel

[permalink] [raw]
Subject: [RFC PATCH v2 10/11] clocksource: clint: Add support for ACLINT MTIMER device

The RISC-V ACLINT specification is a modular specification and the
ACLINT MTIMER device is backward compatible with the M-mode timer
functionality of the CLINT device. This patch extends the CLINT
timer driver to support both CLINT device and ACLINT MTIMER device.

Signed-off-by: Anup Patel <[email protected]>
Reviewed-by: Bin Meng <[email protected]>
---
drivers/clocksource/timer-clint.c | 35 ++++++++++++++++++++++---------
1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/drivers/clocksource/timer-clint.c b/drivers/clocksource/timer-clint.c
index 3b68ed53fe4a..bf89744cd12a 100644
--- a/drivers/clocksource/timer-clint.c
+++ b/drivers/clocksource/timer-clint.c
@@ -2,8 +2,16 @@
/*
* Copyright (C) 2020 Western Digital Corporation or its affiliates.
*
- * Most of the M-mode (i.e. NoMMU) RISC-V systems usually have a
- * CLINT MMIO timer device.
+ * Most of the M-mode (i.e. NoMMU) RISC-V systems usually have a CLINT
+ * MMIO device which is a composite device capable of injecting M-mode
+ * software interrupts and M-mode timer interrupts.
+ *
+ * The RISC-V ACLINT specification is modular in nature and defines
+ * separate devices for M-mode software interrupt (MSWI), M-mode timer
+ * (MTIMER) and S-mode software interrupt (SSWI).
+ *
+ * This is a common timer driver for the CLINT device and the ACLINT
+ * MTIMER device.
*/

#define pr_fmt(fmt) "clint: " fmt
@@ -21,25 +29,26 @@
#include <linux/smp.h>
#include <linux/timex.h>

-#ifndef CONFIG_RISCV_M_MODE
+#ifdef CONFIG_RISCV_M_MODE
#include <asm/clint.h>
+
+u64 __iomem *clint_time_val;
+EXPORT_SYMBOL(clint_time_val);
#endif

#define CLINT_IPI_OFF 0
#define CLINT_TIMER_CMP_OFF 0x4000
#define CLINT_TIMER_VAL_OFF 0xbff8

+#define ACLINT_MTIMER_CMP_OFF 0x0000
+#define ACLINT_MTIMER_VAL_OFF 0x7ff8
+
/* CLINT manages IPI and Timer for RISC-V M-mode */
static u64 __iomem *clint_timer_cmp;
static u64 __iomem *clint_timer_val;
static unsigned long clint_timer_freq;
static unsigned int clint_timer_irq;

-#ifdef CONFIG_RISCV_M_MODE
-u64 __iomem *clint_time_val;
-EXPORT_SYMBOL(clint_time_val);
-#endif
-
#ifdef CONFIG_64BIT
#define clint_get_cycles() readq_relaxed(clint_timer_val)
#else
@@ -170,8 +179,13 @@ static int __init clint_timer_init_dt(struct device_node *np)
return -ENODEV;
}

- clint_timer_cmp = base + CLINT_TIMER_CMP_OFF;
- clint_timer_val = base + CLINT_TIMER_VAL_OFF;
+ if (of_device_is_compatible(np, "riscv,aclint-mtimer")) {
+ clint_timer_cmp = base + ACLINT_MTIMER_CMP_OFF;
+ clint_timer_val = base + ACLINT_MTIMER_VAL_OFF;
+ } else {
+ clint_timer_cmp = base + CLINT_TIMER_CMP_OFF;
+ clint_timer_val = base + CLINT_TIMER_VAL_OFF;
+ }
clint_timer_freq = riscv_timebase;

#ifdef CONFIG_RISCV_M_MODE
@@ -219,3 +233,4 @@ static int __init clint_timer_init_dt(struct device_node *np)

TIMER_OF_DECLARE(clint_timer, "riscv,clint0", clint_timer_init_dt);
TIMER_OF_DECLARE(clint_timer1, "sifive,clint0", clint_timer_init_dt);
+TIMER_OF_DECLARE(clint_timer2, "riscv,aclint-mtimer", clint_timer_init_dt);
--
2.25.1

2021-06-18 13:08:03

by Anup Patel

[permalink] [raw]
Subject: [RFC PATCH v2 11/11] MAINTAINERS: Add entry for RISC-V ACLINT drivers

Add myself as maintainer for RISC-V ACLINT drivers.

Signed-off-by: Anup Patel <[email protected]>
Reviewed-by: Bin Meng <[email protected]>
---
MAINTAINERS | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index bc0ceef87b73..64a5ee2fb20c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15693,6 +15693,15 @@ S: Maintained
F: drivers/mtd/nand/raw/r852.c
F: drivers/mtd/nand/raw/r852.h

+RISC-V ACLINT DRIVERS
+M: Anup Patel <[email protected]>
+L: [email protected]
+S: Supported
+F: Documentation/devicetree/bindings/interrupt-controller/riscv,aclint-swi.yaml
+F: Documentation/devicetree/bindings/timer/riscv,aclint-mtimer.yaml
+F: drivers/clocksource/timer-clint.c
+F: drivers/irqchip/irq-aclint-swi.c
+
RISC-V ARCHITECTURE
M: Paul Walmsley <[email protected]>
M: Palmer Dabbelt <[email protected]>
--
2.25.1

2021-06-18 16:07:02

by Anup Patel

[permalink] [raw]
Subject: [RFC PATCH v2 09/11] dt-bindings: timer: Add ACLINT MTIMER bindings

We add DT bindings documentation for the ACLINT MTIMER device
found on RISC-V SOCs.

Signed-off-by: Anup Patel <[email protected]>
Reviewed-by: Bin Meng <[email protected]>
---
.../bindings/timer/riscv,aclint-mtimer.yaml | 55 +++++++++++++++++++
1 file changed, 55 insertions(+)
create mode 100644 Documentation/devicetree/bindings/timer/riscv,aclint-mtimer.yaml

diff --git a/Documentation/devicetree/bindings/timer/riscv,aclint-mtimer.yaml b/Documentation/devicetree/bindings/timer/riscv,aclint-mtimer.yaml
new file mode 100644
index 000000000000..1f467c869d44
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/riscv,aclint-mtimer.yaml
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/timer/riscv,aclint-mtimer.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: RISC-V ACLINT M-level Timer
+
+maintainers:
+ - Anup Patel <[email protected]>
+
+description:
+ RISC-V SOCs include an implementation of the M-level timer (MTIMER) defined
+ in the RISC-V Advanced Core Local Interruptor (ACLINT) specification. The
+ ACLINT MTIMER device is documented in the RISC-V ACLINT specification found
+ at https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc.
+
+ The ACLINT MTIMER device directly connects to the M-level timer interrupt
+ lines of various HARTs (or CPUs) so the RISC-V per-HART (or per-CPU) local
+ interrupt controller is the parent interrupt controller for the ACLINT
+ MTIMER device.
+
+ The clock frequency of ACLINT is specified via "timebase-frequency" DT
+ property of "/cpus" DT node. The "timebase-frequency" DT property is
+ described in Documentation/devicetree/bindings/riscv/cpus.yaml
+
+properties:
+ compatible:
+ items:
+ - const: riscv,aclint-mtimer
+
+ description:
+ Should be "<vendor>,<chip>-aclint-mtimer" and "riscv,aclint-mtimer".
+
+ reg:
+ maxItems: 1
+
+ interrupts-extended:
+ minItems: 1
+
+additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - interrupts-extended
+
+examples:
+ - |
+ timer@2004000 {
+ compatible = "riscv,aclint-mtimer";
+ interrupts-extended = <&cpu1intc 7 &cpu2intc 7 &cpu3intc 7 &cpu4intc 7>;
+ reg = <0x2004000 0x8000>;
+ };
+...
--
2.25.1

2021-06-18 16:07:13

by Anup Patel

[permalink] [raw]
Subject: [RFC PATCH v2 02/11] RISC-V: Use common print prefix in smp.c

We add "#define pr_fmt()" in smp.c to use "riscv:" as common
print prefix for all pr_xyz() statements in this file.

Signed-off-by: Anup Patel <[email protected]>
Reviewed-by: Bin Meng <[email protected]>
---
arch/riscv/kernel/smp.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
index 547dc508f7d1..eea0c9d11d9f 100644
--- a/arch/riscv/kernel/smp.c
+++ b/arch/riscv/kernel/smp.c
@@ -8,6 +8,7 @@
* Copyright (C) 2017 SiFive
*/

+#define pr_fmt(fmt) "riscv: " fmt
#include <linux/cpu.h>
#include <linux/clockchips.h>
#include <linux/interrupt.h>
@@ -114,7 +115,7 @@ static void send_ipi_mask(const struct cpumask *mask, enum ipi_message_type op)
if (ipi_ops && ipi_ops->ipi_inject)
ipi_ops->ipi_inject(mask);
else
- pr_warn("SMP: IPI inject method not available\n");
+ pr_warn("IPI inject method not available\n");
}

static void send_ipi_single(int cpu, enum ipi_message_type op)
@@ -126,7 +127,7 @@ static void send_ipi_single(int cpu, enum ipi_message_type op)
if (ipi_ops && ipi_ops->ipi_inject)
ipi_ops->ipi_inject(cpumask_of(cpu));
else
- pr_warn("SMP: IPI inject method not available\n");
+ pr_warn("IPI inject method not available\n");
}

#ifdef CONFIG_IRQ_WORK
@@ -242,7 +243,7 @@ void smp_send_stop(void)
cpumask_clear_cpu(smp_processor_id(), &mask);

if (system_state <= SYSTEM_RUNNING)
- pr_crit("SMP: stopping secondary CPUs\n");
+ pr_crit("stopping secondary CPUs\n");
send_ipi_mask(&mask, IPI_CPU_STOP);
}

@@ -252,7 +253,7 @@ void smp_send_stop(void)
udelay(1);

if (num_online_cpus() > 1)
- pr_warn("SMP: failed to stop secondary CPUs %*pbl\n",
+ pr_warn("failed to stop secondary CPUs %*pbl\n",
cpumask_pr_args(cpu_online_mask));
}

--
2.25.1

2021-07-12 19:24:38

by Rob Herring

[permalink] [raw]
Subject: Re: [RFC PATCH v2 06/11] dt-bindings: interrupt-controller: Add ACLINT MSWI and SSWI bindings

On Fri, Jun 18, 2021 at 06:08:46PM +0530, Anup Patel wrote:
> We add DT bindings documentation for the ACLINT MSWI and SSWI
> devices found on RISC-V SOCs.
>
> Signed-off-by: Anup Patel <[email protected]>
> Reviewed-by: Bin Meng <[email protected]>
> ---
> .../riscv,aclint-swi.yaml | 82 +++++++++++++++++++
> 1 file changed, 82 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,aclint-swi.yaml
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/riscv,aclint-swi.yaml b/Documentation/devicetree/bindings/interrupt-controller/riscv,aclint-swi.yaml
> new file mode 100644
> index 000000000000..b74025542866
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,aclint-swi.yaml
> @@ -0,0 +1,82 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interrupt-controller/riscv,aclint-swi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: RISC-V ACLINT Software Interrupt Devices
> +
> +maintainers:
> + - Anup Patel <[email protected]>
> +
> +description:
> + RISC-V SOCs include an implementation of the M-level software interrupt
> + (MSWI) device and the S-level software interrupt (SSWI) device defined
> + in the RISC-V Advanced Core Local Interruptor (ACLINT) specification.
> +
> + The ACLINT MSWI and SSWI devices are documented in the RISC-V ACLINT
> + specification located at
> + https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc.
> +
> + The ACLINT MSWI and SSWI devices directly connect to the M-level and
> + S-level software interrupt lines of various HARTs (or CPUs) respectively
> + so the RISC-V per-HART (or per-CPU) local interrupt controller is the
> + parent interrupt controller for the ACLINT MSWI and SSWI devices.
> +
> +allOf:
> + - $ref: /schemas/interrupt-controller.yaml#
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - riscv,aclint-mswi
> + - riscv,aclint-sswi
> +
> + description:
> + Should be "<vendor>,<chip>-aclint-mswi" and "riscv,aclint-mswi" OR
> + "<vendor>,<chip>-aclint-sswi" and "riscv,aclint-sswi".

The schema doesn't match the description.

There's no actual vendor implementation yet? You could do:

items:
- {}
- const: riscv,aclint-mswi

But then your example will fail.

> +
> + reg:
> + maxItems: 1
> +
> + "#interrupt-cells":
> + const: 0
> +
> + interrupts-extended:
> + minItems: 1

You need maxItems too. I guess this based on number of cores, so just
pick a 'should be enough' value.

> +
> + interrupt-controller: true
> +
> +additionalProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - interrupts-extended
> + - interrupt-controller
> + - "#interrupt-cells"
> +
> +examples:
> + - |
> + // Example 1 (RISC-V MSWI device used by Linux RISC-V NoMMU kernel):
> +
> + interrupt-controller@2000000 {
> + compatible = "riscv,aclint-mswi";
> + interrupts-extended = <&cpu1intc 3 &cpu2intc 3 &cpu3intc 3 &cpu4intc 3>;

interrupts-extended = <&cpu1intc 3>, <&cpu2intc 3>, <&cpu3intc 3>, <&cpu4intc 3>;

> + reg = <0x2000000 0x4000>;
> + interrupt-controller;
> + #interrupt-cells = <0>;
> + };
> +
> + - |
> + // Example 2 (RISC-V SSWI device used by Linux RISC-V MMU kernel):
> +
> + interrupt-controller@2100000 {
> + compatible = "riscv,aclint-sswi";
> + interrupts-extended = <&cpu1intc 1 &cpu2intc 1 &cpu3intc 1 &cpu4intc 1>;

Same here.

> + reg = <0x2100000 0x4000>;
> + interrupt-controller;
> + #interrupt-cells = <0>;
> + };
> +...
> --
> 2.25.1
>
>

2021-07-14 05:54:14

by Anup Patel

[permalink] [raw]
Subject: Re: [RFC PATCH v2 06/11] dt-bindings: interrupt-controller: Add ACLINT MSWI and SSWI bindings

On Tue, Jul 13, 2021 at 12:52 AM Rob Herring <[email protected]> wrote:
>
> On Fri, Jun 18, 2021 at 06:08:46PM +0530, Anup Patel wrote:
> > We add DT bindings documentation for the ACLINT MSWI and SSWI
> > devices found on RISC-V SOCs.
> >
> > Signed-off-by: Anup Patel <[email protected]>
> > Reviewed-by: Bin Meng <[email protected]>
> > ---
> > .../riscv,aclint-swi.yaml | 82 +++++++++++++++++++
> > 1 file changed, 82 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,aclint-swi.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/riscv,aclint-swi.yaml b/Documentation/devicetree/bindings/interrupt-controller/riscv,aclint-swi.yaml
> > new file mode 100644
> > index 000000000000..b74025542866
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,aclint-swi.yaml
> > @@ -0,0 +1,82 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/interrupt-controller/riscv,aclint-swi.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: RISC-V ACLINT Software Interrupt Devices
> > +
> > +maintainers:
> > + - Anup Patel <[email protected]>
> > +
> > +description:
> > + RISC-V SOCs include an implementation of the M-level software interrupt
> > + (MSWI) device and the S-level software interrupt (SSWI) device defined
> > + in the RISC-V Advanced Core Local Interruptor (ACLINT) specification.
> > +
> > + The ACLINT MSWI and SSWI devices are documented in the RISC-V ACLINT
> > + specification located at
> > + https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc.
> > +
> > + The ACLINT MSWI and SSWI devices directly connect to the M-level and
> > + S-level software interrupt lines of various HARTs (or CPUs) respectively
> > + so the RISC-V per-HART (or per-CPU) local interrupt controller is the
> > + parent interrupt controller for the ACLINT MSWI and SSWI devices.
> > +
> > +allOf:
> > + - $ref: /schemas/interrupt-controller.yaml#
> > +
> > +properties:
> > + compatible:
> > + items:
> > + - enum:
> > + - riscv,aclint-mswi
> > + - riscv,aclint-sswi
> > +
> > + description:
> > + Should be "<vendor>,<chip>-aclint-mswi" and "riscv,aclint-mswi" OR
> > + "<vendor>,<chip>-aclint-sswi" and "riscv,aclint-sswi".
>
> The schema doesn't match the description.
>
> There's no actual vendor implementation yet? You could do:
>
> items:
> - {}
> - const: riscv,aclint-mswi
>
> But then your example will fail.

Is it okay to have optional vendor compatible string ?

Vendors can add their specific compatible string if there is some
special handling required. If there is not special handling required
then the two compatible strings are enough.

>
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + "#interrupt-cells":
> > + const: 0
> > +
> > + interrupts-extended:
> > + minItems: 1
>
> You need maxItems too. I guess this based on number of cores, so just
> pick a 'should be enough' value.

There is a limit on the maximum number of connections between the
device and HARTs or CPUs so this will be the maxItems over here.

I will update this in the next patch revision.

>
> > +
> > + interrupt-controller: true
> > +
> > +additionalProperties: false
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts-extended
> > + - interrupt-controller
> > + - "#interrupt-cells"
> > +
> > +examples:
> > + - |
> > + // Example 1 (RISC-V MSWI device used by Linux RISC-V NoMMU kernel):
> > +
> > + interrupt-controller@2000000 {
> > + compatible = "riscv,aclint-mswi";
> > + interrupts-extended = <&cpu1intc 3 &cpu2intc 3 &cpu3intc 3 &cpu4intc 3>;
>
> interrupts-extended = <&cpu1intc 3>, <&cpu2intc 3>, <&cpu3intc 3>, <&cpu4intc 3>;

Okay, will update.

>
> > + reg = <0x2000000 0x4000>;
> > + interrupt-controller;
> > + #interrupt-cells = <0>;
> > + };
> > +
> > + - |
> > + // Example 2 (RISC-V SSWI device used by Linux RISC-V MMU kernel):
> > +
> > + interrupt-controller@2100000 {
> > + compatible = "riscv,aclint-sswi";
> > + interrupts-extended = <&cpu1intc 1 &cpu2intc 1 &cpu3intc 1 &cpu4intc 1>;
>
> Same here.

Okay, will update here as well.

>
> > + reg = <0x2100000 0x4000>;
> > + interrupt-controller;
> > + #interrupt-cells = <0>;
> > + };
> > +...
> > --
> > 2.25.1
> >
> >

Regards,
Anup

2021-07-26 12:47:28

by Anup Patel

[permalink] [raw]
Subject: Re: [RFC PATCH v2 00/11] Linux RISC-V ACLINT Support

Hi Marc,

I have taken the approach of IPI domains (like you suggested) in this series.

What do you think ?

Regards,
Anup

On Fri, Jun 18, 2021 at 6:09 PM Anup Patel <[email protected]> wrote:
>
> Most of the existing RISC-V platforms use SiFive CLINT to provide M-level
> timer and IPI support whereas S-level uses SBI calls for timer and IPI
> support. Also, the SiFive CLINT device is a single device providing both
> timer and IPI functionality so RISC-V platforms can't partially implement
> SiFive CLINT device and provide alternate mechanism for timer and IPI.
>
> The RISC-V Advacned Core Local Interruptor (ACLINT) tries to address the
> limitations of SiFive CLINT by:
> 1) Taking modular approach and defining timer and IPI functionality as
> separate devices so that RISC-V platforms can include only required
> devices
> 2) Providing dedicated MMIO device for S-level IPIs so that SBI calls
> can be avoided for IPIs in Linux RISC-V
> 3) Allowing multiple instances of timer and IPI devices for a
> multi-socket (or multi-die) NUMA systems
> 4) Being backward compatible to SiFive CLINT so that existing RISC-V
> platforms stay compliant with RISC-V ACLINT specification
>
> Latest RISC-V ACLINT specification (will be frozen in a month) can be
> found at:
> https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc
>
> This series adds RISC-V ACLINT support and can be found in riscv_aclint_v2
> branch at:
> https://github.com/avpatel/linux
>
> To test this series, the RISC-V ACLINT support for QEMU and OpenSBI
> can be found in the riscv_aclint_v1 branch at:
> https://github.com/avpatel/qemu
> https://github.com/avpatel/opensbi
>
> Changes since v1:
> - Added a new PATCH3 to treat IPIs as normal Linux IRQs for RISC-V kernel
> - New SBI IPI call based irqchip driver in PATCH3 which is only initialized
> by riscv_ipi_setup() when no Linux IRQ numbers are available for IPIs
> - Moved DT bindings patches before corresponding driver patches
> - Implemented ACLINT SWI driver as a irqchip driver in PATCH7
> - Minor nit fixes pointed by Bin Meng
>
> Anup Patel (11):
> RISC-V: Clear SIP bit only when using SBI IPI operations
> RISC-V: Use common print prefix in smp.c
> RISC-V: Treat IPIs as normal Linux IRQs
> RISC-V: Allow marking IPIs as suitable for remote FENCEs
> RISC-V: Use IPIs for remote TLB flush when possible
> dt-bindings: interrupt-controller: Add ACLINT MSWI and SSWI bindings
> irqchip: Add ACLINT software interrupt driver
> RISC-V: Select ACLINT SWI driver for virt machine
> dt-bindings: timer: Add ACLINT MTIMER bindings
> clocksource: clint: Add support for ACLINT MTIMER device
> MAINTAINERS: Add entry for RISC-V ACLINT drivers
>
> .../riscv,aclint-swi.yaml | 82 ++++++
> .../bindings/timer/riscv,aclint-mtimer.yaml | 55 ++++
> MAINTAINERS | 9 +
> arch/riscv/Kconfig | 1 +
> arch/riscv/Kconfig.socs | 1 +
> arch/riscv/include/asm/sbi.h | 2 +
> arch/riscv/include/asm/smp.h | 48 +++-
> arch/riscv/kernel/Makefile | 1 +
> arch/riscv/kernel/cpu-hotplug.c | 2 +
> arch/riscv/kernel/irq.c | 1 +
> arch/riscv/kernel/sbi-ipi.c | 223 ++++++++++++++
> arch/riscv/kernel/sbi.c | 15 -
> arch/riscv/kernel/smp.c | 171 +++++------
> arch/riscv/kernel/smpboot.c | 4 +-
> arch/riscv/mm/tlbflush.c | 62 +++-
> drivers/clocksource/timer-clint.c | 58 ++--
> drivers/irqchip/Kconfig | 11 +
> drivers/irqchip/Makefile | 1 +
> drivers/irqchip/irq-aclint-swi.c | 271 ++++++++++++++++++
> drivers/irqchip/irq-riscv-intc.c | 55 ++--
> 20 files changed, 879 insertions(+), 194 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,aclint-swi.yaml
> create mode 100644 Documentation/devicetree/bindings/timer/riscv,aclint-mtimer.yaml
> create mode 100644 arch/riscv/kernel/sbi-ipi.c
> create mode 100644 drivers/irqchip/irq-aclint-swi.c
>
> --
> 2.25.1
>

2021-07-26 13:47:02

by Marc Zyngier

[permalink] [raw]
Subject: Re: [RFC PATCH v2 02/11] RISC-V: Use common print prefix in smp.c

On Fri, 18 Jun 2021 13:38:42 +0100,
Anup Patel <[email protected]> wrote:
>
> We add "#define pr_fmt()" in smp.c to use "riscv:" as common
> print prefix for all pr_xyz() statements in this file.
>
> Signed-off-by: Anup Patel <[email protected]>
> Reviewed-by: Bin Meng <[email protected]>
> ---
> arch/riscv/kernel/smp.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
> index 547dc508f7d1..eea0c9d11d9f 100644
> --- a/arch/riscv/kernel/smp.c
> +++ b/arch/riscv/kernel/smp.c
> @@ -8,6 +8,7 @@
> * Copyright (C) 2017 SiFive
> */
>
> +#define pr_fmt(fmt) "riscv: " fmt
> #include <linux/cpu.h>
> #include <linux/clockchips.h>
> #include <linux/interrupt.h>
> @@ -114,7 +115,7 @@ static void send_ipi_mask(const struct cpumask *mask, enum ipi_message_type op)
> if (ipi_ops && ipi_ops->ipi_inject)
> ipi_ops->ipi_inject(mask);
> else
> - pr_warn("SMP: IPI inject method not available\n");
> + pr_warn("IPI inject method not available\n");
> }
>
> static void send_ipi_single(int cpu, enum ipi_message_type op)
> @@ -126,7 +127,7 @@ static void send_ipi_single(int cpu, enum ipi_message_type op)
> if (ipi_ops && ipi_ops->ipi_inject)
> ipi_ops->ipi_inject(cpumask_of(cpu));
> else
> - pr_warn("SMP: IPI inject method not available\n");
> + pr_warn("IPI inject method not available\n");

"SMP:" made a lot more sense. I assume that the user knows that they
are using a RISC-V machine. On the other hand, seeing a "SMP:" prefix
for a message indicates the provenance of the message.

I honestly don't see the point in this change.

M.

--
Without deviation from the norm, progress is not possible.

2021-07-26 14:28:41

by Marc Zyngier

[permalink] [raw]
Subject: Re: [RFC PATCH v2 07/11] irqchip: Add ACLINT software interrupt driver

On Fri, 18 Jun 2021 13:38:47 +0100,
Anup Patel <[email protected]> wrote:
>
> The RISC-V ACLINT provides MSWI and SSWI devices for M-mode and
> S-mode software interrupts respectively. We add irqchip driver
> which provide IPI operations based on ACLINT [M|S]SWI devices
> to the Linux RISC-V kernel.
>
> Signed-off-by: Anup Patel <[email protected]>
> ---
> drivers/irqchip/Kconfig | 11 ++
> drivers/irqchip/Makefile | 1 +
> drivers/irqchip/irq-aclint-swi.c | 271 +++++++++++++++++++++++++++++++
> 3 files changed, 283 insertions(+)
> create mode 100644 drivers/irqchip/irq-aclint-swi.c
>
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 62543a4eccc0..2010d493b03b 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -508,6 +508,17 @@ config RISCV_INTC
>
> If you don't know what to do here, say Y.
>
> +config RISCV_ACLINT_SWI
> + bool "RISC-V Advanced Core Local Interruptor Software Interrupts"
> + depends on RISCV
> + help
> + This enables support for software interrupts using the Advanced
> + Core Local Interruptor (ACLINT) found in RISC-V systems. The
> + RISC-V ACLINT provides devices for inter-process interrupt and
> + timer functionality.
> +
> + If you don't know what to do here, say Y.

Let's face it, nobody knows what to say. So instead of asking the
question, how about selecting it from the platform support config
instead?

> +
> config SIFIVE_PLIC
> bool "SiFive Platform-Level Interrupt Controller"
> depends on RISCV
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index f88cbf36a9d2..a6edf6733c1d 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -97,6 +97,7 @@ obj-$(CONFIG_QCOM_PDC) += qcom-pdc.o
> 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_ACLINT_SWI) += irq-aclint-swi.o
> obj-$(CONFIG_SIFIVE_PLIC) += irq-sifive-plic.o
> obj-$(CONFIG_IMX_IRQSTEER) += irq-imx-irqsteer.o
> obj-$(CONFIG_IMX_INTMUX) += irq-imx-intmux.o
> diff --git a/drivers/irqchip/irq-aclint-swi.c b/drivers/irqchip/irq-aclint-swi.c
> new file mode 100644
> index 000000000000..a31a7fc504d1
> --- /dev/null
> +++ b/drivers/irqchip/irq-aclint-swi.c
> @@ -0,0 +1,271 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2021 Western Digital Corporation or its affiliates.
> + */
> +
> +#define pr_fmt(fmt) "aclint-swi: " fmt
> +#include <linux/cpu.h>
> +#include <linux/cpumask.h>
> +#include <linux/io.h>
> +#include <linux/init.h>
> +#include <linux/irq.h>
> +#include <linux/irqchip.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/smp.h>
> +
> +struct aclint_swi {
> + void __iomem *sip_reg;
> + unsigned long bits;
> +};
> +
> +static int aclint_swi_parent_irq __ro_after_init;
> +static struct irq_domain *aclint_swi_domain __ro_after_init;
> +static DEFINE_PER_CPU(struct aclint_swi, aclint_swis);
> +
> +static void aclint_swi_dummy_mask_unmask(struct irq_data *d)
> +{
> +}
> +
> +static void aclint_swi_send_mask(struct irq_data *d,
> + const struct cpumask *mask)
> +{
> + int cpu;
> + struct aclint_swi *swi;
> +
> + /* Barrier before doing atomic bit update to IPI bits */
> + smp_mb__before_atomic();
> +
> + for_each_cpu(cpu, mask) {
> + swi = per_cpu_ptr(&aclint_swis, cpu);
> + set_bit(d->hwirq, &swi->bits);
> + writel(1, swi->sip_reg);
> + }
> +
> + /* Barrier after doing atomic bit update to IPI bits */
> + smp_mb__after_atomic();
> +}
> +
> +static struct irq_chip aclint_swi_chip = {
> + .name = "RISC-V ACLINT SWI",
> + .irq_mask = aclint_swi_dummy_mask_unmask,

Please call this function something that doesn't immediately
contradict the callback it is assigned to.

> + .irq_unmask = aclint_swi_dummy_mask_unmask,
> + .ipi_send_mask = aclint_swi_send_mask,
> +};
> +
> +static int aclint_swi_domain_map(struct irq_domain *d, unsigned int irq,
> + irq_hw_number_t hwirq)
> +{
> + irq_set_percpu_devid(irq);
> + irq_domain_set_info(d, irq, hwirq, &aclint_swi_chip, d->host_data,
> + handle_percpu_devid_irq, NULL, NULL);
> +
> + return 0;
> +}
> +
> +static int aclint_swi_domain_alloc(struct irq_domain *d, unsigned int virq,
> + unsigned int nr_irqs, void *arg)
> +{
> + int i, ret;
> + irq_hw_number_t hwirq;
> + unsigned int type = IRQ_TYPE_NONE;
> + struct irq_fwspec *fwspec = arg;
> +
> + ret = irq_domain_translate_onecell(d, fwspec, &hwirq, &type);
> + if (ret)
> + return ret;
> +
> + for (i = 0; i < nr_irqs; i++) {
> + ret = aclint_swi_domain_map(d, virq + i, hwirq + i);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static const struct irq_domain_ops aclint_swi_domain_ops = {
> + .translate = irq_domain_translate_onecell,
> + .alloc = aclint_swi_domain_alloc,
> + .free = irq_domain_free_irqs_top,
> +};
> +
> +static void aclint_swi_handle_irq(struct irq_desc *desc)
> +{
> + int irq;
> + unsigned long irqs;
> + irq_hw_number_t hwirq;
> + struct irq_chip *chip = irq_desc_get_chip(desc);
> + struct aclint_swi *swi = this_cpu_ptr(&aclint_swis);
> +
> + chained_irq_enter(chip, desc);
> +
> + writel(0, swi->sip_reg);
> +
> + while (true) {
> + /* Order bit clearing and data access. */
> + mb();
> +
> + irqs = xchg(&swi->bits, 0);
> + if (!irqs)
> + goto done;
> +
> + for (hwirq = 0; hwirq < BITS_PER_LONG; hwirq++) {
> + if (!(BIT(hwirq) & irqs))
> + continue;

for_each_set_bit(hwirq, &irqs, BITS_PER_LONG) {

> +
> + irq = irq_find_mapping(aclint_swi_domain, hwirq);
> + if (unlikely(irq <= 0))
> + pr_warn_ratelimited(
> + "can't find mapping for hwirq %lu\n",
> + hwirq);
> + else
> + generic_handle_irq(irq);

You can now convert this over to generic_handle_domain_irq().

> + }
> + }

So you can loop here and consume bits forever, but only ack the
interrupt once? This feels sketchy.

> +
> +done:
> + chained_irq_exit(chip, desc);
> +}
> +
> +static int aclint_swi_dying_cpu(unsigned int cpu)
> +{
> + disable_percpu_irq(aclint_swi_parent_irq);
> + return 0;
> +}
> +
> +static int aclint_swi_starting_cpu(unsigned int cpu)
> +{
> + enable_percpu_irq(aclint_swi_parent_irq,
> + irq_get_trigger_type(aclint_swi_parent_irq));
> + return 0;
> +}
> +
> +static int __init aclint_swi_set_virq(void)
> +{
> + int virq;
> + struct irq_fwspec ipi = {
> + .fwnode = aclint_swi_domain->fwnode,
> + .param_count = 1,
> + .param[0] = 0,
> + };
> +
> + virq = __irq_domain_alloc_irqs(aclint_swi_domain, -1, BITS_PER_LONG,
> + NUMA_NO_NODE, &ipi,
> + false, NULL);
> + if (virq <= 0) {
> + pr_err("unable to alloc IRQs from SBI IPI IRQ domain\n");
> + return -ENOMEM;
> + }
> +
> + riscv_ipi_set_virq_range(virq, BITS_PER_LONG, true);
> +
> + return 0;
> +}
> +
> +static int __init aclint_swi_domain_init(struct device_node *node)
> +{
> + /*
> + * We can have multiple ACLINT SWI devices but we only need
> + * one IRQ domain for providing per-HART (or per-CPU) IPIs.
> + */
> + if (aclint_swi_domain)
> + return 0;
> +
> + aclint_swi_domain = irq_domain_add_linear(node, BITS_PER_LONG,
> + &aclint_swi_domain_ops, NULL);
> + if (!aclint_swi_domain) {
> + pr_err("unable to add ACLINT SWI IRQ domain\n");
> + return -ENOMEM;
> + }
> +
> + return aclint_swi_set_virq();
> +}
> +
> +static int __init aclint_swi_init(struct device_node *node,
> + struct device_node *parent)
> +{
> + int rc;
> + void __iomem *base;
> + struct aclint_swi *swi;
> + u32 i, nr_irqs, nr_cpus = 0;
> +
> + /* Map the registers */
> + base = of_iomap(node, 0);
> + if (!base) {
> + pr_err("%pOFP: could not map registers\n", node);
> + return -ENODEV;
> + }
> +
> + /* Iterarte over each target CPU connected with this ACLINT */
> + nr_irqs = of_irq_count(node);
> + for (i = 0; i < nr_irqs; i++) {
> + struct of_phandle_args parent;
> + int cpu, hartid;
> +
> + if (of_irq_parse_one(node, i, &parent)) {
> + pr_err("%pOFP: failed to parse irq %d.\n",
> + node, i);
> + continue;
> + }
> +
> + if (parent.args[0] != RV_IRQ_SOFT) {
> + pr_err("%pOFP: invalid irq %d (hwirq %d)\n",
> + node, i, parent.args[0]);
> + continue;
> + }
> +
> + hartid = riscv_of_parent_hartid(parent.np);
> + if (hartid < 0) {
> + pr_warn("failed to parse hart ID for irq %d.\n", i);
> + continue;
> + }
> +
> + cpu = riscv_hartid_to_cpuid(hartid);
> + if (cpu < 0) {
> + pr_warn("Invalid cpuid for irq %d\n", i);
> + continue;
> + }
> +
> + /* Find parent domain and register chained handler */
> + if (!aclint_swi_parent_irq && irq_find_host(parent.np)) {
> + aclint_swi_parent_irq = irq_of_parse_and_map(node, i);
> + if (aclint_swi_parent_irq) {

What is the point of describing all the interrupts for each and every
CPU if you only need *one* to establish the routing on behalf of all
CPUs?

> + irq_set_chained_handler(aclint_swi_parent_irq,
> + aclint_swi_handle_irq);
> + cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
> + "irqchip/riscv/aclint-swi:starting",
> + aclint_swi_starting_cpu,
> + aclint_swi_dying_cpu);
> + }
> + }

else?

> +
> + swi = per_cpu_ptr(&aclint_swis, cpu);
> + swi->sip_reg = base + i * sizeof(u32);
> + writel(0, swi->sip_reg);
> +
> + nr_cpus++;
> + }
> +
> + /* Create the IPI domain for ACLINT SWI device */
> + rc = aclint_swi_domain_init(node);
> + if (rc)
> + return rc;
> +
> + /* Announce the ACLINT SWI device */
> + pr_info("%pOFP: providing IPIs for %d CPUs\n", node, nr_cpus);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_RISCV_M_MODE
> +IRQCHIP_DECLARE(riscv_aclint_swi, "riscv,clint0", aclint_swi_init);
> +IRQCHIP_DECLARE(riscv_aclint_swi1, "sifive,clint0", aclint_swi_init);
> +IRQCHIP_DECLARE(riscv_aclint_swi2, "riscv,aclint-mswi", aclint_swi_init);
> +#else
> +IRQCHIP_DECLARE(riscv_aclint_swi, "riscv,aclint-sswi", aclint_swi_init);
> +#endif
> --
> 2.25.1
>
>

Thanks,

M.

--
Without deviation from the norm, progress is not possible.

2021-07-26 14:34:54

by Marc Zyngier

[permalink] [raw]
Subject: Re: [RFC PATCH v2 00/11] Linux RISC-V ACLINT Support

On Mon, 26 Jul 2021 13:45:20 +0100,
Anup Patel <[email protected]> wrote:
>
> Hi Marc,
>
> I have taken the approach of IPI domains (like you suggested) in this series.
>
> What do you think ?

I have commented on the irqchip driver.

As for the RISC-V specific code, I'll let the architecture maintainers
look into it. I guess the elephant in the room is that this spec seems
to be evolving, and that there is no HW implementation (how this
driver maps on SF's CLINT is anybody's guess).

M.

--
Without deviation from the norm, progress is not possible.

2021-07-26 15:14:16

by Anup Patel

[permalink] [raw]
Subject: Re: [RFC PATCH v2 00/11] Linux RISC-V ACLINT Support

Hi Marc,

On Mon, Jul 26, 2021 at 8:02 PM Marc Zyngier <[email protected]> wrote:
>
> On Mon, 26 Jul 2021 13:45:20 +0100,
> Anup Patel <[email protected]> wrote:
> >
> > Hi Marc,
> >
> > I have taken the approach of IPI domains (like you suggested) in this series.
> >
> > What do you think ?
>
> I have commented on the irqchip driver.
>
> As for the RISC-V specific code, I'll let the architecture maintainers
> look into it. I guess the elephant in the room is that this spec seems
> to be evolving, and that there is no HW implementation (how this
> driver maps on SF's CLINT is anybody's guess).

The SiFive CLINT is a more convoluted device and provides M-level
timer functionality and M-level IPI functionality in one MMIO device.

The RISC-V ACLINT specification is more modular and backward
compatible with the SiFive CLINT. In fact, a SiFive CLINT device
can be viewed as a ACLINT MSWI device + ACLINT MTIMER device.
This means existing RISC-V boards having SiFive CLINT will be
automatically compliant to the RISC-V ACLINT specification.

Here's the RISC-V ACLINT spec:
https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc

The RISC-V ACLINT spec is quite stable and we are not seeing any
further changes hence I sent out RFC PATCHes to get feedback. The
RISC-V ACLINT spec will be frozen before 2021 end (i.e. before next
RISC-V summit).

The Linux NoMMU kernel (M-level) will use an ACLINT MSWI device
for IPI support whereas the regular Linux MMU kernel (S-level) will
use an ACLINT SSWI device for IPI support.

The ACLINT SWI driver is a common IPI driver for both ACLINT
MSWI (Linux NoMMU) and ACLINT SSWI (Linux MMU). In fact,
the ACLINT SWI also works for IPI part (i.e. MSWI) of SiFive CLINT.

Regards,
Anup

>
> M.
>
> --
> Without deviation from the norm, progress is not possible.

2021-07-26 15:23:30

by Anup Patel

[permalink] [raw]
Subject: Re: [RFC PATCH v2 02/11] RISC-V: Use common print prefix in smp.c

On Mon, Jul 26, 2021 at 7:14 PM Marc Zyngier <[email protected]> wrote:
>
> On Fri, 18 Jun 2021 13:38:42 +0100,
> Anup Patel <[email protected]> wrote:
> >
> > We add "#define pr_fmt()" in smp.c to use "riscv:" as common
> > print prefix for all pr_xyz() statements in this file.
> >
> > Signed-off-by: Anup Patel <[email protected]>
> > Reviewed-by: Bin Meng <[email protected]>
> > ---
> > arch/riscv/kernel/smp.c | 9 +++++----
> > 1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
> > index 547dc508f7d1..eea0c9d11d9f 100644
> > --- a/arch/riscv/kernel/smp.c
> > +++ b/arch/riscv/kernel/smp.c
> > @@ -8,6 +8,7 @@
> > * Copyright (C) 2017 SiFive
> > */
> >
> > +#define pr_fmt(fmt) "riscv: " fmt
> > #include <linux/cpu.h>
> > #include <linux/clockchips.h>
> > #include <linux/interrupt.h>
> > @@ -114,7 +115,7 @@ static void send_ipi_mask(const struct cpumask *mask, enum ipi_message_type op)
> > if (ipi_ops && ipi_ops->ipi_inject)
> > ipi_ops->ipi_inject(mask);
> > else
> > - pr_warn("SMP: IPI inject method not available\n");
> > + pr_warn("IPI inject method not available\n");
> > }
> >
> > static void send_ipi_single(int cpu, enum ipi_message_type op)
> > @@ -126,7 +127,7 @@ static void send_ipi_single(int cpu, enum ipi_message_type op)
> > if (ipi_ops && ipi_ops->ipi_inject)
> > ipi_ops->ipi_inject(cpumask_of(cpu));
> > else
> > - pr_warn("SMP: IPI inject method not available\n");
> > + pr_warn("IPI inject method not available\n");
>
> "SMP:" made a lot more sense. I assume that the user knows that they
> are using a RISC-V machine. On the other hand, seeing a "SMP:" prefix
> for a message indicates the provenance of the message.
>
> I honestly don't see the point in this change.

The intention was to distinguish arch specific prints from
generic kernel prints at boot-time because "SMP: " prefix
was not making it obvious that these are arch specific prints.

I am certainly fine dropping this patch as well.

Regards,
Anup

>
> M.
>
> --
> Without deviation from the norm, progress is not possible.

2021-07-26 16:26:40

by Anup Patel

[permalink] [raw]
Subject: Re: [RFC PATCH v2 07/11] irqchip: Add ACLINT software interrupt driver

On Mon, Jul 26, 2021 at 7:55 PM Marc Zyngier <[email protected]> wrote:
>
> On Fri, 18 Jun 2021 13:38:47 +0100,
> Anup Patel <[email protected]> wrote:
> >
> > The RISC-V ACLINT provides MSWI and SSWI devices for M-mode and
> > S-mode software interrupts respectively. We add irqchip driver
> > which provide IPI operations based on ACLINT [M|S]SWI devices
> > to the Linux RISC-V kernel.
> >
> > Signed-off-by: Anup Patel <[email protected]>
> > ---
> > drivers/irqchip/Kconfig | 11 ++
> > drivers/irqchip/Makefile | 1 +
> > drivers/irqchip/irq-aclint-swi.c | 271 +++++++++++++++++++++++++++++++
> > 3 files changed, 283 insertions(+)
> > create mode 100644 drivers/irqchip/irq-aclint-swi.c
> >
> > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> > index 62543a4eccc0..2010d493b03b 100644
> > --- a/drivers/irqchip/Kconfig
> > +++ b/drivers/irqchip/Kconfig
> > @@ -508,6 +508,17 @@ config RISCV_INTC
> >
> > If you don't know what to do here, say Y.
> >
> > +config RISCV_ACLINT_SWI
> > + bool "RISC-V Advanced Core Local Interruptor Software Interrupts"
> > + depends on RISCV
> > + help
> > + This enables support for software interrupts using the Advanced
> > + Core Local Interruptor (ACLINT) found in RISC-V systems. The
> > + RISC-V ACLINT provides devices for inter-process interrupt and
> > + timer functionality.
> > +
> > + If you don't know what to do here, say Y.
>
> Let's face it, nobody knows what to say. So instead of asking the
> question, how about selecting it from the platform support config
> instead?

Yes, this will be mostly selected by platform support config. In fact,
the PATCH8 already selects it for the virt machine.

Let me remove this last line.

>
> > +
> > config SIFIVE_PLIC
> > bool "SiFive Platform-Level Interrupt Controller"
> > depends on RISCV
> > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> > index f88cbf36a9d2..a6edf6733c1d 100644
> > --- a/drivers/irqchip/Makefile
> > +++ b/drivers/irqchip/Makefile
> > @@ -97,6 +97,7 @@ obj-$(CONFIG_QCOM_PDC) += qcom-pdc.o
> > 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_ACLINT_SWI) += irq-aclint-swi.o
> > obj-$(CONFIG_SIFIVE_PLIC) += irq-sifive-plic.o
> > obj-$(CONFIG_IMX_IRQSTEER) += irq-imx-irqsteer.o
> > obj-$(CONFIG_IMX_INTMUX) += irq-imx-intmux.o
> > diff --git a/drivers/irqchip/irq-aclint-swi.c b/drivers/irqchip/irq-aclint-swi.c
> > new file mode 100644
> > index 000000000000..a31a7fc504d1
> > --- /dev/null
> > +++ b/drivers/irqchip/irq-aclint-swi.c
> > @@ -0,0 +1,271 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2021 Western Digital Corporation or its affiliates.
> > + */
> > +
> > +#define pr_fmt(fmt) "aclint-swi: " fmt
> > +#include <linux/cpu.h>
> > +#include <linux/cpumask.h>
> > +#include <linux/io.h>
> > +#include <linux/init.h>
> > +#include <linux/irq.h>
> > +#include <linux/irqchip.h>
> > +#include <linux/irqchip/chained_irq.h>
> > +#include <linux/irqdomain.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/smp.h>
> > +
> > +struct aclint_swi {
> > + void __iomem *sip_reg;
> > + unsigned long bits;
> > +};
> > +
> > +static int aclint_swi_parent_irq __ro_after_init;
> > +static struct irq_domain *aclint_swi_domain __ro_after_init;
> > +static DEFINE_PER_CPU(struct aclint_swi, aclint_swis);
> > +
> > +static void aclint_swi_dummy_mask_unmask(struct irq_data *d)
> > +{
> > +}
> > +
> > +static void aclint_swi_send_mask(struct irq_data *d,
> > + const struct cpumask *mask)
> > +{
> > + int cpu;
> > + struct aclint_swi *swi;
> > +
> > + /* Barrier before doing atomic bit update to IPI bits */
> > + smp_mb__before_atomic();
> > +
> > + for_each_cpu(cpu, mask) {
> > + swi = per_cpu_ptr(&aclint_swis, cpu);
> > + set_bit(d->hwirq, &swi->bits);
> > + writel(1, swi->sip_reg);
> > + }
> > +
> > + /* Barrier after doing atomic bit update to IPI bits */
> > + smp_mb__after_atomic();
> > +}
> > +
> > +static struct irq_chip aclint_swi_chip = {
> > + .name = "RISC-V ACLINT SWI",
> > + .irq_mask = aclint_swi_dummy_mask_unmask,
>
> Please call this function something that doesn't immediately
> contradict the callback it is assigned to.

Okay, I will choose a better name for this callback.

>
> > + .irq_unmask = aclint_swi_dummy_mask_unmask,
> > + .ipi_send_mask = aclint_swi_send_mask,
> > +};
> > +
> > +static int aclint_swi_domain_map(struct irq_domain *d, unsigned int irq,
> > + irq_hw_number_t hwirq)
> > +{
> > + irq_set_percpu_devid(irq);
> > + irq_domain_set_info(d, irq, hwirq, &aclint_swi_chip, d->host_data,
> > + handle_percpu_devid_irq, NULL, NULL);
> > +
> > + return 0;
> > +}
> > +
> > +static int aclint_swi_domain_alloc(struct irq_domain *d, unsigned int virq,
> > + unsigned int nr_irqs, void *arg)
> > +{
> > + int i, ret;
> > + irq_hw_number_t hwirq;
> > + unsigned int type = IRQ_TYPE_NONE;
> > + struct irq_fwspec *fwspec = arg;
> > +
> > + ret = irq_domain_translate_onecell(d, fwspec, &hwirq, &type);
> > + if (ret)
> > + return ret;
> > +
> > + for (i = 0; i < nr_irqs; i++) {
> > + ret = aclint_swi_domain_map(d, virq + i, hwirq + i);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static const struct irq_domain_ops aclint_swi_domain_ops = {
> > + .translate = irq_domain_translate_onecell,
> > + .alloc = aclint_swi_domain_alloc,
> > + .free = irq_domain_free_irqs_top,
> > +};
> > +
> > +static void aclint_swi_handle_irq(struct irq_desc *desc)
> > +{
> > + int irq;
> > + unsigned long irqs;
> > + irq_hw_number_t hwirq;
> > + struct irq_chip *chip = irq_desc_get_chip(desc);
> > + struct aclint_swi *swi = this_cpu_ptr(&aclint_swis);
> > +
> > + chained_irq_enter(chip, desc);
> > +
> > + writel(0, swi->sip_reg);
> > +
> > + while (true) {
> > + /* Order bit clearing and data access. */
> > + mb();
> > +
> > + irqs = xchg(&swi->bits, 0);
> > + if (!irqs)
> > + goto done;
> > +
> > + for (hwirq = 0; hwirq < BITS_PER_LONG; hwirq++) {
> > + if (!(BIT(hwirq) & irqs))
> > + continue;
>
> for_each_set_bit(hwirq, &irqs, BITS_PER_LONG) {

Sure, I will use for_each_set_bit() here.

>
> > +
> > + irq = irq_find_mapping(aclint_swi_domain, hwirq);
> > + if (unlikely(irq <= 0))
> > + pr_warn_ratelimited(
> > + "can't find mapping for hwirq %lu\n",
> > + hwirq);
> > + else
> > + generic_handle_irq(irq);
>
> You can now convert this over to generic_handle_domain_irq().

Sure, I will use generic_handle_domain_irq().

>
> > + }
> > + }
>
> So you can loop here and consume bits forever, but only ack the
> interrupt once? This feels sketchy.

Aargh, I accidently put writel() out of the loop. We should have one
writel() everytime we consume the bits.

I will update this.

>
> > +
> > +done:
> > + chained_irq_exit(chip, desc);
> > +}
> > +
> > +static int aclint_swi_dying_cpu(unsigned int cpu)
> > +{
> > + disable_percpu_irq(aclint_swi_parent_irq);
> > + return 0;
> > +}
> > +
> > +static int aclint_swi_starting_cpu(unsigned int cpu)
> > +{
> > + enable_percpu_irq(aclint_swi_parent_irq,
> > + irq_get_trigger_type(aclint_swi_parent_irq));
> > + return 0;
> > +}
> > +
> > +static int __init aclint_swi_set_virq(void)
> > +{
> > + int virq;
> > + struct irq_fwspec ipi = {
> > + .fwnode = aclint_swi_domain->fwnode,
> > + .param_count = 1,
> > + .param[0] = 0,
> > + };
> > +
> > + virq = __irq_domain_alloc_irqs(aclint_swi_domain, -1, BITS_PER_LONG,
> > + NUMA_NO_NODE, &ipi,
> > + false, NULL);
> > + if (virq <= 0) {
> > + pr_err("unable to alloc IRQs from SBI IPI IRQ domain\n");
> > + return -ENOMEM;
> > + }
> > +
> > + riscv_ipi_set_virq_range(virq, BITS_PER_LONG, true);
> > +
> > + return 0;
> > +}
> > +
> > +static int __init aclint_swi_domain_init(struct device_node *node)
> > +{
> > + /*
> > + * We can have multiple ACLINT SWI devices but we only need
> > + * one IRQ domain for providing per-HART (or per-CPU) IPIs.
> > + */
> > + if (aclint_swi_domain)
> > + return 0;
> > +
> > + aclint_swi_domain = irq_domain_add_linear(node, BITS_PER_LONG,
> > + &aclint_swi_domain_ops, NULL);
> > + if (!aclint_swi_domain) {
> > + pr_err("unable to add ACLINT SWI IRQ domain\n");
> > + return -ENOMEM;
> > + }
> > +
> > + return aclint_swi_set_virq();
> > +}
> > +
> > +static int __init aclint_swi_init(struct device_node *node,
> > + struct device_node *parent)
> > +{
> > + int rc;
> > + void __iomem *base;
> > + struct aclint_swi *swi;
> > + u32 i, nr_irqs, nr_cpus = 0;
> > +
> > + /* Map the registers */
> > + base = of_iomap(node, 0);
> > + if (!base) {
> > + pr_err("%pOFP: could not map registers\n", node);
> > + return -ENODEV;
> > + }
> > +
> > + /* Iterarte over each target CPU connected with this ACLINT */
> > + nr_irqs = of_irq_count(node);
> > + for (i = 0; i < nr_irqs; i++) {
> > + struct of_phandle_args parent;
> > + int cpu, hartid;
> > +
> > + if (of_irq_parse_one(node, i, &parent)) {
> > + pr_err("%pOFP: failed to parse irq %d.\n",
> > + node, i);
> > + continue;
> > + }
> > +
> > + if (parent.args[0] != RV_IRQ_SOFT) {
> > + pr_err("%pOFP: invalid irq %d (hwirq %d)\n",
> > + node, i, parent.args[0]);
> > + continue;
> > + }
> > +
> > + hartid = riscv_of_parent_hartid(parent.np);
> > + if (hartid < 0) {
> > + pr_warn("failed to parse hart ID for irq %d.\n", i);
> > + continue;
> > + }
> > +
> > + cpu = riscv_hartid_to_cpuid(hartid);
> > + if (cpu < 0) {
> > + pr_warn("Invalid cpuid for irq %d\n", i);
> > + continue;
> > + }
> > +
> > + /* Find parent domain and register chained handler */
> > + if (!aclint_swi_parent_irq && irq_find_host(parent.np)) {
> > + aclint_swi_parent_irq = irq_of_parse_and_map(node, i);
> > + if (aclint_swi_parent_irq) {
>
> What is the point of describing all the interrupts for each and every
> CPU if you only need *one* to establish the routing on behalf of all
> CPUs?

The parent interrupt controller for ACLINT is RISC-V local interrupt
controller (INTC) which is within each HART (or CPU). We have one
RISC-V INTC DT node under each CPU DT node so the RISC-V INTC
driver registers a single IRQ domain only for the boot CPU (which could
be HART with any HART ID not necessarily zero).

The RISC-V INTC IRQ domain provides the parent IRQ for the IPI domain
registered by the ACLINT device. The interrupts-extended property of
ACLINT DT node points to a set of CPUs which the given ACLINT device
targets so we loop over all the IRQs to find the RISC-V INTC IRQ domain.
(Note: This is very similar to how SiFive PLIC driver finds parent IRQ
domain using interrupts-extended DT property)

Also, on multi-cluster (or multi-socket) platforms we will have multiple
ACLINT devices where each ACLINT device targets different sets of
CPUs (or HARTs).

Here are ACLINT DT node examples from DT bindings patch:

// Example 1 (RISC-V MSWI device used by Linux RISC-V NoMMU kernel):

interrupt-controller@2000000 {
compatible = "riscv,aclint-mswi";
interrupts-extended = <&cpu1intc 3 &cpu2intc 3 &cpu3intc 3 &cpu4intc 3>;
reg = <0x2000000 0x4000>;
interrupt-controller;
#interrupt-cells = <0>;
};

// Example 2 (RISC-V SSWI device used by Linux RISC-V MMU kernel):

interrupt-controller@2100000 {
compatible = "riscv,aclint-sswi";
interrupts-extended = <&cpu1intc 1 &cpu2intc 1 &cpu3intc 1 &cpu4intc 1>;
reg = <0x2100000 0x4000>;
interrupt-controller;
#interrupt-cells = <0>;
};

>
> > + irq_set_chained_handler(aclint_swi_parent_irq,
> > + aclint_swi_handle_irq);
> > + cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
> > + "irqchip/riscv/aclint-swi:starting",
> > + aclint_swi_starting_cpu,
> > + aclint_swi_dying_cpu);
> > + }
> > + }
>
> else?
>
> > +
> > + swi = per_cpu_ptr(&aclint_swis, cpu);
> > + swi->sip_reg = base + i * sizeof(u32);
> > + writel(0, swi->sip_reg);
> > +
> > + nr_cpus++;
> > + }
> > +
> > + /* Create the IPI domain for ACLINT SWI device */
> > + rc = aclint_swi_domain_init(node);
> > + if (rc)
> > + return rc;
> > +
> > + /* Announce the ACLINT SWI device */
> > + pr_info("%pOFP: providing IPIs for %d CPUs\n", node, nr_cpus);
> > +
> > + return 0;
> > +}
> > +
> > +#ifdef CONFIG_RISCV_M_MODE
> > +IRQCHIP_DECLARE(riscv_aclint_swi, "riscv,clint0", aclint_swi_init);
> > +IRQCHIP_DECLARE(riscv_aclint_swi1, "sifive,clint0", aclint_swi_init);
> > +IRQCHIP_DECLARE(riscv_aclint_swi2, "riscv,aclint-mswi", aclint_swi_init);
> > +#else
> > +IRQCHIP_DECLARE(riscv_aclint_swi, "riscv,aclint-sswi", aclint_swi_init);
> > +#endif
> > --
> > 2.25.1
> >
> >
>
> Thanks,
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.

Regards,
Anup

2021-07-27 06:34:03

by Sean Anderson

[permalink] [raw]
Subject: Re: [RFC PATCH v2 06/11] dt-bindings: interrupt-controller: Add ACLINT MSWI and SSWI bindings

On 7/13/21 11:27 AM, Anup Patel wrote:
> On Tue, Jul 13, 2021 at 12:52 AM Rob Herring <[email protected]> wrote:
>>
>> On Fri, Jun 18, 2021 at 06:08:46PM +0530, Anup Patel wrote:
>>> We add DT bindings documentation for the ACLINT MSWI and SSWI
>>> devices found on RISC-V SOCs.
>>>
>>> Signed-off-by: Anup Patel <[email protected]>
>>> Reviewed-by: Bin Meng <[email protected]>
>>> ---
>>> .../riscv,aclint-swi.yaml | 82 +++++++++++++++++++
>>> 1 file changed, 82 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,aclint-swi.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/riscv,aclint-swi.yaml b/Documentation/devicetree/bindings/interrupt-controller/riscv,aclint-swi.yaml
>>> new file mode 100644
>>> index 000000000000..b74025542866
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,aclint-swi.yaml
>>> @@ -0,0 +1,82 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/interrupt-controller/riscv,aclint-swi.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: RISC-V ACLINT Software Interrupt Devices
>>> +
>>> +maintainers:
>>> + - Anup Patel <[email protected]>
>>> +
>>> +description:
>>> + RISC-V SOCs include an implementation of the M-level software interrupt
>>> + (MSWI) device and the S-level software interrupt (SSWI) device defined
>>> + in the RISC-V Advanced Core Local Interruptor (ACLINT) specification.
>>> +
>>> + The ACLINT MSWI and SSWI devices are documented in the RISC-V ACLINT
>>> + specification located at
>>> + https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc.
>>> +
>>> + The ACLINT MSWI and SSWI devices directly connect to the M-level and
>>> + S-level software interrupt lines of various HARTs (or CPUs) respectively
>>> + so the RISC-V per-HART (or per-CPU) local interrupt controller is the
>>> + parent interrupt controller for the ACLINT MSWI and SSWI devices.
>>> +
>>> +allOf:
>>> + - $ref: /schemas/interrupt-controller.yaml#
>>> +
>>> +properties:
>>> + compatible:
>>> + items:
>>> + - enum:
>>> + - riscv,aclint-mswi
>>> + - riscv,aclint-sswi
>>> +
>>> + description:
>>> + Should be "<vendor>,<chip>-aclint-mswi" and "riscv,aclint-mswi" OR
>>> + "<vendor>,<chip>-aclint-sswi" and "riscv,aclint-sswi".
>>
>> The schema doesn't match the description.
>>
>> There's no actual vendor implementation yet? You could do:
>>
>> items:
>> - {}
>> - const: riscv,aclint-mswi
>>
>> But then your example will fail.
>
> Is it okay to have optional vendor compatible string ?

I think you can express this with something like

properties:
compatible:
contains:
enum:
- ...

>
> Vendors can add their specific compatible string if there is some
> special handling required. If there is not special handling required
> then the two compatible strings are enough.
>
>>
>>> +
>>> + reg:
>>> + maxItems: 1
>>> +
>>> + "#interrupt-cells":
>>> + const: 0
>>> +
>>> + interrupts-extended:
>>> + minItems: 1
>>
>> You need maxItems too. I guess this based on number of cores, so just
>> pick a 'should be enough' value.
>
> There is a limit on the maximum number of connections between the
> device and HARTs or CPUs so this will be the maxItems over here.
>
> I will update this in the next patch revision.
>
>>
>>> +
>>> + interrupt-controller: true
>>> +
>>> +additionalProperties: false
>>> +
>>> +required:
>>> + - compatible
>>> + - reg
>>> + - interrupts-extended
>>> + - interrupt-controller
>>> + - "#interrupt-cells"
>>> +
>>> +examples:
>>> + - |
>>> + // Example 1 (RISC-V MSWI device used by Linux RISC-V NoMMU kernel):
>>> +
>>> + interrupt-controller@2000000 {
>>> + compatible = "riscv,aclint-mswi";
>>> + interrupts-extended = <&cpu1intc 3 &cpu2intc 3 &cpu3intc 3 &cpu4intc 3>;
>>
>> interrupts-extended = <&cpu1intc 3>, <&cpu2intc 3>, <&cpu3intc 3>, <&cpu4intc 3>;
>
> Okay, will update.
>
>>
>>> + reg = <0x2000000 0x4000>;
>>> + interrupt-controller;
>>> + #interrupt-cells = <0>;
>>> + };
>>> +
>>> + - |
>>> + // Example 2 (RISC-V SSWI device used by Linux RISC-V MMU kernel):
>>> +
>>> + interrupt-controller@2100000 {
>>> + compatible = "riscv,aclint-sswi";
>>> + interrupts-extended = <&cpu1intc 1 &cpu2intc 1 &cpu3intc 1 &cpu4intc 1>;
>>
>> Same here.
>
> Okay, will update here as well.
>
>>
>>> + reg = <0x2100000 0x4000>;
>>> + interrupt-controller;
>>> + #interrupt-cells = <0>;
>>> + };
>>> +...
>>> --
>>> 2.25.1
>>>
>>>
>
> Regards,
> Anup
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv
>

2021-07-29 04:32:37

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [RFC PATCH v2 00/11] Linux RISC-V ACLINT Support

On Mon, 26 Jul 2021 06:01:01 PDT (-0700), [email protected] wrote:
> Hi Marc,
>
> On Mon, Jul 26, 2021 at 8:02 PM Marc Zyngier <[email protected]> wrote:
>>
>> On Mon, 26 Jul 2021 13:45:20 +0100,
>> Anup Patel <[email protected]> wrote:
>> >
>> > Hi Marc,
>> >
>> > I have taken the approach of IPI domains (like you suggested) in this series.
>> >
>> > What do you think ?
>>
>> I have commented on the irqchip driver.
>>
>> As for the RISC-V specific code, I'll let the architecture maintainers
>> look into it. I guess the elephant in the room is that this spec seems
>> to be evolving, and that there is no HW implementation (how this
>> driver maps on SF's CLINT is anybody's guess).

There's a long history of interrupt controller efforts from the RISC-V
foundation, and we've yet to have any of them result in hardware.

> The SiFive CLINT is a more convoluted device and provides M-level
> timer functionality and M-level IPI functionality in one MMIO device.
>
> The RISC-V ACLINT specification is more modular and backward
> compatible with the SiFive CLINT. In fact, a SiFive CLINT device
> can be viewed as a ACLINT MSWI device + ACLINT MTIMER device.
> This means existing RISC-V boards having SiFive CLINT will be
> automatically compliant to the RISC-V ACLINT specification.

So is there any hardware that this new specification enables? It seems
to be a more convoluted way to describe the same mess we're already in.
I'm not really inclined to take a bunch of code that just does the same
thing via a more complicated specification.

> Here's the RISC-V ACLINT spec:
> https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc
>
> The RISC-V ACLINT spec is quite stable and we are not seeing any
> further changes hence I sent out RFC PATCHes to get feedback. The
> RISC-V ACLINT spec will be frozen before 2021 end (i.e. before next
> RISC-V summit).

Have you talked to the other ISA folks about that?

As far as I can tell this new spec allows for multiple MTIME registers,
which seems to be in direct contradiction to the single -MTIME register
as defined in the ISA manual. It also seems to be vaguely incompatible
WRT the definition of SSIP, but I'm not sure that one really matters all
that much as it's not like old software can write the new registers.

I just talked to Krste and Andrew, they say they haven't heard of any of
this. I don't know what's going on over there, but it's very hard to
review anything when I can't even tell where the ISA is defined.

> The Linux NoMMU kernel (M-level) will use an ACLINT MSWI device
> for IPI support whereas the regular Linux MMU kernel (S-level) will
> use an ACLINT SSWI device for IPI support.
>
> The ACLINT SWI driver is a common IPI driver for both ACLINT
> MSWI (Linux NoMMU) and ACLINT SSWI (Linux MMU). In fact,
> the ACLINT SWI also works for IPI part (i.e. MSWI) of SiFive CLINT.
>
> Regards,
> Anup
>
>>
>> M.
>>
>> --
>> Without deviation from the norm, progress is not possible.

2021-07-29 04:57:55

by Anup Patel

[permalink] [raw]
Subject: Re: [RFC PATCH v2 00/11] Linux RISC-V ACLINT Support

On Thu, Jul 29, 2021 at 10:00 AM Palmer Dabbelt
<[email protected]> wrote:
>
> On Mon, 26 Jul 2021 06:01:01 PDT (-0700), [email protected] wrote:
> > Hi Marc,
> >
> > On Mon, Jul 26, 2021 at 8:02 PM Marc Zyngier <[email protected]> wrote:
> >>
> >> On Mon, 26 Jul 2021 13:45:20 +0100,
> >> Anup Patel <[email protected]> wrote:
> >> >
> >> > Hi Marc,
> >> >
> >> > I have taken the approach of IPI domains (like you suggested) in this series.
> >> >
> >> > What do you think ?
> >>
> >> I have commented on the irqchip driver.
> >>
> >> As for the RISC-V specific code, I'll let the architecture maintainers
> >> look into it. I guess the elephant in the room is that this spec seems
> >> to be evolving, and that there is no HW implementation (how this
> >> driver maps on SF's CLINT is anybody's guess).
>
> There's a long history of interrupt controller efforts from the RISC-V
> foundation, and we've yet to have any of them result in hardware.

The RISC-V AIA group was formed last year. Can you point me to which
interrupt controller efforts you are referring to.

>
> > The SiFive CLINT is a more convoluted device and provides M-level
> > timer functionality and M-level IPI functionality in one MMIO device.
> >
> > The RISC-V ACLINT specification is more modular and backward
> > compatible with the SiFive CLINT. In fact, a SiFive CLINT device
> > can be viewed as a ACLINT MSWI device + ACLINT MTIMER device.
> > This means existing RISC-V boards having SiFive CLINT will be
> > automatically compliant to the RISC-V ACLINT specification.
>
> So is there any hardware that this new specification enables? It seems
> to be a more convoluted way to describe the same mess we're already in.
> I'm not really inclined to take a bunch of code that just does the same
> thing via a more complicated specification.

Nope, it is much cleaner and modular compared to SiFive CLINT and
it is also backward compatible to SiFive CLINT.

Can you elaborate what part of the code you are not okay with ?

>
> > Here's the RISC-V ACLINT spec:
> > https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc
> >
> > The RISC-V ACLINT spec is quite stable and we are not seeing any
> > further changes hence I sent out RFC PATCHes to get feedback. The
> > RISC-V ACLINT spec will be frozen before 2021 end (i.e. before next
> > RISC-V summit).
>
> Have you talked to the other ISA folks about that?

This spec is being developed by the RISC-V AIA group based on the
feedback from ISA folks and HW architects.

>
> As far as I can tell this new spec allows for multiple MTIME registers,
> which seems to be in direct contradiction to the single -MTIME register
> as defined in the ISA manual. It also seems to be vaguely incompatible
> WRT the definition of SSIP, but I'm not sure that one really matters all
> that much as it's not like old software can write the new registers.

The ACLINT spec clearly defines that if we have multiple MTIME registers
then these registers must be synchronized to meet the architecture
requirements. The spec also defines a software mechanism for MTIME
synchronization. It is also possible for multiple MTIMER devices to share
same MTIME register. Please refer to the latest ACLINT spec.

>
> I just talked to Krste and Andrew, they say they haven't heard of any of
> this. I don't know what's going on over there, but it's very hard to
> review anything when I can't even tell where the ISA is defined.

I am surprised by the respone you got because the ACLINT spec is
being developed by a working group of RISC-V International. In fact,
it is hosted on the RISC-V International GitHub. How can we host it on
RISC-V GitHub if it is not an official spec being developed by RVI.

Regards,
Anup

>
> > The Linux NoMMU kernel (M-level) will use an ACLINT MSWI device
> > for IPI support whereas the regular Linux MMU kernel (S-level) will
> > use an ACLINT SSWI device for IPI support.
> >
> > The ACLINT SWI driver is a common IPI driver for both ACLINT
> > MSWI (Linux NoMMU) and ACLINT SSWI (Linux MMU). In fact,
> > the ACLINT SWI also works for IPI part (i.e. MSWI) of SiFive CLINT.
> >
> > Regards,
> > Anup
> >
> >>
> >> M.
> >>
> >> --
> >> Without deviation from the norm, progress is not possible.

2021-07-29 05:37:41

by Anup Patel

[permalink] [raw]
Subject: Re: [RFC PATCH v2 00/11] Linux RISC-V ACLINT Support

On Thu, Jul 29, 2021 at 10:00 AM Palmer Dabbelt
<[email protected]> wrote:
>
> On Mon, 26 Jul 2021 06:01:01 PDT (-0700), [email protected] wrote:
> > Hi Marc,
> >
> > On Mon, Jul 26, 2021 at 8:02 PM Marc Zyngier <[email protected]> wrote:
> >>
> >> On Mon, 26 Jul 2021 13:45:20 +0100,
> >> Anup Patel <[email protected]> wrote:
> >> >
> >> > Hi Marc,
> >> >
> >> > I have taken the approach of IPI domains (like you suggested) in this series.
> >> >
> >> > What do you think ?
> >>
> >> I have commented on the irqchip driver.
> >>
> >> As for the RISC-V specific code, I'll let the architecture maintainers
> >> look into it. I guess the elephant in the room is that this spec seems
> >> to be evolving, and that there is no HW implementation (how this
> >> driver maps on SF's CLINT is anybody's guess).
>
> There's a long history of interrupt controller efforts from the RISC-V
> foundation, and we've yet to have any of them result in hardware.
>
> > The SiFive CLINT is a more convoluted device and provides M-level
> > timer functionality and M-level IPI functionality in one MMIO device.
> >
> > The RISC-V ACLINT specification is more modular and backward
> > compatible with the SiFive CLINT. In fact, a SiFive CLINT device
> > can be viewed as a ACLINT MSWI device + ACLINT MTIMER device.
> > This means existing RISC-V boards having SiFive CLINT will be
> > automatically compliant to the RISC-V ACLINT specification.
>
> So is there any hardware that this new specification enables? It seems
> to be a more convoluted way to describe the same mess we're already in.
> I'm not really inclined to take a bunch of code that just does the same
> thing via a more complicated specification.
>
> > Here's the RISC-V ACLINT spec:
> > https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc
> >
> > The RISC-V ACLINT spec is quite stable and we are not seeing any
> > further changes hence I sent out RFC PATCHes to get feedback. The
> > RISC-V ACLINT spec will be frozen before 2021 end (i.e. before next
> > RISC-V summit).
>
> Have you talked to the other ISA folks about that?
>
> As far as I can tell this new spec allows for multiple MTIME registers,
> which seems to be in direct contradiction to the single -MTIME register

The RISC-V ISA spec only mandates a single view of MTIME registers
for all the HARTs

The RISC-V ISA spec does not mandate a single physical MTIME register.

In fact, multi-socket and multi-die systems will end-up having multiple
physical MTIME registers so on such systems these physical MTIME
registers need to be synchronized with hardware assistance. Please
refer to the MTIME synchronization section of ACLINT specification.

> as defined in the ISA manual. It also seems to be vaguely incompatible
> WRT the definition of SSIP, but I'm not sure that one really matters all
> that much as it's not like old software can write the new registers.

Please loot at the ACLINT SSWI spec again. The SSIP bit definition
is to be modified where mip.SSIP bit is logical OR of software writable
bit and external SSIP signal. This way older software which uses
SBI call based IPI injection will work fine. In fact, this aspect of SSIP
bit is tested on QEMU RISC-V as well.

>
> I just talked to Krste and Andrew, they say they haven't heard of any of
> this. I don't know what's going on over there, but it's very hard to
> review anything when I can't even tell where the ISA is defined.

Like mentioned in other thread, please first to talk to the actual
spec owners first. There are lot of working groups and activities
going on in RVI.

Regards,
Anup

>
> > The Linux NoMMU kernel (M-level) will use an ACLINT MSWI device
> > for IPI support whereas the regular Linux MMU kernel (S-level) will
> > use an ACLINT SSWI device for IPI support.
> >
> > The ACLINT SWI driver is a common IPI driver for both ACLINT
> > MSWI (Linux NoMMU) and ACLINT SSWI (Linux MMU). In fact,
> > the ACLINT SWI also works for IPI part (i.e. MSWI) of SiFive CLINT.
> >
> > Regards,
> > Anup
> >
> >>
> >> M.
> >>
> >> --
> >> Without deviation from the norm, progress is not possible.