On 8/3/23 19:48, Paolo Bonzini wrote: > On 8/3/23 02:13, Michal Luczaj wrote: >> Anyway, while there, could you take a look at __set_sregs_common()? >> >> *mmu_reset_needed |= kvm_read_cr0(vcpu) != sregs->cr0; >> static_call(kvm_x86_set_cr0)(vcpu, sregs->cr0); >> vcpu->arch.cr0 = sregs->cr0; >> >> That last assignment seems redundant as both vmx_set_cr0() and svm_set_cr0() >> take care of it, but I may be missing something (even if selftests pass with >> that line removed). > > kvm_set_cr0 assumes that the static call sets vcpu->arch.cr0, so indeed > it can be removed: I guess the same can be done in enter_smm()? cr0 = vcpu->arch.cr0 & ~(X86_CR0_PE | X86_CR0_EM | X86_CR0_TS | X86_CR0_PG); static_call(kvm_x86_set_cr0)(vcpu, cr0); vcpu->arch.cr0 = cr0; > Neither __set_sregs_common nor its callers does not call > kvm_post_set_cr0... Not great, even though most uses of KVM_SET_SREGS > are probably limited to reset in most "usual" VMMs. It's probably > enough to replace this line: > > *mmu_reset_needed |= kvm_read_cr0(vcpu) != sregs->cr0; > > with a call to the function just before __set_sregs_common returns. What about kvm_post_set_cr4() then? Should it be introduced to __set_sregs_common() as well? thanks, Michal
