Re: [Xen-devel] [PATCH 1/2] x86/vMSI-X: honor all mask requests

["Wu, Feng" <feng.wu@xxxxxxxxx>]

> -----Original Message-----
> From: Wu, Feng
> Sent: Wednesday, March 25, 2015 1:41 PM
> To: Jan Beulich; xen-devel
> Cc: Andrew Cooper; Keir Fraser; Wu, Feng
> Subject: RE: [Xen-devel] [PATCH 1/2] x86/vMSI-X: honor all mask requests
> 
> 
> 
> > -----Original Message-----
> > From: xen-devel-bounces@xxxxxxxxxxxxx
> > [mailto:xen-devel-bounces@xxxxxxxxxxxxx] On Behalf Of Jan Beulich
> > Sent: Saturday, March 21, 2015 12:27 AM
> > To: xen-devel
> > Cc: Andrew Cooper; Keir Fraser
> > Subject: [Xen-devel] [PATCH 1/2] x86/vMSI-X: honor all mask requests
> >
> > Commit 74fd0036de ("x86: properly handle MSI-X unmask operation from
> > guests") didn't go far enough: it fixed an issue with unmasking, but
> > left an issue with masking in place: Due to the (late) point in time
> > when qemu requests the hypervisor to set up MSI-X interrupts (which is
> > where the MMIO intercept gets put in place), the hypervisor doesn't
> > see all guest writes, and hence shouldn't make assumptions on the state
> > the virtual MSI-X resources are in. Bypassing the rest of the logic on
> > a guest mask operation leads to
> >
> > [00:04.0] pci_msix_write: Error: Can't update msix entry 1 since MSI-X is
> > already enabled.
> >
> > which surprisingly enough doesn't lead to the device not working
> > anymore (I didn't dig in deep enough to figure out why that is). But it
> > does prevent the IRQ to be migrated inside the guest, i.e. all
> > interrupts will always arrive in vCPU 0.
> >
> > Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> >
> > --- a/xen/arch/x86/hvm/vmsi.c
> > +++ b/xen/arch/x86/hvm/vmsi.c
> > @@ -286,11 +286,11 @@ static int msixtbl_write(struct vcpu *v,
> >          goto out;
> >      }
> >
> > -    /* exit to device model if address/data has been modified */
> > -    if ( test_and_clear_bit(nr_entry, &entry->table_flags) )
> > +    /* Exit to device model when unmasking and address/data got
> modified.
> > */
> > +    if ( !(val & PCI_MSIX_VECTOR_BITMASK) &&
> > +         test_and_clear_bit(nr_entry, &entry->table_flags) )
> 
> Is it more accurate as the follows?
> 
> If ( (offset == PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET) &&
>    !(val & PCI_MSIX_VECTOR_BITMASK) &&
>    test_and_clear_bit(nr_entry, &entry->table_flags) )

Oh, just found "offset == PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET" is
not needed here, it guarantees this when we get here.

Thanks,
Feng

> 
> BTW, how can I reproduce this issue?
> 
> Thanks,
> Feng
> 
> >      {
> > -        if ( !(val & PCI_MSIX_VECTOR_BITMASK) )
> > -            v->arch.hvm_vcpu.hvm_io.msix_unmask_address = address;
> > +        v->arch.hvm_vcpu.hvm_io.msix_unmask_address = address;
> >          goto out;
> >      }
> >
> >
> >


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel