[kvm-unit-tests PATCH v3 0/5] riscv: sbi: Add support to test timer extension

[kvm-unit-tests PATCH v3 0/5] riscv: sbi: Add support to test timer extension

[James Raphael Tiovalen]

This patch series adds support for testing the timer extension as
defined in the RISC-V SBI specification. The first 2 patches add
infrastructural support for handling interrupts, the next 2 patches add
some helper routines that can be used by SBI extension tests, while the
last patch adds the actual test for the timer extension.

v3:
- Addressed all of Andrew's comments on v2.
- Added 2 new patches to add sbi_probe and the delay and timer routines.

v2:
- Addressed all of the previous comments from Andrew.
- Updated the test to get the timer frequency value from the device tree
  and allow the test parameters to be specified in microseconds instead of
  cycles.

Andrew Jones (1):
  riscv: Extend exception handling support for interrupts

James Raphael Tiovalen (4):
  riscv: Update exception cause list
  riscv: Add method to probe for SBI extensions
  riscv: Add some delay and timer routines
  riscv: sbi: Add test for timer extension

 riscv/Makefile            |   2 +
 lib/riscv/asm/csr.h       |  21 ++++++
 lib/riscv/asm/delay.h     |  15 +++++
 lib/riscv/asm/processor.h |  15 ++++-
 lib/riscv/asm/sbi.h       |   6 ++
 lib/riscv/asm/setup.h     |   1 +
 lib/riscv/asm/timer.h     |  24 +++++++
 lib/riscv/delay.c         |  16 +++++
 lib/riscv/processor.c     |  27 ++++++--
 lib/riscv/sbi.c           |  10 +++
 lib/riscv/setup.c         |   4 ++
 lib/riscv/timer.c         |  26 ++++++++
 riscv/sbi.c               | 135 ++++++++++++++++++++++++++++++++++++++
 13 files changed, 297 insertions(+), 5 deletions(-)
 create mode 100644 lib/riscv/asm/delay.h
 create mode 100644 lib/riscv/asm/timer.h
 create mode 100644 lib/riscv/delay.c
 create mode 100644 lib/riscv/timer.c

--
2.43.0

[kvm-unit-tests PATCH v3 1/5] riscv: Extend exception handling support for interrupts

[James Raphael Tiovalen]

From: Andrew Jones <andrew.jones@linux.dev>

Add install_irq_handler() to enable tests to install interrupt handlers.
Also add local_irq_enable() and local_irq_disable() to respectively
enable and disable IRQs via the sstatus.SIE bit.

Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
Signed-off-by: James Raphael Tiovalen <jamestiotio@gmail.com>
---
 lib/riscv/asm/csr.h       |  2 ++
 lib/riscv/asm/processor.h | 13 +++++++++++++
 lib/riscv/processor.c     | 27 +++++++++++++++++++++++----
 3 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/lib/riscv/asm/csr.h b/lib/riscv/asm/csr.h
index 52608512..d6909d93 100644
--- a/lib/riscv/asm/csr.h
+++ b/lib/riscv/asm/csr.h
@@ -11,6 +11,8 @@
 #define CSR_STVAL		0x143
 #define CSR_SATP		0x180
 
+#define SR_SIE			_AC(0x00000002, UL)
+
 /* Exception cause high bit - is an interrupt if set */
 #define CAUSE_IRQ_FLAG		(_AC(1, UL) << (__riscv_xlen - 1))
 
diff --git a/lib/riscv/asm/processor.h b/lib/riscv/asm/processor.h
index 32c499d0..6451adb5 100644
--- a/lib/riscv/asm/processor.h
+++ b/lib/riscv/asm/processor.h
@@ -5,6 +5,7 @@
 #include <asm/ptrace.h>
 
 #define EXCEPTION_CAUSE_MAX	16
+#define INTERRUPT_CAUSE_MAX	16
 
 typedef void (*exception_fn)(struct pt_regs *);
 
@@ -13,6 +14,7 @@ struct thread_info {
 	unsigned long hartid;
 	unsigned long isa[1];
 	exception_fn exception_handlers[EXCEPTION_CAUSE_MAX];
+	exception_fn interrupt_handlers[INTERRUPT_CAUSE_MAX];
 };
 
 static inline struct thread_info *current_thread_info(void)
@@ -20,7 +22,18 @@ static inline struct thread_info *current_thread_info(void)
 	return (struct thread_info *)csr_read(CSR_SSCRATCH);
 }
 
+static inline void local_irq_enable(void)
+{
+	csr_set(CSR_SSTATUS, SR_SIE);
+}
+
+static inline void local_irq_disable(void)
+{
+	csr_clear(CSR_SSTATUS, SR_SIE);
+}
+
 void install_exception_handler(unsigned long cause, void (*handler)(struct pt_regs *));
+void install_irq_handler(unsigned long cause, void (*handler)(struct pt_regs *));
 void do_handle_exception(struct pt_regs *regs);
 void thread_info_init(void);
 
diff --git a/lib/riscv/processor.c b/lib/riscv/processor.c
index ece7cbff..0dffadc7 100644
--- a/lib/riscv/processor.c
+++ b/lib/riscv/processor.c
@@ -36,10 +36,21 @@ void do_handle_exception(struct pt_regs *regs)
 {
 	struct thread_info *info = current_thread_info();
 
-	assert(regs->cause < EXCEPTION_CAUSE_MAX);
-	if (info->exception_handlers[regs->cause]) {
-		info->exception_handlers[regs->cause](regs);
-		return;
+	if (regs->cause & CAUSE_IRQ_FLAG) {
+		unsigned long irq_cause = regs->cause & ~CAUSE_IRQ_FLAG;
+
+		assert(irq_cause < INTERRUPT_CAUSE_MAX);
+		if (info->interrupt_handlers[irq_cause]) {
+			info->interrupt_handlers[irq_cause](regs);
+			return;
+		}
+	} else {
+		assert(regs->cause < EXCEPTION_CAUSE_MAX);
+
+		if (info->exception_handlers[regs->cause]) {
+			info->exception_handlers[regs->cause](regs);
+			return;
+		}
 	}
 
 	show_regs(regs);
@@ -47,6 +58,14 @@ void do_handle_exception(struct pt_regs *regs)
 	abort();
 }
 
+void install_irq_handler(unsigned long cause, void (*handler)(struct pt_regs *))
+{
+	struct thread_info *info = current_thread_info();
+
+	assert(cause < INTERRUPT_CAUSE_MAX);
+	info->interrupt_handlers[cause] = handler;
+}
+
 void install_exception_handler(unsigned long cause, void (*handler)(struct pt_regs *))
 {
 	struct thread_info *info = current_thread_info();
-- 
2.43.0

[kvm-unit-tests PATCH v3 2/5] riscv: Update exception cause list

[James Raphael Tiovalen]

Update the list of exception and interrupt causes to follow the latest
RISC-V privileged ISA specification (version 20240411 section 18.6.1).

Reviewed-by: Andrew Jones <andrew.jones@linux.dev>
Signed-off-by: James Raphael Tiovalen <jamestiotio@gmail.com>
---
 lib/riscv/asm/csr.h       | 10 ++++++++++
 lib/riscv/asm/processor.h |  2 +-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/lib/riscv/asm/csr.h b/lib/riscv/asm/csr.h
index d6909d93..ba810c9f 100644
--- a/lib/riscv/asm/csr.h
+++ b/lib/riscv/asm/csr.h
@@ -36,6 +36,16 @@
 #define EXC_VIRTUAL_INST_FAULT		22
 #define EXC_STORE_GUEST_PAGE_FAULT	23
 
+/* Interrupt causes */
+#define IRQ_S_SOFT		1
+#define IRQ_VS_SOFT		2
+#define IRQ_S_TIMER		5
+#define IRQ_VS_TIMER		6
+#define IRQ_S_EXT		9
+#define IRQ_VS_EXT		10
+#define IRQ_S_GEXT		12
+#define IRQ_PMU_OVF		13
+
 #ifndef __ASSEMBLY__
 
 #define csr_swap(csr, val)					\
diff --git a/lib/riscv/asm/processor.h b/lib/riscv/asm/processor.h
index 6451adb5..4c9ad968 100644
--- a/lib/riscv/asm/processor.h
+++ b/lib/riscv/asm/processor.h
@@ -4,7 +4,7 @@
 #include <asm/csr.h>
 #include <asm/ptrace.h>
 
-#define EXCEPTION_CAUSE_MAX	16
+#define EXCEPTION_CAUSE_MAX	24
 #define INTERRUPT_CAUSE_MAX	16
 
 typedef void (*exception_fn)(struct pt_regs *);
-- 
2.43.0

[kvm-unit-tests PATCH v3 3/5] riscv: Add method to probe for SBI extensions

[James Raphael Tiovalen]

Add a `sbi_probe` helper method that can be used by SBI extension tests
to check if a given extension is available.

Signed-off-by: James Raphael Tiovalen <jamestiotio@gmail.com>
---
 lib/riscv/asm/sbi.h |  1 +
 lib/riscv/sbi.c     | 10 ++++++++++
 2 files changed, 11 insertions(+)

diff --git a/lib/riscv/asm/sbi.h b/lib/riscv/asm/sbi.h
index d82a384d..5e1a674a 100644
--- a/lib/riscv/asm/sbi.h
+++ b/lib/riscv/asm/sbi.h
@@ -49,6 +49,7 @@ struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0,
 
 void sbi_shutdown(void);
 struct sbiret sbi_hart_start(unsigned long hartid, unsigned long entry, unsigned long sp);
+long sbi_probe(int ext);
 
 #endif /* !__ASSEMBLY__ */
 #endif /* _ASMRISCV_SBI_H_ */
diff --git a/lib/riscv/sbi.c b/lib/riscv/sbi.c
index f39134c4..7d7d09c3 100644
--- a/lib/riscv/sbi.c
+++ b/lib/riscv/sbi.c
@@ -38,3 +38,13 @@ struct sbiret sbi_hart_start(unsigned long hartid, unsigned long entry, unsigned
 {
 	return sbi_ecall(SBI_EXT_HSM, SBI_EXT_HSM_HART_START, hartid, entry, sp, 0, 0, 0);
 }
+
+long sbi_probe(int ext)
+{
+	struct sbiret ret;
+
+	ret = sbi_ecall(SBI_EXT_BASE, SBI_EXT_BASE_PROBE_EXT, ext, 0, 0, 0, 0, 0);
+	assert(!ret.error);
+
+	return ret.value;
+}
-- 
2.43.0

[kvm-unit-tests PATCH v3 4/5] riscv: Add some delay and timer routines

[James Raphael Tiovalen]

Add a delay method that would allow tests to wait for some specified
number of cycles. Also add a conversion helper method between
microseconds and cycles. This conversion is done by using the timebase
frequency, which is obtained during setup via the device tree.

Signed-off-by: James Raphael Tiovalen <jamestiotio@gmail.com>
---
 riscv/Makefile        |  2 ++
 lib/riscv/asm/csr.h   |  1 +
 lib/riscv/asm/delay.h | 15 +++++++++++++++
 lib/riscv/asm/setup.h |  1 +
 lib/riscv/asm/timer.h | 14 ++++++++++++++
 lib/riscv/delay.c     | 16 ++++++++++++++++
 lib/riscv/setup.c     |  4 ++++
 lib/riscv/timer.c     | 26 ++++++++++++++++++++++++++
 8 files changed, 79 insertions(+)
 create mode 100644 lib/riscv/asm/delay.h
 create mode 100644 lib/riscv/asm/timer.h
 create mode 100644 lib/riscv/delay.c
 create mode 100644 lib/riscv/timer.c

diff --git a/riscv/Makefile b/riscv/Makefile
index 919a3ebb..b0cd613f 100644
--- a/riscv/Makefile
+++ b/riscv/Makefile
@@ -30,6 +30,7 @@ cflatobjs += lib/memregions.o
 cflatobjs += lib/on-cpus.o
 cflatobjs += lib/vmalloc.o
 cflatobjs += lib/riscv/bitops.o
+cflatobjs += lib/riscv/delay.o
 cflatobjs += lib/riscv/io.o
 cflatobjs += lib/riscv/isa.o
 cflatobjs += lib/riscv/mmu.o
@@ -38,6 +39,7 @@ cflatobjs += lib/riscv/sbi.o
 cflatobjs += lib/riscv/setup.o
 cflatobjs += lib/riscv/smp.o
 cflatobjs += lib/riscv/stack.o
+cflatobjs += lib/riscv/timer.o
 ifeq ($(ARCH),riscv32)
 cflatobjs += lib/ldiv32.o
 endif
diff --git a/lib/riscv/asm/csr.h b/lib/riscv/asm/csr.h
index ba810c9f..a9b1bd42 100644
--- a/lib/riscv/asm/csr.h
+++ b/lib/riscv/asm/csr.h
@@ -10,6 +10,7 @@
 #define CSR_SCAUSE		0x142
 #define CSR_STVAL		0x143
 #define CSR_SATP		0x180
+#define CSR_TIME		0xc01
 
 #define SR_SIE			_AC(0x00000002, UL)
 
diff --git a/lib/riscv/asm/delay.h b/lib/riscv/asm/delay.h
new file mode 100644
index 00000000..ce540f4c
--- /dev/null
+++ b/lib/riscv/asm/delay.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef _ASMRISCV_DELAY_H_
+#define _ASMRISCV_DELAY_H_
+
+#include <libcflat.h>
+#include <asm/setup.h>
+
+extern void delay(u64 cycles);
+
+static inline uint64_t usec_to_cycles(uint64_t usec)
+{
+	return (timebase_frequency * usec) / 1000000;
+}
+
+#endif /* _ASMRISCV_DELAY_H_ */
diff --git a/lib/riscv/asm/setup.h b/lib/riscv/asm/setup.h
index 7f81a705..a13159bf 100644
--- a/lib/riscv/asm/setup.h
+++ b/lib/riscv/asm/setup.h
@@ -7,6 +7,7 @@
 #define NR_CPUS 16
 extern struct thread_info cpus[NR_CPUS];
 extern int nr_cpus;
+extern uint64_t timebase_frequency;
 int hartid_to_cpu(unsigned long hartid);
 
 void io_init(void);
diff --git a/lib/riscv/asm/timer.h b/lib/riscv/asm/timer.h
new file mode 100644
index 00000000..2e319391
--- /dev/null
+++ b/lib/riscv/asm/timer.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef _ASMRISCV_TIMER_H_
+#define _ASMRISCV_TIMER_H_
+
+#include <asm/csr.h>
+
+extern void timer_get_frequency(const void *fdt);
+
+static inline uint64_t timer_get_cycles(void)
+{
+	return csr_read(CSR_TIME);
+}
+
+#endif /* _ASMRISCV_TIMER_H_ */
diff --git a/lib/riscv/delay.c b/lib/riscv/delay.c
new file mode 100644
index 00000000..6b5c78da
--- /dev/null
+++ b/lib/riscv/delay.c
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2024, James Raphael Tiovalen <jamestiotio@gmail.com>
+ */
+#include <libcflat.h>
+#include <asm/barrier.h>
+#include <asm/delay.h>
+#include <asm/timer.h>
+
+void delay(uint64_t cycles)
+{
+	uint64_t start = timer_get_cycles();
+
+	while ((timer_get_cycles() - start) < cycles)
+		cpu_relax();
+}
diff --git a/lib/riscv/setup.c b/lib/riscv/setup.c
index 50ffb0d0..905ea708 100644
--- a/lib/riscv/setup.c
+++ b/lib/riscv/setup.c
@@ -20,6 +20,7 @@
 #include <asm/page.h>
 #include <asm/processor.h>
 #include <asm/setup.h>
+#include <asm/timer.h>
 
 #define VA_BASE			((phys_addr_t)3 * SZ_1G)
 #if __riscv_xlen == 64
@@ -38,6 +39,7 @@ u32 initrd_size;
 
 struct thread_info cpus[NR_CPUS];
 int nr_cpus;
+uint64_t timebase_frequency;
 
 static struct mem_region riscv_mem_regions[NR_MEM_REGIONS + 1];
 
@@ -199,6 +201,7 @@ void setup(const void *fdt, phys_addr_t freemem_start)
 
 	mem_init(PAGE_ALIGN(__pa(freemem)));
 	cpu_init();
+	timer_get_frequency(dt_fdt());
 	thread_info_init();
 	io_init();
 
@@ -264,6 +267,7 @@ efi_status_t setup_efi(efi_bootinfo_t *efi_bootinfo)
 	}
 
 	cpu_init();
+	timer_get_frequency(dt_fdt());
 	thread_info_init();
 	io_init();
 	initrd_setup();
diff --git a/lib/riscv/timer.c b/lib/riscv/timer.c
new file mode 100644
index 00000000..db8dbb36
--- /dev/null
+++ b/lib/riscv/timer.c
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2024, James Raphael Tiovalen <jamestiotio@gmail.com>
+ */
+#include <libcflat.h>
+#include <devicetree.h>
+#include <asm/setup.h>
+#include <asm/timer.h>
+
+void timer_get_frequency(const void *fdt)
+{
+	const struct fdt_property *prop;
+	u32 *data;
+	int cpus;
+
+	assert_msg(dt_available(), "ACPI not yet supported");
+
+	cpus = fdt_path_offset(fdt, "/cpus");
+	assert(cpus >= 0);
+
+	prop = fdt_get_property(fdt, cpus, "timebase-frequency", NULL);
+	assert(prop != NULL);
+
+	data = (u32 *)prop->data;
+	timebase_frequency = fdt32_to_cpu(*data);
+}
-- 
2.43.0

[kvm-unit-tests PATCH v3 5/5] riscv: sbi: Add test for timer extension

[James Raphael Tiovalen]

Add a test for the set_timer function of the time extension. The test
checks that:
- The time extension is available
- The time counter monotonically increases
- The installed timer interrupt handler is called
- The timer interrupt is received within a reasonable time interval
- The timer interrupt pending bit is cleared after the set_timer SBI
  call is made
- The timer interrupt can be cleared either by requesting a timer
  interrupt infinitely far into the future or by masking the timer
  interrupt

The timer interrupt delay can be set using the TIMER_DELAY environment
variable in microseconds. The default delay value is 1 second. Since the
interrupt can arrive a little later than the specified delay, allow some
margin of error. This margin of error can be specified via the
TIMER_MARGIN environment variable in microseconds. The default margin of
error is 200 milliseconds.

This test has been verified on RV32 and RV64 with OpenSBI using QEMU.

Signed-off-by: James Raphael Tiovalen <jamestiotio@gmail.com>
---
 lib/riscv/asm/csr.h   |   8 +++
 lib/riscv/asm/sbi.h   |   5 ++
 lib/riscv/asm/timer.h |  10 ++++
 riscv/sbi.c           | 136 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 159 insertions(+)

diff --git a/lib/riscv/asm/csr.h b/lib/riscv/asm/csr.h
index a9b1bd42..052c0412 100644
--- a/lib/riscv/asm/csr.h
+++ b/lib/riscv/asm/csr.h
@@ -4,13 +4,17 @@
 #include <linux/const.h>
 
 #define CSR_SSTATUS		0x100
+#define CSR_SIE			0x104
 #define CSR_STVEC		0x105
 #define CSR_SSCRATCH		0x140
 #define CSR_SEPC		0x141
 #define CSR_SCAUSE		0x142
 #define CSR_STVAL		0x143
+#define CSR_SIP			0x144
 #define CSR_SATP		0x180
 #define CSR_TIME		0xc01
+#define CSR_STIMECMP		0x14d
+#define CSR_STIMECMPH		0x15d
 
 #define SR_SIE			_AC(0x00000002, UL)
 
@@ -47,6 +51,10 @@
 #define IRQ_S_GEXT		12
 #define IRQ_PMU_OVF		13
 
+#define IE_TIE			(_AC(0x1, UL) << IRQ_S_TIMER)
+
+#define IP_TIP			IE_TIE
+
 #ifndef __ASSEMBLY__
 
 #define csr_swap(csr, val)					\
diff --git a/lib/riscv/asm/sbi.h b/lib/riscv/asm/sbi.h
index 5e1a674a..73ab5438 100644
--- a/lib/riscv/asm/sbi.h
+++ b/lib/riscv/asm/sbi.h
@@ -16,6 +16,7 @@
 
 enum sbi_ext_id {
 	SBI_EXT_BASE = 0x10,
+	SBI_EXT_TIME = 0x54494d45,
 	SBI_EXT_HSM = 0x48534d,
 	SBI_EXT_SRST = 0x53525354,
 };
@@ -37,6 +38,10 @@ enum sbi_ext_hsm_fid {
 	SBI_EXT_HSM_HART_SUSPEND,
 };
 
+enum sbi_ext_time_fid {
+	SBI_EXT_TIME_SET_TIMER = 0,
+};
+
 struct sbiret {
 	long error;
 	long value;
diff --git a/lib/riscv/asm/timer.h b/lib/riscv/asm/timer.h
index 2e319391..cd20262f 100644
--- a/lib/riscv/asm/timer.h
+++ b/lib/riscv/asm/timer.h
@@ -11,4 +11,14 @@ static inline uint64_t timer_get_cycles(void)
 	return csr_read(CSR_TIME);
 }
 
+static inline void timer_irq_enable(void)
+{
+	csr_set(CSR_SIE, IE_TIE);
+}
+
+static inline void timer_irq_disable(void)
+{
+	csr_clear(CSR_SIE, IE_TIE);
+}
+
 #endif /* _ASMRISCV_TIMER_H_ */
diff --git a/riscv/sbi.c b/riscv/sbi.c
index 762e9711..9798b989 100644
--- a/riscv/sbi.c
+++ b/riscv/sbi.c
@@ -6,7 +6,21 @@
  */
 #include <libcflat.h>
 #include <stdlib.h>
+#include <limits.h>
+#include <asm/barrier.h>
+#include <asm/csr.h>
+#include <asm/delay.h>
+#include <asm/isa.h>
+#include <asm/processor.h>
 #include <asm/sbi.h>
+#include <asm/smp.h>
+#include <asm/timer.h>
+
+static bool timer_works;
+static bool mask_timer_irq;
+static bool timer_irq_set;
+static bool timer_irq_cleared;
+static unsigned long timer_irq_count;
 
 static void help(void)
 {
@@ -19,6 +33,33 @@ static struct sbiret __base_sbi_ecall(int fid, unsigned long arg0)
 	return sbi_ecall(SBI_EXT_BASE, fid, arg0, 0, 0, 0, 0, 0);
 }
 
+static struct sbiret __time_sbi_ecall(unsigned long stime_value)
+{
+	return sbi_ecall(SBI_EXT_TIME, SBI_EXT_TIME_SET_TIMER, stime_value, 0, 0, 0, 0, 0);
+}
+
+static inline bool timer_irq_pending(void)
+{
+	return csr_read(CSR_SIP) & IP_TIP;
+}
+
+static void timer_irq_handler(struct pt_regs *regs)
+{
+	timer_irq_count = (timer_irq_count == ULONG_MAX) ? ULONG_MAX : timer_irq_count + 1;
+
+	timer_works = true;
+	if (timer_irq_pending())
+		timer_irq_set = true;
+
+	if (mask_timer_irq)
+		timer_irq_disable();
+	else {
+		__time_sbi_ecall(ULONG_MAX);
+		if (!timer_irq_pending())
+			timer_irq_cleared = true;
+	}
+}
+
 static bool env_or_skip(const char *env)
 {
 	if (!getenv(env)) {
@@ -112,6 +153,100 @@ static void check_base(void)
 	report_prefix_pop();
 }
 
+static void check_time(void)
+{
+	struct sbiret ret;
+	unsigned long begin, end, duration;
+	unsigned long d = getenv("TIMER_DELAY") ? strtol(getenv("TIMER_DELAY"), NULL, 0)
+						: 1000000;
+	unsigned long margin = getenv("TIMER_MARGIN") ? strtol(getenv("TIMER_MARGIN"), NULL, 0)
+						      : 200000;
+
+	d = usec_to_cycles(d);
+	margin = usec_to_cycles(margin);
+
+	report_prefix_push("time");
+
+	if (!sbi_probe(SBI_EXT_TIME)) {
+		report_skip("time extension not available");
+		report_prefix_pop();
+		return;
+	}
+
+	begin = timer_get_cycles();
+	delay(d);
+	end = timer_get_cycles();
+	assert(begin + d <= end);
+
+	report_prefix_push("set_timer");
+
+	install_irq_handler(IRQ_S_TIMER, timer_irq_handler);
+	local_irq_enable();
+	if (cpu_has_extension(smp_processor_id(), ISA_SSTC))
+#if __riscv_xlen == 64
+		csr_write(CSR_STIMECMP, ULONG_MAX);
+#else
+		csr_write(CSR_STIMECMPH, ULONG_MAX);
+#endif
+	timer_irq_enable();
+
+	begin = timer_get_cycles();
+	ret = __time_sbi_ecall(begin + d);
+
+	report(!ret.error, "set timer");
+	if (ret.error)
+		report_info("set timer failed with %ld\n", ret.error);
+
+	report(!timer_irq_pending(), "pending timer interrupt bit cleared");
+
+	while ((end = timer_get_cycles()) <= (begin + d + margin) && !timer_works)
+		cpu_relax();
+
+	report(timer_works, "timer interrupt received");
+	report(timer_irq_set, "pending timer interrupt bit set in irq handler");
+	report(timer_irq_cleared, "pending timer interrupt bit cleared by setting timer to -1");
+
+	if (timer_works) {
+		duration = end - begin;
+		report(duration >= d && duration <= (d + margin), "timer delay honored");
+	}
+
+	if (timer_irq_count > 1)
+		report_fail("timer interrupt received multiple times");
+
+	timer_works = false;
+	timer_irq_set = false;
+	timer_irq_count = 0;
+	mask_timer_irq = true;
+	begin = timer_get_cycles();
+	ret = __time_sbi_ecall(begin + d);
+
+	report(!ret.error, "set timer for mask irq test");
+	if (ret.error)
+		report_info("set timer for mask irq test failed with %ld\n", ret.error);
+
+	while ((end = timer_get_cycles()) <= (begin + d + margin) && !timer_works)
+		cpu_relax();
+
+	report(timer_works, "timer interrupt received for mask irq test");
+	report(timer_irq_set, "pending timer interrupt bit set in irq handler for mask irq test");
+
+	if (timer_works) {
+		duration = end - begin;
+		report(duration >= d && duration <= (d + margin),
+		       "timer delay honored for mask irq test");
+	}
+
+	if (timer_irq_count > 1)
+		report_fail("timer interrupt received multiple times for mask irq test");
+
+	local_irq_disable();
+	install_irq_handler(IRQ_S_TIMER, NULL);
+
+	report_prefix_pop();
+	report_prefix_pop();
+}
+
 int main(int argc, char **argv)
 {
 
@@ -122,6 +257,7 @@ int main(int argc, char **argv)
 
 	report_prefix_push("sbi");
 	check_base();
+	check_time();
 
 	return report_summary();
 }
-- 
2.43.0

Re: [kvm-unit-tests PATCH v3 3/5] riscv: Add method to probe for SBI extensions

[Andrew Jones]

On Fri, Jul 19, 2024 at 10:39:45AM GMT, James Raphael Tiovalen wrote:
> Add a `sbi_probe` helper method that can be used by SBI extension tests
> to check if a given extension is available.
>

Suggested-by: Andrew Jones <andrew.jones@linux.dev>

> Signed-off-by: James Raphael Tiovalen <jamestiotio@gmail.com>
> ---
>  lib/riscv/asm/sbi.h |  1 +
>  lib/riscv/sbi.c     | 10 ++++++++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/lib/riscv/asm/sbi.h b/lib/riscv/asm/sbi.h
> index d82a384d..5e1a674a 100644
> --- a/lib/riscv/asm/sbi.h
> +++ b/lib/riscv/asm/sbi.h
> @@ -49,6 +49,7 @@ struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0,
>  
>  void sbi_shutdown(void);
>  struct sbiret sbi_hart_start(unsigned long hartid, unsigned long entry, unsigned long sp);
> +long sbi_probe(int ext);
>  
>  #endif /* !__ASSEMBLY__ */
>  #endif /* _ASMRISCV_SBI_H_ */
> diff --git a/lib/riscv/sbi.c b/lib/riscv/sbi.c
> index f39134c4..7d7d09c3 100644
> --- a/lib/riscv/sbi.c
> +++ b/lib/riscv/sbi.c
> @@ -38,3 +38,13 @@ struct sbiret sbi_hart_start(unsigned long hartid, unsigned long entry, unsigned
>  {
>  	return sbi_ecall(SBI_EXT_HSM, SBI_EXT_HSM_HART_START, hartid, entry, sp, 0, 0, 0);
>  }
> +
> +long sbi_probe(int ext)
> +{
> +	struct sbiret ret;

Let's also add

  ret = sbi_ecall(SBI_EXT_BASE, SBI_EXT_BASE_GET_SPEC_VERSION, 0, 0, 0, 0, 0, 0);
  assert(!ret.error && ret.value >= 2);

> +
> +	ret = sbi_ecall(SBI_EXT_BASE, SBI_EXT_BASE_PROBE_EXT, ext, 0, 0, 0, 0, 0);
> +	assert(!ret.error);
> +
> +	return ret.value;
> +}
> -- 
> 2.43.0
>

With that addition,

Reviewed-by: Andrew Jones <andrew.jones@linux.dev>

Thanks,
drew

Re: [kvm-unit-tests PATCH v3 4/5] riscv: Add some delay and timer routines

[Andrew Jones]

On Fri, Jul 19, 2024 at 10:39:46AM GMT, James Raphael Tiovalen wrote:
> Add a delay method that would allow tests to wait for some specified
> number of cycles. Also add a conversion helper method between
> microseconds and cycles. This conversion is done by using the timebase
> frequency, which is obtained during setup via the device tree.
> 
> Signed-off-by: James Raphael Tiovalen <jamestiotio@gmail.com>
> ---
>  riscv/Makefile        |  2 ++
>  lib/riscv/asm/csr.h   |  1 +
>  lib/riscv/asm/delay.h | 15 +++++++++++++++
>  lib/riscv/asm/setup.h |  1 +
>  lib/riscv/asm/timer.h | 14 ++++++++++++++
>  lib/riscv/delay.c     | 16 ++++++++++++++++
>  lib/riscv/setup.c     |  4 ++++
>  lib/riscv/timer.c     | 26 ++++++++++++++++++++++++++
>  8 files changed, 79 insertions(+)
>  create mode 100644 lib/riscv/asm/delay.h
>  create mode 100644 lib/riscv/asm/timer.h
>  create mode 100644 lib/riscv/delay.c
>  create mode 100644 lib/riscv/timer.c
> 
> diff --git a/riscv/Makefile b/riscv/Makefile
> index 919a3ebb..b0cd613f 100644
> --- a/riscv/Makefile
> +++ b/riscv/Makefile
> @@ -30,6 +30,7 @@ cflatobjs += lib/memregions.o
>  cflatobjs += lib/on-cpus.o
>  cflatobjs += lib/vmalloc.o
>  cflatobjs += lib/riscv/bitops.o
> +cflatobjs += lib/riscv/delay.o
>  cflatobjs += lib/riscv/io.o
>  cflatobjs += lib/riscv/isa.o
>  cflatobjs += lib/riscv/mmu.o
> @@ -38,6 +39,7 @@ cflatobjs += lib/riscv/sbi.o
>  cflatobjs += lib/riscv/setup.o
>  cflatobjs += lib/riscv/smp.o
>  cflatobjs += lib/riscv/stack.o
> +cflatobjs += lib/riscv/timer.o
>  ifeq ($(ARCH),riscv32)
>  cflatobjs += lib/ldiv32.o
>  endif
> diff --git a/lib/riscv/asm/csr.h b/lib/riscv/asm/csr.h
> index ba810c9f..a9b1bd42 100644
> --- a/lib/riscv/asm/csr.h
> +++ b/lib/riscv/asm/csr.h
> @@ -10,6 +10,7 @@
>  #define CSR_SCAUSE		0x142
>  #define CSR_STVAL		0x143
>  #define CSR_SATP		0x180
> +#define CSR_TIME		0xc01
>  
>  #define SR_SIE			_AC(0x00000002, UL)
>  
> diff --git a/lib/riscv/asm/delay.h b/lib/riscv/asm/delay.h
> new file mode 100644
> index 00000000..ce540f4c
> --- /dev/null
> +++ b/lib/riscv/asm/delay.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef _ASMRISCV_DELAY_H_
> +#define _ASMRISCV_DELAY_H_
> +
> +#include <libcflat.h>
> +#include <asm/setup.h>
> +
> +extern void delay(u64 cycles);
> +
> +static inline uint64_t usec_to_cycles(uint64_t usec)
> +{
> +	return (timebase_frequency * usec) / 1000000;
> +}
> +
> +#endif /* _ASMRISCV_DELAY_H_ */
> diff --git a/lib/riscv/asm/setup.h b/lib/riscv/asm/setup.h
> index 7f81a705..a13159bf 100644
> --- a/lib/riscv/asm/setup.h
> +++ b/lib/riscv/asm/setup.h
> @@ -7,6 +7,7 @@
>  #define NR_CPUS 16
>  extern struct thread_info cpus[NR_CPUS];
>  extern int nr_cpus;
> +extern uint64_t timebase_frequency;
>  int hartid_to_cpu(unsigned long hartid);
>  
>  void io_init(void);
> diff --git a/lib/riscv/asm/timer.h b/lib/riscv/asm/timer.h
> new file mode 100644
> index 00000000..2e319391
> --- /dev/null
> +++ b/lib/riscv/asm/timer.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef _ASMRISCV_TIMER_H_
> +#define _ASMRISCV_TIMER_H_
> +
> +#include <asm/csr.h>
> +
> +extern void timer_get_frequency(const void *fdt);

This shouldn't take an fdt pointer as input since it should also work for
ACPI. And, it doesn't need the fdt pointer since everything can get the
pointer with dt_fdt() if it needs it.

> +
> +static inline uint64_t timer_get_cycles(void)
> +{
> +	return csr_read(CSR_TIME);
> +}
> +
> +#endif /* _ASMRISCV_TIMER_H_ */
> diff --git a/lib/riscv/delay.c b/lib/riscv/delay.c
> new file mode 100644
> index 00000000..6b5c78da
> --- /dev/null
> +++ b/lib/riscv/delay.c
> @@ -0,0 +1,16 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2024, James Raphael Tiovalen <jamestiotio@gmail.com>
> + */
> +#include <libcflat.h>
> +#include <asm/barrier.h>
> +#include <asm/delay.h>
> +#include <asm/timer.h>
> +
> +void delay(uint64_t cycles)
> +{
> +	uint64_t start = timer_get_cycles();
> +
> +	while ((timer_get_cycles() - start) < cycles)
> +		cpu_relax();
> +}
> diff --git a/lib/riscv/setup.c b/lib/riscv/setup.c
> index 50ffb0d0..905ea708 100644
> --- a/lib/riscv/setup.c
> +++ b/lib/riscv/setup.c
> @@ -20,6 +20,7 @@
>  #include <asm/page.h>
>  #include <asm/processor.h>
>  #include <asm/setup.h>
> +#include <asm/timer.h>
>  
>  #define VA_BASE			((phys_addr_t)3 * SZ_1G)
>  #if __riscv_xlen == 64
> @@ -38,6 +39,7 @@ u32 initrd_size;
>  
>  struct thread_info cpus[NR_CPUS];
>  int nr_cpus;
> +uint64_t timebase_frequency;
>  
>  static struct mem_region riscv_mem_regions[NR_MEM_REGIONS + 1];
>  
> @@ -199,6 +201,7 @@ void setup(const void *fdt, phys_addr_t freemem_start)
>  
>  	mem_init(PAGE_ALIGN(__pa(freemem)));
>  	cpu_init();
> +	timer_get_frequency(dt_fdt());
>  	thread_info_init();
>  	io_init();
>  
> @@ -264,6 +267,7 @@ efi_status_t setup_efi(efi_bootinfo_t *efi_bootinfo)
>  	}
>  
>  	cpu_init();
> +	timer_get_frequency(dt_fdt());
>  	thread_info_init();
>  	io_init();
>  	initrd_setup();
> diff --git a/lib/riscv/timer.c b/lib/riscv/timer.c
> new file mode 100644
> index 00000000..db8dbb36
> --- /dev/null
> +++ b/lib/riscv/timer.c
> @@ -0,0 +1,26 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2024, James Raphael Tiovalen <jamestiotio@gmail.com>
> + */
> +#include <libcflat.h>
> +#include <devicetree.h>
> +#include <asm/setup.h>
> +#include <asm/timer.h>
> +
> +void timer_get_frequency(const void *fdt)
> +{
> +	const struct fdt_property *prop;
> +	u32 *data;
> +	int cpus;
> +
> +	assert_msg(dt_available(), "ACPI not yet supported");
> +
> +	cpus = fdt_path_offset(fdt, "/cpus");
> +	assert(cpus >= 0);
> +
> +	prop = fdt_get_property(fdt, cpus, "timebase-frequency", NULL);
> +	assert(prop != NULL);

I usually also pass &len to fdt_get_property and then assert that the
length matches my expectations.

> +
> +	data = (u32 *)prop->data;
> +	timebase_frequency = fdt32_to_cpu(*data);
> +}
> -- 
> 2.43.0
>

This patch looks great. With the two changes,

Reviewed-by: Andrew Jones <andrew.jones@linux.dev>

Thanks,
drew

Re: [kvm-unit-tests PATCH v3 5/5] riscv: sbi: Add test for timer extension

[Andrew Jones]

On Fri, Jul 19, 2024 at 10:39:47AM GMT, James Raphael Tiovalen wrote:
> Add a test for the set_timer function of the time extension. The test
> checks that:
> - The time extension is available
> - The time counter monotonically increases
> - The installed timer interrupt handler is called
> - The timer interrupt is received within a reasonable time interval
> - The timer interrupt pending bit is cleared after the set_timer SBI
>   call is made
> - The timer interrupt can be cleared either by requesting a timer
>   interrupt infinitely far into the future or by masking the timer
>   interrupt
> 
> The timer interrupt delay can be set using the TIMER_DELAY environment
> variable in microseconds. The default delay value is 1 second. Since the

Do we need a whole second? I'd reduce the default delay for the interrupt
in order to ensure speedy test execution. I think this can be 200 ms as
well.

> interrupt can arrive a little later than the specified delay, allow some
> margin of error. This margin of error can be specified via the
> TIMER_MARGIN environment variable in microseconds. The default margin of
> error is 200 milliseconds.
> 
> This test has been verified on RV32 and RV64 with OpenSBI using QEMU.

Sentences like these belong in the cover letter or under the --- below
your signoff as they don't belong in the commit (particularly because
it doesn't include opensbi and qemu versions so it's not that useful
of information and because all patches should be tested on rv32 and
rv64, both with and without EFI, before they're merged :-)

> 
> Signed-off-by: James Raphael Tiovalen <jamestiotio@gmail.com>
> ---
>  lib/riscv/asm/csr.h   |   8 +++
>  lib/riscv/asm/sbi.h   |   5 ++
>  lib/riscv/asm/timer.h |  10 ++++
>  riscv/sbi.c           | 136 ++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 159 insertions(+)
> 
> diff --git a/lib/riscv/asm/csr.h b/lib/riscv/asm/csr.h
> index a9b1bd42..052c0412 100644
> --- a/lib/riscv/asm/csr.h
> +++ b/lib/riscv/asm/csr.h
> @@ -4,13 +4,17 @@
>  #include <linux/const.h>
>  
>  #define CSR_SSTATUS		0x100
> +#define CSR_SIE			0x104
>  #define CSR_STVEC		0x105
>  #define CSR_SSCRATCH		0x140
>  #define CSR_SEPC		0x141
>  #define CSR_SCAUSE		0x142
>  #define CSR_STVAL		0x143
> +#define CSR_SIP			0x144
>  #define CSR_SATP		0x180
>  #define CSR_TIME		0xc01
> +#define CSR_STIMECMP		0x14d
> +#define CSR_STIMECMPH		0x15d

Please put these CSRs in numeric order, below CSR_SIP.

>  
>  #define SR_SIE			_AC(0x00000002, UL)
>  
> @@ -47,6 +51,10 @@
>  #define IRQ_S_GEXT		12
>  #define IRQ_PMU_OVF		13
>  
> +#define IE_TIE			(_AC(0x1, UL) << IRQ_S_TIMER)
> +
> +#define IP_TIP			IE_TIE
> +
>  #ifndef __ASSEMBLY__
>  
>  #define csr_swap(csr, val)					\
> diff --git a/lib/riscv/asm/sbi.h b/lib/riscv/asm/sbi.h
> index 5e1a674a..73ab5438 100644
> --- a/lib/riscv/asm/sbi.h
> +++ b/lib/riscv/asm/sbi.h
> @@ -16,6 +16,7 @@
>  
>  enum sbi_ext_id {
>  	SBI_EXT_BASE = 0x10,
> +	SBI_EXT_TIME = 0x54494d45,
>  	SBI_EXT_HSM = 0x48534d,
>  	SBI_EXT_SRST = 0x53525354,
>  };
> @@ -37,6 +38,10 @@ enum sbi_ext_hsm_fid {
>  	SBI_EXT_HSM_HART_SUSPEND,
>  };
>  
> +enum sbi_ext_time_fid {
> +	SBI_EXT_TIME_SET_TIMER = 0,
> +};
> +
>  struct sbiret {
>  	long error;
>  	long value;
> diff --git a/lib/riscv/asm/timer.h b/lib/riscv/asm/timer.h
> index 2e319391..cd20262f 100644
> --- a/lib/riscv/asm/timer.h
> +++ b/lib/riscv/asm/timer.h
> @@ -11,4 +11,14 @@ static inline uint64_t timer_get_cycles(void)
>  	return csr_read(CSR_TIME);
>  }
>  
> +static inline void timer_irq_enable(void)
> +{
> +	csr_set(CSR_SIE, IE_TIE);
> +}
> +
> +static inline void timer_irq_disable(void)
> +{
> +	csr_clear(CSR_SIE, IE_TIE);
> +}
> +
>  #endif /* _ASMRISCV_TIMER_H_ */
> diff --git a/riscv/sbi.c b/riscv/sbi.c
> index 762e9711..9798b989 100644
> --- a/riscv/sbi.c
> +++ b/riscv/sbi.c
> @@ -6,7 +6,21 @@
>   */
>  #include <libcflat.h>
>  #include <stdlib.h>
> +#include <limits.h>
> +#include <asm/barrier.h>
> +#include <asm/csr.h>
> +#include <asm/delay.h>
> +#include <asm/isa.h>
> +#include <asm/processor.h>
>  #include <asm/sbi.h>
> +#include <asm/smp.h>
> +#include <asm/timer.h>
> +
> +static bool timer_works;
> +static bool mask_timer_irq;
> +static bool timer_irq_set;
> +static bool timer_irq_cleared;
> +static unsigned long timer_irq_count;
>  
>  static void help(void)
>  {
> @@ -19,6 +33,33 @@ static struct sbiret __base_sbi_ecall(int fid, unsigned long arg0)
>  	return sbi_ecall(SBI_EXT_BASE, fid, arg0, 0, 0, 0, 0, 0);
>  }
>  
> +static struct sbiret __time_sbi_ecall(unsigned long stime_value)
> +{
> +	return sbi_ecall(SBI_EXT_TIME, SBI_EXT_TIME_SET_TIMER, stime_value, 0, 0, 0, 0, 0);
> +}
> +
> +static inline bool timer_irq_pending(void)
> +{
> +	return csr_read(CSR_SIP) & IP_TIP;
> +}
> +
> +static void timer_irq_handler(struct pt_regs *regs)
> +{
> +	timer_irq_count = (timer_irq_count == ULONG_MAX) ? ULONG_MAX : timer_irq_count + 1;

Unnecessary () and it might read better as

  if (timer_irq_count < ULONG_MAX)
      ++timer_irq_count;

> +
> +	timer_works = true;
> +	if (timer_irq_pending())
> +		timer_irq_set = true;
> +
> +	if (mask_timer_irq)
> +		timer_irq_disable();

If we use {} on one arm of an if-else then we should use them on both,
even if one arm is only a single statement.

> +	else {
> +		__time_sbi_ecall(ULONG_MAX);
> +		if (!timer_irq_pending())
> +			timer_irq_cleared = true;
> +	}
> +}
> +
>  static bool env_or_skip(const char *env)
>  {
>  	if (!getenv(env)) {
> @@ -112,6 +153,100 @@ static void check_base(void)
>  	report_prefix_pop();
>  }
>  
> +static void check_time(void)
> +{
> +	struct sbiret ret;
> +	unsigned long begin, end, duration;
> +	unsigned long d = getenv("TIMER_DELAY") ? strtol(getenv("TIMER_DELAY"), NULL, 0)
> +						: 1000000;
> +	unsigned long margin = getenv("TIMER_MARGIN") ? strtol(getenv("TIMER_MARGIN"), NULL, 0)
> +						      : 200000;
> +
> +	d = usec_to_cycles(d);
> +	margin = usec_to_cycles(margin);

You can also add udelay() to delay.c to provide a convenience
to users, i.e. we could use udelay(d) below without first
converting d from usec to cycles. See lib/arm/delay.c for some
inspiration.

> +
> +	report_prefix_push("time");
> +
> +	if (!sbi_probe(SBI_EXT_TIME)) {
> +		report_skip("time extension not available");
> +		report_prefix_pop();
> +		return;
> +	}
> +
> +	begin = timer_get_cycles();
> +	delay(d);
> +	end = timer_get_cycles();
> +	assert(begin + d <= end);

I don't think we need this begin/end capture and assert because it's only
testing our delay function. We should test our delay function before we
merge it to lib/riscv and then unit tests should be able to trust it
since it's in the lib.

> +
> +	report_prefix_push("set_timer");
> +
> +	install_irq_handler(IRQ_S_TIMER, timer_irq_handler);
> +	local_irq_enable();
> +	if (cpu_has_extension(smp_processor_id(), ISA_SSTC))
> +#if __riscv_xlen == 64
> +		csr_write(CSR_STIMECMP, ULONG_MAX);
> +#else
> +		csr_write(CSR_STIMECMPH, ULONG_MAX);
> +#endif

This should be

	if (cpu_has_extension(smp_processor_id(), ISA_SSTC)) {
		csr_write(CSR_STIMECMP, ULONG_MAX);
 #if __riscv_xlen == 32
		csr_write(CSR_STIMECMPH, ULONG_MAX);
 #endif
        }

since rv32 needs to write both registers.

> +	timer_irq_enable();
> +
> +	begin = timer_get_cycles();
> +	ret = __time_sbi_ecall(begin + d);
> +
> +	report(!ret.error, "set timer");
> +	if (ret.error)
> +		report_info("set timer failed with %ld\n", ret.error);
> +
> +	report(!timer_irq_pending(), "pending timer interrupt bit cleared");
> +
> +	while ((end = timer_get_cycles()) <= (begin + d + margin) && !timer_works)
> +		cpu_relax();
> +
> +	report(timer_works, "timer interrupt received");
> +	report(timer_irq_set, "pending timer interrupt bit set in irq handler");
> +	report(timer_irq_cleared, "pending timer interrupt bit cleared by setting timer to -1");
> +
> +	if (timer_works) {
> +		duration = end - begin;
> +		report(duration >= d && duration <= (d + margin), "timer delay honored");
> +	}
> +
> +	if (timer_irq_count > 1)
> +		report_fail("timer interrupt received multiple times");
> +
> +	timer_works = false;
> +	timer_irq_set = false;
> +	timer_irq_count = 0;
> +	mask_timer_irq = true;
> +	begin = timer_get_cycles();
> +	ret = __time_sbi_ecall(begin + d);
> +
> +	report(!ret.error, "set timer for mask irq test");
> +	if (ret.error)
> +		report_info("set timer for mask irq test failed with %ld\n", ret.error);
> +
> +	while ((end = timer_get_cycles()) <= (begin + d + margin) && !timer_works)
> +		cpu_relax();
> +
> +	report(timer_works, "timer interrupt received for mask irq test");
> +	report(timer_irq_set, "pending timer interrupt bit set in irq handler for mask irq test");
> +
> +	if (timer_works) {
> +		duration = end - begin;
> +		report(duration >= d && duration <= (d + margin),
> +		       "timer delay honored for mask irq test");
> +	}
> +
> +	if (timer_irq_count > 1)
> +		report_fail("timer interrupt received multiple times for mask irq test");
> +
> +	local_irq_disable();
> +	install_irq_handler(IRQ_S_TIMER, NULL);
> +
> +	report_prefix_pop();
> +	report_prefix_pop();
> +}
> +
>  int main(int argc, char **argv)
>  {
>  
> @@ -122,6 +257,7 @@ int main(int argc, char **argv)
>  
>  	report_prefix_push("sbi");
>  	check_base();
> +	check_time();
>  
>  	return report_summary();
>  }
> -- 
> 2.43.0
>

Looks great! Only a couple tweaks remaining and it'll be ready for merge.

Thanks,
drew