[RfC,03/15] virtio-pci: make pci bars configurable

Add msix_bar and modern_mem_bar fields to VirtIOPCIProxy.  They can be
used to configure which pci regions are used for the virtio 1.0 memory
bar and the msix bar.

For legacy/transitional devices the legacy bar is region 0 and the msix
bar is region 1.  Only the modern bar can be configured, and it must be
2 or larger.  Default is 2.

For legacy-free devices the modern bar is region 0 by default and the
msix bar is 2 by default.

Use case: For VirtIOPCIProxy subclasses which need additional pci bars,
such as virtio-vga.  With the new fields they can make sure the regions
do not conflict.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/virtio/virtio-pci.c | 25 +++++++++++++++++++++----
 hw/virtio/virtio-pci.h |  2 ++
 2 files changed, 23 insertions(+), 4 deletions(-)

On Mon, Feb 23, 2015 at 11:23:19AM +0100, Gerd Hoffmann wrote:
> Add msix_bar and modern_mem_bar fields to VirtIOPCIProxy.  They can be
> used to configure which pci regions are used for the virtio 1.0 memory
> bar and the msix bar.
> 
> For legacy/transitional devices the legacy bar is region 0 and the msix
> bar is region 1.  Only the modern bar can be configured, and it must be
> 2 or larger.  Default is 2.
> 
> For legacy-free devices the modern bar is region 0 by default and the
> msix bar is 2 by default.
> 
> Use case: For VirtIOPCIProxy subclasses which need additional pci bars,
> such as virtio-vga.  With the new fields they can make sure the regions
> do not conflict.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Hmm, I'd rather add an API to register a free BAR.
What's wrong with that?

> ---
>  hw/virtio/virtio-pci.c | 25 +++++++++++++++++++++----
>  hw/virtio/virtio-pci.h |  2 ++
>  2 files changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index cd7c777..f97baf2 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -965,8 +965,6 @@ static void virtio_pci_add_mem_cap(VirtIOPCIProxy *proxy,
>      PCIDevice *dev = &proxy->pci_dev;
>      int offset;
>  
> -    cap->bar = 2;
> -
>      offset = pci_add_capability(dev, PCI_CAP_ID_VNDR, 0, cap->cap_len);
>      assert(offset > 0);
>  
> @@ -1243,11 +1241,21 @@ static void virtio_pci_device_plugged(DeviceState *d)
>          pci_config_set_class(config, proxy->class_code);
>      }
>  
> +    if (proxy->modern_mem_bar > 5) {
> +        proxy->modern_mem_bar = 5;
> +    }
> +    if (proxy->msix_bar > 5) {
> +        proxy->msix_bar = 5;
> +    }
>      if (legacy) {
>          /* legacy and transitional */
>          pci_set_word(config + PCI_SUBSYSTEM_VENDOR_ID,
>                       pci_get_word(config + PCI_VENDOR_ID));
>          pci_set_word(config + PCI_SUBSYSTEM_ID, virtio_bus_get_vdev_id(bus));
> +        proxy->msix_bar = 1;
> +        if (proxy->modern_mem_bar < 2) {
> +            proxy->modern_mem_bar = 2;
> +        }
>      } else {
>          /* pure virtio-1.0 */
>          pci_set_word(config + PCI_VENDOR_ID,
> @@ -1255,6 +1263,9 @@ static void virtio_pci_device_plugged(DeviceState *d)
>          pci_set_word(config + PCI_DEVICE_ID,
>                       0x1040 + virtio_bus_get_vdev_id(bus));
>          pci_config_set_revision(config, 1);
> +        if (proxy->msix_bar == proxy->modern_mem_bar) {
> +            proxy->msix_bar = (proxy->msix_bar + 2) % 6;
> +        }
>      }
>      config[PCI_INTERRUPT_PIN] = 1;
>  
> @@ -1263,24 +1274,28 @@ static void virtio_pci_device_plugged(DeviceState *d)
>          struct virtio_pci_cap common = {
>              .cfg_type = VIRTIO_PCI_CAP_COMMON_CFG,
>              .cap_len = sizeof common,
> +            .bar = proxy->modern_mem_bar,
>              .offset = cpu_to_le32(0x0),
>              .length = cpu_to_le32(0x1000),
>          };
>          struct virtio_pci_cap isr = {
>              .cfg_type = VIRTIO_PCI_CAP_ISR_CFG,
>              .cap_len = sizeof isr,
> +            .bar = proxy->modern_mem_bar,
>              .offset = cpu_to_le32(0x1000),
>              .length = cpu_to_le32(0x1000),
>          };
>          struct virtio_pci_cap device = {
>              .cfg_type = VIRTIO_PCI_CAP_DEVICE_CFG,
>              .cap_len = sizeof device,
> +            .bar = proxy->modern_mem_bar,
>              .offset = cpu_to_le32(0x2000),
>              .length = cpu_to_le32(0x1000),
>          };
>          struct virtio_pci_notify_cap notify = {
>              .cap.cfg_type = VIRTIO_PCI_CAP_NOTIFY_CFG,
>              .cap.cap_len = sizeof notify,
> +            .cap.bar = proxy->modern_mem_bar,
>              .cap.offset = cpu_to_le32(0x3000),
>              .cap.length = cpu_to_le32(QEMU_VIRTIO_PCI_QUEUE_MEM_MULT *
>                                        VIRTIO_PCI_QUEUE_MAX),
> @@ -1359,12 +1374,14 @@ static void virtio_pci_device_plugged(DeviceState *d)
>                                QEMU_VIRTIO_PCI_QUEUE_MEM_MULT *
>                                VIRTIO_PCI_QUEUE_MAX);
>          memory_region_add_subregion(&proxy->modern_bar, 0x3000, &proxy->notify);
> -        pci_register_bar(&proxy->pci_dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY,
> +        pci_register_bar(&proxy->pci_dev, proxy->modern_mem_bar,
> +                         PCI_BASE_ADDRESS_SPACE_MEMORY,
>                           &proxy->modern_bar);
>      }
>  
>      if (proxy->nvectors &&
> -        msix_init_exclusive_bar(&proxy->pci_dev, proxy->nvectors, 1)) {
> +        msix_init_exclusive_bar(&proxy->pci_dev, proxy->nvectors,
> +                                proxy->msix_bar)) {
>          error_report("unable to init msix vectors to %" PRIu32,
>                       proxy->nvectors);
>          proxy->nvectors = 0;
> diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
> index 3068a63..a273c33 100644
> --- a/hw/virtio/virtio-pci.h
> +++ b/hw/virtio/virtio-pci.h
> @@ -102,6 +102,8 @@ struct VirtIOPCIProxy {
>      uint32_t flags;
>      uint32_t class_code;
>      uint32_t nvectors;
> +    uint32_t msix_bar;
> +    uint32_t modern_mem_bar;
>      uint64_t host_features;
>      uint32_t dfselect;
>      uint32_t gfselect;
> -- 
> 1.8.3.1

Hi,

> > Use case: For VirtIOPCIProxy subclasses which need additional pci bars,
> > such as virtio-vga.  With the new fields they can make sure the regions
> > do not conflict.
> > 
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> 
> Hmm, I'd rather add an API to register a free BAR.
> What's wrong with that?

I want a fixed bar (for the legacy vga framebuffer), because that makes
things easier for vgabios etc.  Currently virtio-vga uses bar #2, and
I'd much prefer to not change that as support for this is in seabios
already.

I don't mind much how that is actually implemented though.  Reserving
bar #2 somehow, then have virtio_pci_device_plugged check this and place
the bars automatically in a non-conflicting way is fine with me too.

cheers,
  Gerd