Re: [PATCH v1 13/15] xen/riscv: implement reprogram_timer() using SBI

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 1/14/26 4:04 PM, Jan Beulich wrote:
> On 14.01.2026 13:41, Oleksii Kurochko wrote:
> > On 1/14/26 12:17 PM, Jan Beulich wrote:
> > > On 14.01.2026 11:33, Oleksii Kurochko wrote:
> > > > On the other hand, if some
> > > > other negative error code is returned, it might be better to return 0 and simply
> > > > allow the timer programming to be retried later.
> > > > However, if we look at the comments for other architectures, the meaning of a
> > > > return value of 0 from this function is:
> > > >     Returns 1 on success; 0 if the timeout is too soon or is in the past.
> > > > In that case, it becomes difficult to distinguish whether 0 was returned due to
> > > > an error or because the timeout was too soon or already in the past.
> > > Well, your problem is that neither Arm nor x86 can actually fail. Hence
> > > calling code isn't presently prepared for that. With panic() (and hence
> > > also BUG()) and domain_crash() ruled out, maybe generic infrastructure
> > > needs touching first (in a different way than making the function's return
> > > type "bool")?
> > I think making the function's return still is fine and it is only question to
> > arch-specific reprogram_timer() what to do when an error happens.
> >
> > Still doesn't clear to me what should be a reaction on failure of
> > reprogram_timer().
> > Considering that SBI spec doesn't specify a list of possible errors and now
> > the only possible error is -ENOSUPP it seems to me it is fine
> > to have panic() as we don't have any other mechanism to set a timer
> > except SBI call
> panic() (or BUG_ON()) is pretty drastic a measure when possibly the system
> could be kept alive. If is pretty certain that future SBI timer calls also
> aren't going to work, then I'd agree that panic()ing might be appropriate.
> If otoh a subsequent call might work, a less heavyweight action would seem
> preferable. (Welcome to the funs of relying on lower-level software.)
I don’t know how OpenSBI will be updated in the future, but looking at the 
current
situation, since SBI timer calls have existed from very early OpenSBI versions 
up
to the latest ones, repeatedly issuing the SBI timer call will always return the
same negative error code indicating that the timer call is not supported.

I can add a comment above panic(), or include an explanation in the commit 
message.

> > (except the case SSTC is supported then we can use just
> > supervisor timer register directly without SBI call).
> So maybe a good first step would be to use that extension if available?
> Might even think about requiring it for the time being ...
I initially started working on SSTC support, but then stopped because an almost
production-ready board to which I do have indirect access does not support this
extension. As a result, I decided to proceed with SBI approach as it covers both
cases when SSTC is available and when no.

~ Oleksii