| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAJFTR8Q0rruNDSC0_biPhHQqnHobTaZc=6so2kAuw5jbJcs-9g@mail.gmail.com>
Date: Wed, 26 Feb 2025 12:33:55 -0500
From: Jesse T <mr.bossman075@...il.com>
To: linux-riscv@...ts.infradead.org
Cc: Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt <palmer@...belt.com>,
Albert Ou <aou@...s.berkeley.edu>, Daniel Lezcano <daniel.lezcano@...aro.org>,
Thomas Gleixner <tglx@...utronix.de>, Samuel Holland <samuel.holland@...ive.com>,
Conor Dooley <conor@...nel.org>, Icenowy Zheng <uwu@...nowy.me>, Jisheng Zhang <jszhang@...nel.org>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5] clocksource/drivers/timer-clint: Add T-Head C9xx clint
On Fri, Jan 17, 2025 at 11:45 AM Jesse Taube <mr.bossman075@...il.com> wrote:
>
> From: Jisheng Zhang <jszhang@...nel.org>
>
> To use the T-HEAD C9xx clint in RISCV-M NOMMU env, we need to take
> care two points:
>
> 1.The mtimecmp in T-Head C9xx clint only supports 32bit read/write,
> implement such support.
>
> 2. As pointed out by commit ca7810aecdba ("lib: utils/timer: mtimer:
> add a quirk for lacking mtime register") of opensbi:
>
> "T-Head developers surely have a different understanding of time CSR and
> CLINT's mtime register with SiFive ones, that they did not implement
> the mtime register at all -- as shown in openC906 source code, their
> time CSR value is just exposed at the top of their processor IP block
> and expects an external continous counter, which makes it not
> overrideable, and thus mtime register is not implemented, even not for
> reading. However, if CLINTEE is not enabled in T-Head's MXSTATUS
> extended CSR, these systems still rely on the mtimecmp registers to
> generate timer interrupts. This makes it necessary to implement T-Head
> C9xx CLINT support in OpenSBI MTIMER driver, which skips implementing
> reading mtime register and falls back to default code that reads time
> CSR."
>
> So, we need to fall back to read time CSR instead of mtime register.
> Add riscv_csr_time_available static key for this purpose.
>
> Signed-off-by: Jisheng Zhang <jszhang@...nel.org>
> Signed-off-by: Jesse Taube <Mr.Bossman075@...il.com>
> ---
> Treat this as a completely new patch, as it is mostly rewritten.
> Original:
> https://lore.kernel.org/all/20240410142347.964-3-jszhang@kernel.org/
> V3 -> V4:
> - Add riscv,csr-clint
> - Allow using of CSRs in S mode
> - Change if return else return to if return return
> - Change static_branch_likely to static_branch_unlikely
> - Fix 32-bit clint_get_cycles64 csr_available check being inverted
> - Fix is_c900_clint being uninitialized
> V4 -> V5:
> - Remove riscv,csr-clint
> - Remove riscv_csr_time_available
> - Replace all static_branch_unlikely(&riscv_csr_time_available) with
> riscv_has_extension_likely(RISCV_ISA_EXT_ZICNTR)
> ---
> arch/riscv/include/asm/timex.h | 12 +++++++-
> drivers/clocksource/timer-clint.c | 50 +++++++++++++++++++++++++++----
> 2 files changed, 56 insertions(+), 6 deletions(-)
>
> diff --git a/arch/riscv/include/asm/timex.h b/arch/riscv/include/asm/timex.h
> index a06697846e69..d78075351bf4 100644
> --- a/arch/riscv/include/asm/timex.h
> +++ b/arch/riscv/include/asm/timex.h
> @@ -17,17 +17,26 @@ typedef unsigned long cycles_t;
> #ifdef CONFIG_64BIT
> static inline cycles_t get_cycles(void)
> {
> + if (riscv_has_extension_likely(RISCV_ISA_EXT_ZICNTR))
> + return csr_read(CSR_TIME);
> +
> return readq_relaxed(clint_time_val);
> }
> #else /* !CONFIG_64BIT */
> static inline u32 get_cycles(void)
> {
> + if (riscv_has_extension_likely(RISCV_ISA_EXT_ZICNTR))
> + return csr_read(CSR_TIME);
> +
> return readl_relaxed(((u32 *)clint_time_val));
> }
> #define get_cycles get_cycles
>
> static inline u32 get_cycles_hi(void)
> {
> + if (riscv_has_extension_likely(RISCV_ISA_EXT_ZICNTR))
> + return csr_read(CSR_TIMEH);
> +
> return readl_relaxed(((u32 *)clint_time_val) + 1);
> }
> #define get_cycles_hi get_cycles_hi
> @@ -40,7 +49,8 @@ static inline u32 get_cycles_hi(void)
> */
> static inline unsigned long random_get_entropy(void)
> {
> - if (unlikely(clint_time_val == NULL))
> + if (!riscv_has_extension_likely(RISCV_ISA_EXT_ZICNTR) &&
> + (unlikely(clint_time_val == NULL)))
> return random_get_entropy_fallback();
> return get_cycles();
> }
> diff --git a/drivers/clocksource/timer-clint.c b/drivers/clocksource/timer-clint.c
> index 0bdd9d7ec545..cdf8b33d2dd2 100644
> --- a/drivers/clocksource/timer-clint.c
> +++ b/drivers/clocksource/timer-clint.c
> @@ -39,6 +39,7 @@ static u64 __iomem *clint_timer_cmp;
> static u64 __iomem *clint_timer_val;
> static unsigned long clint_timer_freq;
> static unsigned int clint_timer_irq;
> +static bool is_c900_clint;
>
> #ifdef CONFIG_RISCV_M_MODE
> u64 __iomem *clint_time_val;
> @@ -79,6 +80,9 @@ static void clint_ipi_interrupt(struct irq_desc *desc)
> #ifdef CONFIG_64BIT
> static u64 notrace clint_get_cycles64(void)
> {
> + if (riscv_has_extension_likely(RISCV_ISA_EXT_ZICNTR))
> + return csr_read(CSR_TIME);
> +
> return clint_get_cycles();
> }
> #else /* CONFIG_64BIT */
> @@ -86,10 +90,17 @@ static u64 notrace clint_get_cycles64(void)
> {
> u32 hi, lo;
>
> - do {
> - hi = clint_get_cycles_hi();
> - lo = clint_get_cycles();
> - } while (hi != clint_get_cycles_hi());
> + if (riscv_has_extension_likely(RISCV_ISA_EXT_ZICNTR)) {
> + do {
> + hi = csr_read(CSR_TIMEH);
> + lo = csr_read(CSR_TIME);
> + } while (hi != csr_read(CSR_TIMEH));
> + } else {
> + do {
> + hi = clint_get_cycles_hi();
> + lo = clint_get_cycles();
> + } while (hi != clint_get_cycles_hi());
> + }
>
> return ((u64)hi << 32) | lo;
> }
> @@ -119,6 +130,19 @@ static int clint_clock_next_event(unsigned long delta,
> return 0;
> }
>
> +static int c900_clint_clock_next_event(unsigned long delta,
> + struct clock_event_device *ce)
> +{
> + void __iomem *r = clint_timer_cmp +
> + cpuid_to_hartid_map(smp_processor_id());
> + u64 val = clint_get_cycles64() + delta;
> +
> + csr_set(CSR_IE, IE_TIE);
> + writel_relaxed(val, r);
> + writel_relaxed(val >> 32, r + 4);
> + return 0;
> +}
> +
> static DEFINE_PER_CPU(struct clock_event_device, clint_clock_event) = {
> .name = "clint_clockevent",
> .features = CLOCK_EVT_FEAT_ONESHOT,
> @@ -130,6 +154,9 @@ static int clint_timer_starting_cpu(unsigned int cpu)
> {
> struct clock_event_device *ce = per_cpu_ptr(&clint_clock_event, cpu);
>
> + if (is_c900_clint)
> + ce->set_next_event = c900_clint_clock_next_event;
> +
> ce->cpumask = cpumask_of(cpu);
> clockevents_config_and_register(ce, clint_timer_freq, 100, ULONG_MAX);
>
> @@ -161,7 +188,7 @@ static irqreturn_t clint_timer_interrupt(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> -static int __init clint_timer_init_dt(struct device_node *np)
> +static int __init clint_timer_init(struct device_node *np)
> {
> int rc;
> u32 i, nr_irqs;
> @@ -273,5 +300,18 @@ static int __init clint_timer_init_dt(struct device_node *np)
> return rc;
> }
>
> +static int __init clint_timer_init_dt(struct device_node *np)
> +{
> + is_c900_clint = false;
> + return clint_timer_init(np);
> +}
> +
> +static int __init c900_clint_timer_init_dt(struct device_node *np)
> +{
> + is_c900_clint = true;
> + return clint_timer_init(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, "thead,c900-clint", c900_clint_timer_init_dt);
> --
> 2.45.2
>
Any updates on this palmer?
It seems to be marked as handled elsewhere on patchwork, but i'm not
sure where and iirc i didn't mark it as that.
https://patchwork.kernel.org/project/linux-riscv/patch/20250117164536.3918949-1-Mr.Bossman075@gmail.com/
Thanks,
Jesse Taube
Powered by blists - more mailing lists