Re: [PATCH 1/2] msix: Allow msix_init on a device with existing MSI-X capability

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



 On 10/23/2010 06:55 PM, Alex Williamson wrote:
On Sat, 2010-10-23 at 18:18 +0200, Michael S. Tsirkin wrote:
>  On Fri, Oct 22, 2010 at 02:40:31PM -0600, Alex Williamson wrote:
>  >  To enable common msix support to be used with pass through devices,
>  >  don't attempt to change the BAR if the device already has an
>  >  MSI-X capability.  This also means we want to pay closer attention
>  >  to the size when we map the msix table page, as it isn't necessarily
>  >  covering the entire end of the BAR.
>  >
>  >  Signed-off-by: Alex Williamson<alex.williamson@xxxxxxxxxx>
>  >  ---
>  >
>  >   hw/msix.c |   67 +++++++++++++++++++++++++++++++++++--------------------------
>  >   1 files changed, 38 insertions(+), 29 deletions(-)
>  >
>  >  diff --git a/hw/msix.c b/hw/msix.c
>  >  index 43efbd2..4122395 100644
>  >  --- a/hw/msix.c
>  >  +++ b/hw/msix.c
>  >  @@ -167,35 +167,43 @@ static int msix_add_config(struct PCIDevice *pdev, unsigned short nentries,
>  >   {
>  >       int config_offset;
>  >       uint8_t *config;
>  >  -    uint32_t new_size;
>  >
>  >  -    if (nentries<  1 || nentries>  PCI_MSIX_FLAGS_QSIZE + 1)
>  >  -        return -EINVAL;
>  >  -    if (bar_size>  0x80000000)
>  >  -        return -ENOSPC;
>  >  -
>  >  -    /* Add space for MSI-X structures */
>  >  -    if (!bar_size) {
>  >  -        new_size = MSIX_PAGE_SIZE;
>  >  -    } else if (bar_size<  MSIX_PAGE_SIZE) {
>  >  -        bar_size = MSIX_PAGE_SIZE;
>  >  -        new_size = MSIX_PAGE_SIZE * 2;
>  >  -    } else {
>  >  -        new_size = bar_size * 2;
>  >  -    }
>  >  -
>  >  -    pdev->msix_bar_size = new_size;
>  >  -    config_offset = pci_add_capability(pdev, PCI_CAP_ID_MSIX, MSIX_CAP_LENGTH);
>  >  -    if (config_offset<  0)
>  >  -        return config_offset;
>  >  -    config = pdev->config + config_offset;
>  >  -
>  >  -    pci_set_word(config + PCI_MSIX_FLAGS, nentries - 1);
>  >  -    /* Table on top of BAR */
>  >  -    pci_set_long(config + MSIX_TABLE_OFFSET, bar_size | bar_nr);
>  >  -    /* Pending bits on top of that */
>  >  -    pci_set_long(config + MSIX_PBA_OFFSET, (bar_size + MSIX_PAGE_PENDING) |
>  >  -                 bar_nr);
>  >  +    pdev->msix_bar_size = bar_size;
>  >  +
>  >  +    config_offset = pci_find_capability(pdev, PCI_CAP_ID_MSIX);
>  >  +
>  >  +    if (!config_offset) {
>  >  +        uint32_t new_size;
>  >  +
>  >  +        if (nentries<  1 || nentries>  PCI_MSIX_FLAGS_QSIZE + 1)
>  >  +            return -EINVAL;
>  >  +        if (bar_size>  0x80000000)
>  >  +            return -ENOSPC;
>  >  +
>  >  +        /* Add space for MSI-X structures */
>  >  +        if (!bar_size) {
>  >  +            new_size = MSIX_PAGE_SIZE;
>  >  +        } else if (bar_size<  MSIX_PAGE_SIZE) {
>  >  +            bar_size = MSIX_PAGE_SIZE;
>  >  +            new_size = MSIX_PAGE_SIZE * 2;
>  >  +        } else {
>  >  +            new_size = bar_size * 2;
>  >  +        }
>  >  +
>  >  +        pdev->msix_bar_size = new_size;
>  >  +        config_offset = pci_add_capability(pdev, PCI_CAP_ID_MSIX,
>  >  +                                           MSIX_CAP_LENGTH);
>  >  +        if (config_offset<  0)
>  >  +            return config_offset;
>  >  +        config = pdev->config + config_offset;
>  >  +
>  >  +        pci_set_word(config + PCI_MSIX_FLAGS, nentries - 1);
>  >  +        /* Table on top of BAR */
>  >  +        pci_set_long(config + MSIX_TABLE_OFFSET, bar_size | bar_nr);
>  >  +        /* Pending bits on top of that */
>  >  +        pci_set_long(config + MSIX_PBA_OFFSET, (bar_size + MSIX_PAGE_PENDING) |
>  >  +                     bar_nr);
>  >  +    }
>  >       pdev->msix_cap = config_offset;
>  >       /* Make flags bit writeable. */
>  >       pdev->wmask[config_offset + MSIX_CONTROL_OFFSET] |= MSIX_ENABLE_MASK |
>  >  @@ -337,7 +345,8 @@ void msix_mmio_map(PCIDevice *d, int region_num,
>  >           return;
>  >       if (size<= offset)
>  >           return;
>  >  -    cpu_register_physical_memory(addr + offset, size - offset,
>  >  +    cpu_register_physical_memory(addr + offset,
>  >  +                                 MIN(size - offset, MSIX_PAGE_SIZE),
>
>  This is wrong I think, the table might not fit in a single page.
>  You would need to read table size out of from device config.

That's true, but I was hoping to save that for later since we don't seem
to be running into that problem yet.  Current device assignment code
assumes a single page, and I haven't heard of anyone with a vector table
that exceeds that yet.  Thanks,


Ok; applied. Please add some warning if the condition happens so the breakage is at least not silent.

--
error compiling committee.c: too many arguments to function

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux