Re: [PATCH v1 08/15] xen/riscv: introduce vtimer_set_timer() and vtimer_expired()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.01.2026 10:30, Oleksii Kurochko wrote:
> 
> On 1/15/26 8:52 AM, Jan Beulich wrote:
>> On 14.01.2026 16:59, Oleksii Kurochko wrote:
>>> On 1/14/26 3:57 PM, Jan Beulich wrote:
>>>> On 14.01.2026 13:27, Oleksii Kurochko wrote:
>>>>> On 1/13/26 4:12 PM, Jan Beulich wrote:
>>>>>> On 13.01.2026 15:44, Oleksii Kurochko wrote:
>>>>>>> On 1/8/26 11:28 AM, Jan Beulich wrote:
>>>>>>>> On 24.12.2025 18:03, Oleksii Kurochko wrote:
>>>>>>>>> +    {
>>>>>>>>> +        stop_timer(&t->timer);
>>>>>>>>> +
>>>>>>>>> +        return;
>>>>>>>>> +    }
>>>>>>>>> +
>>>>>>>>> +    set_timer(&t->timer, expires);
>>>>>>>> See the handling of VCPUOP_set_singleshot_timer for what you may want 
>>>>>>>> to
>>>>>>>> do if the expiry asked for is (perhaps just very slightly) into the 
>>>>>>>> past.
>>>>>>> I got an idea why we want to check if "expires" already expired, but ...
>>>>>>>
>>>>>>>> There you'll also find a use of migrate_timer(), which you will want to
>>>>>>>> at least consider using here as well.
>>>>>>> ... I don't get why we want to migrate timer before set_timer() here.
>>>>>>> Could you please explain that?
>>>>>> Didn't I see you use migrate_timer() in other patches (making me assume
>>>>>> you understand)? Having the timer tied to the pCPU where the vCPU runs
>>>>>> means the signalling to that vCPU will (commonly) be cheaper.
>>>>> I thought that migrate_timer() is needed only when a vCPU changes the pCPU
>>>>> it is running on to ensure that it is running on correct pCPU after 
>>>>> migrations,
>>>>> hotplug events, or scheduling changes. That is why I placed it in
>>>>> vtimer_restore(), as there is no guarantee that the vCPU will run on the
>>>>> same pCPU it was running on previously.
>>>>>
>>>>> So that is why ...
>>>>>
>>>>>> Whether
>>>>>> that actually matters depends on what vtimer_expired() will eventually
>>>>>> contain. Hence why I said "consider using".
>>>>> ... I didn't get why I might need vtimer_expired() in vtimer_set_timer()
>>>>> before set_timer().
>>>>>
>>>>> vtimer_expired() will only notify the vCPU that a timer interrupt has
>>>>> occurred by setting bit in irqs_pending bitmap which then will be synced
>>>>> with vcpu->hvip, but I still do not understand whether migrate_timer()
>>>>> is needed before calling set_timer() here.
>>>> Just to repeat - it's not needed. It may be wanted.
>>>>
>>>>> Considering that vtimer_set_timer() is called from the vCPU while it is
>>>>> running on the current pCPU, and assuming no pCPU rescheduling has
>>>>> occurred for this vCPU, we are already on the correct pCPU.
>>>>> If pCPU rescheduling for the vCPU did occur, then migrate_timer() would
>>>>> have been called in context_switch(),
>>>> Even if the timer wasn't active?
>>> Yes, migrate_timer() is called unconditionally in vtimer_restore() called
>>> from context_switch(). migrate_timer() will activate the timer.
>> Which is wrong?
> 
> I don't know, based on the comment above migrate_timer():
>    /* Migrate a timer to a different CPU. The timer may be currently active. 
> */
> 
> it doesn't mention that it shouldn't be called if the timer wasn't active.
> All around other cases where migrate_timer() is used I don't see also that
> anyone checks if a timer is active or not.

Hmm, I'm sorry, I was mis-remembering. Migrating is indeed fine for inactive
timers.

Jan