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

Michael S. Tsirkin
2010-10-23T16:25:28Z

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@redhat.com>
> ---
>
> 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.

> d->msix_mmio_index);
> }
>