Hi Joey, On Fri, Feb 24, 2023 at 05:39:15PM +0000, Joey Gouly wrote: > I'm having an issue with this commit where a VCPU is getting a CNTVOFF_EL2 set > to 0, so it sees the same time as the host system, and the other VCPU has the > correct offset. Yikes! > The flow of execution looks like this: > KVM_CREATE_VCPU 0 (VMM) -> > kvm_timer_vcpu_init -> > update_vtimer_cntvoff (VCPU0.CNTVOFF_EL2=kvm_phys_timer_read) > KVM_ARM_VCPU_INIT (VMM) -> > kvm_arch_vcpu_ioctl_vcpu_init -> > kvm_vcpu_set_target -> > kvm_reset_vcpu -> > kvm_reset_sys_regs (VCPU0.CNTVOFF_EL2=0) > > KVM_CREATE_VCPU 1 (VMM) -> > kvm_timer_vcpu_init -> > update_vtimer_cntvoff (VCPU0.CNTVOFF_EL2=VCPU1.CNTVOFF_EL2=kvm_phys_timer_read) > KVM_ARM_VCPU_INIT (VMM) -> > kvm_arch_vcpu_ioctl_vcpu_init -> > kvm_vcpu_set_target -> > kvm_reset_vcpu -> > kvm_reset_sys_regs (VCPU1.CNTVOFF_EL2=0) > > At this point VCPU0 has CNTVOFF_EL2 == kvm_phys_timer_read, and VCPU1 > has CNTVOFF_EL2 == 0. > > The VCPUs having different CNTVOFF_EL2 valuess is just a symptom of the fact that > CNTVOFF_EL2 is now reset in kvm_reset_sys_regs. Right, and the fundamental problem at hand is that we used CNTVOFF_EL2 to stash the _host's_ offset even though we are trying to change the meaning of it to be part of the virtual EL2's context. > The following patch gets it booting again, but I'm sure it's not the right fix. I'd rather we just break the host away from using the shadow reg altogether and separately track the host offset. As it so happens Marc has a patch that does exactly that [*]. Marc, do you want to resend that patch in isolation addressing the feedback and link to this bug report? [*] https://lore.kernel.org/kvmarm/20230216142123.2638675-6-maz@xxxxxxxxxx/ -- Thanks, Oliver
