|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [PATCH] xen/riscv: allow Xen to use SSTC while hiding it from guests
OpenSBI currently does not advertise the SSTC extension via the device tree, so if SSTC support is detected by Xen the riscv_isa bitmap is updated manually. Furthermore, removing the "sstc" string from riscv,isa is not a reliable way to disable SSTC, because OpenSBI probes support by attempting to access CSR_STIMECMP. Introduce a runtime probe in Xen to determine whether SSTC is available. The probe attempts to read CSR_STIMECMP using csr_read_safe(). If the access succeeds, SSTC is considered available; if a trap occurs, it is treated as unsupported. When SSTC is detected, Xen may use it internally to program timers. However, the extension is not exposed to guests because the required context switch handling for the SSTC CSRs is not yet implemented. Note: clearing RISCV_ISA_EXT_sstc from the DTS riscv,isa property is deferred to a follow-up patch. Also, the corresponding HENVCFG bit is not set so guests fall back to the SBI timer interface. Timer requests are then handled by Xen via the usual SBI interception path. Introduce set_xen_timer() to abstract how the timer is programmed, either via the SSTC extension or an SBI call. Drop sbi_set_timer() as it is more than enough to have only introduced set_xen_timer(). Drop "SBI v0.2 TIME extension detected" message to avoid confusion which set timer function is really used. Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx> --- CI tests: https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/2437772678 Also, I've manually ran QEMU with SSTC=on/off and built Xen with a newer RISC-V compileer (in comparison with the version used by CI's docker container) to verify CONFIG_CC_HAS_ASM_GOTO_OUTPUT branch inside csr_read_safe(). --- Changes in v3: - Reword print message when SSTC extension is detected. - s/__clear_bit/__set_bit() for the case when SSTC is detected in riscv_fill_hwcap(). Update also the comment above __set_bit(). - Drop BUG_ON()s in vtimer.c. - s/printk/dprintk for the message: SSTC detected... - Drop sbi_set_timer global variable, it is enough just to have set_xen_timer. - As we set bit in riscv_isa bitmap there is no need to use csr_read_safe(CSR_STIMECMP) second time. - Move init of CSR_VSTIMECMP in preinit_xen_time as it looks more correct place. - Update the commit message. --- Changes in v2: - Minor style fixes. - Drop from vcpu_csr_init() setting of SSTC bit in HENVCFG register. Add it back when SSTC for guests will be available. - Add static to set_xen_timer function pointer. - Refactor sstc_set_xen_timer(). - s/csr_allowed_read/csr_read_safe() --- xen/arch/riscv/cpufeature.c | 18 ++++++++ xen/arch/riscv/include/asm/cpufeature.h | 1 + xen/arch/riscv/include/asm/riscv_encoding.h | 2 + xen/arch/riscv/include/asm/sbi.h | 18 -------- xen/arch/riscv/include/asm/time.h | 3 ++ xen/arch/riscv/sbi.c | 29 ++++++++++--- xen/arch/riscv/time.c | 48 ++++++++++++++------- xen/arch/riscv/vtimer.c | 1 + 8 files changed, 81 insertions(+), 39 deletions(-) diff --git a/xen/arch/riscv/cpufeature.c b/xen/arch/riscv/cpufeature.c index 03e27b037be0..92235fdfd5ab 100644 --- a/xen/arch/riscv/cpufeature.c +++ b/xen/arch/riscv/cpufeature.c @@ -17,6 +17,7 @@ #include <xen/sections.h> #include <asm/cpufeature.h> +#include <asm/csr.h> #ifdef CONFIG_ACPI # error "cpufeature.c functions should be updated to support ACPI" @@ -139,6 +140,7 @@ const struct riscv_isa_ext_data __initconst riscv_isa_ext[] = { RISCV_ISA_EXT_DATA(smaia), RISCV_ISA_EXT_DATA(smstateen), RISCV_ISA_EXT_DATA(ssaia), + RISCV_ISA_EXT_DATA(sstc), RISCV_ISA_EXT_DATA(svade), RISCV_ISA_EXT_DATA(svpbmt), }; @@ -483,6 +485,7 @@ void __init riscv_fill_hwcap(void) unsigned int i; const size_t req_extns_amount = ARRAY_SIZE(required_extensions); bool all_extns_available = true; + unsigned long tmp; riscv_fill_hwcap_from_isa_string(); @@ -495,6 +498,21 @@ void __init riscv_fill_hwcap(void) panic("HW capabilities parsing failed: %s\n", failure_msg); } + if ( csr_read_safe(CSR_STIMECMP, &tmp) ) + { + dprintk(XENLOG_DEBUG, + "SSTC detected; supported for Xen use, but not for guests\n"); + + /* + * As there is no any guarantee that SSTC will be added to riscv,isa + * property by OpenSBI(it doesn't add it now) or whatever ran before + * Xen, it is needed to set this bit manually. + * + * Guest isolation is maintained by not setting ENVCFG_STCE in henvcfg. + */ + __set_bit(RISCV_ISA_EXT_sstc, riscv_isa); + } + for ( i = 0; i < req_extns_amount; i++ ) { const struct riscv_isa_ext_data ext = required_extensions[i]; diff --git a/xen/arch/riscv/include/asm/cpufeature.h b/xen/arch/riscv/include/asm/cpufeature.h index ef02a3e26d2c..0c48d57a03bb 100644 --- a/xen/arch/riscv/include/asm/cpufeature.h +++ b/xen/arch/riscv/include/asm/cpufeature.h @@ -38,6 +38,7 @@ enum riscv_isa_ext_id { RISCV_ISA_EXT_smaia, RISCV_ISA_EXT_smstateen, RISCV_ISA_EXT_ssaia, + RISCV_ISA_EXT_sstc, RISCV_ISA_EXT_svade, RISCV_ISA_EXT_svpbmt, RISCV_ISA_EXT_MAX diff --git a/xen/arch/riscv/include/asm/riscv_encoding.h b/xen/arch/riscv/include/asm/riscv_encoding.h index dd15731a86fa..d0d60ba15e62 100644 --- a/xen/arch/riscv/include/asm/riscv_encoding.h +++ b/xen/arch/riscv/include/asm/riscv_encoding.h @@ -396,6 +396,8 @@ #define CSR_VSTVAL 0x243 #define CSR_VSIP 0x244 #define CSR_VSATP 0x280 +#define CSR_VSTIMECMP 0x24d +#define CSR_VSTIMECMPH 0x25d /* Virtual Interrupts and Interrupt Priorities (H-extension with AIA) */ #define CSR_HVIEN 0x608 diff --git a/xen/arch/riscv/include/asm/sbi.h b/xen/arch/riscv/include/asm/sbi.h index ed7af200288f..1952868e963c 100644 --- a/xen/arch/riscv/include/asm/sbi.h +++ b/xen/arch/riscv/include/asm/sbi.h @@ -13,7 +13,6 @@ #define ASM__RISCV__SBI_H #include <xen/cpumask.h> -#include <xen/sections.h> /* SBI-defined implementation ID */ #define SBI_XEN_IMPID 7 @@ -139,23 +138,6 @@ int sbi_remote_hfence_gvma(const cpumask_t *cpu_mask, vaddr_t start, int sbi_remote_hfence_gvma_vmid(const cpumask_t *cpu_mask, vaddr_t start, size_t size, unsigned long vmid); -/* - * Programs the clock for next event at (or after) stime_value. stime_value is - * in absolute time. This function must clear the pending timer interrupt bit - * as well. - * - * If the supervisor wishes to clear the timer interrupt without scheduling the - * next timer event, it can either request a timer interrupt infinitely far - * into the future (i.e., (uint64_t)-1), or it can instead mask the timer - * interrupt by clearing sie.STIE CSR bit. - * - * The stime_value parameter represents absolute time measured in ticks. - * - * This SBI call returns 0 upon success or an implementation specific negative - * error code. - */ -extern int (* __ro_after_init sbi_set_timer)(uint64_t stime_value); - /* * Initialize SBI library * diff --git a/xen/arch/riscv/include/asm/time.h b/xen/arch/riscv/include/asm/time.h index be3875b9984e..4d68900151a7 100644 --- a/xen/arch/riscv/include/asm/time.h +++ b/xen/arch/riscv/include/asm/time.h @@ -4,6 +4,7 @@ #include <xen/bug.h> #include <xen/muldiv64.h> +#include <xen/sections.h> #include <asm/csr.h> @@ -26,6 +27,8 @@ static inline cycles_t get_cycles(void) void preinit_xen_time(void); +extern int (* __ro_after_init set_xen_timer)(uint64_t deadline); + #endif /* ASM__RISCV__TIME_H */ /* diff --git a/xen/arch/riscv/sbi.c b/xen/arch/riscv/sbi.c index b4a7ae6940c1..3576e26033a5 100644 --- a/xen/arch/riscv/sbi.c +++ b/xen/arch/riscv/sbi.c @@ -22,6 +22,7 @@ #include <asm/processor.h> #include <asm/sbi.h> +#include <asm/time.h> static unsigned long __ro_after_init sbi_spec_version = SBI_SPEC_VERSION_DEFAULT; @@ -249,6 +250,21 @@ static int (* __ro_after_init sbi_rfence)(unsigned long fid, unsigned long arg4, unsigned long arg5); +/* + * Programs the clock for next event at (or after) stime_value. stime_value is + * in absolute time. This function must clear the pending timer interrupt bit + * as well. + * + * If the supervisor wishes to clear the timer interrupt without scheduling the + * next timer event, it can either request a timer interrupt infinitely far + * into the future (i.e., (uint64_t)-1), or it can instead mask the timer + * interrupt by clearing sie.STIE CSR bit. + * + * The stime_value parameter represents absolute time measured in ticks. + * + * This SBI call returns 0 upon success or an implementation specific negative + * error code. + */ static int cf_check sbi_set_timer_v02(uint64_t stime_value) { struct sbiret ret; @@ -264,6 +280,10 @@ static int cf_check sbi_set_timer_v02(uint64_t stime_value) return sbi_err_map_xen_errno(ret.error); } +/* + * Legacy SBI v0.1 SET_TIMER; functionally equivalent to sbi_set_timer_v02 + * from Xen's perspective. + */ static int cf_check sbi_set_timer_v01(uint64_t stime_value) { struct sbiret ret; @@ -279,8 +299,6 @@ static int cf_check sbi_set_timer_v01(uint64_t stime_value) return sbi_err_map_xen_errno(ret.error); } -int (* __ro_after_init sbi_set_timer)(uint64_t stime_value) = sbi_set_timer_v01; - int sbi_remote_sfence_vma(const cpumask_t *cpu_mask, vaddr_t start, size_t size) { @@ -360,10 +378,9 @@ int __init sbi_init(void) } if ( sbi_probe_extension(SBI_EXT_TIME) > 0 ) - { - sbi_set_timer = sbi_set_timer_v02; - dprintk(XENLOG_INFO, "SBI v0.2 TIME extension detected\n"); - } + set_xen_timer = sbi_set_timer_v02; + else + set_xen_timer = sbi_set_timer_v01; } else panic("Ooops. SBI spec version 0.1 detected. Need to add support"); diff --git a/xen/arch/riscv/time.c b/xen/arch/riscv/time.c index 7efa76fdbcb1..80f0e9ddae6a 100644 --- a/xen/arch/riscv/time.c +++ b/xen/arch/riscv/time.c @@ -7,12 +7,24 @@ #include <xen/time.h> #include <xen/types.h> +#include <asm/cpufeature.h> #include <asm/csr.h> -#include <asm/sbi.h> unsigned long __ro_after_init cpu_khz; /* CPU clock frequency in kHz. */ uint64_t __ro_after_init boot_clock_cycles; +static int cf_check sstc_set_xen_timer(uint64_t deadline) +{ + csr_write(CSR_STIMECMP, deadline); +#ifdef CONFIG_RISCV_32 + csr_write(CSR_STIMECMPH, deadline >> 32); +#endif + + return 0; +} + +int (* __ro_after_init set_xen_timer)(uint64_t deadline); + s_time_t get_s_time(void) { uint64_t ticks = get_cycles() - boot_clock_cycles; @@ -61,20 +73,7 @@ int reprogram_timer(s_time_t timeout) if ( deadline <= now ) return 0; - /* - * TODO: When the SSTC extension is supported, it would be preferable to - * use the supervisor timer registers directly here for better - * performance, since an SBI call and mode switch would no longer - * be required. - * - * This would also reduce reliance on a specific SBI implementation. - * For example, it is not ideal to panic() if sbi_set_timer() returns - * a non-zero value. Currently it can return 0 or -ENOSUPP, and - * without SSTC we still need an implementation because only the - * M-mode timer is available, and it can only be programmed in - * M-mode. - */ - if ( (rc = sbi_set_timer(deadline)) ) + if ( (rc = set_xen_timer(deadline)) ) panic("%s: timer wasn't set because: %d\n", __func__, rc); /* Enable timer interrupt */ @@ -91,4 +90,23 @@ void __init preinit_xen_time(void) panic("%s: ACPI isn't supported\n", __func__); boot_clock_cycles = get_cycles(); + + /* set_xen_timer must have been set by sbi_init() already */ + ASSERT(set_xen_timer); + + if ( riscv_isa_extension_available(NULL, RISCV_ISA_EXT_sstc) ) + { + set_xen_timer = sstc_set_xen_timer; + + /* + * A VS-timer interrupt becomes pending whenever the value of + * (time + htimedelta) is greater than or equal to vstimecmp CSR. + * Thereby to avoid spurious VS-timer irqs set vstimecmp CSR to + * ULONG_MAX. + */ + csr_write(CSR_VSTIMECMP, ULONG_MAX); +#ifdef CONFIG_RISCV_32 + csr_write(CSR_VSTIMECMPH, ULONG_MAX); +#endif + } } diff --git a/xen/arch/riscv/vtimer.c b/xen/arch/riscv/vtimer.c index afd8a53a7387..d5a8dfcb2edb 100644 --- a/xen/arch/riscv/vtimer.c +++ b/xen/arch/riscv/vtimer.c @@ -4,6 +4,7 @@ #include <xen/sched.h> #include <xen/timer.h> +#include <asm/cpufeature.h> #include <asm/vtimer.h> static void vtimer_expired(void *data) -- 2.53.0
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |