* [PATCH] kvm/x86 : set meaningful return value
@ 2018-09-28 16:41 Peng Hao
2018-10-01 14:06 ` Paolo Bonzini
0 siblings, 1 reply; 2+ messages in thread
From: Peng Hao @ 2018-09-28 16:41 UTC (permalink / raw)
To: pbonzini, rkrcmar, tglx, mingo, hpa; +Cc: linux-kernel, kvm, x86, Peng Hao
From: Peng Hao <peng.hao2@zte.com.cn>
kvm_irq_delivery_to_apic_fast()-->
kvm_apic_map_get_dest_lapic()-->
kvm_apic_disabled_lapic_found()
kvm_apic_map_get_dest_lapic return with bitmap==0 and dst[i]==NULL,
then (*r == -1) will be returned to qemu and "KVM: injection failed,
MSI lost(Operation not permitted)" will be recorded in qemu log. The
output is puzzling.
Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
---
arch/x86/kvm/lapic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index fbb0e6d..a8896b3 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -944,7 +944,7 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
int i;
bool ret;
- *r = -1;
+ *r = -ENXIO;
if (irq->shorthand == APIC_DEST_SELF) {
*r = kvm_apic_set_irq(src->vcpu, irq, dest_map);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] kvm/x86 : set meaningful return value
2018-09-28 16:41 [PATCH] kvm/x86 : set meaningful return value Peng Hao
@ 2018-10-01 14:06 ` Paolo Bonzini
0 siblings, 0 replies; 2+ messages in thread
From: Paolo Bonzini @ 2018-10-01 14:06 UTC (permalink / raw)
To: Peng Hao, rkrcmar, tglx, mingo, hpa; +Cc: linux-kernel, kvm, x86, Peng Hao
On 28/09/2018 18:41, Peng Hao wrote:
> From: Peng Hao <peng.hao2@zte.com.cn>
>
> kvm_irq_delivery_to_apic_fast()-->
> kvm_apic_map_get_dest_lapic()-->
> kvm_apic_disabled_lapic_found()
> kvm_apic_map_get_dest_lapic return with bitmap==0 and dst[i]==NULL,
> then (*r == -1) will be returned to qemu and "KVM: injection failed,
> MSI lost(Operation not permitted)" will be recorded in qemu log. The
> output is puzzling.
This makes sense, but I am not sure that ENXIO is any better...
In fact, in this case I think it's okay to just return zero. What about
this fix:
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index b756f1237ce3..d02515b8018f 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -957,14 +957,14 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm
*kvm, struct kvm_lapic *src,
map = rcu_dereference(kvm->arch.apic_map);
ret = kvm_apic_map_get_dest_lapic(kvm, &src, irq, map, &dst, &bitmap);
- if (ret)
+ if (ret) {
+ *r = 0;
for_each_set_bit(i, &bitmap, 16) {
if (!dst[i])
continue;
- if (*r < 0)
- *r = 0;
*r += kvm_apic_set_irq(dst[i]->vcpu, irq, dest_map);
}
+ }
rcu_read_unlock();
return ret;
Paolo
> Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
> ---
> arch/x86/kvm/lapic.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index fbb0e6d..a8896b3 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -944,7 +944,7 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
> int i;
> bool ret;
>
> - *r = -1;
> + *r = -ENXIO;
>
> if (irq->shorthand == APIC_DEST_SELF) {
> *r = kvm_apic_set_irq(src->vcpu, irq, dest_map);
>
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-10-01 14:06 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-28 16:41 [PATCH] kvm/x86 : set meaningful return value Peng Hao
2018-10-01 14:06 ` Paolo Bonzini
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).