diff mbox series

[2/7] KVM: x86: inhibit APICv/AVIC when the guest and/or host changes either apic id or the apic base from their default values.

Message ID 20220606180829.102503-3-mlevitsk@redhat.com (mailing list archive)
State New, archived
Headers show
Series KVM: x86: AVIC/APICv patch queue | expand

Commit Message

Maxim Levitsky June 6, 2022, 6:08 p.m. UTC 
Neither of these settings should be changed by the guest and it is
a burden to support it in the acceleration code, so just inhibit
this code instead.

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 arch/x86/include/asm/kvm_host.h |  9 +++++++++
 arch/x86/kvm/lapic.c            | 27 +++++++++++++++++++++++----
 arch/x86/kvm/svm/avic.c         |  4 +++-
 arch/x86/kvm/vmx/vmx.c          |  4 +++-
 4 files changed, 38 insertions(+), 6 deletions(-)

Comments

Chao Gao June 7, 2022, 7:05 a.m. UTC | #1 
On Mon, Jun 06, 2022 at 09:08:24PM +0300, Maxim Levitsky wrote:
>+	/*
>+	 * For simplicity, the APIC acceleration is inhibited
>+	 * first time either APIC ID or APIC base are changed by the guest
>+	 * from their reset values.
>+	 */
>+	APICV_INHIBIT_REASON_APIC_ID_MODIFIED,
>+	APICV_INHIBIT_REASON_APIC_BASE_MODIFIED,
>+
>+

Remove one newline.

> void kvm_apic_update_apicv(struct kvm_vcpu *vcpu)
>@@ -2666,6 +2683,8 @@ static int kvm_apic_state_fixup(struct kvm_vcpu *vcpu,
> 			icr = __kvm_lapic_get_reg64(s->regs, APIC_ICR);
> 			__kvm_lapic_set_reg(s->regs, APIC_ICR2, icr >> 32);
> 		}
>+	} else {
>+		kvm_lapic_xapic_id_updated(vcpu->arch.apic);

Strictly speaking, this is needed only for "set" cases.
Maxim Levitsky June 7, 2022, 7:53 a.m. UTC | #2 
On Tue, 2022-06-07 at 15:05 +0800, Chao Gao wrote:
> On Mon, Jun 06, 2022 at 09:08:24PM +0300, Maxim Levitsky wrote:
> > +       /*
> > +        * For simplicity, the APIC acceleration is inhibited
> > +        * first time either APIC ID or APIC base are changed by
> > the guest
> > +        * from their reset values.
> > +        */
> > +       APICV_INHIBIT_REASON_APIC_ID_MODIFIED,
> > +       APICV_INHIBIT_REASON_APIC_BASE_MODIFIED,
> > +
> > +
> 
> Remove one newline.
Will do.
> 
> > void kvm_apic_update_apicv(struct kvm_vcpu *vcpu)
> > @@ -2666,6 +2683,8 @@ static int kvm_apic_state_fixup(struct
> > kvm_vcpu *vcpu,
> >                         icr = __kvm_lapic_get_reg64(s->regs,
> > APIC_ICR);
> >                         __kvm_lapic_set_reg(s->regs, APIC_ICR2, icr
> > >> 32);
> >                 }
> > +       } else {
> > +               kvm_lapic_xapic_id_updated(vcpu->arch.apic);
> 
> Strictly speaking, this is needed only for "set" cases.
> 
True, thanks!


Best regards,
	Maxim Levitsky
diff mbox series

Patch

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index d7fb3ade44501..971db02e8ed86 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1094,6 +1094,15 @@  enum kvm_apicv_inhibit {
 	 */
 	APICV_INHIBIT_REASON_BLOCKIRQ,
 
+	/*
+	 * For simplicity, the APIC acceleration is inhibited
+	 * first time either APIC ID or APIC base are changed by the guest
+	 * from their reset values.
+	 */
+	APICV_INHIBIT_REASON_APIC_ID_MODIFIED,
+	APICV_INHIBIT_REASON_APIC_BASE_MODIFIED,
+
+
 	/******************************************************/
 	/* INHIBITs that are relevant only to the AMD's AVIC. */
 	/******************************************************/
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index e69b83708f050..a413a1d8df4c1 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -2040,6 +2040,19 @@  static void apic_manage_nmi_watchdog(struct kvm_lapic *apic, u32 lvt0_val)
 	}
 }
 
+static void kvm_lapic_xapic_id_updated(struct kvm_lapic *apic)
+{
+	struct kvm *kvm = apic->vcpu->kvm;
+
+	if (KVM_BUG_ON(apic_x2apic_mode(apic), kvm))
+		return;
+
+	if (kvm_xapic_id(apic) == apic->vcpu->vcpu_id)
+		return;
+
+	kvm_set_apicv_inhibit(apic->vcpu->kvm, APICV_INHIBIT_REASON_APIC_ID_MODIFIED);
+}
+
 static int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
 {
 	int ret = 0;
@@ -2048,10 +2061,12 @@  static int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
 
 	switch (reg) {
 	case APIC_ID:		/* Local APIC ID */
-		if (!apic_x2apic_mode(apic))
+		if (!apic_x2apic_mode(apic)) {
 			kvm_apic_set_xapic_id(apic, val >> 24);
-		else
+			kvm_lapic_xapic_id_updated(apic);
+		} else {
 			ret = 1;
+		}
 		break;
 
 	case APIC_TASKPRI:
@@ -2354,8 +2369,10 @@  void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64 value)
 			     MSR_IA32_APICBASE_BASE;
 
 	if ((value & MSR_IA32_APICBASE_ENABLE) &&
-	     apic->base_address != APIC_DEFAULT_PHYS_BASE)
-		pr_warn_once("APIC base relocation is unsupported by KVM");
+	     apic->base_address != APIC_DEFAULT_PHYS_BASE) {
+		kvm_set_apicv_inhibit(apic->vcpu->kvm,
+				      APICV_INHIBIT_REASON_APIC_BASE_MODIFIED);
+	}
 }
 
 void kvm_apic_update_apicv(struct kvm_vcpu *vcpu)
@@ -2666,6 +2683,8 @@  static int kvm_apic_state_fixup(struct kvm_vcpu *vcpu,
 			icr = __kvm_lapic_get_reg64(s->regs, APIC_ICR);
 			__kvm_lapic_set_reg(s->regs, APIC_ICR2, icr >> 32);
 		}
+	} else {
+		kvm_lapic_xapic_id_updated(vcpu->arch.apic);
 	}
 
 	return 0;
diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
index 54fe03714f8a6..8dffd67f60862 100644
--- a/arch/x86/kvm/svm/avic.c
+++ b/arch/x86/kvm/svm/avic.c
@@ -910,7 +910,9 @@  bool avic_check_apicv_inhibit_reasons(enum kvm_apicv_inhibit reason)
 			  BIT(APICV_INHIBIT_REASON_PIT_REINJ) |
 			  BIT(APICV_INHIBIT_REASON_X2APIC) |
 			  BIT(APICV_INHIBIT_REASON_BLOCKIRQ) |
-			  BIT(APICV_INHIBIT_REASON_SEV);
+			  BIT(APICV_INHIBIT_REASON_SEV      |
+			  BIT(APICV_INHIBIT_REASON_APIC_ID_MODIFIED) |
+			  BIT(APICV_INHIBIT_REASON_APIC_BASE_MODIFIED));
 
 	return supported & BIT(reason);
 }
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index fd2e707faf2bf..48440f73c3352 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7860,7 +7860,9 @@  static bool vmx_check_apicv_inhibit_reasons(enum kvm_apicv_inhibit reason)
 	ulong supported = BIT(APICV_INHIBIT_REASON_DISABLE) |
 			  BIT(APICV_INHIBIT_REASON_ABSENT) |
 			  BIT(APICV_INHIBIT_REASON_HYPERV) |
-			  BIT(APICV_INHIBIT_REASON_BLOCKIRQ);
+			  BIT(APICV_INHIBIT_REASON_BLOCKIRQ) |
+			  BIT(APICV_INHIBIT_REASON_APIC_ID_MODIFIED) |
+			  BIT(APICV_INHIBIT_REASON_APIC_BASE_MODIFIED);
 
 	return supported & BIT(reason);
 }