2020-10-19 10:45:50

by Zhenzhong Duan

[permalink] [raw]
Subject: [PATCH 1/2] 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 <[email protected]>
---
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


2020-10-20 09:42:30

by Jason Wang

[permalink] [raw]
Subject: Re: [PATCH 1/2] KVM: not register a IRQ bypass producer if unsupported or disabled


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 <[email protected]>
> ---
> 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);
> }
>

2020-10-20 11:29:15

by Zhenzhong Duan

[permalink] [raw]
Subject: Re: [PATCH 1/2] KVM: not register a IRQ bypass producer if unsupported or disabled

On Tue, Oct 20, 2020 at 2:23 PM Jason Wang <[email protected]> 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