* [PATCH] KVM: arm/arm64: GICv4: Do not perform an map to a mapped vLPI @ 2023-11-16 12:52 Kunkun Jiang 2023-11-16 14:10 ` Marc Zyngier 0 siblings, 1 reply; 4+ messages in thread From: Kunkun Jiang @ 2023-11-16 12:52 UTC (permalink / raw) To: Marc Zyngier, Oliver Upton, James Morse, Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon, Jean-Philippe Brucker, eric.auger Cc: linux-arm-kernel, kvmarm, wanghaibin.wang, Kunkun Jiang Before performing an unmap, let's check whether the vLPI has been mapped. This corresponds to checking whether a vLPI is valid before unmap it. Signed-off-by: Kunkun Jiang <jiangkunkun@huawei.com> --- arch/arm64/kvm/vgic/vgic-v4.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/arch/arm64/kvm/vgic/vgic-v4.c b/arch/arm64/kvm/vgic/vgic-v4.c index 339a55194b2c..824f4baf50ee 100644 --- a/arch/arm64/kvm/vgic/vgic-v4.c +++ b/arch/arm64/kvm/vgic/vgic-v4.c @@ -436,6 +436,11 @@ int kvm_vgic_v4_set_forwarding(struct kvm *kvm, int virq, if (ret) goto out; + if (irq->hw) { + ret = -EBUSY; + goto out; + } + /* * Emit the mapping request. If it fails, the ITS probably * isn't v4 compatible, so let's silently bail out. Holding -- 2.33.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: arm/arm64: GICv4: Do not perform an map to a mapped vLPI 2023-11-16 12:52 [PATCH] KVM: arm/arm64: GICv4: Do not perform an map to a mapped vLPI Kunkun Jiang @ 2023-11-16 14:10 ` Marc Zyngier 2023-11-17 9:54 ` Kunkun Jiang 0 siblings, 1 reply; 4+ messages in thread From: Marc Zyngier @ 2023-11-16 14:10 UTC (permalink / raw) To: Kunkun Jiang Cc: Oliver Upton, James Morse, Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon, Jean-Philippe Brucker, eric.auger, linux-arm-kernel, kvmarm, wanghaibin.wang On Thu, 16 Nov 2023 12:52:15 +0000, Kunkun Jiang <jiangkunkun@huawei.com> wrote: > > Before performing an unmap, let's check whether the vLPI has been > mapped. This corresponds to checking whether a vLPI is valid before > unmap it. > > Signed-off-by: Kunkun Jiang <jiangkunkun@huawei.com> > --- > arch/arm64/kvm/vgic/vgic-v4.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/arch/arm64/kvm/vgic/vgic-v4.c b/arch/arm64/kvm/vgic/vgic-v4.c > index 339a55194b2c..824f4baf50ee 100644 > --- a/arch/arm64/kvm/vgic/vgic-v4.c > +++ b/arch/arm64/kvm/vgic/vgic-v4.c > @@ -436,6 +436,11 @@ int kvm_vgic_v4_set_forwarding(struct kvm *kvm, int virq, > if (ret) > goto out; > > + if (irq->hw) { > + ret = -EBUSY; > + goto out; > + } > + So this code affects the mapping side, not the unmapping. Even more confusingly, the subject of the patch doesn't match the commit message. Furthermore, I'm not sure we want to return an error here. Userspace has no knowledge of GICv4, and there are numerous other cases where we don't return any error. This is certainly a userspace visible change. Thanks, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: arm/arm64: GICv4: Do not perform an map to a mapped vLPI 2023-11-16 14:10 ` Marc Zyngier @ 2023-11-17 9:54 ` Kunkun Jiang 2023-11-17 11:47 ` Marc Zyngier 0 siblings, 1 reply; 4+ messages in thread From: Kunkun Jiang @ 2023-11-17 9:54 UTC (permalink / raw) To: Marc Zyngier Cc: Oliver Upton, James Morse, Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon, Jean-Philippe Brucker, eric.auger, linux-arm-kernel, kvmarm, wanghaibin.wang, jiangkunkun Hi Marc， On 2023/11/16 22:10, Marc Zyngier wrote: > On Thu, 16 Nov 2023 12:52:15 +0000, > Kunkun Jiang <jiangkunkun@huawei.com> wrote: >> Before performing an unmap, let's check whether the vLPI has been s/unmap/map >> mapped. This corresponds to checking whether a vLPI is valid before >> unmap it. >> >> Signed-off-by: Kunkun Jiang <jiangkunkun@huawei.com> >> --- >> arch/arm64/kvm/vgic/vgic-v4.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/arch/arm64/kvm/vgic/vgic-v4.c b/arch/arm64/kvm/vgic/vgic-v4.c >> index 339a55194b2c..824f4baf50ee 100644 >> --- a/arch/arm64/kvm/vgic/vgic-v4.c >> +++ b/arch/arm64/kvm/vgic/vgic-v4.c >> @@ -436,6 +436,11 @@ int kvm_vgic_v4_set_forwarding(struct kvm *kvm, int virq, >> if (ret) >> goto out; >> >> + if (irq->hw) { >> + ret = -EBUSY; >> + goto out; >> + } >> + > So this code affects the mapping side, not the unmapping. Even more > confusingly, the subject of the patch doesn't match the commit > message. > > Furthermore, I'm not sure we want to return an error here. Userspace > has no knowledge of GICv4, and there are numerous other cases where we > don't return any error. This is certainly a userspace visible change. Yes, Userspace has no knowledge of GICv4. In the current implementation, this error will not be presented to userspace. Only a log will be printed in kvm_irqfd_assign(). > #ifdef CONFIG_HAVE_KVM_IRQ_BYPASS > if (kvm_arch_has_irq_bypass()) { > irqfd->consumer.token = (void *)irqfd->eventfd; > irqfd->consumer.add_producer = kvm_arch_irq_bypass_add_producer; > irqfd->consumer.del_producer = kvm_arch_irq_bypass_del_producer; > irqfd->consumer.stop = kvm_arch_irq_bypass_stop; > irqfd->consumer.start = kvm_arch_irq_bypass_start; > ret = irq_bypass_register_consumer(&irqfd->consumer); > if (ret) > pr_info("irq bypass consumer (token %p) registration fails: %d\n", > irqfd->consumer.token, ret); > } > #endif If you think it is better not to return an error, I will modify it in the next version. Looking forwarding to your reply. Thanks, Kunkun Jiang _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: arm/arm64: GICv4: Do not perform an map to a mapped vLPI 2023-11-17 9:54 ` Kunkun Jiang @ 2023-11-17 11:47 ` Marc Zyngier 0 siblings, 0 replies; 4+ messages in thread From: Marc Zyngier @ 2023-11-17 11:47 UTC (permalink / raw) To: Kunkun Jiang Cc: Oliver Upton, James Morse, Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon, Jean-Philippe Brucker, eric.auger, linux-arm-kernel, kvmarm, wanghaibin.wang On Fri, 17 Nov 2023 09:54:37 +0000, Kunkun Jiang <jiangkunkun@huawei.com> wrote: > > Hi Marc， > > On 2023/11/16 22:10, Marc Zyngier wrote: > > On Thu, 16 Nov 2023 12:52:15 +0000, > > Kunkun Jiang <jiangkunkun@huawei.com> wrote: > >> Before performing an unmap, let's check whether the vLPI has been > s/unmap/map > >> mapped. This corresponds to checking whether a vLPI is valid before > >> unmap it. > >> > >> Signed-off-by: Kunkun Jiang <jiangkunkun@huawei.com> > >> --- > >> arch/arm64/kvm/vgic/vgic-v4.c | 5 +++++ > >> 1 file changed, 5 insertions(+) > >> > >> diff --git a/arch/arm64/kvm/vgic/vgic-v4.c b/arch/arm64/kvm/vgic/vgic-v4.c > >> index 339a55194b2c..824f4baf50ee 100644 > >> --- a/arch/arm64/kvm/vgic/vgic-v4.c > >> +++ b/arch/arm64/kvm/vgic/vgic-v4.c > >> @@ -436,6 +436,11 @@ int kvm_vgic_v4_set_forwarding(struct kvm *kvm, int virq, > >> if (ret) > >> goto out; > >> + if (irq->hw) { > >> + ret = -EBUSY; > >> + goto out; > >> + } > >> + > > So this code affects the mapping side, not the unmapping. Even more > > confusingly, the subject of the patch doesn't match the commit > > message. > > > > Furthermore, I'm not sure we want to return an error here. Userspace > > has no knowledge of GICv4, and there are numerous other cases where we > > don't return any error. This is certainly a userspace visible change. > Yes, Userspace has no knowledge of GICv4. In the current implementation, > this error will not be presented to userspace. Only a log will be printed > in kvm_irqfd_assign(). > > #ifdef CONFIG_HAVE_KVM_IRQ_BYPASS > > if (kvm_arch_has_irq_bypass()) { > > irqfd->consumer.token = (void *)irqfd->eventfd; > > irqfd->consumer.add_producer = > kvm_arch_irq_bypass_add_producer; > > irqfd->consumer.del_producer = > kvm_arch_irq_bypass_del_producer; > > irqfd->consumer.stop = kvm_arch_irq_bypass_stop; > > irqfd->consumer.start = kvm_arch_irq_bypass_start; > > ret = irq_bypass_register_consumer(&irqfd->consumer); > > if (ret) > > pr_info("irq bypass consumer (token %p) > registration fails: %d\n", > > irqfd->consumer.token, ret); > > } > > #endif > > If you think it is better not to return an error, I will modify it in > the next version. I don't think returning an error is worth it, given that it is not actionable from userspace. The pr_info() itself is ugly and should probably be at least ratelimited, but that's for another patch. Please resend this with a 0 return value, a fixed commit message, and a Fixes: tag. Thanks, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-11-17 11:48 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-11-16 12:52 [PATCH] KVM: arm/arm64: GICv4: Do not perform an map to a mapped vLPI Kunkun Jiang 2023-11-16 14:10 ` Marc Zyngier 2023-11-17 9:54 ` Kunkun Jiang 2023-11-17 11:47 ` Marc Zyngier
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).