netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] KVM: not register a IRQ bypass producer if unsupported or disabled
@ 2020-10-19  9:06 Zhenzhong Duan
  2020-10-19  9:06 ` [PATCH 2/2] KVM: not link irqfd with a fake IRQ bypass producer Zhenzhong Duan
  2020-10-20  6:23 ` [PATCH 1/2] KVM: not register a IRQ bypass producer if unsupported or disabled Jason Wang
  0 siblings, 2 replies; 6+ messages in thread
From: Zhenzhong Duan @ 2020-10-19  9:06 UTC (permalink / raw)
  To: linux-kernel, virtualization, kvm
  Cc: netdev, pbonzini, sean.j.christopherson, vkuznets, wanpengli,
	jmattson, joro, tglx, mingo, bp, mst, jasowang, Zhenzhong Duan

If Post interrupt is disabled due to hardware limit or forcely disabled
by "intremap=nopost" parameter, return -EINVAL so that the legacy mode IRQ
isn't registered as IRQ bypass producer.

With this change, below message is printed:
"vfio-pci 0000:db:00.0: irq bypass producer (token 0000000060c8cda5) registration fails: -22"

..which also hints us if a vfio or vdpa device works in PI mode or legacy
remapping mode.

Add a print to vdpa code just like what vfio_msi_set_vector_signal() does.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@gmail.com>
---
 arch/x86/kvm/svm/avic.c | 3 +--
 arch/x86/kvm/vmx/vmx.c  | 5 ++---
 drivers/vhost/vdpa.c    | 5 +++++
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
index ac830cd..316142a 100644
--- a/arch/x86/kvm/svm/avic.c
+++ b/arch/x86/kvm/svm/avic.c
@@ -814,7 +814,7 @@ int svm_update_pi_irte(struct kvm *kvm, unsigned int host_irq,
 
 	if (!kvm_arch_has_assigned_device(kvm) ||
 	    !irq_remapping_cap(IRQ_POSTING_CAP))
-		return 0;
+		return ret;
 
 	pr_debug("SVM: %s: host_irq=%#x, guest_irq=%#x, set=%#x\n",
 		 __func__, host_irq, guest_irq, set);
@@ -899,7 +899,6 @@ int svm_update_pi_irte(struct kvm *kvm, unsigned int host_irq,
 		}
 	}
 
-	ret = 0;
 out:
 	srcu_read_unlock(&kvm->irq_srcu, idx);
 	return ret;
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index f0a9954..1fed6d6 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7716,12 +7716,12 @@ static int vmx_update_pi_irte(struct kvm *kvm, unsigned int host_irq,
 	struct kvm_lapic_irq irq;
 	struct kvm_vcpu *vcpu;
 	struct vcpu_data vcpu_info;
-	int idx, ret = 0;
+	int idx, ret = -EINVAL;
 
 	if (!kvm_arch_has_assigned_device(kvm) ||
 		!irq_remapping_cap(IRQ_POSTING_CAP) ||
 		!kvm_vcpu_apicv_active(kvm->vcpus[0]))
-		return 0;
+		return ret;
 
 	idx = srcu_read_lock(&kvm->irq_srcu);
 	irq_rt = srcu_dereference(kvm->irq_routing, &kvm->irq_srcu);
@@ -7787,7 +7787,6 @@ static int vmx_update_pi_irte(struct kvm *kvm, unsigned int host_irq,
 		}
 	}
 
-	ret = 0;
 out:
 	srcu_read_unlock(&kvm->irq_srcu, idx);
 	return ret;
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 62a9bb0..b20060a 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -107,6 +107,11 @@ static void vhost_vdpa_setup_vq_irq(struct vhost_vdpa *v, u16 qid)
 	vq->call_ctx.producer.token = vq->call_ctx.ctx;
 	vq->call_ctx.producer.irq = irq;
 	ret = irq_bypass_register_producer(&vq->call_ctx.producer);
+	if (unlikely(ret))
+		dev_info(&vdpa->dev,
+		"irq bypass producer (token %p) registration fails: %d\n",
+		vq->call_ctx.producer.token, ret);
+
 	spin_unlock(&vq->call_ctx.ctx_lock);
 }
 
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 2/2] KVM: not link irqfd with a fake IRQ bypass producer
  2020-10-19  9:06 [PATCH 1/2] KVM: not register a IRQ bypass producer if unsupported or disabled Zhenzhong Duan
@ 2020-10-19  9:06 ` Zhenzhong Duan
  2020-10-20  6:32   ` Jason Wang
  2020-10-20  6:23 ` [PATCH 1/2] KVM: not register a IRQ bypass producer if unsupported or disabled Jason Wang
  1 sibling, 1 reply; 6+ messages in thread
From: Zhenzhong Duan @ 2020-10-19  9:06 UTC (permalink / raw)
  To: linux-kernel, virtualization, kvm
  Cc: netdev, pbonzini, sean.j.christopherson, vkuznets, wanpengli,
	jmattson, joro, tglx, mingo, bp, mst, jasowang, Zhenzhong Duan

In case failure to setup Post interrupt for an IRQ, it make no sense
to assign irqfd->producer to the producer.

This change makes code more robust.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@gmail.com>
---
 arch/x86/kvm/x86.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ce856e0..277e961 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -10683,13 +10683,14 @@ int kvm_arch_irq_bypass_add_producer(struct irq_bypass_consumer *cons,
 		container_of(cons, struct kvm_kernel_irqfd, consumer);
 	int ret;
 
-	irqfd->producer = prod;
 	kvm_arch_start_assignment(irqfd->kvm);
 	ret = kvm_x86_ops.update_pi_irte(irqfd->kvm,
 					 prod->irq, irqfd->gsi, 1);
 
 	if (ret)
 		kvm_arch_end_assignment(irqfd->kvm);
+	else
+		irqfd->producer = prod;
 
 	return ret;
 }
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] KVM: not register a IRQ bypass producer if unsupported or disabled
  2020-10-19  9:06 [PATCH 1/2] KVM: not register a IRQ bypass producer if unsupported or disabled Zhenzhong Duan
  2020-10-19  9:06 ` [PATCH 2/2] KVM: not link irqfd with a fake IRQ bypass producer Zhenzhong Duan
@ 2020-10-20  6:23 ` Jason Wang
  2020-10-20 10:05   ` Zhenzhong Duan
  1 sibling, 1 reply; 6+ messages in thread
From: Jason Wang @ 2020-10-20  6:23 UTC (permalink / raw)
  To: Zhenzhong Duan, linux-kernel, virtualization, kvm
  Cc: netdev, pbonzini, sean.j.christopherson, vkuznets, wanpengli,
	jmattson, joro, tglx, mingo, bp, mst


On 2020/10/19 下午5:06, Zhenzhong Duan wrote:
> If Post interrupt is disabled due to hardware limit or forcely disabled
> by "intremap=nopost" parameter, return -EINVAL so that the legacy mode IRQ
> isn't registered as IRQ bypass producer.


Is there any side effect if it was still registered?


>
> With this change, below message is printed:
> "vfio-pci 0000:db:00.0: irq bypass producer (token 0000000060c8cda5) registration fails: -22"


I may miss something, but the patch only touches vhost-vDPA instead of VFIO?

Thanks


>
> ..which also hints us if a vfio or vdpa device works in PI mode or legacy
> remapping mode.
>
> Add a print to vdpa code just like what vfio_msi_set_vector_signal() does.
>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@gmail.com>
> ---
>   arch/x86/kvm/svm/avic.c | 3 +--
>   arch/x86/kvm/vmx/vmx.c  | 5 ++---
>   drivers/vhost/vdpa.c    | 5 +++++
>   3 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> index ac830cd..316142a 100644
> --- a/arch/x86/kvm/svm/avic.c
> +++ b/arch/x86/kvm/svm/avic.c
> @@ -814,7 +814,7 @@ int svm_update_pi_irte(struct kvm *kvm, unsigned int host_irq,
>   
>   	if (!kvm_arch_has_assigned_device(kvm) ||
>   	    !irq_remapping_cap(IRQ_POSTING_CAP))
> -		return 0;
> +		return ret;
>   
>   	pr_debug("SVM: %s: host_irq=%#x, guest_irq=%#x, set=%#x\n",
>   		 __func__, host_irq, guest_irq, set);
> @@ -899,7 +899,6 @@ int svm_update_pi_irte(struct kvm *kvm, unsigned int host_irq,
>   		}
>   	}
>   
> -	ret = 0;
>   out:
>   	srcu_read_unlock(&kvm->irq_srcu, idx);
>   	return ret;
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index f0a9954..1fed6d6 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -7716,12 +7716,12 @@ static int vmx_update_pi_irte(struct kvm *kvm, unsigned int host_irq,
>   	struct kvm_lapic_irq irq;
>   	struct kvm_vcpu *vcpu;
>   	struct vcpu_data vcpu_info;
> -	int idx, ret = 0;
> +	int idx, ret = -EINVAL;
>   
>   	if (!kvm_arch_has_assigned_device(kvm) ||
>   		!irq_remapping_cap(IRQ_POSTING_CAP) ||
>   		!kvm_vcpu_apicv_active(kvm->vcpus[0]))
> -		return 0;
> +		return ret;
>   
>   	idx = srcu_read_lock(&kvm->irq_srcu);
>   	irq_rt = srcu_dereference(kvm->irq_routing, &kvm->irq_srcu);
> @@ -7787,7 +7787,6 @@ static int vmx_update_pi_irte(struct kvm *kvm, unsigned int host_irq,
>   		}
>   	}
>   
> -	ret = 0;
>   out:
>   	srcu_read_unlock(&kvm->irq_srcu, idx);
>   	return ret;
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index 62a9bb0..b20060a 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -107,6 +107,11 @@ static void vhost_vdpa_setup_vq_irq(struct vhost_vdpa *v, u16 qid)
>   	vq->call_ctx.producer.token = vq->call_ctx.ctx;
>   	vq->call_ctx.producer.irq = irq;
>   	ret = irq_bypass_register_producer(&vq->call_ctx.producer);
> +	if (unlikely(ret))
> +		dev_info(&vdpa->dev,
> +		"irq bypass producer (token %p) registration fails: %d\n",
> +		vq->call_ctx.producer.token, ret);
> +
>   	spin_unlock(&vq->call_ctx.ctx_lock);
>   }
>   


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] KVM: not link irqfd with a fake IRQ bypass producer
  2020-10-19  9:06 ` [PATCH 2/2] KVM: not link irqfd with a fake IRQ bypass producer Zhenzhong Duan
@ 2020-10-20  6:32   ` Jason Wang
  2020-10-20 10:12     ` Zhenzhong Duan
  0 siblings, 1 reply; 6+ messages in thread
From: Jason Wang @ 2020-10-20  6:32 UTC (permalink / raw)
  To: Zhenzhong Duan, linux-kernel, virtualization, kvm
  Cc: netdev, pbonzini, sean.j.christopherson, vkuznets, wanpengli,
	jmattson, joro, tglx, mingo, bp, mst


On 2020/10/19 下午5:06, Zhenzhong Duan wrote:
> In case failure to setup Post interrupt for an IRQ, it make no sense
> to assign irqfd->producer to the producer.
>
> This change makes code more robust.


It's better to describe what issue we will get without this patch.

Thanks


>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@gmail.com>
> ---
>   arch/x86/kvm/x86.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index ce856e0..277e961 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -10683,13 +10683,14 @@ int kvm_arch_irq_bypass_add_producer(struct irq_bypass_consumer *cons,
>   		container_of(cons, struct kvm_kernel_irqfd, consumer);
>   	int ret;
>   
> -	irqfd->producer = prod;
>   	kvm_arch_start_assignment(irqfd->kvm);
>   	ret = kvm_x86_ops.update_pi_irte(irqfd->kvm,
>   					 prod->irq, irqfd->gsi, 1);
>   
>   	if (ret)
>   		kvm_arch_end_assignment(irqfd->kvm);
> +	else
> +		irqfd->producer = prod;
>   
>   	return ret;
>   }


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] KVM: not register a IRQ bypass producer if unsupported or disabled
  2020-10-20  6:23 ` [PATCH 1/2] KVM: not register a IRQ bypass producer if unsupported or disabled Jason Wang
@ 2020-10-20 10:05   ` Zhenzhong Duan
  0 siblings, 0 replies; 6+ messages in thread
From: Zhenzhong Duan @ 2020-10-20 10:05 UTC (permalink / raw)
  To: Jason Wang
  Cc: linux-kernel, virtualization, kvm, netdev, pbonzini,
	sean.j.christopherson, vkuznets, wanpengli, jmattson,
	Joerg Roedel, Thomas Gleixner, Ingo Molnar, Borislav Petkov, mst

On Tue, Oct 20, 2020 at 2:23 PM Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 2020/10/19 下午5:06, Zhenzhong Duan wrote:
> > If Post interrupt is disabled due to hardware limit or forcely disabled
> > by "intremap=nopost" parameter, return -EINVAL so that the legacy mode IRQ
> > isn't registered as IRQ bypass producer.
>
>
> Is there any side effect if it was still registered?

Not much side effect in theory, just some legacy mode IRQs in producer
list and it's not easy to distinguish them with PI interrupt mode IRQ.
The main purpose of this patch is to provide a way for people to know
if a device IRQ is really offloaded from kernel by a print.
>
>
> >
> > With this change, below message is printed:
> > "vfio-pci 0000:db:00.0: irq bypass producer (token 0000000060c8cda5) registration fails: -22"
>
>
> I may miss something, but the patch only touches vhost-vDPA instead of VFIO?

VFIO already has above print in vfio_msi_set_vector_signal() but vhost-vDPA not.

Regards
Zhenzhong

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] KVM: not link irqfd with a fake IRQ bypass producer
  2020-10-20  6:32   ` Jason Wang
@ 2020-10-20 10:12     ` Zhenzhong Duan
  0 siblings, 0 replies; 6+ messages in thread
From: Zhenzhong Duan @ 2020-10-20 10:12 UTC (permalink / raw)
  To: Jason Wang
  Cc: linux-kernel, virtualization, kvm, netdev, pbonzini,
	sean.j.christopherson, vkuznets, wanpengli, jmattson,
	Joerg Roedel, Thomas Gleixner, Ingo Molnar, Borislav Petkov, mst

On Tue, Oct 20, 2020 at 2:32 PM Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 2020/10/19 下午5:06, Zhenzhong Duan wrote:
> > In case failure to setup Post interrupt for an IRQ, it make no sense
> > to assign irqfd->producer to the producer.
> >
> > This change makes code more robust.
>
>
> It's better to describe what issue we will get without this patch.

I didn't see an issue without this patch.

Regards
Zhenzhong

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-10-20 10:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-19  9:06 [PATCH 1/2] KVM: not register a IRQ bypass producer if unsupported or disabled Zhenzhong Duan
2020-10-19  9:06 ` [PATCH 2/2] KVM: not link irqfd with a fake IRQ bypass producer Zhenzhong Duan
2020-10-20  6:32   ` Jason Wang
2020-10-20 10:12     ` Zhenzhong Duan
2020-10-20  6:23 ` [PATCH 1/2] KVM: not register a IRQ bypass producer if unsupported or disabled Jason Wang
2020-10-20 10:05   ` Zhenzhong Duan

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).