[bug report] KVM: x86: enable event window in inject_pending_event

[bug report] KVM: x86: enable event window in inject_pending_event

[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          /*
 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

Re: [bug report] KVM: x86: enable event window in inject_pending_event

[Paolo Bonzini]

On 03/06/20 12:34, Dan Carpenter wrote:
> 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.

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

Paolo