[PATCH 2/3] lib: utils/timer: mtimer: add a quirk for lacking mtime register
Anup Patel
anup at brainfault.org
Sat Dec 10 08:58:37 PST 2022
On Fri, Dec 9, 2022 at 12:31 PM Icenowy Zheng <uwu at icenowy.me> wrote:
>
> 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.
>
> Add a quirk into MTIMER driver, which represents a mtime register is
> lacking and time CSR value should be used instead.
>
> Signed-off-by: Icenowy Zheng <uwu at icenowy.me>
> ---
> include/sbi_utils/timer/aclint_mtimer.h | 1 +
> lib/utils/timer/aclint_mtimer.c | 19 +++++++++++-----
> lib/utils/timer/fdt_timer_mtimer.c | 29 +++++++++++++++++--------
> 3 files changed, 35 insertions(+), 14 deletions(-)
>
> diff --git a/include/sbi_utils/timer/aclint_mtimer.h b/include/sbi_utils/timer/aclint_mtimer.h
> index 6ab8799..9724f53 100644
> --- a/include/sbi_utils/timer/aclint_mtimer.h
> +++ b/include/sbi_utils/timer/aclint_mtimer.h
> @@ -35,6 +35,7 @@ struct aclint_mtimer_data {
> u32 hart_count;
> bool has_64bit_mmio;
> bool has_shared_mtime;
> + bool has_mtime;
No need for a separate "has_mtime" flag.
If "mtime_size != 0" then MTIME mmio register is present else it is absent.
Update this patch accordingly.
> /* Private details (initialized and used by ACLINT MTIMER library) */
> struct aclint_mtimer_data *time_delta_reference;
> unsigned long time_delta_computed;
> diff --git a/lib/utils/timer/aclint_mtimer.c b/lib/utils/timer/aclint_mtimer.c
> index 3f00c21..f99857b 100644
> --- a/lib/utils/timer/aclint_mtimer.c
> +++ b/lib/utils/timer/aclint_mtimer.c
> @@ -95,7 +95,7 @@ void aclint_mtimer_sync(struct aclint_mtimer_data *mt)
> struct aclint_mtimer_data *reference;
>
> /* Sync-up non-shared MTIME if reference is available */
> - if (mt->has_shared_mtime || !mt->time_delta_reference)
> + if (!mt->has_mtime || mt->has_shared_mtime || !mt->time_delta_reference)
> return;
>
> reference = mt->time_delta_reference;
> @@ -149,10 +149,10 @@ int aclint_mtimer_cold_init(struct aclint_mtimer_data *mt,
> int rc;
>
> /* Sanity checks */
> - if (!mt || !mt->mtime_size ||
> + if (!mt || (mt->has_mtime && !mt->mtime_size) ||
> (mt->hart_count && !mt->mtimecmp_size) ||
> - (mt->mtime_addr & (ACLINT_MTIMER_ALIGN - 1)) ||
> - (mt->mtime_size & (ACLINT_MTIMER_ALIGN - 1)) ||
> + (mt->has_mtime && (mt->mtime_addr & (ACLINT_MTIMER_ALIGN - 1))) ||
> + (mt->has_mtime && (mt->mtime_size & (ACLINT_MTIMER_ALIGN - 1))) ||
> (mt->mtimecmp_addr & (ACLINT_MTIMER_ALIGN - 1)) ||
> (mt->mtimecmp_size & (ACLINT_MTIMER_ALIGN - 1)) ||
> (mt->first_hartid >= SBI_HARTMASK_MAX_BITS) ||
> @@ -179,7 +179,16 @@ int aclint_mtimer_cold_init(struct aclint_mtimer_data *mt,
> mtimer_hartid2data[mt->first_hartid + i] = mt;
>
> /* Add MTIMER regions to the root domain */
> - if (mt->mtime_addr == (mt->mtimecmp_addr + mt->mtimecmp_size)) {
> + if (!mt->has_mtime) {
> + rc = sbi_domain_root_add_memrange(mt->mtimecmp_addr,
> + mt->mtimecmp_size, MTIMER_REGION_ALIGN,
> + SBI_DOMAIN_MEMREGION_MMIO);
> + if (rc)
> + return rc;
> +
> + /* Disable reading mtime when mtime is not available */
> + mtimer.timer_value = NULL;
> + } else if (mt->mtime_addr == (mt->mtimecmp_addr + mt->mtimecmp_size)) {
No need for this "if ()" block since "mt->mtime_size == 0" can imply
MTIME is absent.
You only need a separate "if ()" for setting "mtimer.timer_value = NULL;".
> rc = sbi_domain_root_add_memrange(mt->mtimecmp_addr,
> mt->mtime_size + mt->mtimecmp_size,
> MTIMER_REGION_ALIGN,
> diff --git a/lib/utils/timer/fdt_timer_mtimer.c b/lib/utils/timer/fdt_timer_mtimer.c
> index 7b8546b..7ec3907 100644
> --- a/lib/utils/timer/fdt_timer_mtimer.c
> +++ b/lib/utils/timer/fdt_timer_mtimer.c
> @@ -18,6 +18,7 @@
> struct timer_mtimer_quirks {
> unsigned int mtime_offset;
> bool has_64bit_mmio;
> + bool without_mtime;
> };
>
> static unsigned long mtimer_count = 0;
> @@ -53,14 +54,19 @@ static int timer_mtimer_cold_init(void *fdt, int nodeoff,
> /* Set CLINT addresses */
> mt->mtimecmp_addr = addr[0] + ACLINT_DEFAULT_MTIMECMP_OFFSET;
> mt->mtimecmp_size = ACLINT_DEFAULT_MTIMECMP_SIZE;
> - mt->mtime_addr = addr[0] + ACLINT_DEFAULT_MTIME_OFFSET;
> - mt->mtime_size = size[0] - mt->mtimecmp_size;
> + if (!quirks->without_mtime) {
> + mt->mtime_addr = addr[0] + ACLINT_DEFAULT_MTIME_OFFSET;
> + mt->mtime_size = size[0] - mt->mtimecmp_size;
> + }
> /* Adjust MTIMER address and size for CLINT device */
> - mt->mtime_addr += quirks->mtime_offset;
> + if (!quirks->without_mtime) {
> + mt->mtime_addr += quirks->mtime_offset;
> + mt->mtime_size -= quirks->mtime_offset;
> + }
> mt->mtimecmp_addr += quirks->mtime_offset;
> - mt->mtime_size -= quirks->mtime_offset;
> /* Apply additional CLINT quirks */
> mt->has_64bit_mmio = quirks->has_64bit_mmio;
> + mt->has_mtime = !quirks->without_mtime;
> } else { /* RISC-V ACLINT MTIMER */
> /* Set ACLINT MTIMER addresses */
> mt->mtime_addr = addr[0];
> @@ -70,12 +76,17 @@ static int timer_mtimer_cold_init(void *fdt, int nodeoff,
> }
>
> /* Check if MTIMER device has shared MTIME address */
> - mt->has_shared_mtime = false;
> - for (i = 0; i < mtimer_count; i++) {
> - if (mtimer[i].mtime_addr == mt->mtime_addr) {
> - mt->has_shared_mtime = true;
> - break;
> + if (mt->has_mtime) {
> + mt->has_shared_mtime = false;
> + for (i = 0; i < mtimer_count; i++) {
> + if (mtimer[i].mtime_addr == mt->mtime_addr) {
> + mt->has_shared_mtime = true;
> + break;
> + }
> }
> + } else {
> + /* Assume shared time CSR */
> + mt->has_shared_mtime = true;
> }
>
> /* Initialize the MTIMER device */
> --
> 2.38.1
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
Regards,
Anup
More information about the opensbi
mailing list