> On 23 Dec 2019, at 19:09, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote: > > On 23/12/19 15:48, Liran Alon wrote: >>> Should userspace expose the CR pining CPUID feature bit, it must zero CR >>> pinned MSRs on reboot. If it does not, it runs the risk of having the >>> guest enable pinning and subsequently cause general protection faults on >>> next boot due to early boot code setting control registers to values >>> which do not contain the pinned bits. >> >> Why reset CR pinned MSRs by userspace instead of KVM INIT handling? > > Most MSRs are not reset by INIT, are they? > > Paolo > MSR_KVM_SYSTEM_TIME saved in vcpu->arch.time is reset at kvmclock_reset() which is called by kvm_vcpu_reset() (KVM INIT handler). In addition, vmx_vcpu_reset(), called from kvm_vcpu_reset(), also resets multiple MSRs such as: MSR_IA32_SPEC_CTRL (vmx->spec_ctrl) and MSR_IA32_UMWAIT_CONTROL (msr_ia32_umwait_control). Having said that, I see indeed that most of MSRs are being set by QEMU in kvm_put_msrs() when level >= KVM_PUT_RESET_STATE. When is triggered by qemu_system_reset() -> cpu_synchronize_all_post_reset -> cpu_synchronize_post_reset() -> kvm_cpu_synchronize_post_reset(). So given current design, OK I agree with you that CR pinned MSRs should be zeroed by userspace VMM. It does though seems kinda weird to me that part of CPU state is initialised on KVM INIT handler and part of it in userspace VMM. It could lead to inconsistent (i.e. diverging from spec) CPU behaviour. -Liran
