[PATCH 0/3] KVM: SVM: Fix guest not booting w/ AVIC enabled

public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH 0/3] KVM: SVM: Fix guest not booting w/ AVIC enabled
@ 2017-09-06  3:39 Suravee Suthikulpanit
  2017-09-06  3:39 ` [PATCH 1/3] KVM: SVM: Refactor AVIC vcpu initialization into avic_init_vcpu() Suravee Suthikulpanit
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Suravee Suthikulpanit @ 2017-09-06  3:39 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: pbonzini, rkrcmar, joro, Suravee Suthikulpanit

Certain QEMU options fails to boot VM guest w/ SVM AVIC enabled
(e.g. modprobe kvm_amd avic=1). Investigation shows that this mainly
due to AVIC hardware does not trap into hypervisor when guest OS
writes to APIC_EOI register. 

The boot hang is caused by missing timer interrupt when using in-kernel
PIT model (e.g. launch qemu w/ '-no-hpet' option) since it requires
irq acknowledgmen before injecting another interrupt in case
irq re-injection is enabled (normally default).

Suravee Suthikulpanit (3):
  KVM: SVM: Refactor AVIC vcpu initialization into avic_init_vcpu()
  KVM: Add struct kvm_vcpu pointer parameter to get_enable_apicv()
  KVM: SVM: Add irqchip_split() checks before enabling AVIC

 arch/x86/include/asm/kvm_host.h |  2 +-
 arch/x86/kvm/svm.c              | 43 ++++++++++++++++++++++++++++-------------
 arch/x86/kvm/vmx.c              |  2 +-
 arch/x86/kvm/x86.c              |  2 +-
 4 files changed, 33 insertions(+), 16 deletions(-)

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/3] KVM: SVM: Refactor AVIC vcpu initialization into avic_init_vcpu()
  2017-09-06  3:39 [PATCH 0/3] KVM: SVM: Fix guest not booting w/ AVIC enabled Suravee Suthikulpanit
@ 2017-09-06  3:39 ` Suravee Suthikulpanit
  2017-09-06  3:39 ` [PATCH 2/3] KVM: Add struct kvm_vcpu pointer parameter to get_enable_apicv() Suravee Suthikulpanit
  2017-09-06  3:39 ` [PATCH 3/3] KVM: SVM: Add irqchip_split() checks before enabling AVIC Suravee Suthikulpanit
  2 siblings, 0 replies; 9+ messages in thread
From: Suravee Suthikulpanit @ 2017-09-06  3:39 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: pbonzini, rkrcmar, joro, Suravee Suthikulpanit

Preparing the base code for subsequent changes. This does not change
existing logic.

Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 arch/x86/kvm/svm.c | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index af256b7..316edbf 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -1587,6 +1587,23 @@ static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
 		avic_update_vapic_bar(svm, APIC_DEFAULT_PHYS_BASE);
 }
 
+static int avic_init_vcpu(struct vcpu_svm *svm)
+{
+	int ret;
+
+	if (!avic)
+		return 0;
+
+	ret = avic_init_backing_page(&svm->vcpu);
+	if (ret)
+		return ret;
+
+	INIT_LIST_HEAD(&svm->ir_list);
+	spin_lock_init(&svm->ir_list_lock);
+
+	return ret;
+}
+
 static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id)
 {
 	struct vcpu_svm *svm;
@@ -1623,14 +1640,9 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id)
 	if (!hsave_page)
 		goto free_page3;
 
-	if (avic) {
-		err = avic_init_backing_page(&svm->vcpu);
-		if (err)
-			goto free_page4;
-
-		INIT_LIST_HEAD(&svm->ir_list);
-		spin_lock_init(&svm->ir_list_lock);
-	}
+	err = avic_init_vcpu(svm);
+	if (err)
+		goto free_page4;
 
 	/* We initialize this flag to true to make sure that the is_running
 	 * bit would be set the first time the vcpu is loaded.
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/3] KVM: Add struct kvm_vcpu pointer parameter to get_enable_apicv()
  2017-09-06  3:39 [PATCH 0/3] KVM: SVM: Fix guest not booting w/ AVIC enabled Suravee Suthikulpanit
  2017-09-06  3:39 ` [PATCH 1/3] KVM: SVM: Refactor AVIC vcpu initialization into avic_init_vcpu() Suravee Suthikulpanit
@ 2017-09-06  3:39 ` Suravee Suthikulpanit
  2017-09-06  3:39 ` [PATCH 3/3] KVM: SVM: Add irqchip_split() checks before enabling AVIC Suravee Suthikulpanit
  2 siblings, 0 replies; 9+ messages in thread
From: Suravee Suthikulpanit @ 2017-09-06  3:39 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: pbonzini, rkrcmar, joro, Suravee Suthikulpanit

Modify struct kvm_x86_ops.arch.apicv_active() to take struct kvm_vcpu
pointer as parameter in preparation to subsequent changes.

Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 arch/x86/include/asm/kvm_host.h | 2 +-
 arch/x86/kvm/svm.c              | 2 +-
 arch/x86/kvm/vmx.c              | 2 +-
 arch/x86/kvm/x86.c              | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index f4d120a..3cdde44 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -969,7 +969,7 @@ struct kvm_x86_ops {
 	void (*enable_nmi_window)(struct kvm_vcpu *vcpu);
 	void (*enable_irq_window)(struct kvm_vcpu *vcpu);
 	void (*update_cr8_intercept)(struct kvm_vcpu *vcpu, int tpr, int irr);
-	bool (*get_enable_apicv)(void);
+	bool (*get_enable_apicv)(struct kvm_vcpu *vcpu);
 	void (*refresh_apicv_exec_ctrl)(struct kvm_vcpu *vcpu);
 	void (*hwapic_irr_update)(struct kvm_vcpu *vcpu, int max_irr);
 	void (*hwapic_isr_update)(struct kvm_vcpu *vcpu, int isr);
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 316edbf..d1b3eb4 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -4386,7 +4386,7 @@ static void svm_set_virtual_x2apic_mode(struct kvm_vcpu *vcpu, bool set)
 	return;
 }
 
-static bool svm_get_enable_apicv(void)
+static bool svm_get_enable_apicv(struct kvm_vcpu *vcpu)
 {
 	return avic;
 }
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index c6ef294..8a7ab16 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -4983,7 +4983,7 @@ static void vmx_disable_intercept_msr_x2apic(u32 msr, int type, bool apicv_activ
 	}
 }
 
-static bool vmx_get_enable_apicv(void)
+static bool vmx_get_enable_apicv(struct kvm_vcpu *vcpu)
 {
 	return enable_apicv;
 }
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 05a5e57..b8c40ca 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7946,7 +7946,7 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
 	BUG_ON(vcpu->kvm == NULL);
 	kvm = vcpu->kvm;
 
-	vcpu->arch.apicv_active = kvm_x86_ops->get_enable_apicv();
+	vcpu->arch.apicv_active = kvm_x86_ops->get_enable_apicv(vcpu);
 	vcpu->arch.pv.pv_unhalted = false;
 	vcpu->arch.emulate_ctxt.ops = &emulate_ops;
 	if (!irqchip_in_kernel(kvm) || kvm_vcpu_is_reset_bsp(vcpu))
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/3] KVM: SVM: Add irqchip_split() checks before enabling AVIC
  2017-09-06  3:39 [PATCH 0/3] KVM: SVM: Fix guest not booting w/ AVIC enabled Suravee Suthikulpanit
  2017-09-06  3:39 ` [PATCH 1/3] KVM: SVM: Refactor AVIC vcpu initialization into avic_init_vcpu() Suravee Suthikulpanit
  2017-09-06  3:39 ` [PATCH 2/3] KVM: Add struct kvm_vcpu pointer parameter to get_enable_apicv() Suravee Suthikulpanit
@ 2017-09-06  3:39 ` Suravee Suthikulpanit
  2017-09-08 15:53   ` Radim Krčmář
  2 siblings, 1 reply; 9+ messages in thread
From: Suravee Suthikulpanit @ 2017-09-06  3:39 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: pbonzini, rkrcmar, joro, Suravee Suthikulpanit

SVM AVIC hardware accelerates guest write to APIC_EOI register
(for edge-trigger interrupt), which means it does not trap to KVM.

So, only enable SVM AVIC only in split irqchip mode.
(e.g. launching qemu w/ option '-machine kernel_irqchip=split').

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 arch/x86/kvm/svm.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index d1b3eb4..7ce191b 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -1176,7 +1176,6 @@ static void avic_init_vmcb(struct vcpu_svm *svm)
 	vmcb->control.avic_physical_id = ppa & AVIC_HPA_MASK;
 	vmcb->control.avic_physical_id |= AVIC_MAX_PHYSICAL_ID_COUNT;
 	vmcb->control.int_ctl |= AVIC_ENABLE_MASK;
-	svm->vcpu.arch.apicv_active = true;
 }
 
 static void init_vmcb(struct vcpu_svm *svm)
@@ -1292,7 +1291,7 @@ static void init_vmcb(struct vcpu_svm *svm)
 		set_intercept(svm, INTERCEPT_PAUSE);
 	}
 
-	if (avic)
+	if (kvm_vcpu_apicv_active(&svm->vcpu))
 		avic_init_vmcb(svm);
 
 	/*
@@ -1594,6 +1593,12 @@ static int avic_init_vcpu(struct vcpu_svm *svm)
 	if (!avic)
 		return 0;
 
+	if (!irqchip_split(svm->vcpu.kvm)) {
+		pr_debug("%s: Disable AVIC due to non-split irqchip.\n",
+			 __func__);
+		return 0;
+	}
+
 	ret = avic_init_backing_page(&svm->vcpu);
 	if (ret)
 		return ret;
@@ -4388,7 +4393,7 @@ static void svm_set_virtual_x2apic_mode(struct kvm_vcpu *vcpu, bool set)
 
 static bool svm_get_enable_apicv(struct kvm_vcpu *vcpu)
 {
-	return avic;
+	return avic && irqchip_split(vcpu->kvm);
 }
 
 static void svm_hwapic_irr_update(struct kvm_vcpu *vcpu, int max_irr)
@@ -4405,7 +4410,7 @@ static void svm_refresh_apicv_exec_ctrl(struct kvm_vcpu *vcpu)
 	struct vcpu_svm *svm = to_svm(vcpu);
 	struct vmcb *vmcb = svm->vmcb;
 
-	if (!avic)
+	if (!avic || !irqchip_split(svm->vcpu.kvm))
 		return;
 
 	vmcb->control.int_ctl &= ~AVIC_ENABLE_MASK;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] KVM: SVM: Add irqchip_split() checks before enabling AVIC
  2017-09-06  3:39 ` [PATCH 3/3] KVM: SVM: Add irqchip_split() checks before enabling AVIC Suravee Suthikulpanit
@ 2017-09-08 15:53   ` Radim Krčmář
  2017-09-12  8:59     ` Suravee Suthikulpanit
  0 siblings, 1 reply; 9+ messages in thread
From: Radim Krčmář @ 2017-09-08 15:53 UTC (permalink / raw)
  To: Suravee Suthikulpanit; +Cc: linux-kernel, kvm, pbonzini, joro

2017-09-05 22:39-0500, Suravee Suthikulpanit:
> SVM AVIC hardware accelerates guest write to APIC_EOI register
> (for edge-trigger interrupt), which means it does not trap to KVM.
> 
> So, only enable SVM AVIC only in split irqchip mode.
> (e.g. launching qemu w/ option '-machine kernel_irqchip=split').

Yeah, hacking TMR to get the VM exit could result in future bugs.
We have to push split irqchip as the deafult in userspaces with this
change.

> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
>  arch/x86/kvm/svm.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index d1b3eb4..7ce191b 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -1292,7 +1291,7 @@ static void init_vmcb(struct vcpu_svm *svm)
>  		set_intercept(svm, INTERCEPT_PAUSE);
>  	}
>  
> -	if (avic)
> +	if (kvm_vcpu_apicv_active(&svm->vcpu))
>  		avic_init_vmcb(svm);
>  	/*
> @@ -1594,6 +1593,12 @@ static int avic_init_vcpu(struct vcpu_svm *svm)
>  	if (!avic)
>  		return 0;
>  
> +	if (!irqchip_split(svm->vcpu.kvm)) {

The other place used kvm_vcpu_apicv_active() instead of checking
irqchip_split() directly, so I think it would be better to be consistent
and do it here too.
I'd also like if this workaround used !irqchip_split() exactly once.

> +		pr_debug("%s: Disable AVIC due to non-split irqchip.\n",
> +			 __func__);

There is going to be too much of those.  pr_debug_once() would be a
better notification.  We can also report it in svm_get_enable_apicv().

> +		return 0;
> +	}
> +
>  	ret = avic_init_backing_page(&svm->vcpu);
>  	if (ret)
>  		return ret;
> @@ -4388,7 +4393,7 @@ static void svm_set_virtual_x2apic_mode(struct kvm_vcpu *vcpu, bool set)
>  
>  static bool svm_get_enable_apicv(struct kvm_vcpu *vcpu)
>  {
> -	return avic;
> +	return avic && irqchip_split(vcpu->kvm);
>  }
>  
>  static void svm_hwapic_irr_update(struct kvm_vcpu *vcpu, int max_irr)
> @@ -4405,7 +4410,7 @@ static void svm_refresh_apicv_exec_ctrl(struct kvm_vcpu *vcpu)
>  	struct vcpu_svm *svm = to_svm(vcpu);
>  	struct vmcb *vmcb = svm->vmcb;
>  
> -	if (!avic)
> +	if (!avic || !irqchip_split(svm->vcpu.kvm))

Seems like we want !kvm_vcpu_apicv_active() here too.

>  		return;
>  
>  	vmcb->control.int_ctl &= ~AVIC_ENABLE_MASK;

(separate bug: refresh should be able to enable avic as well.)

thanks.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] KVM: SVM: Add irqchip_split() checks before enabling AVIC
  2017-09-08 15:53   ` Radim Krčmář
@ 2017-09-12  8:59     ` Suravee Suthikulpanit
  2017-09-12 13:17       ` Radim Krčmář
  0 siblings, 1 reply; 9+ messages in thread
From: Suravee Suthikulpanit @ 2017-09-12  8:59 UTC (permalink / raw)
  To: Radim Krčmář; +Cc: linux-kernel, kvm, pbonzini, joro

Hi Radim,

On 9/8/17 08:53, Radim Krčmář wrote:
> 2017-09-05 22:39-0500, Suravee Suthikulpanit:
>> SVM AVIC hardware accelerates guest write to APIC_EOI register
>> (for edge-trigger interrupt), which means it does not trap to KVM.
>>
>> So, only enable SVM AVIC only in split irqchip mode.
>> (e.g. launching qemu w/ option '-machine kernel_irqchip=split').
>
> Yeah, hacking TMR to get the VM exit could result in future bugs.
> We have to push split irqchip as the deafult in userspaces with this
> change.

Actually, I'm not quite sure about the advantages/disadvantages with split 
irqchip, and how it would affect other cases, and why it was not used as default 
currently.

>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>> ---
>>  arch/x86/kvm/svm.c | 13 +++++++++----
>>  1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
>> index d1b3eb4..7ce191b 100644
>> --- a/arch/x86/kvm/svm.c
>> +++ b/arch/x86/kvm/svm.c
>> @@ -1292,7 +1291,7 @@ static void init_vmcb(struct vcpu_svm *svm)
>>  		set_intercept(svm, INTERCEPT_PAUSE);
>>  	}
>>
>> -	if (avic)
>> +	if (kvm_vcpu_apicv_active(&svm->vcpu))
>>  		avic_init_vmcb(svm);
>>  	/*
>> @@ -1594,6 +1593,12 @@ static int avic_init_vcpu(struct vcpu_svm *svm)
>>  	if (!avic)
>>  		return 0;
>>
>> +	if (!irqchip_split(svm->vcpu.kvm)) {
>
> The other place used kvm_vcpu_apicv_active() instead of checking
> irqchip_split() directly, so I think it would be better to be consistent
> and do it here too.
> I'd also like if this workaround used !irqchip_split() exactly once.

Ok, I'll clean up in V2.

>> +		pr_debug("%s: Disable AVIC due to non-split irqchip.\n",
>> +			 __func__);
>
> There is going to be too much of those.  pr_debug_once() would be a
> better notification.  We can also report it in svm_get_enable_apicv().

pr_debug_once does not use dynamic debug APIs. I think I can call pr_debug only 
when vcpu_id == 0.

>> +		return 0;
>> +	}
>> +
>>  	ret = avic_init_backing_page(&svm->vcpu);
>>  	if (ret)
>>  		return ret;
>> @@ -4388,7 +4393,7 @@ static void svm_set_virtual_x2apic_mode(struct kvm_vcpu *vcpu, bool set)
>>
>>  static bool svm_get_enable_apicv(struct kvm_vcpu *vcpu)
>>  {
>> -	return avic;
>> +	return avic && irqchip_split(vcpu->kvm);
>>  }
>>
>>  static void svm_hwapic_irr_update(struct kvm_vcpu *vcpu, int max_irr)
>> @@ -4405,7 +4410,7 @@ static void svm_refresh_apicv_exec_ctrl(struct kvm_vcpu *vcpu)
>>  	struct vcpu_svm *svm = to_svm(vcpu);
>>  	struct vmcb *vmcb = svm->vmcb;
>>
>> -	if (!avic)
>> +	if (!avic || !irqchip_split(svm->vcpu.kvm))
>
> Seems like we want !kvm_vcpu_apicv_active() here too.

Right.

>
>>  		return;
>>
>>  	vmcb->control.int_ctl &= ~AVIC_ENABLE_MASK;
>
> (separate bug: refresh should be able to enable avic as well.)
>
> thanks.
>

Thanks,
Suravee

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] KVM: SVM: Add irqchip_split() checks before enabling AVIC
  2017-09-12  8:59     ` Suravee Suthikulpanit
@ 2017-09-12 13:17       ` Radim Krčmář
  2017-09-12 14:22         ` Paolo Bonzini
  0 siblings, 1 reply; 9+ messages in thread
From: Radim Krčmář @ 2017-09-12 13:17 UTC (permalink / raw)
  To: Suravee Suthikulpanit; +Cc: linux-kernel, kvm, pbonzini, joro

2017-09-12 01:59-0700, Suravee Suthikulpanit:
> On 9/8/17 08:53, Radim Krčmář wrote:
>> 2017-09-05 22:39-0500, Suravee Suthikulpanit:
>> > SVM AVIC hardware accelerates guest write to APIC_EOI register
>> > (for edge-trigger interrupt), which means it does not trap to KVM.
>> > 
>> > So, only enable SVM AVIC only in split irqchip mode.
>> > (e.g. launching qemu w/ option '-machine kernel_irqchip=split').
>> 
>> Yeah, hacking TMR to get the VM exit could result in future bugs.
>> We have to push split irqchip as the deafult in userspaces with this
>> change.
> 
> Actually, I'm not quite sure about the advantages/disadvantages with split
> irqchip, and how it would affect other cases, and why it was not used as
> default currently.

The main advantage of split irqchip is that we're moving code out of the
kernel, and QEMU's irqchip currently has more features too.

I think it is not the default as the support for split irqchip is recent
(v4.3) and has lower performance, so it is only used in cases that need
the extra features.

> > > +		pr_debug("%s: Disable AVIC due to non-split irqchip.\n",
> > > +			 __func__);
> > 
> > There is going to be too much of those.  pr_debug_once() would be a
> > better notification.  We can also report it in svm_get_enable_apicv().
> 
> pr_debug_once does not use dynamic debug APIs. I think I can call pr_debug
> only when vcpu_id == 0.

I see, the rest uses dynamic debug.  It is not printing by default, so
v1 is ok.  (I'd rather remove the line than to add a condition.)

Thanks.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] KVM: SVM: Add irqchip_split() checks before enabling AVIC
  2017-09-12 13:17       ` Radim Krčmář
@ 2017-09-12 14:22         ` Paolo Bonzini
  2017-09-12 15:07           ` Suravee Suthikulpanit
  0 siblings, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2017-09-12 14:22 UTC (permalink / raw)
  To: Radim Krčmář, Suravee Suthikulpanit
  Cc: linux-kernel, kvm, joro

On 12/09/2017 15:17, Radim Krčmář wrote:
>>> Yeah, hacking TMR to get the VM exit could result in future bugs.
>>> We have to push split irqchip as the deafult in userspaces with this
>>> change.
>> Actually, I'm not quite sure about the advantages/disadvantages with split
>> irqchip, and how it would affect other cases, and why it was not used as
>> default currently.
> The main advantage of split irqchip is that we're moving code out of the
> kernel, and QEMU's irqchip currently has more features too.
> 
> I think it is not the default as the support for split irqchip is recent
> (v4.3) and has lower performance, so it is only used in cases that need
> the extra features.

One other difference is that in-kernel PIT is not supported with
split-irqchip, and the QEMU PIT lacks support for reinjecting lost
ticks.  But this should only be needed for very old guests at this point.

Paolo

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] KVM: SVM: Add irqchip_split() checks before enabling AVIC
  2017-09-12 14:22         ` Paolo Bonzini
@ 2017-09-12 15:07           ` Suravee Suthikulpanit
  0 siblings, 0 replies; 9+ messages in thread
From: Suravee Suthikulpanit @ 2017-09-12 15:07 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář; +Cc: linux-kernel, kvm, joro



On 9/12/17 07:22, Paolo Bonzini wrote:
> On 12/09/2017 15:17, Radim Krčmář wrote:
>>>> Yeah, hacking TMR to get the VM exit could result in future bugs.
>>>> We have to push split irqchip as the deafult in userspaces with this
>>>> change.
>>> Actually, I'm not quite sure about the advantages/disadvantages with split
>>> irqchip, and how it would affect other cases, and why it was not used as
>>> default currently.
>> The main advantage of split irqchip is that we're moving code out of the
>> kernel, and QEMU's irqchip currently has more features too.
>>
>> I think it is not the default as the support for split irqchip is recent
>> (v4.3) and has lower performance, so it is only used in cases that need
>> the extra features.
>
> One other difference is that in-kernel PIT is not supported with
> split-irqchip, and the QEMU PIT lacks support for reinjecting lost
> ticks.  But this should only be needed for very old guests at this point.
>
> Paolo
>

Thanks. I'll look into patch for changing the default in QEMU and follow up with 
that separately then.

Suravee

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-09-12 15:07 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-06  3:39 [PATCH 0/3] KVM: SVM: Fix guest not booting w/ AVIC enabled Suravee Suthikulpanit
2017-09-06  3:39 ` [PATCH 1/3] KVM: SVM: Refactor AVIC vcpu initialization into avic_init_vcpu() Suravee Suthikulpanit
2017-09-06  3:39 ` [PATCH 2/3] KVM: Add struct kvm_vcpu pointer parameter to get_enable_apicv() Suravee Suthikulpanit
2017-09-06  3:39 ` [PATCH 3/3] KVM: SVM: Add irqchip_split() checks before enabling AVIC Suravee Suthikulpanit
2017-09-08 15:53   ` Radim Krčmář
2017-09-12  8:59     ` Suravee Suthikulpanit
2017-09-12 13:17       ` Radim Krčmář
2017-09-12 14:22         ` Paolo Bonzini
2017-09-12 15:07           ` Suravee Suthikulpanit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox