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

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

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.

~ Oleksii