[08/15] riscv: provide native clint access for M-mode

Message ID	20190813154747.24256-9-hch@lst.de (mailing list archive)
State	New, archived
Headers	show Return-Path: <linux-riscv-bounces+patchwork-linux-riscv=patchwork.kernel.org@lists.infradead.org> From: Christoph Hellwig <hch@lst.de> To: Palmer Dabbelt <palmer@sifive.com>, Paul Walmsley <paul.walmsley@sifive.com> Subject: [PATCH 08/15] riscv: provide native clint access for M-mode Date: Tue, 13 Aug 2019 17:47:40 +0200 Message-Id: <20190813154747.24256-9-hch@lst.de> In-Reply-To: <20190813154747.24256-1-hch@lst.de> References: <20190813154747.24256-1-hch@lst.de> MIME-Version: 1.0 Precedence: list Cc: Damien Le Moal <damien.lemoal@wdc.com>, linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-riscv" <linux-riscv-bounces@lists.infradead.org> Errors-To: linux-riscv-bounces+patchwork-linux-riscv=patchwork.kernel.org@lists.infradead.org
Series	[01/15] irqchip/sifive-plic: set max threshold for ignored handlers \| expand [01/15] irqchip/sifive-plic: set max threshold for ignored handlers [02/15] riscv: use CSR_SATP instead of the legacy sptbr name in switch_mm [03/15] riscv: refactor the IPI code [04/15] riscv: abstract out CSR names for supervisor vs machine mode [05/15] riscv: improve the default power off implementation [06/15] riscv: provide a flat entry loader [07/15] riscv: read the hart ID from mhartid on boot [08/15] riscv: provide native clint access for M-mode [09/15] riscv: implement remote sfence.i natively for M-mode [10/15] riscv: poison SBI calls for M-mode [11/15] riscv: don't allow selecting SBI-based drivers for M-mode [12/15] riscv: use the correct interrupt levels for M-mode [13/15] riscv: clear the instruction cache and all registers when booting [14/15] riscv: add nommu support [15/15] riscv: disable the EFI PECOFF header for M-mode

Christoph Hellwig Aug. 13, 2019, 3:47 p.m. UTC

RISC-V has the concept of a cpu level interrupt controller.  Part of it
is expose as bits in the status registers, and 2 new CSRs per privilege
level in the instruction set, but the machanisms to trigger IPIs and
timer events, as well as reading the actual timer value are not
specified in the RISC-V spec but usually delegated to a block of MMIO
registers.  This patch adds support for those MMIO registers in the
timer and IPI code.  For now only the SiFive layout also supported by
a few other implementations is supported, but the code should be
easily extensible to others in the future.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/riscv/include/asm/clint.h    | 40 +++++++++++++++++++++++++++
 arch/riscv/include/asm/timex.h    | 17 ++++++++++++
 arch/riscv/kernel/Makefile        |  1 +
 arch/riscv/kernel/clint.c         | 45 +++++++++++++++++++++++++++++++
 arch/riscv/kernel/setup.c         |  2 ++
 arch/riscv/kernel/smp.c           | 24 +++++++++++++++++
 arch/riscv/kernel/smpboot.c       |  3 +++
 drivers/clocksource/timer-riscv.c | 16 ++++++++---
 8 files changed, 144 insertions(+), 4 deletions(-)
 create mode 100644 arch/riscv/include/asm/clint.h
 create mode 100644 arch/riscv/kernel/clint.c

Mark Rutland Aug. 13, 2019, 4:29 p.m. UTC | #1

On Tue, Aug 13, 2019 at 05:47:40PM +0200, Christoph Hellwig wrote:
> RISC-V has the concept of a cpu level interrupt controller.  Part of it
> is expose as bits in the status registers, and 2 new CSRs per privilege
> level in the instruction set, but the machanisms to trigger IPIs and
> timer events, as well as reading the actual timer value are not
> specified in the RISC-V spec but usually delegated to a block of MMIO
> registers.  This patch adds support for those MMIO registers in the
> timer and IPI code.  For now only the SiFive layout also supported by
> a few other implementations is supported, but the code should be
> easily extensible to others in the future.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

> +/*
> + * This is the layout used by the SiFive clint, which is also shared by the qemu
> + * virt platform, and the Kendryte KD210 at least.
> + */
> +#define CLINT_IPI_OFF		0
> +#define CLINT_TIME_VAL_OFF	0xbff8
> +#define CLINT_TIME_CMP_OFF	0x4000;
> +
> +u32 __iomem *clint_ipi_base;
> +u64 __iomem *clint_time_val;
> +u64 __iomem *clint_time_cmp;
> +
> +void clint_init_boot_cpu(void)
> +{
> +	struct device_node *np;
> +	void __iomem *base;
> +
> +	np = of_find_compatible_node(NULL, NULL, "riscv,clint0");

Since the MMIO layout is that of the SiFive clint, the compatible string
should be specific to that. e.g. "sifive,clint". That way it will be
possible to distinguish it from other implementations.

What exactly is the "0" suffix for? Is that a version number?

If that's a CPU index, then I don't think that's the right way to encode
this unless the programming interface actually differs across CPUs. It
would be better to use an explicit phandle to express the affinity.

Thanks,
Mark.

Christoph Hellwig Aug. 19, 2019, 10:16 a.m. UTC | #2

On Tue, Aug 13, 2019 at 05:29:58PM +0100, Mark Rutland wrote:
> > +	np = of_find_compatible_node(NULL, NULL, "riscv,clint0");
> 
> Since the MMIO layout is that of the SiFive clint, the compatible string
> should be specific to that. e.g. "sifive,clint". That way it will be
> possible to distinguish it from other implementations.
> 
> What exactly is the "0" suffix for? Is that a version number?
> 
> If that's a CPU index, then I don't think that's the right way to encode
> this unless the programming interface actually differs across CPUs. It
> would be better to use an explicit phandle to express the affinity.

It isn't a cpu index, I suspect a version number.  These show up
in a lot of the early RISC-V DTs coming from the UCB/SiFive sphere.
They've now spread everywhere unfortunately.

Atish Patra Aug. 21, 2019, 12:24 a.m. UTC | #3

On Tue, 2019-08-13 at 17:47 +0200, Christoph Hellwig wrote:
> RISC-V has the concept of a cpu level interrupt controller.  Part of
> it
> is expose as bits in the status registers, and 2 new CSRs per 
//is expose/is to expose/

> privilege
> level in the instruction set, but the machanisms to trigger IPIs and
> timer events, as well as reading the actual timer value are not
> specified in the RISC-V spec but usually delegated to a block of MMIO
> registers.  This patch adds support for those MMIO registers in the
> timer and IPI code.  For now only the SiFive layout also supported by
> a few other implementations is supported, but the code should be
> easily extensible to others in the future.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  arch/riscv/include/asm/clint.h    | 40 +++++++++++++++++++++++++++
>  arch/riscv/include/asm/timex.h    | 17 ++++++++++++
>  arch/riscv/kernel/Makefile        |  1 +
>  arch/riscv/kernel/clint.c         | 45
> +++++++++++++++++++++++++++++++
>  arch/riscv/kernel/setup.c         |  2 ++
>  arch/riscv/kernel/smp.c           | 24 +++++++++++++++++
>  arch/riscv/kernel/smpboot.c       |  3 +++
>  drivers/clocksource/timer-riscv.c | 16 ++++++++---
>  8 files changed, 144 insertions(+), 4 deletions(-)
>  create mode 100644 arch/riscv/include/asm/clint.h
>  create mode 100644 arch/riscv/kernel/clint.c
> 
> diff --git a/arch/riscv/include/asm/clint.h
> b/arch/riscv/include/asm/clint.h
> new file mode 100644
> index 000000000000..46d182d9a4db
> --- /dev/null
> +++ b/arch/riscv/include/asm/clint.h
> @@ -0,0 +1,40 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#ifndef _ASM_CLINT_H
> +#define _ASM_CLINT_H 1
> +
> +#include <linux/smp.h>
> +
> +#ifdef CONFIG_M_MODE
> +extern u32 __iomem *clint_ipi_base;
> +extern u64 __iomem *clint_time_val;
> +extern u64 __iomem *clint_time_cmp;
> +
> +void clint_init_boot_cpu(void);
> +
> +static inline void clint_send_ipi(unsigned long hartid)
> +{
> +	writel(1, clint_ipi_base + hartid);
> +}
> +
> +static inline void clint_clear_ipi(unsigned long hartid)
> +{
> +	writel(0, clint_ipi_base + hartid);
> +}
> +
> +static inline u64 clint_read_timer(void)
> +{
> +	return readq_relaxed(clint_time_val);
> +}
> +
> +static inline void clint_set_timer(unsigned long delta)
> +{
> +	writeq_relaxed(clint_read_timer() + delta,
> +		clint_time_cmp +
> cpuid_to_hartid_map(smp_processor_id()));'

This is not compatible with 32 bit mode. IIRC, timecmp is a 64 bit on
RV32 as well. Here is the implementation in OpenSBI.

https://github.com/riscv/opensbi/blob/master/lib/utils/sys/clint.c#L104

> +}
> +
> +#else
> +#define clint_init_boot_cpu()	do { } while (0)
> +#define clint_clear_ipi(hartid)	do { } while (0)
> +#endif /* CONFIG_M_MODE */
> +
> +#endif /* _ASM_CLINT_H */
> diff --git a/arch/riscv/include/asm/timex.h
> b/arch/riscv/include/asm/timex.h
> index 6a703ec9d796..bf907997f107 100644
> --- a/arch/riscv/include/asm/timex.h
> +++ b/arch/riscv/include/asm/timex.h
> @@ -10,6 +10,22 @@
>  
>  typedef unsigned long cycles_t;
>  
> +#ifdef CONFIG_M_MODE
> +
> +#include <linux/io-64-nonatomic-lo-hi.h>
> +#include <asm/clint.h>
> +
> +static inline cycles_t get_cycles(void)
> +{
> +#ifdef CONFIG_64BIT
> +	return readq_relaxed(clint_time_val);
> +#else
> +	return readl_relaxed(clint_time_val);
> +#endif

Same comment as above. Both RV32 & RV64 bit have 64 bit have 64 bit
precission for timer val. You have to read 32 bits at a time and "or"
them to get 64 bit value. Here is the implementation from OpenSBI

https://github.com/riscv/opensbi/blob/master/lib/utils/sys/clint.c#L69

> +}
> +#define get_cycles	get_cycles
> +
> +#else /* CONFIG_M_MODE */
>  static inline cycles_t get_cycles_inline(void)
>  {
>  	cycles_t n;
> @@ -40,6 +56,7 @@ static inline uint64_t get_cycles64(void)
>  	return ((u64)hi << 32) | lo;
>  }
>  #endif
> +#endif /* CONFIG_M_MODE */
>  
>  #define ARCH_HAS_READ_CURRENT_TIMER
>  
> diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
> index 2420d37d96de..f933c04f89db 100644
> --- a/arch/riscv/kernel/Makefile
> +++ b/arch/riscv/kernel/Makefile
> @@ -29,6 +29,7 @@ obj-y	+= vdso.o
>  obj-y	+= cacheinfo.o
>  obj-y	+= vdso/
>  
> +obj-$(CONFIG_M_MODE)		+= clint.o
>  obj-$(CONFIG_FPU)		+= fpu.o
>  obj-$(CONFIG_SMP)		+= smpboot.o
>  obj-$(CONFIG_SMP)		+= smp.o
> diff --git a/arch/riscv/kernel/clint.c b/arch/riscv/kernel/clint.c
> new file mode 100644
> index 000000000000..15b9e7fa5416
> --- /dev/null
> +++ b/arch/riscv/kernel/clint.c
> @@ -0,0 +1,45 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019 Christoph Hellwig.
> + */
> +
> +#include <linux/io.h>
> +#include <linux/of_address.h>
> +#include <linux/types.h>
> +#include <asm/csr.h>
> +#include <asm/irq.h>
> +#include <asm/timex.h>
> +
> +/*
> + * This is the layout used by the SiFive clint, which is also shared
> by the qemu
> + * virt platform, and the Kendryte KD210 at least.
> + */
> +#define CLINT_IPI_OFF		0
> +#define CLINT_TIME_VAL_OFF	0xbff8
> +#define CLINT_TIME_CMP_OFF	0x4000;
> +
> +u32 __iomem *clint_ipi_base;
> +u64 __iomem *clint_time_val;
> +u64 __iomem *clint_time_cmp;
> +
> +void clint_init_boot_cpu(void)
> +{
> +	struct device_node *np;
> +	void __iomem *base;
> +
> +	np = of_find_compatible_node(NULL, NULL, "riscv,clint0");
> +	if (!np) {
> +		panic("clint not found");
> +		return;
> +	}
> +
> +	base = of_iomap(np, 0);
> +	if (!base)
> +		panic("could not map CLINT");
> +
> +	clint_ipi_base = base + CLINT_IPI_OFF;
> +	clint_time_val = base + CLINT_TIME_VAL_OFF;
> +	clint_time_cmp = base + CLINT_TIME_CMP_OFF;
> +
> +	clint_clear_ipi(boot_cpu_hartid);
> +}
> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> index a990a6cb184f..f4ba71b66c73 100644
> --- a/arch/riscv/kernel/setup.c
> +++ b/arch/riscv/kernel/setup.c
> @@ -17,6 +17,7 @@
>  #include <linux/sched/task.h>
>  #include <linux/swiotlb.h>
>  
> +#include <asm/clint.h>
>  #include <asm/setup.h>
>  #include <asm/sections.h>
>  #include <asm/pgtable.h>
> @@ -65,6 +66,7 @@ void __init setup_arch(char **cmdline_p)
>  	setup_bootmem();
>  	paging_init();
>  	unflatten_device_tree();
> +	clint_init_boot_cpu();
>  
>  #ifdef CONFIG_SWIOTLB
>  	swiotlb_init(1);
> diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
> index 8cd730239613..ee8599a7ca48 100644
> --- a/arch/riscv/kernel/smp.c
> +++ b/arch/riscv/kernel/smp.c
> @@ -13,7 +13,9 @@
>  #include <linux/sched.h>
>  #include <linux/seq_file.h>
>  #include <linux/delay.h>
> +#include <linux/io.h>
>  
> +#include <asm/clint.h>
>  #include <asm/sbi.h>
>  #include <asm/tlbflush.h>
>  #include <asm/cacheflush.h>
> @@ -78,6 +80,27 @@ static void ipi_stop(void)
>  		wait_for_interrupt();
>  }
>  
> +#ifdef CONFIG_M_MODE
> +static inline void send_ipi_single(int cpu, enum ipi_message_type
> op)
> +{
> +	set_bit(op, &ipi_data[cpu].bits);
> +	clint_send_ipi(cpuid_to_hartid_map(cpu));
> +}
> +
> +static inline void send_ipi_mask(const struct cpumask *mask,
> +		enum ipi_message_type op)
> +{
> +	int cpu;
> +
> +	for_each_cpu(cpu, mask)
> +		send_ipi_single(cpu, op);
> +}
> +
> +static inline void clear_ipi(void)
> +{
> +	clint_clear_ipi(cpuid_to_hartid_map(smp_processor_id()));
> +}
> +#else /* CONFIG_M_MODE */
>  static void send_ipi_mask(const struct cpumask *mask, enum
> ipi_message_type op)
>  {
>  	int cpuid, hartid;
> @@ -103,6 +126,7 @@ static inline void clear_ipi(void)
>  {
>  	csr_clear(CSR_SIP, SIE_SSIE);
>  }
> +#endif /* CONFIG_M_MODE */
>  
>  void riscv_software_interrupt(void)
>  {
> diff --git a/arch/riscv/kernel/smpboot.c
> b/arch/riscv/kernel/smpboot.c
> index 7462a44304fe..1b7678d86ec8 100644
> --- a/arch/riscv/kernel/smpboot.c
> +++ b/arch/riscv/kernel/smpboot.c
> @@ -23,6 +23,7 @@
>  #include <linux/of.h>
>  #include <linux/sched/task_stack.h>
>  #include <linux/sched/mm.h>
> +#include <asm/clint.h>
>  #include <asm/irq.h>
>  #include <asm/mmu_context.h>
>  #include <asm/tlbflush.h>
> @@ -132,6 +133,8 @@ asmlinkage void __init smp_callin(void)
>  {
>  	struct mm_struct *mm = &init_mm;
>  
> +	clint_clear_ipi(cpuid_to_hartid_map(smp_processor_id()));
> +
>  	/* All kernel threads share the same mm context.  */
>  	mmgrab(mm);
>  	current->active_mm = mm;
> diff --git a/drivers/clocksource/timer-riscv.c
> b/drivers/clocksource/timer-riscv.c
> index 3ad2fa52bac9..09f2165bd0a5 100644
> --- a/drivers/clocksource/timer-riscv.c
> +++ b/drivers/clocksource/timer-riscv.c
> @@ -24,12 +24,16 @@
>   * operations on the current hart.  There is guaranteed to be
> exactly one timer
>   * per hart on all RISC-V systems.
>   */
> -
>  static int riscv_clock_next_event(unsigned long delta,
>  		struct clock_event_device *ce)
>  {
>  	csr_set(CSR_XIE, XIE_XTIE);
> +
> +#ifdef CONFIG_M_MODE
> +	clint_set_timer(delta);
> +#else
>  	sbi_set_timer(get_cycles64() + delta);
> +#endif
>  	return 0;
>  }
>  
> @@ -45,14 +49,18 @@ static DEFINE_PER_CPU(struct clock_event_device,
> riscv_clock_event) = {
>   * within one tick of each other, so while this could technically go
>   * backwards when hopping between CPUs, practically it won't happen.
>   */
> -static unsigned long long riscv_clocksource_rdtime(struct
> clocksource *cs)
> +static u64 riscv_sched_clock(void)
>  {
> +#ifdef CONFIG_M_MODE
> +	return clint_read_timer();
> +#else
>  	return get_cycles64();
> +#endif
>  }
>  
> -static u64 riscv_sched_clock(void)
> +static unsigned long long riscv_clocksource_rdtime(struct
> clocksource *cs)
>  {
> -	return get_cycles64();
> +	return riscv_sched_clock();
>  }
>  
>  static struct clocksource riscv_clocksource = {

Christoph Hellwig Aug. 21, 2019, 12:42 a.m. UTC | #4

On Wed, Aug 21, 2019 at 12:24:31AM +0000, Atish Patra wrote:
> > +static inline void clint_set_timer(unsigned long delta)
> > +{
> > +	writeq_relaxed(clint_read_timer() + delta,
> > +		clint_time_cmp +
> > cpuid_to_hartid_map(smp_processor_id()));'
> 
> This is not compatible with 32 bit mode. IIRC, timecmp is a 64 bit on
> RV32 as well. Here is the implementation in OpenSBI.

writeq alwasy writes 64-bit anyway, but the deltas is just 32-bit
per the Linux clocksource API.

> > +static inline cycles_t get_cycles(void)
> > +{
> > +#ifdef CONFIG_64BIT
> > +	return readq_relaxed(clint_time_val);
> > +#else
> > +	return readl_relaxed(clint_time_val);
> > +#endif
> 
> Same comment as above. Both RV32 & RV64 bit have 64 bit have 64 bit
> precission for timer val. You have to read 32 bits at a time and "or"
> them to get 64 bit value. Here is the implementation from OpenSBI

But the Linux API is only going to read 32-bits of that, same as
for the rdtime pseudo-instruction used by the current SBI-based code.

Note that I've reworked this area a bit for v4, which I'm going to
send out soon, including cleanups to the existing code to make a few
of these things more obvious:

http://git.infradead.org/users/hch/riscv.git/shortlog/refs/heads/riscv-nommu.4

Palmer Dabbelt Aug. 27, 2019, 11:37 p.m. UTC | #5

On Mon, 19 Aug 2019 03:16:48 PDT (-0700), Christoph Hellwig wrote:
> On Tue, Aug 13, 2019 at 05:29:58PM +0100, Mark Rutland wrote:
>> > +	np = of_find_compatible_node(NULL, NULL, "riscv,clint0");
>>
>> Since the MMIO layout is that of the SiFive clint, the compatible string
>> should be specific to that. e.g. "sifive,clint". That way it will be
>> possible to distinguish it from other implementations.
>>
>> What exactly is the "0" suffix for? Is that a version number?
>>
>> If that's a CPU index, then I don't think that's the right way to encode
>> this unless the programming interface actually differs across CPUs. It
>> would be better to use an explicit phandle to express the affinity.
>
> It isn't a cpu index, I suspect a version number.  These show up
> in a lot of the early RISC-V DTs coming from the UCB/SiFive sphere.
> They've now spread everywhere unfortunately.

clint0 would be version 0 of the clint, with is the core-local interrupt 
controller in rocket chip.  It should be "sifive,clint-1.0.0", not 
"riscv,clint0", as it's a SiFive widget.  Unfortunately there are a lot of 
legacy device trees floating around, but I'm only considering what's been 
upstream to be actually part of the spec.

In this case the code should match on a "sifive,clint-1.0.0", and the device 
trees should be fixed up to match.  We match on "riscv,plic0" for legacy 
systems, and I guess it makes sense to do something similar here.

Christoph Hellwig Aug. 28, 2019, 6:11 a.m. UTC | #6

On Tue, Aug 27, 2019 at 04:37:16PM -0700, Palmer Dabbelt wrote:
> clint0 would be version 0 of the clint, with is the core-local interrupt 
> controller in rocket chip.  It should be "sifive,clint-1.0.0", not 
> "riscv,clint0", as it's a SiFive widget.  Unfortunately there are a lot of 
> legacy device trees floating around, but I'm only considering what's been 
> upstream to be actually part of the spec.
>
> In this case the code should match on a "sifive,clint-1.0.0", and the 
> device trees should be fixed up to match.  We match on "riscv,plic0" for 
> legacy systems, and I guess it makes sense to do something similar here.

IFF we decided to change it I'd rather separate DT noes for the ipi
bank vs timecmp register vs timeval to support variable layouts.  The
downside is that we can't just boot on unmodified upstream qemu, which
has used the "riscv,clint0" for years.

Palmer Dabbelt Sept. 3, 2019, 6:48 p.m. UTC | #7

On Tue, 27 Aug 2019 23:11:46 PDT (-0700), Christoph Hellwig wrote:
> On Tue, Aug 27, 2019 at 04:37:16PM -0700, Palmer Dabbelt wrote:
>> clint0 would be version 0 of the clint, with is the core-local interrupt
>> controller in rocket chip.  It should be "sifive,clint-1.0.0", not
>> "riscv,clint0", as it's a SiFive widget.  Unfortunately there are a lot of
>> legacy device trees floating around, but I'm only considering what's been
>> upstream to be actually part of the spec.
>>
>> In this case the code should match on a "sifive,clint-1.0.0", and the
>> device trees should be fixed up to match.  We match on "riscv,plic0" for
>> legacy systems, and I guess it makes sense to do something similar here.
>
> IFF we decided to change it I'd rather separate DT noes for the ipi
> bank vs timecmp register vs timeval to support variable layouts.  The
> downside is that we can't just boot on unmodified upstream qemu, which
> has used the "riscv,clint0" for years.

Like I alluded to above, matching on "riscv,clint0" seems reasonable to me as 
it's a defacto standard -- we'll just have to make sure that if we ever end up 
with a RISC-V CLINT that the DT entry is something else.

As far as splitting the memory maps goes, I don't have a strong opinion but it 
seems like that'll introduce more complexity than it's worth.

Alan Kao Sept. 4, 2019, 2:05 a.m. UTC | #8

On Tue, Sep 03, 2019 at 11:48:52AM -0700, Palmer Dabbelt wrote:
> On Tue, 27 Aug 2019 23:11:46 PDT (-0700), Christoph Hellwig wrote:
> >On Tue, Aug 27, 2019 at 04:37:16PM -0700, Palmer Dabbelt wrote:
> >>clint0 would be version 0 of the clint, with is the core-local interrupt
> >>controller in rocket chip.  It should be "sifive,clint-1.0.0", not
> >>"riscv,clint0", as it's a SiFive widget.  Unfortunately there are a lot of
> >>legacy device trees floating around, but I'm only considering what's been
> >>upstream to be actually part of the spec.
> >>
> >>In this case the code should match on a "sifive,clint-1.0.0", and the
> >>device trees should be fixed up to match.  We match on "riscv,plic0" for
> >>legacy systems, and I guess it makes sense to do something similar here.
> >
> >IFF we decided to change it I'd rather separate DT noes for the ipi
> >bank vs timecmp register vs timeval to support variable layouts.  The
> >downside is that we can't just boot on unmodified upstream qemu, which
> >has used the "riscv,clint0" for years.
> 
> Like I alluded to above, matching on "riscv,clint0" seems reasonable to me
> as it's a defacto standard -- we'll just have to make sure that if we ever
> end up with a RISC-V CLINT that the DT entry is something else.

De facto, but not mandatory.

> 
> As far as splitting the memory maps goes, I don't have a strong opinion but
> it seems like that'll introduce more complexity than it's worth.
> 

At least the splitting can keep reminding us and any new comers in the future
that CLINT is not (yet) a must in RISC-V landscape.  A previous discussion
FYI: ( https://lkml.org/lkml/2019/8/20/1361 )

> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

[08/15] riscv: provide native clint access for M-mode

Commit Message

Comments

Patch