Re: [PATCH 2/2] KVM: nVMX: Update vmcs01 TPR_THRESHOLD if L2 changed L1 TPR

[Liran Alon <liran.alon@oracle.com>]

> On 11 Nov 2019, at 18:07, Paolo Bonzini <pbonzini@redhat.com> 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