Re: [PULL 57/62] hw/xen: Support MSI mapping to PIRQ

[Peter Maydell]

On Thu, 2 Mar 2023 at 12:37, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> From: David Woodhouse <dwmw@amazon.co.uk>
>
> The way that Xen handles MSI PIRQs is kind of awful.

> Now that this is working we can finally enable XENFEAT_hvm_pirqs and
> let the guest use it all.
>

Hi; Coverity points out a logic error in this code (CID 1507603):

> @@ -1638,6 +1877,7 @@ int xen_physdev_unmap_pirq(struct physdev_unmap_pirq 
> *unmap)
>
>      /* We can only unmap GSI PIRQs */
>      if (gsi < 0) {
> +        qemu_mutex_unlock(&s->port_lock);
>          return -EINVAL;
>      }

One of the things xen_physdev_unmap_pirq() does early is return
if gsi is a negative value...

> @@ -1646,6 +1886,12 @@ int xen_physdev_unmap_pirq(struct physdev_unmap_pirq 
> *unmap)
>      pirq_inuse_word(s, pirq) &= ~pirq_inuse_bit(pirq);
>
>      trace_kvm_xen_unmap_pirq(pirq, gsi);
> +    qemu_mutex_unlock(&s->port_lock);
> +
> +    if (gsi == IRQ_MSI_EMU) {

...but then later we try to test to see if it is IRQ_MSI_EMU.
IRQ_MSI_EMU is -3, so this condition can never be true.

> +        kvm_update_msi_routes_all(NULL, true, 0, 0);
> +    }

What was the intention here ?

> +
>      return 0;
>  }

thanks
-- PMM