Re: [PATCH v3 12/16] xen/riscv: introduce sbi_set_timer()

[Oleksii Kurochko <oleksii.kurochko@gmail.com>]

On 2/11/26 4:03 PM, Jan Beulich wrote:
> On 09.02.2026 17:52, Oleksii Kurochko wrote:
>> Introduce a function pointer for sbi_set_timer(), since different OpenSBI
>> versions may implement the TIME extension with different extension IDs
>> and/or function IDs.
>>
>> If the TIME extension is not available, fall back to the legacy timer
>> mechanism. This is useful when Xen runs as a guest under another Xen,
>> because the TIME extension is not currently virtualised and therefore
>> will not appear as available.
>> Despite of the fact that sbi_set_timer_v01 is introduced and used as
>> fall back, SBI v0.1 still isn't fully supported (with the current SBI
>> calls usage, sbi_rfence_v01 should be introduced too), so panic()
>> in sbi_init() isn't removed.
>>
>> The sbi_set_timer() pointer will be used by reprogram_timer() to program
>> Xen’s physical timer as without SSTC extension there is no any other
>> option except SBI call to do that as only M-timer is available for us.
>>
>> Use dprintk() for all the cases to print that a speicifc SBI extension
>> is available as it isn't really necessary in case of release builds.
>>
>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>
> with one further request:
>
>> @@ -134,6 +138,20 @@ 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 after stime_value time. This function also
>> + * clears the pending timer interrupt bit.
> "after stime_value time" reads as if this was a relative input, stime_value units
> of time need to pass until the event. Iirc values passed are absolute, though.
> Furthermore it was my understanding that the interrupt being raised is dependent
> upon clock >= value, not clock > value (where the latter is what "after" means
> when taken to apply to an absolute value).

Interesting that sbi_set_timer() has different description for OpenSBI v0.1 and v0.2.
What you see in the comment it is what v0.1 tells, but v0.2 tells that:
   
   "Programs the clock for next event after stime_value time.*stime_value is in absolute time.* This function must clear the pending timer interrupt bit as well."

"stime_value is in absolute time" has been added in v0.2. I'll update the comment for
a clarity.

Thanks.

~ Oleksii