Re: [PATCH v1 3/4] xen/riscv: allow Xen to use SSTC while hiding it from guests

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 3/24/26 3:32 PM, Jan Beulich wrote:
> On 13.03.2026 17:44, Oleksii Kurochko wrote:
> > OpenSBI currently does not advertise the SSTC extension via the device
> > tree. Additionally, SSTC can no longer be reliably disabled by removing
> > the "sstc" string from riscv,isa, as OpenSBI probes support by attempting
> > to access CSR_STIMECMP.
> Still don't yopu need to remove that string from what guests get to see, ...
>
> > Introduce a runtime probe in Xen to determine whether SSTC is available.
> > The probe attempts to read CSR_STIMECMP using csr_allowed_read(). 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.
> >
> > To prevent guests from using SSTC, RISCV_ISA_EXT_sstc is cleared from the
> > riscv_isa bitmap. As a result, the corresponding HENVCFG bit is not set
> > and guests fall back to the SBI timer interface. Timer requests are then
> > handled by Xen via the usual SBI interception path.
> ... alongside the riscv_isa adjustment?
Right, I have to mentioned that in the commit message. It will be 
skipped for riscv_isa property here:
 
https://lore.kernel.org/xen-devel/cover.1773157782.git.oleksii.kurochko@xxxxxxxxx/T/#m6e45279d24258fe78c6aebf29379fa9135ec9f1c
(what will require to add SSTC extension to guest_unsupp_exts.)

I will add to commit message that except HEVFCFG bit shouldn't be set to 
not let a guest try to access VSTIMECMP register, it should be droppped 
(or not added) from riscv,isa property.
> > --- a/xen/arch/riscv/domain.c
> > +++ b/xen/arch/riscv/domain.c
> > @@ -99,6 +99,9 @@ static void vcpu_csr_init(struct vcpu *v)
> >       if ( riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svpbmt) )
> >           v->arch.henvcfg = ENVCFG_PBMTE & csr_masks.henvcfg;
> >   
> > +    if ( riscv_isa_extension_available(NULL, RISCV_ISA_EXT_sstc) )
> > +        v->arch.henvcfg |= ENVCFG_STCE & csr_masks.henvcfg;
> Wouldn't this better be part of the (future) patch enabling SSTC for guests?
Probably, it will be better. Lets drop these changes and re-introduce 
later when guests will support SSTC.
> > --- 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
> I think it would be nice if throughout the CSR definitions you settled on
> using upper case hex digits uniformly, or all lower case ones (personally
> I'd prefer the latter).
>
> > --- a/xen/arch/riscv/time.c
> > +++ b/xen/arch/riscv/time.c
> > @@ -13,6 +13,20 @@
> >   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)
> > +{
> > +#ifdef CONFIG_RISCV_32
> > +    csr_write(CSR_STIMECMP, deadline & 0xFFFFFFFF);
> The "& 0x..." isn't needed here, is it? I.e. the whole function could be ...
I think you are right, it "& 0x..." could be dropped as it anyway will 
be truncated by (unsigned long) cast inside csr_write().
Thanks.

~ Oleksii