| Message ID | 1480584341-32340-1-git-send-email-marc.zyngier@arm.com |
|---|---|
| State | New |
| Headers | show |
Hi Marc, > -----Original Message----- > From: kvmarm-bounces@lists.cs.columbia.edu [mailto:kvmarm- > bounces@lists.cs.columbia.edu] On Behalf Of Marc Zyngier > Sent: Thursday, December 01, 2016 9:26 AM > To: Paolo Bonzini; Radim Krčmář > Cc: Catalin Marinas; kvmarm@lists.cs.columbia.edu; linux-arm- > kernel@lists.infradead.org; kvm@vger.kernel.org > Subject: [PATCH] KVM: arm/arm64: vgic: Don't notify EOI for non-SPIs > > When we inject a level triggerered interrupt (and unless it is backed > by the physical distributor - timer style), we request a maintenance > interrupt. Part of the processing for that interrupt is to feed to the > rest of KVM (and to the eventfd subsystem) the information that the > interrupt has been EOIed. > > But that notification only makes sense for SPIs, and not PPIs (such as > the PMU interrupt). Skip over the notification if the interrupt is not > an SPI. Just to clarify my understanding, the maintenance interrupt is generated for cases where there is no mapping of virt to phys interrupts (ie, ICH_LR HW bit is not set). And I was under the impression that kvm_notify_acked_irq will eventually deactivate the interrupt on distributor for such cases. Its not clear to me how the deactivation is done otherwise. Could you please help me to understand this better. Thanks, Shameer > Cc: stable@vger.kernel.org # 4.7+ > Fixes: 140b086dd197 ("KVM: arm/arm64: vgic-new: Add GICv2 world switch > backend") > Fixes: 59529f69f504 ("KVM: arm/arm64: vgic-new: Add GICv3 world switch > backend") > Reported-by: Catalin Marinas <catalin.marinas@arm.com> > Tested-by: Catalin Marinas <catalin.marinas@arm.com> > Acked-by: Christoffer Dall <christoffer.dall@linaro.org> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > --- > virt/kvm/arm/vgic/vgic-v2.c | 6 ++++-- > virt/kvm/arm/vgic/vgic-v3.c | 6 ++++-- > 2 files changed, 8 insertions(+), 4 deletions(-) > > diff --git a/virt/kvm/arm/vgic/vgic-v2.c b/virt/kvm/arm/vgic/vgic-v2.c > index 0a063af..9bab867 100644 > --- a/virt/kvm/arm/vgic/vgic-v2.c > +++ b/virt/kvm/arm/vgic/vgic-v2.c > @@ -50,8 +50,10 @@ void vgic_v2_process_maintenance(struct kvm_vcpu > *vcpu) > > WARN_ON(cpuif->vgic_lr[lr] & GICH_LR_STATE); > > - kvm_notify_acked_irq(vcpu->kvm, 0, > - intid - VGIC_NR_PRIVATE_IRQS); > + /* Only SPIs require notification */ > + if (vgic_valid_spi(vcpu->kvm, intid)) > + kvm_notify_acked_irq(vcpu->kvm, 0, > + intid - VGIC_NR_PRIVATE_IRQS); > } > } > > diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c > index 9f0dae3..5c9f974 100644 > --- a/virt/kvm/arm/vgic/vgic-v3.c > +++ b/virt/kvm/arm/vgic/vgic-v3.c > @@ -41,8 +41,10 @@ void vgic_v3_process_maintenance(struct kvm_vcpu > *vcpu) > > WARN_ON(cpuif->vgic_lr[lr] & ICH_LR_STATE); > > - kvm_notify_acked_irq(vcpu->kvm, 0, > - intid - VGIC_NR_PRIVATE_IRQS); > + /* Only SPIs require notification */ > + if (vgic_valid_spi(vcpu->kvm, intid)) > + kvm_notify_acked_irq(vcpu->kvm, 0, > + intid - VGIC_NR_PRIVATE_IRQS); > } > > /* > -- > 2.1.4 > > _______________________________________________ > kvmarm mailing list > kvmarm@lists.cs.columbia.edu > https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
On 01/12/16 10:28, Shameerali Kolothum Thodi wrote: > Hi Marc, > >> -----Original Message----- >> From: kvmarm-bounces@lists.cs.columbia.edu [mailto:kvmarm- >> bounces@lists.cs.columbia.edu] On Behalf Of Marc Zyngier >> Sent: Thursday, December 01, 2016 9:26 AM >> To: Paolo Bonzini; Radim Krčmář >> Cc: Catalin Marinas; kvmarm@lists.cs.columbia.edu; linux-arm- >> kernel@lists.infradead.org; kvm@vger.kernel.org >> Subject: [PATCH] KVM: arm/arm64: vgic: Don't notify EOI for non-SPIs >> >> When we inject a level triggerered interrupt (and unless it is backed >> by the physical distributor - timer style), we request a maintenance >> interrupt. Part of the processing for that interrupt is to feed to the >> rest of KVM (and to the eventfd subsystem) the information that the >> interrupt has been EOIed. >> >> But that notification only makes sense for SPIs, and not PPIs (such as >> the PMU interrupt). Skip over the notification if the interrupt is not >> an SPI. > > Just to clarify my understanding, the maintenance interrupt is generated > for cases where there is no mapping of virt to phys interrupts > (ie, ICH_LR HW bit is not set). And I was under the impression that > kvm_notify_acked_irq will eventually deactivate the interrupt on distributor > for such cases. Its not clear to me how the deactivation is done > otherwise. > > Could you please help me to understand this better. kvm_notify_acked_irq() doesn't do *anything* at the distributor level, ever (it has no idea of anything GIC-specific anyway). It's sole job is to signal the rest of the stack that an interrupt has been EOIed in the guest. For these interrupts, which are purely virtual, there is absolutely nothing to do at the physical distributor level anyway. Furthermore, kvm_notify_acked_irq doesn't know about per-cpu interrupt, which is why we cannot notify them. Thanks, M.
> -----Original Message----- > From: Marc Zyngier [mailto:marc.zyngier@arm.com] > Sent: Thursday, December 01, 2016 10:58 AM > To: Shameerali Kolothum Thodi; Paolo Bonzini; Radim Krčmář > Cc: Catalin Marinas; kvmarm@lists.cs.columbia.edu; linux-arm- > kernel@lists.infradead.org; kvm@vger.kernel.org > Subject: Re: [PATCH] KVM: arm/arm64: vgic: Don't notify EOI for non- > SPIs > > On 01/12/16 10:28, Shameerali Kolothum Thodi wrote: > > Hi Marc, > > > >> -----Original Message----- > >> From: kvmarm-bounces@lists.cs.columbia.edu [mailto:kvmarm- > >> bounces@lists.cs.columbia.edu] On Behalf Of Marc Zyngier > >> Sent: Thursday, December 01, 2016 9:26 AM > >> To: Paolo Bonzini; Radim Krčmář > >> Cc: Catalin Marinas; kvmarm@lists.cs.columbia.edu; linux-arm- > >> kernel@lists.infradead.org; kvm@vger.kernel.org > >> Subject: [PATCH] KVM: arm/arm64: vgic: Don't notify EOI for non-SPIs > >> > >> When we inject a level triggerered interrupt (and unless it is > backed > >> by the physical distributor - timer style), we request a maintenance > >> interrupt. Part of the processing for that interrupt is to feed to > the > >> rest of KVM (and to the eventfd subsystem) the information that the > >> interrupt has been EOIed. > >> > >> But that notification only makes sense for SPIs, and not PPIs (such > as > >> the PMU interrupt). Skip over the notification if the interrupt is > not > >> an SPI. > > > > Just to clarify my understanding, the maintenance interrupt is > generated > > for cases where there is no mapping of virt to phys interrupts > > (ie, ICH_LR HW bit is not set). And I was under the impression that > > kvm_notify_acked_irq will eventually deactivate the interrupt on > distributor > > for such cases. Its not clear to me how the deactivation is done > > otherwise. > > > > Could you please help me to understand this better. > > kvm_notify_acked_irq() doesn't do *anything* at the distributor level, > ever (it has no idea of anything GIC-specific anyway). It's sole job is > to signal the rest of the stack that an interrupt has been EOIed in the > guest. Thanks Marc. Understood. I got confused by the kvm_set_irq in the kvm_notify_acked_irq path. > For these interrupts, which are purely virtual, there is absolutely > nothing to do at the physical distributor level anyway. Furthermore, > kvm_notify_acked_irq doesn't know about per-cpu interrupt, which is why > we cannot notify them. Just to confirm, that means for any phys interrupt(PPI/SPI) to be injected to the Guest, the mapping bit has(HW bit set) to be used. Thanks, Shameer
On 01/12/16 12:15, Shameerali Kolothum Thodi wrote: >> For these interrupts, which are purely virtual, there is absolutely >> nothing to do at the physical distributor level anyway. Furthermore, >> kvm_notify_acked_irq doesn't know about per-cpu interrupt, which is why >> we cannot notify them. > > Just to confirm, that means for any phys interrupt(PPI/SPI) to be injected > to the Guest, the mapping bit has(HW bit set) to be used. Yup. M.
2016-12-01 09:25+0000, Marc Zyngier: > Paolo, Radim, > > Hopefully, this is the last update for 4.9. This time, a single patch > that prevents bogus acknoledgement of interrupts. > > It'd be great if this could make it into v4.9-final Pulled, thanks.
Paolo, Radim, Hopefully, this is the last update for 4.9. This time, a single patch that prevents bogus acknoledgement of interrupts. It'd be great if this could make it into v4.9-final Thanks, M. The following changes since commit b112c84a6ff035271d41d548c10215f18443d6a6: KVM: arm64: Fix the issues when guest PMCCFILTR is configured (2016-11-18 09:06:58 +0000) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git tags/kvm-arm-for-4.9-rc7 for you to fetch changes up to 8ca18eec2b2276b449c1dc86b98bf083c5fe4e09: KVM: arm/arm64: vgic: Don't notify EOI for non-SPIs (2016-11-24 13:12:07 +0000) ---------------------------------------------------------------- KVM/ARM updates for v4.9-rc7 - Do not call kvm_notify_acked for PPIs ---------------------------------------------------------------- Marc Zyngier (1): KVM: arm/arm64: vgic: Don't notify EOI for non-SPIs virt/kvm/arm/vgic/vgic-v2.c | 6 ++++-- virt/kvm/arm/vgic/vgic-v3.c | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-)