KVM: not register a IRQ bypass producer if unsupported or disabled

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 <redacted>
---
 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

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 <redacted>
---
 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

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

37 more quoted lines

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

On Tue, Oct 20, 2020 at 2:23 PM Jason Wang [off-list ref] wrote:

On 2020/10/19 下午5:06, Zhenzhong Duan wrote:

quoted

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.

quoted

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

On Tue, Oct 20, 2020 at 2:32 PM Jason Wang [off-list ref] wrote:

On 2020/10/19 下午5:06, Zhenzhong Duan wrote:

quoted

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