Re: [PATCH 13/16] kvm: enable MSI-X capabilty for assigned device

[Sheng Yang <sheng@linux.intel.com>]

On Thursday 09 April 2009 00:13:56 Alex Williamson wrote:
> Hi Sheng,
>
> On Wed, 2009-04-08 at 10:26 +0800, Sheng Yang wrote:
> > On Wednesday 08 April 2009 00:38:10 Alex Williamson wrote:
> > > On Tue, 2009-04-07 at 14:09 +0800, Sheng Yang wrote:
> > > > Could you enable DEVICE_ASSSIGNMENT_DEBUG=1 in
> > > > qemu/hw/device-assignment.c and post the output?
> > >
> > > Yup, see below.  The error comes after I 'ifdown eth0; ifup eth0' in
> > > the guest.  Note bnx2 appears to only turn on MSIX for SMP systems. 
> > > Thanks,
> > >
> > > Alex
> >
> > Seems your "ifdown/ifup" script reload the module?
>
> No, the bnx2 module isn't unloaded on ifdown.
>
> >  Oh god, I found one bug
> > after checked the spec:
> >
> > System software reads this field to determine the MSI-X Table Size *N*,
> > which is encoded as *N-1*. For example, a returned value of “00000000011”
> > indicates a table size of 4.
> >
> > But it seems still can't explain the problem...(OK, it may affect the
> > guest in a unknown way as well...) I would post a fix for it soon.
>
> [snip]
>
> > The writing to MMIO have been intercepted, but code fail to count it?
> > Strange...
> >
> > Could you try this debug?
>
> I added the debug printfs, plus the MSI-X table size patch, and printed
> the value of msg_ctrl as we loop through.  Output below.  This is what
> made me think the MSI-X state isn't getting cleared when the driver
> closes the interface.  Let me know what you think.  Thanks,
>

Thanks Alex, now I know where the problem is. Part of functional haven't been 
implemented...

> address=0052 val=0x00008008 len=2 the MSIX capabilty position is 0x50
> the MSIX entries_max_nr is 0x9
> 0: msg_ctrl: 0001
> 1: msg_ctrl: 0001
> 2: msg_ctrl: 0001
> 3: msg_ctrl: 0001
> 4: msg_ctrl: 0001
> 5: msg_ctrl: 0001
> 6: msg_ctrl: 0001
> 7: msg_ctrl: 0001
> 8: msg_ctrl: 0001
> MSI-X entry number is zero!
> assigned_dev_update_msix_mmio: No such device or address

Driver write to the vectors at first, then enable MSI-X,

> msix_mmio_writel: write to MSI-X entry table mmio offset 0xc, val 0x0
> msix_mmio_writel: write to MSI-X entry table mmio offset 0x1c, val 0x0
> msix_mmio_writel: write to MSI-X entry table mmio offset 0x2c, val 0x0
> msix_mmio_writel: write to MSI-X entry table mmio offset 0x3c, val 0x0
> msix_mmio_writel: write to MSI-X entry table mmio offset 0x4c, val 0x0
> msix_mmio_writel: write to MSI-X entry table mmio offset 0x5c, val 0x0
> msix_mmio_writel: write to MSI-X entry table mmio offset 0x6c, val 0x0
> msix_mmio_writel: write to MSI-X entry table mmio offset 0x7c, val 0x0

And finally clear the mask bit.

For current we didn't implement mask capability in MSI-X vectors, so it won't 
work...

OK. I'd like to remove the check of mask bit and only ignored unused vector 
when msg data is zero now(hope it won't cause more problems). And we would add 
support for per-vector mask later.

Thanks for help to debug!
-- 
regards
Yang, Sheng