Re: [PATCH] LoongArch: KVM: Reload one-shot TVAL on migration destination

在 2026/7/15 14:21, Bibo Mao 写道:
> 
> 
> On 2026/7/15 下午1:53, Tao Cui wrote:
>>
>>
>> 在 2026/7/15 11:49, Bibo Mao 写道:
>>>
>>>
>>> On 2026/7/15 上午11:28, Tao Cui wrote:
>>>> From: Tao Cui <[email protected]>
>>>>
>>>> kvm_restore_timer() rebuilds the remaining timer countdown from
>>>> vcpu->arch.expire, which is host-internal and is not part of the migrated
>>>> vCPU state. On the migration destination it is still 0, so for a one-shot
>>>> timer that has not expired yet the computed delta is 0 and
>>>> write_gcsr_timertick(0) injects the timer interrupt immediately instead of
>>>> after the remaining time. The guest observes a premature timer event right
>>>> after migration.
>>>>
>>>> The expired one-shot case (TVAL = -1) is already handled earlier, so only
>>>> the non-expired one-shot path is affected. When expire has not been set
>>>> (i.e. on the destination), reload the remaining countdown from the
>>>> migrated TVAL. The regular preempt/resume path on the source, where
>>>> expire is valid, is unchanged.
>>>>
>>>> Fixes: a5857b9ff6e0 ("LoongArch: KVM: Implement vcpu timer operations")
>>>> Signed-off-by: Tao Cui <[email protected]>
>>>> ---
>>>>    arch/loongarch/kvm/timer.c | 8 ++++++++
>>>>    1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/arch/loongarch/kvm/timer.c b/arch/loongarch/kvm/timer.c
>>>> index 3829f35a4070..57f53d19a00a 100644
>>>> --- a/arch/loongarch/kvm/timer.c
>>>> +++ b/arch/loongarch/kvm/timer.c
>>>> @@ -132,6 +132,14 @@ void kvm_restore_timer(struct kvm_vcpu *vcpu)
>>>>             * during injecting intr async
>>>>             */
>>>>            kvm_queue_irq(vcpu, INT_TI);
>>>> +    } else if (!expire) {
>>>> +        /*
>>>> +         * One-shot timer on the migration destination: vcpu->arch.expire
>>>> +         * is host-internal and is not migrated, so it is still 0 here.
>>> Good catch, I ever noticed this issue before however without good method. My previous method is to recalculate vcpu->arch.expire in function _kvm_setcsr() when LOONGARCH_CSR_TVAL is set and value of vcpu->arch.expire is 0.
>>>
>> Thanks for the review and for sharing your idea. I also considered
>> recalculating expire in _kvm_setcsr() when TVAL is restored, but ended
>> up keeping the change within kvm_restore_timer() to avoid touching the
>> core CSR write path. Both approaches are functionally equivalent since
>> the first save_timer() after restore will set expire correctly either
>> way.
>>
>> That said, if you prefer the _kvm_setcsr() approach, I'm happy to
> The problem in _kvm_setcsr() is that there is only register LOONGARCH_CSR_TVAL however no context of LOONGARCH_CSR_TCFG register, it seems that it is better in to restore it in kvm_restore_timer() function where all registers about CPU timer are set.
> 
> How about something like this, it should work with period timer mode when migration.
> @@ -119,6 +119,17 @@ void kvm_restore_timer(struct kvm_vcpu *vcpu)
>         delta = 0;
>         now = ktime_get();
>         expire = vcpu->arch.expire;
> +       if (!expire) {
> +             /*
> +              * The field vcpu->arch.expire is host-internal and is not
> +              * migrated, so it is 0 after migration. Reload the
> +              * remaining countdown from the migrated TVAL.
> +              */
> +               if (ticks < cfg)
> +                       delta = tick_to_ns(vcpu, ticks);
> +               expire = ktime_add_ns(ktime_get(), delta);
> +       }
> +

Thank you for the suggestion. I agree that kvm_restore_timer() is the
right place since all timer registers (TCFG/TVAL) are available there.

I've updated the patch to compute expire from the migrated TVAL before
the normal path, which also fixes the period timer case during migration.
Suggested-by added.

Will send v2 shortly.

Thanks,
Tao
>> rework the patch accordingly.
> ok, that is fine. Thanks for catching this issue.
> 
> Regards
> Bibo Mao
>>
>> Thanks,
>> Tao
>>> Regards
>>> Bibo Mao
>>>> +         * Reload the remaining countdown from the migrated TVAL instead
>>>> +         * of firing the timer immediately.
>>>> +         */
>>>> +        delta = ticks;
>>>>        }
>>>>          write_gcsr_timertick(delta);
>>>>
>>>
>