Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1730853 > unrolled thread

Re: [PATCH 3/3] KVM: SVM: Add irqchip_split() checks before enabling AVIC

Started byRadim Krčmář <rkrcmar@redhat.com>
First post2017-09-12 15:20 +0200
Last post2017-09-12 16:30 +0200
Articles 2 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH 3/3] KVM: SVM: Add irqchip_split() checks before enabling  AVIC Radim Krčmář <rkrcmar@redhat.com> - 2017-09-12 15:20 +0200
    Re: [PATCH 3/3] KVM: SVM: Add irqchip_split() checks before enabling  AVIC Paolo Bonzini <pbonzini@redhat.com> - 2017-09-12 16:30 +0200

#1730853 — Re: [PATCH 3/3] KVM: SVM: Add irqchip_split() checks before enabling AVIC

FromRadim Krčmář <rkrcmar@redhat.com>
Date2017-09-12 15:20 +0200
SubjectRe: [PATCH 3/3] KVM: SVM: Add irqchip_split() checks before enabling AVIC
Message-ID<uoTgB-1KR-15@gated-at.bofh.it>
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.

[toc] | [next] | [standalone]


#1730885

FromPaolo Bonzini <pbonzini@redhat.com>
Date2017-09-12 16:30 +0200
Message-ID<uoUmm-2nc-19@gated-at.bofh.it>
In reply to#1730853
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

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web