All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH] KVM: arm64: Restrict CNTP_CVAL/CNTHP_CVAL save/reload when ptimer is used
@ 2026-09-21 11:18 Zhou Wang
  2026-09-21 11:41 ` sashiko-bot
  2026-09-21 13:22 ` Marc Zyngier
  0 siblings, 2 replies; 6+ messages in thread
From: Zhou Wang @ 2026-09-21 11:18 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton, Fuad Tabba, Joey Gouly, Steffen Eiden,
	Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
	Mark Rutland
  Cc: kvmarm, Zhou Wang, Nianyao Tang

A guest's physical timer may be used in nested guest or directly used in
VHE mode. We can check above cases to avoid useless CNTP_CVAL/CNTHP_CVAL save
and reload.

Use lazy detection to determine whether a non-nested VHE guest actually uses
the physical timer.

timer_set_traps() initially traps CNTP_CTL_EL0 writes for non-nested guests.
On the first write that sets CNTP_CTL.ENABLE=1, the trap handler records
ptimer_used and clears the trap in CNTHCTL_EL2, allowing the guest direct
access thereafter.

A test result from KUT in HiSilicon HIP12 is:

                    nopatch        patch
 hvc                758            624
 mmio_read_user     3647           3479
 mmio_read_vgic     949            817
 eoi                29             29
 ipi                2534           2302
 lpi                2684           2442
 timer_10ms         2393           2168

Reported-by: Nianyao Tang <tangnianyao@huawei.com>
Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
---
 arch/arm64/kvm/arch_timer.c     | 15 +++++++++++++++
 arch/arm64/kvm/hyp/vhe/switch.c |  6 ++++--
 include/kvm/arm_arch_timer.h    |  1 +
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
index 6ac3321f4c57..fcbd1e133ad0 100644
--- a/arch/arm64/kvm/arch_timer.c
+++ b/arch/arm64/kvm/arch_timer.c
@@ -832,6 +832,10 @@ static void timer_set_traps(struct kvm_vcpu *vcpu, struct timer_map *map)
 	if (!has_cntpoff() && timer_get_offset(map->direct_ptimer))
 		tpt = tpc = true;
 
+	/* Lazy ptimer detection: trap CTL until guest first enables it */
+	if (has_cntpoff() && !vcpu_has_nv(vcpu) && !vcpu_timer(vcpu)->ptimer_used)
+		tpt = true;
+
 	/*
 	 * For the poor sods that could not correctly subtract one value
 	 * from another, trap the full virtual timer and counter.
@@ -1054,6 +1058,7 @@ void kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu)
 		offs->vm_offset = &vcpu->kvm->arch.timer_data.poffset;
 	}
 
+	timer->ptimer_used = false;
 	if (timer->enabled) {
 		for (int i = 0; i < nr_timers(vcpu); i++)
 			kvm_timer_update_irq(vcpu, false,
@@ -1270,6 +1275,16 @@ void kvm_arm_timer_write_sysreg(struct kvm_vcpu *vcpu,
 		preempt_disable();
 		timer_save_state(timer);
 		kvm_arm_timer_write(vcpu, timer, treg, val);
+
+		/* Lazy ptimer detection: catch first CNTP_CTL enable */
+		if (has_cntpoff() && !vcpu_has_nv(vcpu) &&
+		    tmr == TIMER_PTIMER && treg == TIMER_REG_CTL &&
+		    (val & ARCH_TIMER_CTRL_ENABLE) &&
+		    !vcpu_timer(vcpu)->ptimer_used) {
+			vcpu_timer(vcpu)->ptimer_used = true;
+			sysreg_clear_set(cnthctl_el2, 0,
+					 CNTHCTL_EL1PCEN << 10);
+		}
 		timer_restore_state(timer);
 		preempt_enable();
 	}
diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
index 7875911c0506..0cd73093878a 100644
--- a/arch/arm64/kvm/hyp/vhe/switch.c
+++ b/arch/arm64/kvm/hyp/vhe/switch.c
@@ -122,7 +122,8 @@ static void __activate_traps(struct kvm_vcpu *vcpu)
 
 	___activate_traps(vcpu, __compute_hcr(vcpu));
 
-	if (has_cntpoff()) {
+	if (has_cntpoff() &&
+	    (vcpu_has_nv(vcpu) || vcpu_timer(vcpu)->ptimer_used)) {
 		struct timer_map map;
 
 		get_timer_map(vcpu, &map);
@@ -156,7 +157,8 @@ static void __deactivate_traps(struct kvm_vcpu *vcpu)
 
 	write_sysreg_hcr(HCR_HOST_VHE_FLAGS);
 
-	if (has_cntpoff()) {
+	if (has_cntpoff() &&
+	    (vcpu_has_nv(vcpu) || vcpu_timer(vcpu)->ptimer_used)) {
 		struct timer_map map;
 		u64 val, offset;
 
diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
index bc6f2fdd7ad3..f427246a7726 100644
--- a/include/kvm/arm_arch_timer.h
+++ b/include/kvm/arm_arch_timer.h
@@ -90,6 +90,7 @@ struct arch_timer_cpu {
 
 	/* Is the timer enabled */
 	bool			enabled;
+	bool			ptimer_used;
 };
 
 int __init kvm_timer_hyp_init(bool has_gic);
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] KVM: arm64: Restrict CNTP_CVAL/CNTHP_CVAL save/reload when ptimer is used
  2026-09-21 11:18 [RFC PATCH] KVM: arm64: Restrict CNTP_CVAL/CNTHP_CVAL save/reload when ptimer is used Zhou Wang
@ 2026-09-21 11:41 ` sashiko-bot
  2026-09-21 13:22 ` Marc Zyngier
  1 sibling, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-09-21 11:41 UTC (permalink / raw)
  To: Zhou Wang; +Cc: Marc Zyngier, Oliver Upton, kvmarm

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] kvm: arm64: ptimer_used flag breaks physical timer after live migration
- [High] kvm: arm64: hardcoded cnthctl_el2 write breaks timer traps on NV2p1
--

commit a19fd66b3c0094c0f95419be8e827cd25f55a439
Author: Zhou Wang <wangzhou1@hisilicon.com>

KVM: arm64: Restrict CNTP_CVAL/CNTHP_CVAL save/reload when ptimer is used

This patch introduces a lazy detection mechanism to determine if a
non-nested VHE guest actually uses the physical timer, avoiding useless
timer state saves and reloads. It initially traps CNTP_CTL_EL0 writes
and clears the trap on the first guest enable, granting direct access.

> diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
> index 6ac3321f4c575..fcbd1e133ad0d 100644
> --- a/arch/arm64/kvm/arch_timer.c
> +++ b/arch/arm64/kvm/arch_timer.c
[ ... ]
> @@ -1054,6 +1058,7 @@ void kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu)
>  		offs->vm_offset = &vcpu->kvm->arch.timer_data.poffset;
>  	}
>  
> +	timer->ptimer_used = false;
>  	if (timer->enabled) {
>  		for (int i = 0; i < nr_timers(vcpu); i++)
>  			kvm_timer_update_irq(vcpu, false,
[ ... ]
> @@ -1270,6 +1275,16 @@ void kvm_arm_timer_write_sysreg(struct kvm_vcpu *vcpu,
>  		preempt_disable();
>  		timer_save_state(timer);
>  		kvm_arm_timer_write(vcpu, timer, treg, val);
> +
> +		/* Lazy ptimer detection: catch first CNTP_CTL enable */
> +		if (has_cntpoff() && !vcpu_has_nv(vcpu) &&
> +		    tmr == TIMER_PTIMER && treg == TIMER_REG_CTL &&
> +		    (val & ARCH_TIMER_CTRL_ENABLE) &&
> +		    !vcpu_timer(vcpu)->ptimer_used) {
> +			vcpu_timer(vcpu)->ptimer_used = true;

[Severity: High]
Could this cause the physical timer to break after a live migration?

When a guest enables the physical timer, kvm_arm_timer_write_sysreg()
sets ptimer_used to true. However, during a live migration, userspace
restores state via KVM_SET_ONE_REG, which bypasses this sysreg write
path.

Because kvm_timer_vcpu_reset() initializes ptimer_used to false, it
appears the destination host will permanently leave ptimer_used as
false. Consequently, __activate_traps() in
arch/arm64/kvm/hyp/vhe/switch.c will skip writing the un-offset guest
CVAL to the hardware SYS_CNTP_CVAL. This leaves the hardware retaining
the offset-shifted value, preventing the interrupt from firing at the
correct guest time.

> +			sysreg_clear_set(cnthctl_el2, 0,
> +					 CNTHCTL_EL1PCEN << 10);

[Severity: High]
Does this unconditionally write to the wrong register on hardware with
NV2p1?

If a host is equipped with FEAT_NV2p1 and FEAT_ECV, timer_set_traps() in
arch/arm64/kvm/arch_timer.c correctly checks for ARM64_HAS_NV2P1 and
uses cntkctl_el1 instead of cnthctl_el2 for EL1 trap configuration.

This unconditional write to cnthctl_el2 seems to ignore the NV2p1
hardware requirement. This might fail to clear the trap in hardware
and could potentially corrupt unrelated timer control state in
CNTHCTL_EL2.

> +		}
>  		timer_restore_state(timer);
>  		preempt_enable();
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921111840.3000905-1-wangzhou1@hisilicon.com?part=1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] KVM: arm64: Restrict CNTP_CVAL/CNTHP_CVAL save/reload when ptimer is used
  2026-09-21 11:18 [RFC PATCH] KVM: arm64: Restrict CNTP_CVAL/CNTHP_CVAL save/reload when ptimer is used Zhou Wang
  2026-09-21 11:41 ` sashiko-bot
@ 2026-09-21 13:22 ` Marc Zyngier
  2026-09-22 10:16   ` Zhou Wang
  1 sibling, 1 reply; 6+ messages in thread
From: Marc Zyngier @ 2026-09-21 13:22 UTC (permalink / raw)
  To: Zhou Wang
  Cc: Oliver Upton, Fuad Tabba, Joey Gouly, Steffen Eiden,
	Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
	Mark Rutland, kvmarm, Nianyao Tang

On Mon, 21 Sep 2026 12:18:40 +0100,
Zhou Wang <wangzhou1@hisilicon.com> wrote:
> 
> A guest's physical timer may be used in nested guest or directly used in
> VHE mode. We can check above cases to avoid useless CNTP_CVAL/CNTHP_CVAL save
> and reload.

Why useless? What cases?

> 
> Use lazy detection to determine whether a non-nested VHE guest actually uses
> the physical timer.
> 
> timer_set_traps() initially traps CNTP_CTL_EL0 writes for non-nested guests.
> On the first write that sets CNTP_CTL.ENABLE=1, the trap handler records
> ptimer_used and clears the trap in CNTHCTL_EL2, allowing the guest direct
> access thereafter.

I really don't want to go into these micro optimisation to paper over
the bad state of the architecture.

The options are:

- Don't use any offset (offsets always set to 0), and there will not
  be any trap

- Use CNTPOFF, which will work just fine for an L1 guest, without
  traps

- For nested guests, there is no solution that matches the
  architecture (CNTPOFF doesn't nest), and you must trap.

So where is the cost in your case?

> 
> A test result from KUT in HiSilicon HIP12 is:
> 
>                     nopatch        patch
>  hvc                758            624
>  mmio_read_user     3647           3479
>  mmio_read_vgic     949            817
>  eoi                29             29
>  ipi                2534           2302
>  lpi                2684           2442
>  timer_10ms         2393           2168

I don't know what these numbers are. There is no unit, no indication
of how this is run (L1, L2, Ln...), no base kernel version...

Can you please explain what you are actually trying to do? Are you
saying that accessing the physical timer (two registers) is that
expensive?

Thanks,

	M.

-- 
Jazz isn't dead. It just smells funny.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] KVM: arm64: Restrict CNTP_CVAL/CNTHP_CVAL save/reload when ptimer is used
  2026-09-21 13:22 ` Marc Zyngier
@ 2026-09-22 10:16   ` Zhou Wang
  2026-09-24 17:58     ` Marc Zyngier
  0 siblings, 1 reply; 6+ messages in thread
From: Zhou Wang @ 2026-09-22 10:16 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Oliver Upton, Fuad Tabba, Joey Gouly, Steffen Eiden,
	Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
	Mark Rutland, kvmarm, Nianyao Tang

On 2026/9/21 21:22, Marc Zyngier wrote:
> On Mon, 21 Sep 2026 12:18:40 +0100,
> Zhou Wang <wangzhou1@hisilicon.com> wrote:
>>
>> A guest's physical timer may be used in nested guest or directly used in
>> VHE mode. We can check above cases to avoid useless CNTP_CVAL/CNTHP_CVAL save
>> and reload.
> 
> Why useless? What cases?

I mean we do not use ptimer at least in current Linux kernel, however, for system
which has ECV_CNTPOFF capability, we have to handle related logic here.

So we can detect if ptimer is been used, if it is been used, then we run into the
related codes.

I am not familiar with the nested cases, so just not change related logic.

> 
>>
>> Use lazy detection to determine whether a non-nested VHE guest actually uses
>> the physical timer.
>>
>> timer_set_traps() initially traps CNTP_CTL_EL0 writes for non-nested guests.
>> On the first write that sets CNTP_CTL.ENABLE=1, the trap handler records
>> ptimer_used and clears the trap in CNTHCTL_EL2, allowing the guest direct
>> access thereafter.
> 
> I really don't want to go into these micro optimisation to paper over
> the bad state of the architecture.
> 
> The options are:
> 
> - Don't use any offset (offsets always set to 0), and there will not
>   be any trap

Do you mean that in non-nested case, we can make offset always be 0, so
related logic in __activate_traps/__deactivate_traps can be skipped?

> 
> - Use CNTPOFF, which will work just fine for an L1 guest, without
>   traps
> 
> - For nested guests, there is no solution that matches the
>   architecture (CNTPOFF doesn't nest), and you must trap.

I need more time to figure out how ptimer working in NV(both L1 and L2).

> 
> So where is the cost in your case?
> 
>>
>> A test result from KUT in HiSilicon HIP12 is:
>>
>>                     nopatch        patch
>>  hvc                758            624
>>  mmio_read_user     3647           3479
>>  mmio_read_vgic     949            817
>>  eoi                29             29
>>  ipi                2534           2302
>>  lpi                2684           2442
>>  timer_10ms         2393           2168
> 
> I don't know what these numbers are. There is no unit, no indication
> of how this is run (L1, L2, Ln...), no base kernel version...

The test system does not support NV,so we just run KUT in host based on
the kernel 7.3-rc3. The unit is ns.
> 
> Can you please explain what you are actually trying to do? Are you
> saying that accessing the physical timer (two registers) is that
> expensive?

Well, just as mentioned above, it seems ptimer is rarely used, but we have
to run related codes every time during VM in and out.

Best,
Zhou

> 
> Thanks,
> 
> 	M.
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] KVM: arm64: Restrict CNTP_CVAL/CNTHP_CVAL save/reload when ptimer is used
  2026-09-22 10:16   ` Zhou Wang
@ 2026-09-24 17:58     ` Marc Zyngier
  2026-09-28  8:30       ` Zhou Wang
  0 siblings, 1 reply; 6+ messages in thread
From: Marc Zyngier @ 2026-09-24 17:58 UTC (permalink / raw)
  To: Zhou Wang
  Cc: Oliver Upton, Fuad Tabba, Joey Gouly, Steffen Eiden,
	Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
	Mark Rutland, kvmarm, Nianyao Tang

On Tue, 22 Sep 2026 11:16:36 +0100,
Zhou Wang <wangzhou1@hisilicon.com> wrote:
> 
> On 2026/9/21 21:22, Marc Zyngier wrote:
> > On Mon, 21 Sep 2026 12:18:40 +0100,
> > Zhou Wang <wangzhou1@hisilicon.com> wrote:
> >>
> >> A guest's physical timer may be used in nested guest or directly used in
> >> VHE mode. We can check above cases to avoid useless CNTP_CVAL/CNTHP_CVAL save
> >> and reload.
> > 
> > Why useless? What cases?
> 
> I mean we do not use ptimer at least in current Linux kernel, however, for system
> which has ECV_CNTPOFF capability, we have to handle related logic here.
> 
> So we can detect if ptimer is been used, if it is been used, then we run into the
> related codes.

But where is the actual cost? Is it the saving of these registers?
Something else?

> 
> I am not familiar with the nested cases, so just not change related logic.
> 
> > 
> >>
> >> Use lazy detection to determine whether a non-nested VHE guest actually uses
> >> the physical timer.
> >>
> >> timer_set_traps() initially traps CNTP_CTL_EL0 writes for non-nested guests.
> >> On the first write that sets CNTP_CTL.ENABLE=1, the trap handler records
> >> ptimer_used and clears the trap in CNTHCTL_EL2, allowing the guest direct
> >> access thereafter.
> > 
> > I really don't want to go into these micro optimisation to paper over
> > the bad state of the architecture.
> > 
> > The options are:
> > 
> > - Don't use any offset (offsets always set to 0), and there will not
> >   be any trap
> 
> Do you mean that in non-nested case, we can make offset always be 0, so
> related logic in __activate_traps/__deactivate_traps can be skipped?
>

The *trapping* of the physical timer/counter will be skipped.

	M.

-- 
Without deviation from the norm, progress is not possible.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] KVM: arm64: Restrict CNTP_CVAL/CNTHP_CVAL save/reload when ptimer is used
  2026-09-24 17:58     ` Marc Zyngier
@ 2026-09-28  8:30       ` Zhou Wang
  0 siblings, 0 replies; 6+ messages in thread
From: Zhou Wang @ 2026-09-28  8:30 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Oliver Upton, Fuad Tabba, Joey Gouly, Steffen Eiden,
	Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
	Mark Rutland, kvmarm, Nianyao Tang

On 2026/9/25 1:58, Marc Zyngier wrote:
> On Tue, 22 Sep 2026 11:16:36 +0100,
> Zhou Wang <wangzhou1@hisilicon.com> wrote:
>>
>> On 2026/9/21 21:22, Marc Zyngier wrote:
>>> On Mon, 21 Sep 2026 12:18:40 +0100,
>>> Zhou Wang <wangzhou1@hisilicon.com> wrote:
>>>>
>>>> A guest's physical timer may be used in nested guest or directly used in
>>>> VHE mode. We can check above cases to avoid useless CNTP_CVAL/CNTHP_CVAL save
>>>> and reload.
>>>
>>> Why useless? What cases?
>>
>> I mean we do not use ptimer at least in current Linux kernel, however, for system
>> which has ECV_CNTPOFF capability, we have to handle related logic here.
>>
>> So we can detect if ptimer is been used, if it is been used, then we run into the
>> related codes.
> 
> But where is the actual cost? Is it the saving of these registers?
> Something else?

Just save the cost of the codes in has_cntpoff branch in __activate_traps/__deactivate_traps.

From the KUT test results, for VHE in non-nested mode, it will save:
hvc		758ns		624ns		17.68%
mmio_read_user	3647ns		3479ns		4.61%
mmio_read_vgic	949ns		817ns		13.91%
eoi		29ns		29ns		0.00%
ipi		2534ns		2302ns		9.16%
lpi		2684ns		2442ns		9.02%
timer_10ms	2393ns		2168ns		9.40%

I am not sure it deserves a hacking like this patch, key point is that it seems
we rarely use ptimer, but we have to handle it every VM in/exit.

> 
>>
>> I am not familiar with the nested cases, so just not change related logic.
>>
>>>
>>>>
>>>> Use lazy detection to determine whether a non-nested VHE guest actually uses
>>>> the physical timer.
>>>>
>>>> timer_set_traps() initially traps CNTP_CTL_EL0 writes for non-nested guests.
>>>> On the first write that sets CNTP_CTL.ENABLE=1, the trap handler records
>>>> ptimer_used and clears the trap in CNTHCTL_EL2, allowing the guest direct
>>>> access thereafter.
>>>
>>> I really don't want to go into these micro optimisation to paper over
>>> the bad state of the architecture.
>>>
>>> The options are:
>>>
>>> - Don't use any offset (offsets always set to 0), and there will not
>>>   be any trap
>>
>> Do you mean that in non-nested case, we can make offset always be 0, so
>> related logic in __activate_traps/__deactivate_traps can be skipped?
>>
> 
> The *trapping* of the physical timer/counter will be skipped.

Thanks, I saw related trap setting in timer_set_traps.

Best,
Zhou

> 
> 	M.
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-09-28  8:30 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-21 11:18 [RFC PATCH] KVM: arm64: Restrict CNTP_CVAL/CNTHP_CVAL save/reload when ptimer is used Zhou Wang
2026-09-21 11:41 ` sashiko-bot
2026-09-21 13:22 ` Marc Zyngier
2026-09-22 10:16   ` Zhou Wang
2026-09-24 17:58     ` Marc Zyngier
2026-09-28  8:30       ` Zhou Wang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.