diff mbox

[2/2] kvm: vmx: Basic APIC virtualization controls have three settings

Message ID 20180509205605.2161-3-krish.sadhukhan@oracle.com (mailing list archive)
State New, archived
Headers show

Commit Message

Krish Sadhukhan May 9, 2018, 8:56 p.m. UTC
From: Jim Mattson <jmattson@google.com>

Previously, we toggled between SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE
and SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES, depending on whether or
not the EXTD bit was set in MSR_IA32_APICBASE. However, if the local
APIC is disabled, we should not set either of these APIC
virtualization control bits.

Signed-off-by: Jim Mattson <jmattson@google.com>
Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
---
 arch/x86/include/asm/kvm_host.h |  2 +-
 arch/x86/kvm/lapic.c            | 12 +++++------
 arch/x86/kvm/svm.c              |  4 ++--
 arch/x86/kvm/vmx.c              | 48 +++++++++++++++++++++++++----------------
 4 files changed, 38 insertions(+), 28 deletions(-)

Comments

Paolo Bonzini May 10, 2018, 1:30 p.m. UTC | #1
On 09/05/2018 22:56, Krish Sadhukhan wrote:
> From: Jim Mattson <jmattson@google.com>
> 
> Previously, we toggled between SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE
> and SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES, depending on whether or
> not the EXTD bit was set in MSR_IA32_APICBASE. However, if the local
> APIC is disabled, we should not set either of these APIC
> virtualization control bits.
> 
> Signed-off-by: Jim Mattson <jmattson@google.com>
> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>

What is the buggy behavior that happens due to this?  Can it be turned
into a testcase?

Paolo

> ---
>  arch/x86/include/asm/kvm_host.h |  2 +-
>  arch/x86/kvm/lapic.c            | 12 +++++------
>  arch/x86/kvm/svm.c              |  4 ++--
>  arch/x86/kvm/vmx.c              | 48 +++++++++++++++++++++++++----------------
>  4 files changed, 38 insertions(+), 28 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index c25775f..52e685b 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -995,7 +995,7 @@ struct kvm_x86_ops {
>  	void (*hwapic_irr_update)(struct kvm_vcpu *vcpu, int max_irr);
>  	void (*hwapic_isr_update)(struct kvm_vcpu *vcpu, int isr);
>  	void (*load_eoi_exitmap)(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap);
> -	void (*set_virtual_x2apic_mode)(struct kvm_vcpu *vcpu, bool set);
> +	void (*set_virtual_apic_mode)(struct kvm_vcpu *vcpu);
>  	void (*set_apic_access_page_addr)(struct kvm_vcpu *vcpu, hpa_t hpa);
>  	void (*deliver_posted_interrupt)(struct kvm_vcpu *vcpu, int vector);
>  	int (*sync_pir_to_irr)(struct kvm_vcpu *vcpu);
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index b74c9c1..776391c 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -1990,13 +1990,11 @@ void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64 value)
>  		}
>  	}
>  
> -	if ((old_value ^ value) & X2APIC_ENABLE) {
> -		if (value & X2APIC_ENABLE) {
> -			kvm_apic_set_x2apic_id(apic, vcpu->vcpu_id);
> -			kvm_x86_ops->set_virtual_x2apic_mode(vcpu, true);
> -		} else
> -			kvm_x86_ops->set_virtual_x2apic_mode(vcpu, false);
> -	}
> +	if (((old_value ^ value) & X2APIC_ENABLE) && (value & X2APIC_ENABLE))
> +		kvm_apic_set_x2apic_id(apic, vcpu->vcpu_id);
> +
> +	if ((old_value ^ value) & (MSR_IA32_APICBASE_ENABLE | X2APIC_ENABLE))
> +		kvm_x86_ops->set_virtual_apic_mode(vcpu);
>  
>  	apic->base_address = apic->vcpu->arch.apic_base &
>  			     MSR_IA32_APICBASE_BASE;
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 1fc05e4..220e5a89 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -5036,7 +5036,7 @@ static void update_cr8_intercept(struct kvm_vcpu *vcpu, int tpr, int irr)
>  		set_cr_intercept(svm, INTERCEPT_CR8_WRITE);
>  }
>  
> -static void svm_set_virtual_x2apic_mode(struct kvm_vcpu *vcpu, bool set)
> +static void svm_set_virtual_apic_mode(struct kvm_vcpu *vcpu)
>  {
>  	return;
>  }
> @@ -7076,7 +7076,7 @@ static struct kvm_x86_ops svm_x86_ops __ro_after_init = {
>  	.enable_nmi_window = enable_nmi_window,
>  	.enable_irq_window = enable_irq_window,
>  	.update_cr8_intercept = update_cr8_intercept,
> -	.set_virtual_x2apic_mode = svm_set_virtual_x2apic_mode,
> +	.set_virtual_apic_mode = svm_set_virtual_apic_mode,
>  	.get_enable_apicv = svm_get_enable_apicv,
>  	.refresh_apicv_exec_ctrl = svm_refresh_apicv_exec_ctrl,
>  	.load_eoi_exitmap = svm_load_eoi_exitmap,
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index c766880..c7a3dea 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -481,7 +481,8 @@ struct nested_vmx {
>  	bool sync_shadow_vmcs;
>  	bool dirty_vmcs12;
>  
> -	bool change_vmcs01_virtual_x2apic_mode;
> +	bool change_vmcs01_virtual_apic_mode;
> +
>  	/* L2 must run next, and mustn't decide to exit to L1. */
>  	bool nested_run_pending;
>  
> @@ -9248,31 +9249,43 @@ static void update_cr8_intercept(struct kvm_vcpu *vcpu, int tpr, int irr)
>  	vmcs_write32(TPR_THRESHOLD, irr);
>  }
>  
> -static void vmx_set_virtual_x2apic_mode(struct kvm_vcpu *vcpu, bool set)
> +static void vmx_set_virtual_apic_mode(struct kvm_vcpu *vcpu)
>  {
>  	u32 sec_exec_control;
>  
> +	if (!lapic_in_kernel(vcpu))
> +		return;
> +
>  	/* Postpone execution until vmcs01 is the current VMCS. */
>  	if (is_guest_mode(vcpu)) {
> -		to_vmx(vcpu)->nested.change_vmcs01_virtual_x2apic_mode = true;
> +		to_vmx(vcpu)->nested.change_vmcs01_virtual_apic_mode = true;
>  		return;
>  	}
>  
> -	if (!cpu_has_vmx_virtualize_x2apic_mode())
> -		return;
> -
>  	if (!cpu_need_tpr_shadow(vcpu))
>  		return;
>  
>  	sec_exec_control = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
> +	sec_exec_control &= ~(SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
> +			      SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE);
>  
> -	if (set) {
> -		sec_exec_control &= ~SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES;
> -		sec_exec_control |= SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE;
> -	} else {
> -		sec_exec_control &= ~SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE;
> -		sec_exec_control |= SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES;
> -		vmx_flush_tlb(vcpu, true);
> +	switch (kvm_get_apic_mode(vcpu)) {
> +	case LAPIC_MODE_INVALID:
> +		WARN_ONCE(true, "Invalid local APIC state");
> +	case LAPIC_MODE_DISABLED:
> +		break;
> +	case LAPIC_MODE_XAPIC:
> +		if (flexpriority_enabled) {
> +			sec_exec_control |=
> +				SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES;
> +			vmx_flush_tlb(vcpu, true);
> +		}
> +		break;
> +	case LAPIC_MODE_X2APIC:
> +		if (cpu_has_vmx_virtualize_x2apic_mode())
> +			sec_exec_control |=
> +				SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE;
> +		break;
>  	}
>  	vmcs_write32(SECONDARY_VM_EXEC_CONTROL, sec_exec_control);
>  
> @@ -12060,10 +12073,9 @@ static void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 exit_reason,
>  	if (kvm_has_tsc_control)
>  		decache_tsc_multiplier(vmx);
>  
> -	if (vmx->nested.change_vmcs01_virtual_x2apic_mode) {
> -		vmx->nested.change_vmcs01_virtual_x2apic_mode = false;
> -		vmx_set_virtual_x2apic_mode(vcpu,
> -				vcpu->arch.apic_base & X2APIC_ENABLE);
> +	if (vmx->nested.change_vmcs01_virtual_apic_mode) {
> +		vmx->nested.change_vmcs01_virtual_apic_mode = false;
> +		vmx_set_virtual_apic_mode(vcpu);
>  	} else if (!nested_cpu_has_ept(vmcs12) &&
>  		   nested_cpu_has2(vmcs12,
>  				   SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES)) {
> @@ -12691,7 +12703,7 @@ static struct kvm_x86_ops vmx_x86_ops __ro_after_init = {
>  	.enable_nmi_window = enable_nmi_window,
>  	.enable_irq_window = enable_irq_window,
>  	.update_cr8_intercept = update_cr8_intercept,
> -	.set_virtual_x2apic_mode = vmx_set_virtual_x2apic_mode,
> +	.set_virtual_apic_mode = vmx_set_virtual_apic_mode,
>  	.set_apic_access_page_addr = vmx_set_apic_access_page_addr,
>  	.get_enable_apicv = vmx_get_enable_apicv,
>  	.refresh_apicv_exec_ctrl = vmx_refresh_apicv_exec_ctrl,
>
Jim Mattson May 10, 2018, 3:01 p.m. UTC | #2
I actually have a kvm-unit-test for this. I'll try to get it crossported today.

On Thu, May 10, 2018 at 6:30 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 09/05/2018 22:56, Krish Sadhukhan wrote:
>> From: Jim Mattson <jmattson@google.com>
>>
>> Previously, we toggled between SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE
>> and SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES, depending on whether or
>> not the EXTD bit was set in MSR_IA32_APICBASE. However, if the local
>> APIC is disabled, we should not set either of these APIC
>> virtualization control bits.
>>
>> Signed-off-by: Jim Mattson <jmattson@google.com>
>> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
>
> What is the buggy behavior that happens due to this?  Can it be turned
> into a testcase?
>
> Paolo
>
>> ---
>>  arch/x86/include/asm/kvm_host.h |  2 +-
>>  arch/x86/kvm/lapic.c            | 12 +++++------
>>  arch/x86/kvm/svm.c              |  4 ++--
>>  arch/x86/kvm/vmx.c              | 48 +++++++++++++++++++++++++----------------
>>  4 files changed, 38 insertions(+), 28 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>> index c25775f..52e685b 100644
>> --- a/arch/x86/include/asm/kvm_host.h
>> +++ b/arch/x86/include/asm/kvm_host.h
>> @@ -995,7 +995,7 @@ struct kvm_x86_ops {
>>       void (*hwapic_irr_update)(struct kvm_vcpu *vcpu, int max_irr);
>>       void (*hwapic_isr_update)(struct kvm_vcpu *vcpu, int isr);
>>       void (*load_eoi_exitmap)(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap);
>> -     void (*set_virtual_x2apic_mode)(struct kvm_vcpu *vcpu, bool set);
>> +     void (*set_virtual_apic_mode)(struct kvm_vcpu *vcpu);
>>       void (*set_apic_access_page_addr)(struct kvm_vcpu *vcpu, hpa_t hpa);
>>       void (*deliver_posted_interrupt)(struct kvm_vcpu *vcpu, int vector);
>>       int (*sync_pir_to_irr)(struct kvm_vcpu *vcpu);
>> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
>> index b74c9c1..776391c 100644
>> --- a/arch/x86/kvm/lapic.c
>> +++ b/arch/x86/kvm/lapic.c
>> @@ -1990,13 +1990,11 @@ void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64 value)
>>               }
>>       }
>>
>> -     if ((old_value ^ value) & X2APIC_ENABLE) {
>> -             if (value & X2APIC_ENABLE) {
>> -                     kvm_apic_set_x2apic_id(apic, vcpu->vcpu_id);
>> -                     kvm_x86_ops->set_virtual_x2apic_mode(vcpu, true);
>> -             } else
>> -                     kvm_x86_ops->set_virtual_x2apic_mode(vcpu, false);
>> -     }
>> +     if (((old_value ^ value) & X2APIC_ENABLE) && (value & X2APIC_ENABLE))
>> +             kvm_apic_set_x2apic_id(apic, vcpu->vcpu_id);
>> +
>> +     if ((old_value ^ value) & (MSR_IA32_APICBASE_ENABLE | X2APIC_ENABLE))
>> +             kvm_x86_ops->set_virtual_apic_mode(vcpu);
>>
>>       apic->base_address = apic->vcpu->arch.apic_base &
>>                            MSR_IA32_APICBASE_BASE;
>> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
>> index 1fc05e4..220e5a89 100644
>> --- a/arch/x86/kvm/svm.c
>> +++ b/arch/x86/kvm/svm.c
>> @@ -5036,7 +5036,7 @@ static void update_cr8_intercept(struct kvm_vcpu *vcpu, int tpr, int irr)
>>               set_cr_intercept(svm, INTERCEPT_CR8_WRITE);
>>  }
>>
>> -static void svm_set_virtual_x2apic_mode(struct kvm_vcpu *vcpu, bool set)
>> +static void svm_set_virtual_apic_mode(struct kvm_vcpu *vcpu)
>>  {
>>       return;
>>  }
>> @@ -7076,7 +7076,7 @@ static struct kvm_x86_ops svm_x86_ops __ro_after_init = {
>>       .enable_nmi_window = enable_nmi_window,
>>       .enable_irq_window = enable_irq_window,
>>       .update_cr8_intercept = update_cr8_intercept,
>> -     .set_virtual_x2apic_mode = svm_set_virtual_x2apic_mode,
>> +     .set_virtual_apic_mode = svm_set_virtual_apic_mode,
>>       .get_enable_apicv = svm_get_enable_apicv,
>>       .refresh_apicv_exec_ctrl = svm_refresh_apicv_exec_ctrl,
>>       .load_eoi_exitmap = svm_load_eoi_exitmap,
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index c766880..c7a3dea 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -481,7 +481,8 @@ struct nested_vmx {
>>       bool sync_shadow_vmcs;
>>       bool dirty_vmcs12;
>>
>> -     bool change_vmcs01_virtual_x2apic_mode;
>> +     bool change_vmcs01_virtual_apic_mode;
>> +
>>       /* L2 must run next, and mustn't decide to exit to L1. */
>>       bool nested_run_pending;
>>
>> @@ -9248,31 +9249,43 @@ static void update_cr8_intercept(struct kvm_vcpu *vcpu, int tpr, int irr)
>>       vmcs_write32(TPR_THRESHOLD, irr);
>>  }
>>
>> -static void vmx_set_virtual_x2apic_mode(struct kvm_vcpu *vcpu, bool set)
>> +static void vmx_set_virtual_apic_mode(struct kvm_vcpu *vcpu)
>>  {
>>       u32 sec_exec_control;
>>
>> +     if (!lapic_in_kernel(vcpu))
>> +             return;
>> +
>>       /* Postpone execution until vmcs01 is the current VMCS. */
>>       if (is_guest_mode(vcpu)) {
>> -             to_vmx(vcpu)->nested.change_vmcs01_virtual_x2apic_mode = true;
>> +             to_vmx(vcpu)->nested.change_vmcs01_virtual_apic_mode = true;
>>               return;
>>       }
>>
>> -     if (!cpu_has_vmx_virtualize_x2apic_mode())
>> -             return;
>> -
>>       if (!cpu_need_tpr_shadow(vcpu))
>>               return;
>>
>>       sec_exec_control = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
>> +     sec_exec_control &= ~(SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
>> +                           SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE);
>>
>> -     if (set) {
>> -             sec_exec_control &= ~SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES;
>> -             sec_exec_control |= SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE;
>> -     } else {
>> -             sec_exec_control &= ~SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE;
>> -             sec_exec_control |= SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES;
>> -             vmx_flush_tlb(vcpu, true);
>> +     switch (kvm_get_apic_mode(vcpu)) {
>> +     case LAPIC_MODE_INVALID:
>> +             WARN_ONCE(true, "Invalid local APIC state");
>> +     case LAPIC_MODE_DISABLED:
>> +             break;
>> +     case LAPIC_MODE_XAPIC:
>> +             if (flexpriority_enabled) {
>> +                     sec_exec_control |=
>> +                             SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES;
>> +                     vmx_flush_tlb(vcpu, true);
>> +             }
>> +             break;
>> +     case LAPIC_MODE_X2APIC:
>> +             if (cpu_has_vmx_virtualize_x2apic_mode())
>> +                     sec_exec_control |=
>> +                             SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE;
>> +             break;
>>       }
>>       vmcs_write32(SECONDARY_VM_EXEC_CONTROL, sec_exec_control);
>>
>> @@ -12060,10 +12073,9 @@ static void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 exit_reason,
>>       if (kvm_has_tsc_control)
>>               decache_tsc_multiplier(vmx);
>>
>> -     if (vmx->nested.change_vmcs01_virtual_x2apic_mode) {
>> -             vmx->nested.change_vmcs01_virtual_x2apic_mode = false;
>> -             vmx_set_virtual_x2apic_mode(vcpu,
>> -                             vcpu->arch.apic_base & X2APIC_ENABLE);
>> +     if (vmx->nested.change_vmcs01_virtual_apic_mode) {
>> +             vmx->nested.change_vmcs01_virtual_apic_mode = false;
>> +             vmx_set_virtual_apic_mode(vcpu);
>>       } else if (!nested_cpu_has_ept(vmcs12) &&
>>                  nested_cpu_has2(vmcs12,
>>                                  SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES)) {
>> @@ -12691,7 +12703,7 @@ static struct kvm_x86_ops vmx_x86_ops __ro_after_init = {
>>       .enable_nmi_window = enable_nmi_window,
>>       .enable_irq_window = enable_irq_window,
>>       .update_cr8_intercept = update_cr8_intercept,
>> -     .set_virtual_x2apic_mode = vmx_set_virtual_x2apic_mode,
>> +     .set_virtual_apic_mode = vmx_set_virtual_apic_mode,
>>       .set_apic_access_page_addr = vmx_set_apic_access_page_addr,
>>       .get_enable_apicv = vmx_get_enable_apicv,
>>       .refresh_apicv_exec_ctrl = vmx_refresh_apicv_exec_ctrl,
>>
>
diff mbox

Patch

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index c25775f..52e685b 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -995,7 +995,7 @@  struct kvm_x86_ops {
 	void (*hwapic_irr_update)(struct kvm_vcpu *vcpu, int max_irr);
 	void (*hwapic_isr_update)(struct kvm_vcpu *vcpu, int isr);
 	void (*load_eoi_exitmap)(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap);
-	void (*set_virtual_x2apic_mode)(struct kvm_vcpu *vcpu, bool set);
+	void (*set_virtual_apic_mode)(struct kvm_vcpu *vcpu);
 	void (*set_apic_access_page_addr)(struct kvm_vcpu *vcpu, hpa_t hpa);
 	void (*deliver_posted_interrupt)(struct kvm_vcpu *vcpu, int vector);
 	int (*sync_pir_to_irr)(struct kvm_vcpu *vcpu);
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index b74c9c1..776391c 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1990,13 +1990,11 @@  void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64 value)
 		}
 	}
 
-	if ((old_value ^ value) & X2APIC_ENABLE) {
-		if (value & X2APIC_ENABLE) {
-			kvm_apic_set_x2apic_id(apic, vcpu->vcpu_id);
-			kvm_x86_ops->set_virtual_x2apic_mode(vcpu, true);
-		} else
-			kvm_x86_ops->set_virtual_x2apic_mode(vcpu, false);
-	}
+	if (((old_value ^ value) & X2APIC_ENABLE) && (value & X2APIC_ENABLE))
+		kvm_apic_set_x2apic_id(apic, vcpu->vcpu_id);
+
+	if ((old_value ^ value) & (MSR_IA32_APICBASE_ENABLE | X2APIC_ENABLE))
+		kvm_x86_ops->set_virtual_apic_mode(vcpu);
 
 	apic->base_address = apic->vcpu->arch.apic_base &
 			     MSR_IA32_APICBASE_BASE;
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 1fc05e4..220e5a89 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -5036,7 +5036,7 @@  static void update_cr8_intercept(struct kvm_vcpu *vcpu, int tpr, int irr)
 		set_cr_intercept(svm, INTERCEPT_CR8_WRITE);
 }
 
-static void svm_set_virtual_x2apic_mode(struct kvm_vcpu *vcpu, bool set)
+static void svm_set_virtual_apic_mode(struct kvm_vcpu *vcpu)
 {
 	return;
 }
@@ -7076,7 +7076,7 @@  static struct kvm_x86_ops svm_x86_ops __ro_after_init = {
 	.enable_nmi_window = enable_nmi_window,
 	.enable_irq_window = enable_irq_window,
 	.update_cr8_intercept = update_cr8_intercept,
-	.set_virtual_x2apic_mode = svm_set_virtual_x2apic_mode,
+	.set_virtual_apic_mode = svm_set_virtual_apic_mode,
 	.get_enable_apicv = svm_get_enable_apicv,
 	.refresh_apicv_exec_ctrl = svm_refresh_apicv_exec_ctrl,
 	.load_eoi_exitmap = svm_load_eoi_exitmap,
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index c766880..c7a3dea 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -481,7 +481,8 @@  struct nested_vmx {
 	bool sync_shadow_vmcs;
 	bool dirty_vmcs12;
 
-	bool change_vmcs01_virtual_x2apic_mode;
+	bool change_vmcs01_virtual_apic_mode;
+
 	/* L2 must run next, and mustn't decide to exit to L1. */
 	bool nested_run_pending;
 
@@ -9248,31 +9249,43 @@  static void update_cr8_intercept(struct kvm_vcpu *vcpu, int tpr, int irr)
 	vmcs_write32(TPR_THRESHOLD, irr);
 }
 
-static void vmx_set_virtual_x2apic_mode(struct kvm_vcpu *vcpu, bool set)
+static void vmx_set_virtual_apic_mode(struct kvm_vcpu *vcpu)
 {
 	u32 sec_exec_control;
 
+	if (!lapic_in_kernel(vcpu))
+		return;
+
 	/* Postpone execution until vmcs01 is the current VMCS. */
 	if (is_guest_mode(vcpu)) {
-		to_vmx(vcpu)->nested.change_vmcs01_virtual_x2apic_mode = true;
+		to_vmx(vcpu)->nested.change_vmcs01_virtual_apic_mode = true;
 		return;
 	}
 
-	if (!cpu_has_vmx_virtualize_x2apic_mode())
-		return;
-
 	if (!cpu_need_tpr_shadow(vcpu))
 		return;
 
 	sec_exec_control = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
+	sec_exec_control &= ~(SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
+			      SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE);
 
-	if (set) {
-		sec_exec_control &= ~SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES;
-		sec_exec_control |= SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE;
-	} else {
-		sec_exec_control &= ~SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE;
-		sec_exec_control |= SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES;
-		vmx_flush_tlb(vcpu, true);
+	switch (kvm_get_apic_mode(vcpu)) {
+	case LAPIC_MODE_INVALID:
+		WARN_ONCE(true, "Invalid local APIC state");
+	case LAPIC_MODE_DISABLED:
+		break;
+	case LAPIC_MODE_XAPIC:
+		if (flexpriority_enabled) {
+			sec_exec_control |=
+				SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES;
+			vmx_flush_tlb(vcpu, true);
+		}
+		break;
+	case LAPIC_MODE_X2APIC:
+		if (cpu_has_vmx_virtualize_x2apic_mode())
+			sec_exec_control |=
+				SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE;
+		break;
 	}
 	vmcs_write32(SECONDARY_VM_EXEC_CONTROL, sec_exec_control);
 
@@ -12060,10 +12073,9 @@  static void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 exit_reason,
 	if (kvm_has_tsc_control)
 		decache_tsc_multiplier(vmx);
 
-	if (vmx->nested.change_vmcs01_virtual_x2apic_mode) {
-		vmx->nested.change_vmcs01_virtual_x2apic_mode = false;
-		vmx_set_virtual_x2apic_mode(vcpu,
-				vcpu->arch.apic_base & X2APIC_ENABLE);
+	if (vmx->nested.change_vmcs01_virtual_apic_mode) {
+		vmx->nested.change_vmcs01_virtual_apic_mode = false;
+		vmx_set_virtual_apic_mode(vcpu);
 	} else if (!nested_cpu_has_ept(vmcs12) &&
 		   nested_cpu_has2(vmcs12,
 				   SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES)) {
@@ -12691,7 +12703,7 @@  static struct kvm_x86_ops vmx_x86_ops __ro_after_init = {
 	.enable_nmi_window = enable_nmi_window,
 	.enable_irq_window = enable_irq_window,
 	.update_cr8_intercept = update_cr8_intercept,
-	.set_virtual_x2apic_mode = vmx_set_virtual_x2apic_mode,
+	.set_virtual_apic_mode = vmx_set_virtual_apic_mode,
 	.set_apic_access_page_addr = vmx_set_apic_access_page_addr,
 	.get_enable_apicv = vmx_get_enable_apicv,
 	.refresh_apicv_exec_ctrl = vmx_refresh_apicv_exec_ctrl,