| Message ID | 20211213104634.199141-5-mlevitsk@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | RFC: KVM: SVM: Allow L1's AVIC to co-exist with nesting | expand |
On Mon, Dec 13, 2021, Maxim Levitsky wrote: > kvm_apic_update_apicv is called when AVIC is still active, thus IRR bits > can be set by the CPU after it was called, and won't cause the irr_pending > to be set to true. > > Also the logic in avic_kick_target_vcpu doesn't expect a race with this > function. > > To make it simple, just keep irr_pending set to true and > let the next interrupt injection to the guest clear it. > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > --- > arch/x86/kvm/lapic.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > index baca9fa37a91c..6e1fbbf4c508b 100644 > --- a/arch/x86/kvm/lapic.c > +++ b/arch/x86/kvm/lapic.c > @@ -2312,7 +2312,10 @@ void kvm_apic_update_apicv(struct kvm_vcpu *vcpu) > apic->irr_pending = true; > apic->isr_count = 1; > } else { > - apic->irr_pending = (apic_search_irr(apic) != -1); > + /* > + * Don't touch irr_pending, let it be cleared when > + * we process the interrupt Please don't use pronouns in comments, e.g. who is "we" in this context? Please also say _why_. IIUC, this could more precisely be: /* * Don't clear irr_pending, searching the IRR can race with * updates from the CPU as APICv is still active from hardware's * perspective. The flag will be cleared as appropriate when * KVM injects the interrupt. */ > + */ > apic->isr_count = count_vectors(apic->regs + APIC_ISR); > } > } > -- > 2.26.3 >
On Tue, 2022-01-04 at 22:57 +0000, Sean Christopherson wrote: > On Mon, Dec 13, 2021, Maxim Levitsky wrote: > > kvm_apic_update_apicv is called when AVIC is still active, thus IRR bits > > can be set by the CPU after it was called, and won't cause the irr_pending > > to be set to true. > > > > Also the logic in avic_kick_target_vcpu doesn't expect a race with this > > function. > > > > To make it simple, just keep irr_pending set to true and > > let the next interrupt injection to the guest clear it. > > > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > > --- > > arch/x86/kvm/lapic.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > > index baca9fa37a91c..6e1fbbf4c508b 100644 > > --- a/arch/x86/kvm/lapic.c > > +++ b/arch/x86/kvm/lapic.c > > @@ -2312,7 +2312,10 @@ void kvm_apic_update_apicv(struct kvm_vcpu *vcpu) > > apic->irr_pending = true; > > apic->isr_count = 1; > > } else { > > - apic->irr_pending = (apic_search_irr(apic) != -1); > > + /* > > + * Don't touch irr_pending, let it be cleared when > > + * we process the interrupt > > Please don't use pronouns in comments, e.g. who is "we" in this context? Please > also say _why_. IIUC, this could more precisely be: Yes, good point. I will fix this. Best regards, Maxim Levitsky > > /* > * Don't clear irr_pending, searching the IRR can race with > * updates from the CPU as APICv is still active from hardware's > * perspective. The flag will be cleared as appropriate when > * KVM injects the interrupt. > */ > > > + */ > > apic->isr_count = count_vectors(apic->regs + APIC_ISR); > > } > > } > > -- > > 2.26.3 > >
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index baca9fa37a91c..6e1fbbf4c508b 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -2312,7 +2312,10 @@ void kvm_apic_update_apicv(struct kvm_vcpu *vcpu) apic->irr_pending = true; apic->isr_count = 1; } else { - apic->irr_pending = (apic_search_irr(apic) != -1); + /* + * Don't touch irr_pending, let it be cleared when + * we process the interrupt + */ apic->isr_count = count_vectors(apic->regs + APIC_ISR); } }
kvm_apic_update_apicv is called when AVIC is still active, thus IRR bits can be set by the CPU after it was called, and won't cause the irr_pending to be set to true. Also the logic in avic_kick_target_vcpu doesn't expect a race with this function. To make it simple, just keep irr_pending set to true and let the next interrupt injection to the guest clear it. Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> --- arch/x86/kvm/lapic.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)