On 2/11/21 2:56 AM, Paolo Bonzini wrote: > On 29/01/21 01:43, Babu Moger wrote: >> This support also fixes an issue where a guest may sometimes see an >> inconsistent value for the SPEC_CTRL MSR on processors that support this >> feature. With the current SPEC_CTRL support, the first write to >> SPEC_CTRL is intercepted and the virtualized version of the SPEC_CTRL >> MSR is not updated. > > This is a bit ugly, new features should always be enabled manually (AMD > did it right for vVMLOAD/vVMSAVE for example, even though _in theory_ > assuming that all hypervisors were intercepting VMLOAD/VMSAVE would have > been fine). > > Also regarding nested virtualization: > >> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c >> index 7a605ad8254d..9e51f9e4f631 100644 >> --- a/arch/x86/kvm/svm/nested.c >> +++ b/arch/x86/kvm/svm/nested.c >> @@ -534,6 +534,7 @@ int nested_svm_vmrun(struct vcpu_svm *svm) >> hsave->save.cr3 = vmcb->save.cr3; >> else >> hsave->save.cr3 = kvm_read_cr3(&svm->vcpu); >> + hsave->save.spec_ctrl = vmcb->save.spec_ctrl; >> >> copy_vmcb_control_area(&hsave->control, &vmcb->control); >> >> @@ -675,6 +676,7 @@ int nested_svm_vmexit(struct vcpu_svm *svm) >> kvm_rip_write(&svm->vcpu, hsave->save.rip); >> svm->vmcb->save.dr7 = DR7_FIXED_1; >> svm->vmcb->save.cpl = 0; >> + svm->vmcb->save.spec_ctrl = hsave->save.spec_ctrl; >> svm->vmcb->control.exit_int_info = 0; >> >> vmcb_mark_all_dirty(svm->vmcb); > > I think this is incorrect. Since we don't support this feature in the > nested hypervisor, any writes to the SPEC_CTRL MSR while L2 (nested guest) > runs have to be reflected to L1 (nested hypervisor). In other words, this > new field is more like VMLOAD/VMSAVE state, in that it doesn't change > across VMRUN and VMEXIT. These two hunks can be removed. Makes sense. I have tested removing these two hunks and it worked fine. > > If we want to do it, exposing this feature to the nested hypervisor will > be a bit complicated, because one has to write host SPEC_CTRL | > vmcb01.GuestSpecCtrl in the host MSR, in order to free the vmcb02 > GuestSpecCtrl for the vmcb12 GuestSpecCtrl. > > It would also be possible to emulate it on processors that don't have it. > However I'm not sure it's a good idea because of the problem that you > mentioned with running old kernels on new processors. > > I have queued the patches with the small fix above. However I plan to > only include them in 5.13 because I have a bunch of other SVM patches, Yes. 5.13 is fine. thanks Babu > those have been tested already but I need to send them out for review > before "officially" getting them in kvm.git. > > Paolo >
