Wanpeng Li wrote on 2014-08-04: > This patch fix bug https://bugzilla.kernel.org/show_bug.cgi?id=61411 > > TPR shadow/threshold feature is important to speed up the Windows guest. > Besides, it is a must feature for certain VMM. > > We map virtual APIC page address and TPR threshold from L1 VMCS. If > TPR_BELOW_THRESHOLD VM exit is triggered by L2 guest and L1 interested > in, we inject it into L1 VMM for handling. > > Signed-off-by: Wanpeng Li <wanpeng.li@xxxxxxxxxxxxxxx> > --- > v2 -> v3: > * nested vm entry failure if both tpr shadow and cr8 exiting bits are not set > v1 -> v2: > * don't take L0's "virtualize APIC accesses" setting into account > * virtual_apic_page do exactly the same thing that is done for > apic_access_page > * add the tpr threshold field to the read-write fields for shadow VMCS > > arch/x86/kvm/vmx.c | 38 ++++++++++++++++++++++++++++++++++++-- > 1 file changed, 36 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index c604f3c..7a56e2c 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -379,6 +379,7 @@ struct nested_vmx { > * we must keep them pinned while L2 runs. > */ > struct page *apic_access_page; > + struct page *virtual_apic_page; > u64 msr_ia32_feature_control; > > struct hrtimer preemption_timer; > @@ -533,6 +534,7 @@ static int max_shadow_read_only_fields = > ARRAY_SIZE(shadow_read_only_fields); > > static unsigned long shadow_read_write_fields[] = { > + TPR_THRESHOLD, > GUEST_RIP, > GUEST_RSP, > GUEST_CR0, > @@ -2330,7 +2332,7 @@ static __init void > nested_vmx_setup_ctls_msrs(void) > CPU_BASED_MOV_DR_EXITING | > CPU_BASED_UNCOND_IO_EXITING | > CPU_BASED_USE_IO_BITMAPS | CPU_BASED_MONITOR_EXITING | > CPU_BASED_RDPMC_EXITING | CPU_BASED_RDTSC_EXITING | > - CPU_BASED_PAUSE_EXITING | > + CPU_BASED_PAUSE_EXITING | CPU_BASED_TPR_SHADOW | > CPU_BASED_ACTIVATE_SECONDARY_CONTROLS; > /* > * We can allow some features even when not supported by the > @@ -6148,6 +6150,10 @@ static void free_nested(struct vcpu_vmx *vmx) > nested_release_page(vmx->nested.apic_access_page); > vmx->nested.apic_access_page = 0; > } > + if (vmx->nested.virtual_apic_page) { > + nested_release_page(vmx->nested.virtual_apic_page); > + vmx->nested.virtual_apic_page = 0; > + } > > nested_free_all_saved_vmcss(vmx); > } > @@ -6936,7 +6942,7 @@ static bool nested_vmx_exit_handled(struct > kvm_vcpu *vcpu) > case EXIT_REASON_MCE_DURING_VMENTRY: > return 0; > case EXIT_REASON_TPR_BELOW_THRESHOLD: > - return 1; > + return nested_cpu_has(vmcs12, CPU_BASED_TPR_SHADOW); > case EXIT_REASON_APIC_ACCESS: > return nested_cpu_has2(vmcs12, > SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES); > @@ -7057,6 +7063,9 @@ static int vmx_handle_exit(struct kvm_vcpu *vcpu) > > static void update_cr8_intercept(struct kvm_vcpu *vcpu, int tpr, int irr) > { > + if (is_guest_mode(vcpu)) > + return; > + > if (irr == -1 || tpr < irr) { > vmcs_write32(TPR_THRESHOLD, 0); > return; > @@ -8024,6 +8033,27 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu, > struct vmcs12 *vmcs12) > exec_control &= ~CPU_BASED_VIRTUAL_NMI_PENDING; > exec_control &= ~CPU_BASED_TPR_SHADOW; > exec_control |= vmcs12->cpu_based_vm_exec_control; > + > + if (exec_control & CPU_BASED_TPR_SHADOW) { > + if (vmx->nested.virtual_apic_page) > + nested_release_page(vmx->nested.virtual_apic_page); > + vmx->nested.virtual_apic_page = > + nested_get_page(vcpu, vmcs12->virtual_apic_page_addr); > + if (!vmx->nested.virtual_apic_page) > + exec_control &= > + ~CPU_BASED_TPR_SHADOW; > + else > + vmcs_write64(VIRTUAL_APIC_PAGE_ADDR, > + page_to_phys(vmx->nested.virtual_apic_page)); > + > + if (!(exec_control & CPU_BASED_TPR_SHADOW) && > + !((exec_control & CPU_BASED_CR8_LOAD_EXITING) && > + (exec_control & CPU_BASED_CR8_STORE_EXITING))) > + nested_vmx_failValid(vcpu, > VMXERR_ENTRY_INVALID_CONTROL_FIELD); I think this is not correct. The vmx->nested.virtual_apic_page may not valid due to two reasons: 1. The virtual_apic_page_addr is not a valid gfn. In this case, the vmx failure must be injected to L1 unconditionally regardless of the setting of CR8 load/store exiting. 2. The virtual_apic_page is swapped by L0. In this case, we should not inject failure to L1. > + > + vmcs_write32(TPR_THRESHOLD, vmcs12->tpr_threshold); > + } Miss else here: If L2 owns the APIC and doesn't use TPR_SHADOW, we need to setup the vmcs02 based on vmcs01. For example, if vmcs01 is using TPR_SHADOW, then vmcs02 must set it. Also, we need to use vmcs01's virtual_apic_page and TPR_THRESHOLD for vmcs02. > + > /* > * Merging of IO and MSR bitmaps not currently supported. > * Rather, exit every time. > @@ -8802,6 +8832,10 @@ static void nested_vmx_vmexit(struct kvm_vcpu > *vcpu, u32 exit_reason, > nested_release_page(vmx->nested.apic_access_page); > vmx->nested.apic_access_page = 0; > } > + if (vmx->nested.virtual_apic_page) { > + nested_release_page(vmx->nested.virtual_apic_page); > + vmx->nested.virtual_apic_page = 0; > + } > > /* > * Exiting from L2 to L1, we're now back to L1 which thinks it just > -- > 1.9.1 Best regards, Yang -- 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
