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

[Woodhouse, David]

On Thu, 2023-04-06 at 16:48 +0100, Peter Maydell wrote:
> 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 ?


Hrm.... the way that Xen automatically maps the MSI to a PIRQ by
snooping on the (masked) writes to the MSI target is awful, as noted.

I don't think Linux guests ever do unmap the MSI PIRQ but it might be
possible; I'll have to do some experiments in Xen to see what happens.

Thanks.

smime.p7s
Description: S/MIME cryptographic signature

Amazon Development Centre (London) Ltd.Registered in England and Wales with registration number 04543232 with its registered office at 1 Principal Place, Worship Street, London EC2A 2FA, United Kingdom.