Hi KarimAllah,
On 3/16/20 9:39 AM, KarimAllah Ahmed wrote:
> Use the physical timer object when reading the physical timer counter
> instead of using the virtual timer object. This is only visible when
> reading it from user-space as kvm_arm_timer_get_reg() is only executed on
> the get register patch from user-space.
Have you seen this go wrong?
I agree this looks like this was a typo introduced by:
84135d3d1 ("KVM: arm/arm64: consolidate arch timer trap handlers")
-----------------%<-----------------
case KVM_REG_ARM_PTIMER_CNT:
- return kvm_phys_timer_read();
+ return kvm_arm_timer_read(vcpu,
+ vcpu_vtimer(vcpu), TIMER_REG_CNT);
-----------------%<-----------------
This would be a problem when the guest reads the physical counter
directly, (which doesn't get trapped), and the VMM makes this API call
and gets a number in a totally different ball-park.
Can the VMM actually read these registers with this path?
kvm_arm_get_reg() gets to filter out the coproc registers that aren't in
the sys_reg[], it also uses is_timer_reg() to spot the timer/counter
registers, but is_timer_reg() only matches three of them:
| case KVM_REG_ARM_TIMER_CTL:
| case KVM_REG_ARM_TIMER_CNT:
| case KVM_REG_ARM_TIMER_CVAL:
KVM_REG_ARM_PTIMER_CNT is not one of them.
It looks like when the VMM tries to read this, it fails is_timer_reg(),
and matches in the sys_regs[] and is handled by access_arch_timer(),
which uses kvm_arm_timer_read_sysreg() -> kvm_arm_timer_read(),
bypassing this bug.
... this looks like a bug in dead code ...
Thanks!
James
> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
> index 0d9438e..93bd59b 100644
> --- a/virt/kvm/arm/arch_timer.c
> +++ b/virt/kvm/arm/arch_timer.c
> @@ -788,7 +788,7 @@ u64 kvm_arm_timer_get_reg(struct kvm_vcpu *vcpu, u64 regid)
> vcpu_ptimer(vcpu), TIMER_REG_CTL);
> case KVM_REG_ARM_PTIMER_CNT:
> return kvm_arm_timer_read(vcpu,
> - vcpu_vtimer(vcpu), TIMER_REG_CNT);
> + vcpu_ptimer(vcpu), TIMER_REG_CNT);
> case KVM_REG_ARM_PTIMER_CVAL:
> return kvm_arm_timer_read(vcpu,
> vcpu_ptimer(vcpu), TIMER_REG_CVAL);
>
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
