Received-SPF: pass (google.com: domain of linux-kernel+bounces-118309-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <b063df9b-90b6-4f06-8be5-5a8c267e6c8d@sifive.com>
Date: Mon, 25 Mar 2024 21:39:26 -0500
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 2/5] riscv: nommu: use CSR_TIME* for get_cycles*
 implementation
To: Jisheng Zhang <jszhang@kernel.org>,
 Paul Walmsley <paul.walmsley@sifive.com>, Palmer Dabbelt
 <palmer@dabbelt.com>, Albert Ou <aou@eecs.berkeley.edu>,
 Daniel Lezcano <daniel.lezcano@linaro.org>,
 Thomas Gleixner <tglx@linutronix.de>
Cc: linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org
References: <20240325164021.3229-1-jszhang@kernel.org>
 <20240325164021.3229-3-jszhang@kernel.org>
Content-Language: en-US
From: Samuel Holland <samuel.holland@sifive.com>
In-Reply-To: <20240325164021.3229-3-jszhang@kernel.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

Hi Jisheng,

On 2024-03-25 11:40 AM, Jisheng Zhang wrote:
> Per riscv privileged spec, "The time CSR is a read-only shadow of the
> memory-mapped mtime register", "On RV32I the timeh CSR is a read-only
> shadow of the upper 32 bits of the memory-mapped mtime register, while
> time shadows only the lower 32 bits of mtime." Since get_cycles() only
> reads the timer, it's fine to use CSR_TIME to implement get_cycles().

Unfortunately there are various implementations (e.g. FU740/Unmatched, probably
K210 which this code was originally used for) which do not implement the time
CSR, relying on M-mode software to emulate the CSR so S-mode software doesn't
notice. So this code is needed to support those platforms when running Linux in
M-mode.

Maybe there should be an option to assume the time CSR is/is not implemented,
like there is for misaligned access?

Regards,
Samuel

> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> ---
>  arch/riscv/include/asm/timex.h | 40 ----------------------------------
>  1 file changed, 40 deletions(-)
> 
> diff --git a/arch/riscv/include/asm/timex.h b/arch/riscv/include/asm/timex.h
> index a06697846e69..a3fb85d505d4 100644
> --- a/arch/riscv/include/asm/timex.h
> +++ b/arch/riscv/include/asm/timex.h
> @@ -10,44 +10,6 @@
>  
>  typedef unsigned long cycles_t;
>  
> -#ifdef CONFIG_RISCV_M_MODE
> -
> -#include <asm/clint.h>
> -
> -#ifdef CONFIG_64BIT
> -static inline cycles_t get_cycles(void)
> -{
> -	return readq_relaxed(clint_time_val);
> -}
> -#else /* !CONFIG_64BIT */
> -static inline u32 get_cycles(void)
> -{
> -	return readl_relaxed(((u32 *)clint_time_val));
> -}
> -#define get_cycles get_cycles
> -
> -static inline u32 get_cycles_hi(void)
> -{
> -	return readl_relaxed(((u32 *)clint_time_val) + 1);
> -}
> -#define get_cycles_hi get_cycles_hi
> -#endif /* CONFIG_64BIT */
> -
> -/*
> - * Much like MIPS, we may not have a viable counter to use at an early point
> - * in the boot process. Unfortunately we don't have a fallback, so instead
> - * we just return 0.
> - */
> -static inline unsigned long random_get_entropy(void)
> -{
> -	if (unlikely(clint_time_val == NULL))
> -		return random_get_entropy_fallback();
> -	return get_cycles();
> -}
> -#define random_get_entropy()	random_get_entropy()
> -
> -#else /* CONFIG_RISCV_M_MODE */
> -
>  static inline cycles_t get_cycles(void)
>  {
>  	return csr_read(CSR_TIME);
> @@ -60,8 +22,6 @@ static inline u32 get_cycles_hi(void)
>  }
>  #define get_cycles_hi get_cycles_hi
>  
> -#endif /* !CONFIG_RISCV_M_MODE */
> -
>  #ifdef CONFIG_64BIT
>  static inline u64 get_cycles64(void)
>  {