On Fri, Dec 02, 2011 at 04:34:21PM -0700, Cam Macdonell wrote: > Based on a git bisect, this patch breaks msi-x interrupt delivery in > the ivshmem device. > > On Mon, Nov 21, 2011 at 9:57 AM, Michael S. Tsirkin <m...@redhat.com> wrote: > > Only go over the table when function is masked. > > This is not really important for qemu.git but helps > > fix a bug in qemu-kvm.git. > > > > Signed-off-by: Michael S. Tsirkin <m...@redhat.com> > > --- > > hw/msix.c | 21 ++++++++++++++------- > > hw/pci.h | 2 ++ > > 2 files changed, 16 insertions(+), 7 deletions(-) > > > > diff --git a/hw/msix.c b/hw/msix.c > > index b15bafc..63b41b9 100644 > > --- a/hw/msix.c > > +++ b/hw/msix.c > > @@ -79,6 +79,7 @@ static int msix_add_config(struct PCIDevice *pdev, > > unsigned short nentries, > > /* Make flags bit writable. */ > > pdev->wmask[config_offset + MSIX_CONTROL_OFFSET] |= MSIX_ENABLE_MASK | > > MSIX_MASKALL_MASK; > > + pdev->msix_function_masked = true; > > return 0; > > iiuc, this masks the msix by default.
Yes, because msi-x is disabled by default, that's in the pci spec. > > } > > > > @@ -117,16 +118,11 @@ static void msix_clr_pending(PCIDevice *dev, int > > vector) > > *msix_pending_byte(dev, vector) &= ~msix_pending_mask(vector); > > } > > > > -static int msix_function_masked(PCIDevice *dev) > > -{ > > - return dev->config[dev->msix_cap + MSIX_CONTROL_OFFSET] & > > MSIX_MASKALL_MASK; > > -} > > - > > static int msix_is_masked(PCIDevice *dev, int vector) > > { > > unsigned offset = > > vector * PCI_MSIX_ENTRY_SIZE + PCI_MSIX_ENTRY_VECTOR_CTRL; > > - return msix_function_masked(dev) || > > + return dev->msix_function_masked || > > dev->msix_table_page[offset] & PCI_MSIX_ENTRY_CTRL_MASKBIT; > > } > > > > @@ -138,24 +134,34 @@ static void msix_handle_mask_update(PCIDevice *dev, > > int vector) > > } > > } > > > > +static void msix_update_function_masked(PCIDevice *dev) > > +{ > > + dev->msix_function_masked = !msix_enabled(dev) || > > + (dev->config[dev->msix_cap + MSIX_CONTROL_OFFSET] & > > MSIX_MASKALL_MASK); > > +} > > + > > /* Handle MSI-X capability config write. */ > > void msix_write_config(PCIDevice *dev, uint32_t addr, > > uint32_t val, int len) > > { > > unsigned enable_pos = dev->msix_cap + MSIX_CONTROL_OFFSET; > > int vector; > > + bool was_masked; > > > > if (!range_covers_byte(addr, len, enable_pos)) { > > return; > > } > > > > + was_masked = dev->msix_function_masked; > > + msix_update_function_masked(dev); > > + > > if (!msix_enabled(dev)) { > > return; > > } > > > > pci_device_deassert_intx(dev); > > > > - if (msix_function_masked(dev)) { > > + if (dev->msix_function_masked == was_masked) { > > return; > > } > > So I believe my bug is due to the fact the new logic included in this > patch requires msix_write_config() to be called to unmask the vectors. Not exactly, to enable msi-x really. > Virtio-pci calls msix_write_config(), but ivshmem does not (nor does > PCIe so I'm not sure if it's also affected). At this point PCIe is a stub. > I haven't been able to fix the bug yet, but I wanted to make sure I > was looking in the correct place. Any help of further explanation of > this patch would be greatly appreciated. > > Sincerely, > Cam So I think you just need to call msix_write_config, otherwise msix is not getting enabled. BTW looking at the ivshmem code, this bit looks wrong: pci_conf[PCI_COMMAND] = PCI_COMMAND_IO | PCI_COMMAND_MEMORY; I think the spec says IO/MEMORY must be disabled at init time since BARs are not yet set to anything reasonable. > > > > @@ -300,6 +306,7 @@ void msix_load(PCIDevice *dev, QEMUFile *f) > > msix_free_irq_entries(dev); > > qemu_get_buffer(f, dev->msix_table_page, n * PCI_MSIX_ENTRY_SIZE); > > qemu_get_buffer(f, dev->msix_table_page + MSIX_PAGE_PENDING, (n + 7) / > > 8); > > + msix_update_function_masked(dev); > > } > > > > /* Does device support MSI-X? */ > > diff --git a/hw/pci.h b/hw/pci.h > > index 4b2e785..625e717 100644 > > --- a/hw/pci.h > > +++ b/hw/pci.h > > @@ -178,6 +178,8 @@ struct PCIDevice { > > unsigned *msix_entry_used; > > /* Region including the MSI-X table */ > > uint32_t msix_bar_size; > > + /* MSIX function mask set or MSIX disabled */ > > + bool msix_function_masked; > > /* Version id needed for VMState */ > > int32_t version_id; > > > > -- > > 1.7.5.53.gc233e > > > >