Re: [PATCH 1/4] KVM: VMX: cache secondary exec controls

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Aug 24, 2017 at 6:37 AM, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
> Currently, secondary execution controls are divided in three groups:
>
> - static, depending mostly on the module arguments or the processor
>   (vmx_secondary_exec_control)
>
> - static, depending on CPUID (vmx_cpuid_update)

There should also be:

- static, depending on guest VMX capability MSRs (vmx_set_vmx_msr)

>
> - dynamic, depending on nested VMX or local APIC state
>
> Because walking CPUID is expensive, prepare_vmcs02 is using only
> the first group.  This however is unnecessarily complicated.  Just
> cache the static secondary execution controls, and then prepare_vmcs02
> does not need to compute them every time.  Computation of all static
> secondary execution controls is now kept in a single function,
> vmx_compute_secondary_exec_control.
>
> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> ---
>  arch/x86/kvm/vmx.c | 100 +++++++++++++++++++++++++++++------------------------
>  1 file changed, 54 insertions(+), 46 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 19aa69af7c2d..08381a5d8879 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -576,6 +576,8 @@ struct vcpu_vmx {
>  #endif
>         u32 vm_entry_controls_shadow;
>         u32 vm_exit_controls_shadow;
> +       u32 secondary_exec_control;
> +
>         /*
>          * loaded_vmcs points to the VMCS currently used in this vcpu. For a
>          * non-nested (L1) guest, it always points to vmcs01. For a nested
> @@ -2807,7 +2809,10 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
>         vmx->nested.nested_vmx_procbased_ctls_low &=
>                 ~(CPU_BASED_CR3_LOAD_EXITING | CPU_BASED_CR3_STORE_EXITING);
>
> -       /* secondary cpu-based controls */
> +       /*
> +        * secondary cpu-based controls.  Do not include those that
> +        * depend on CPUID bits, they are added later by vmx_cpuid_update.

I think vmx_cpuid_update should only clear VMX capability bits, and
not set them. See below.

> +        */
>         rdmsr(MSR_IA32_VMX_PROCBASED_CTLS2,
>                 vmx->nested.nested_vmx_secondary_ctls_low,
>                 vmx->nested.nested_vmx_secondary_ctls_high);
> @@ -2815,7 +2820,6 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
>         vmx->nested.nested_vmx_secondary_ctls_high &=
>                 SECONDARY_EXEC_RDRAND | SECONDARY_EXEC_RDSEED |
>                 SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
> -               SECONDARY_EXEC_RDTSCP |
>                 SECONDARY_EXEC_DESC |
>                 SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE |
>                 SECONDARY_EXEC_APIC_REGISTER_VIRT |
> @@ -5269,10 +5273,12 @@ static u32 vmx_exec_control(struct vcpu_vmx *vmx)
>         return exec_control;
>  }
>
> -static u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx)
> +static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
>  {
> +       struct kvm_vcpu *vcpu = &vmx->vcpu;
> +
>         u32 exec_control = vmcs_config.cpu_based_2nd_exec_ctrl;
> -       if (!cpu_need_virtualize_apic_accesses(&vmx->vcpu))
> +       if (!cpu_need_virtualize_apic_accesses(vcpu))
>                 exec_control &= ~SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES;
>         if (vmx->vpid == 0)
>                 exec_control &= ~SECONDARY_EXEC_ENABLE_VPID;
> @@ -5286,7 +5292,7 @@ static u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx)
>                 exec_control &= ~SECONDARY_EXEC_UNRESTRICTED_GUEST;
>         if (!ple_gap)
>                 exec_control &= ~SECONDARY_EXEC_PAUSE_LOOP_EXITING;
> -       if (!kvm_vcpu_apicv_active(&vmx->vcpu))
> +       if (!kvm_vcpu_apicv_active(vcpu))
>                 exec_control &= ~(SECONDARY_EXEC_APIC_REGISTER_VIRT |
>                                   SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY);
>         exec_control &= ~SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE;
> @@ -5300,7 +5306,43 @@ static u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx)
>         if (!enable_pml)
>                 exec_control &= ~SECONDARY_EXEC_ENABLE_PML;
>
> -       return exec_control;
> +       if (vmx_rdtscp_supported()) {
> +               bool rdtscp_enabled = guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP);
> +               if (!rdtscp_enabled)
> +                       exec_control &= ~SECONDARY_EXEC_RDTSCP;
> +
> +               if (nested) {
> +                       if (rdtscp_enabled)
> +                               vmx->nested.nested_vmx_secondary_ctls_high |=
> +                                       SECONDARY_EXEC_RDTSCP;

CPUID.80000001H:EDX.RDTSCP[bit 27] does not imply
IA32_VMX_PROCBASED_CTLS2.[bit 35].

> +                       else
> +                               vmx->nested.nested_vmx_secondary_ctls_high &=
> +                                       ~SECONDARY_EXEC_RDTSCP;
> +               }
> +       }
> +
> +       if (vmx_invpcid_supported()) {
> +               /* Exposing INVPCID only when PCID is exposed */
> +               bool invpcid_enabled =
> +                       guest_cpuid_has(vcpu, X86_FEATURE_INVPCID) &&
> +                       guest_cpuid_has(vcpu, X86_FEATURE_PCID);
> +
> +               if (!invpcid_enabled) {
> +                       exec_control &= ~SECONDARY_EXEC_ENABLE_INVPCID;
> +                       guest_cpuid_clear(vcpu, X86_FEATURE_INVPCID);
> +               }
> +
> +               if (nested) {
> +                       if (invpcid_enabled)
> +                               vmx->nested.nested_vmx_secondary_ctls_high |=
> +                                       SECONDARY_EXEC_ENABLE_INVPCID;

CPUID.(EAX=07H,ECX=0):EBX.INVPCID[bit 10] does not imply
IA32_VMX_PROCBASED_CTLS2.[bit 44].

> +                       else
> +                               vmx->nested.nested_vmx_secondary_ctls_high &=
> +                                       ~SECONDARY_EXEC_ENABLE_INVPCID;
> +               }
> +       }
> +
> +       vmx->secondary_exec_control = exec_control;
>  }
>
>  static void ept_set_mmio_spte_mask(void)
> @@ -5344,8 +5386,9 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx)
>         vmcs_write32(CPU_BASED_VM_EXEC_CONTROL, vmx_exec_control(vmx));
>
>         if (cpu_has_secondary_exec_ctrls()) {
> +               vmx_compute_secondary_exec_control(vmx);
>                 vmcs_write32(SECONDARY_VM_EXEC_CONTROL,
> -                               vmx_secondary_exec_control(vmx));
> +                            vmx->secondary_exec_control);
>         }
>
>         if (kvm_vcpu_apicv_active(&vmx->vcpu)) {
> @@ -9623,47 +9666,12 @@ static void nested_vmx_cr_fixed1_bits_update(struct kvm_vcpu *vcpu)
>  static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
>  {
>         struct vcpu_vmx *vmx = to_vmx(vcpu);
> -       u32 secondary_exec_ctl = vmx_secondary_exec_control(vmx);
> -
> -       if (vmx_rdtscp_supported()) {
> -               bool rdtscp_enabled = guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP);
> -               if (!rdtscp_enabled)
> -                       secondary_exec_ctl &= ~SECONDARY_EXEC_RDTSCP;
> -
> -               if (nested) {
> -                       if (rdtscp_enabled)
> -                               vmx->nested.nested_vmx_secondary_ctls_high |=
> -                                       SECONDARY_EXEC_RDTSCP;
> -                       else
> -                               vmx->nested.nested_vmx_secondary_ctls_high &=
> -                                       ~SECONDARY_EXEC_RDTSCP;
> -               }
> -       }
> -
> -       if (vmx_invpcid_supported()) {
> -               /* Exposing INVPCID only when PCID is exposed */
> -               bool invpcid_enabled =
> -                       guest_cpuid_has(vcpu, X86_FEATURE_INVPCID) &&
> -                       guest_cpuid_has(vcpu, X86_FEATURE_PCID);
> -
> -               if (!invpcid_enabled) {
> -                       secondary_exec_ctl &= ~SECONDARY_EXEC_ENABLE_INVPCID;
> -                       guest_cpuid_clear(vcpu, X86_FEATURE_INVPCID);
> -               }
>
> -               if (nested) {
> -                       if (invpcid_enabled)
> -                               vmx->nested.nested_vmx_secondary_ctls_high |=
> -                                       SECONDARY_EXEC_ENABLE_INVPCID;
> -                       else
> -                               vmx->nested.nested_vmx_secondary_ctls_high &=
> -                                       ~SECONDARY_EXEC_ENABLE_INVPCID;
> -               }
> +       if (cpu_has_secondary_exec_ctrls()) {
> +               vmx_compute_secondary_exec_control(vmx);
> +               vmcs_set_secondary_exec_control(vmx->secondary_exec_control);
>         }
>
> -       if (cpu_has_secondary_exec_ctrls())
> -               vmcs_set_secondary_exec_control(secondary_exec_ctl);
> -
>         if (nested_vmx_allowed(vcpu))
>                 to_vmx(vcpu)->msr_ia32_feature_control_valid_bits |=
>                         FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX;
> @@ -10356,7 +10364,7 @@ static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
>                 enable_ept ? vmcs12->page_fault_error_code_match : 0);
>
>         if (cpu_has_secondary_exec_ctrls()) {
> -               exec_control = vmx_secondary_exec_control(vmx);
> +               exec_control = vmx->secondary_exec_control;
>
>                 /* Take the following fields only from vmcs12 */
>                 exec_control &= ~(SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
> --
> 1.8.3.1
>
>



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux