Skip to contentSkip to conversation

KVM: x86: enable event window in inject_pending_event

bug report Dan Carpenter 2 participants 2 messages Jun 3, 2020 KVM

lore
Share & tools
lore
#0 lore raw

Hello Paolo Bonzini,

The patch c9d40913ac5a: "KVM: x86: enable event window in inject_pending_event" from May 22, 2020, leads to the following static checker warning:

	arch/x86/kvm/x86.c:10530 kvm_can_do_async_pf()
	warn: signedness bug returning '(-16)'

arch/x86/kvm/x86.c
10516 bool kvm_can_do_async_pf(struct kvm_vcpu *vcpu) 10517 {
10518 if (unlikely(!lapic_in_kernel(vcpu) ||
10519 kvm_event_needs_reinjection(vcpu) ||
10520 vcpu->arch.exception.pending))
10521 return false;
10522
10523 if (kvm_hlt_in_guest(vcpu->kvm) && !kvm_can_deliver_async_pf(vcpu))
10524 return false;
10525
10526 /*

 10527           * If interrupts are off we cannot even use an artificial
 10528           * halt state.
 10529           */

10530 return kvm_arch_interrupt_allowed(vcpu);
10531 }

The svm_nmi_allowed() used to return false because interrupts aren't allowed but now it returns -EBUSY so it returns true/allowed.

regards,
dan carpenter

maintainer
#1 lore raw
Quoted 21 lines · Dan Carpenter

Hello Paolo Bonzini,

The patch c9d40913ac5a: "KVM: x86: enable event window in inject_pending_event" from May 22, 2020, leads to the following static checker warning:

	arch/x86/kvm/x86.c:10530 kvm_can_do_async_pf()
	warn: signedness bug returning '(-16)'

arch/x86/kvm/x86.c
10516 bool kvm_can_do_async_pf(struct kvm_vcpu *vcpu) 10517 {
10518 if (unlikely(!lapic_in_kernel(vcpu) ||
10519 kvm_event_needs_reinjection(vcpu) ||
10520 vcpu->arch.exception.pending))
10521 return false;
10522
10523 if (kvm_hlt_in_guest(vcpu->kvm) && !kvm_can_deliver_async_pf(vcpu))
10524 return false;
10525
10526 /*

Quoted 8 lines
 10527           * If interrupts are off we cannot even use an artificial
 10528           * halt state.
 10529           */

10530 return kvm_arch_interrupt_allowed(vcpu);
10531 }

The svm_nmi_allowed() used to return false because interrupts aren't allowed but now it returns -EBUSY so it returns true/allowed.

This is intentional (i.e. not a bug) but it should have an explicit "!= 0".

Paolo