[PATCH 1/3] KVM: arm64: nv: only emulate timers that have not yet fired

[Ganapatrao Kulkarni]

Hi Marc,

On 29-12-2022 06:30 pm, Marc Zyngier wrote:
> On Wed, 24 Aug 2022 07:03:02 +0100,
> Ganapatrao Kulkarni <gankulkarni at os.amperecomputing.com> wrote:
>>
>> From: D Scott Phillips <scott at os.amperecomputing.com>
>>
>> The timer emulation logic goes into an infinite loop when the NestedVM(L2)
>> timer is being emulated.
>>
>> While the CPU is executing in L1 context, the L2 timers are emulated using
>> host hrtimer. When the delta of cval and current time reaches zero, the
>> vtimer interrupt is fired/forwarded to L2, however the emulation function
>> in Host-Hypervisor(L0) is still restarting the hrtimer with an expiry time
>> set to now, triggering hrtimer to fire immediately and resulting in a
>> continuous trigger of hrtimer and endless looping in the timer emulation.
>>
>> Adding a fix to avoid restarting of the hrtimer if the interrupt is
>> already fired.
>>
>> Signed-off-by: D Scott Phillips <scott at os.amperecomputing.com>
>> Signed-off-by: Ganapatrao Kulkarni <gankulkarni at os.amperecomputing.com>
>> ---
>>   arch/arm64/kvm/arch_timer.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
>> index 2371796b1ab5..27a6ec46803a 100644
>> --- a/arch/arm64/kvm/arch_timer.c
>> +++ b/arch/arm64/kvm/arch_timer.c
>> @@ -472,7 +472,8 @@ static void timer_emulate(struct arch_timer_context *ctx)
>>   		return;
>>   	}
>>   
>> -	soft_timer_start(&ctx->hrtimer, kvm_timer_compute_delta(ctx));
>> +	if (!ctx->irq.level)
>> +		soft_timer_start(&ctx->hrtimer, kvm_timer_compute_delta(ctx));
>>   }
>>   
>>   static void timer_save_state(struct arch_timer_context *ctx)
> 
> I think this is a regression introduced by bee038a67487 ("KVM:
> arm/arm64: Rework the timer code to use a timer_map"), and you can see
> it because the comment in this function doesn't make much sense
> anymore.

OK, check was removed while rework in bee038a67487.
> 
> Does the following work for you, mostly restoring the original code?

Below diff too works and avoids the unnecessary soft timer restarts with 
zero delta.
> 
> Thanks,
> 
> 	M.
> 
> diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
> index ad2a5df88810..4945c5b96f05 100644
> --- a/arch/arm64/kvm/arch_timer.c
> +++ b/arch/arm64/kvm/arch_timer.c
> @@ -480,7 +480,7 @@ static void timer_emulate(struct arch_timer_context *ctx)
>   	 * scheduled for the future.  If the timer cannot fire at all,
>   	 * then we also don't need a soft timer.
>   	 */
> -	if (!kvm_timer_irq_can_fire(ctx)) {
> +	if (should_fire || !kvm_timer_irq_can_fire(ctx)) {

Now, aligns to comment.
>   		soft_timer_cancel(&ctx->hrtimer);
>   		return;
>   	}
> 

Shall I resend this patch as regression fix of bee038a67487?

Thanks,
Ganapat