> On 11 Nov 2019, at 18:07, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote: > > On 11/11/19 16:24, Liran Alon wrote: >>> Can you explain why the write shouldn't be done to vmcs02 as well? >> >> Because when L1 don’t use TPR-Shadow, L0 configures vmcs02 without TPR-Shadow. >> Thus, writing to vmcs02->tpr_threshold doesn’t have any effect. >> >> If l1 do use TPR-Shadow, then VMX’s update_cr8_intercept() doesn’t write to vmcs at all, >> because it means L1 defines a vTPR for L2 and thus doesn’t provide it direct access to L1 TPR. > > But I'm still not sure about another aspect of the patch. The write to > vmcs01 can be done even if TPR_SHADOW was set in vmcs12, because no one > takes care of clearing vmx->nested.l1_tpr_threshold. Should > "vmx->nested.l1_tpr_threshold = -1;" be outside the if? If I understand you correctly, you refer to the case where L1 first enters L2 without TPR-Shadow, then L2 lowers L1 TPR directly (which load vmx->nested.l1_tpr_threshold with value), then an emualted exit happen from L2 to L1 which writes to vmcs01->tpr_threshold the value of vmx->nested.l1_tpr_threshold. Then L1 enters again L2 but this time with TPR-Shadow and prepare_vmcs02_early() doesn’t clear vmx->nested.l1_tpr_threshold which will cause next exit from L2 to L1 to wrongly write the value of vmx->nested.l1_tpr_threshold to vmcs01->tpr_threshold. So yes I think you are right. Good catch. We should move vmx->nested.l1_tpr_threshold = -1; outside of the if. Should I send v2 or will you change on apply? > > Also, what happens to_vmx(vcpu)->nested.l1_tpr_threshold if the guest is > migrated while L2 is running without TPR shadow? Perhaps it would be > easier to just rerun update_cr8_intercept on nested_vmx_vmexit. > On restore of state during migration, kvm_apic_set_state() must be called which will also request a KVM_REQ_EVENT which will make sure to call update_cr8_intercept(). If vCPU is currently in guest-mode, this should update vmx->nested.l1_tpr_threshold. -Liran > Paolo
