"Nadav Har'El" <nyh@xxxxxxxxxxxxxxxxxxx> wrote on 11/03/2013 12:43:35 AM: > On Sun, Mar 10, 2013, Abel Gordon wrote about "[PATCH 10/11] KVM: > nVMX: Synchronize VMCS12 content with the shadow vmcs": > > nested_vmx_vmexit(vcpu); > > + if (enable_shadow_vmcs) > > + copy_vmcs12_to_shadow(to_vmx(vcpu)); > > I was curious why your patch adds this call to copy_vmcs12_to_shadow after > every nested_vmx_vmexit (3 times), instead of making this call inside > nested_vmx_vmexit(), say right after prepare_vmcs12(). Until I saw: Because nested code sometimes modifies vmcs fileds "after" nested_vmx_vmexit (see below). I was afraid nested logic may be changed in the future and some field may become out-of-sync. If we do have to call copy_vmcs12_to_shadow explicitly, then, it will be more difficult to miss some field. > > > nested_vmx_vmexit(vcpu); > > vmcs12->vm_exit_reason = EXIT_REASON_EXTERNAL_INTERRUPT; > > vmcs12->vm_exit_intr_info = 0; > > + if (enable_shadow_vmcs) > > + copy_vmcs12_to_shadow(to_vmx(vcpu)); > > where you need to copy this exit-reason override as well... Exactly ;) Also, if I remember properly, nested EPT patches also modify vmcs12 fields outside the nested_vmx_vmexit context. > I wonder if there isn't a less ugly and repetitive way to do this :( We can change nested_vmx_vmexit to get additional parameters so we can set others fields (e.g. exit_reasion and exit_intr_info) as part of this function, however, I am not sure this will be "less ugly"... do you remember the old/original nested code in which nested_vmx_vmexit had an additional "external_interrupt" parameter ? :) -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
