On Wed, Jun 22, 2022, Vitaly Kuznetsov wrote:
> Signed-off-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>
> ---
> arch/x86/kvm/vmx/vmx.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 5e14e4c40007..24da9e93bdab 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -2490,11 +2490,6 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf,
> if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_PROCBASED_CTLS,
> &_cpu_based_exec_control) < 0)
> return -EIO;
> -#ifdef CONFIG_X86_64
> - if (_cpu_based_exec_control & CPU_BASED_TPR_SHADOW)
> - _cpu_based_exec_control &= ~CPU_BASED_CR8_LOAD_EXITING &
> - ~CPU_BASED_CR8_STORE_EXITING;
Eww, who does a double "~" with an "&"?
> -#endif
> if (_cpu_based_exec_control & CPU_BASED_ACTIVATE_SECONDARY_CONTROLS) {
> min2 = 0;
> opt2 = SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
> @@ -4285,6 +4280,12 @@ static u32 vmx_exec_control(struct vcpu_vmx *vmx)
> {
> u32 exec_control = vmcs_config.cpu_based_exec_ctrl;
>
> +#ifdef CONFIG_X86_64
> + if (exec_control & CPU_BASED_TPR_SHADOW)
> + exec_control &= ~CPU_BASED_CR8_LOAD_EXITING &
> + ~CPU_BASED_CR8_STORE_EXITING;
If you shove this done a few lines, then you can have a single set of #ifdefs,
and avoid restoring the controls a few lines later if it turns out KVM isn't
enabling the TPR shadow, e.g. (with fixup to use the more canonical ~(x | y)
pattern).
if (!cpu_need_tpr_shadow(&vmx->vcpu))
exec_control &= ~CPU_BASED_TPR_SHADOW;
#ifdef CONFIG_X86_64
if (exec_control & CPU_BASED_TPR_SHADOW)
exec_control &= ~(CPU_BASED_CR8_LOAD_EXITING |
CPU_BASED_CR8_STORE_EXITING);
else
exec_control |= CPU_BASED_CR8_STORE_EXITING |
CPU_BASED_CR8_LOAD_EXITING;
#endif
