[PATCH v2 1/2] virtio-vga: fix virtio-vga bar ordering

[PATCH v2 1/2] virtio-vga: fix virtio-vga bar ordering

[Anthoine Bourgeois]

With virtio-vga, pci bar are reordered. Bar #2 is used for compatibility
with stdvga. By default, bar #2 is used by virtio modern io bar.
This bar is the last one introduce in the virtio pci bar layout and it's
crushed by the virtio-vga reordering. So virtio-vga and
modern-pio-notify are incompatible because virtio-vga failed to
initialize with this option.

This fix sets the modern io bar to the bar #5 to avoid conflict.

Signed-off-by: Anthoine Bourgeois <anthoine.bourgeois@gmail.com>
---
 hw/display/virtio-vga.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c
index 2b4c2aa126..95757a6619 100644
--- a/hw/display/virtio-vga.c
+++ b/hw/display/virtio-vga.c
@@ -114,6 +114,7 @@ static void virtio_vga_base_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
      */
     vpci_dev->modern_mem_bar_idx = 2;
     vpci_dev->msix_bar_idx = 4;
+    vpci_dev->modern_io_bar_idx = 5;
 
     if (!(vpci_dev->flags & VIRTIO_PCI_FLAG_PAGE_PER_VQ)) {
         /*
-- 
2.20.1

[PATCH v2 2/2] virtio-pci: update virtio pci bar layout documentation

[Anthoine Bourgeois]

The modern io bar was never documented.

Signed-off-by: Anthoine Bourgeois <anthoine.bourgeois@gmail.com>
---
 hw/virtio/virtio-pci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 4cb784389c..d028c17c24 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1705,6 +1705,7 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp)
      *
      *   region 0   --  virtio legacy io bar
      *   region 1   --  msi-x bar
+     *   region 2   --  virtio modern io bar (off by default)
      *   region 4+5 --  virtio modern memory (64bit) bar
      *
      */
-- 
2.20.1

Re: [PATCH v2 1/2] virtio-vga: fix virtio-vga bar ordering

[Gerd Hoffmann]

On Wed, Apr 22, 2020 at 11:54:54PM +0200, Anthoine Bourgeois wrote:
> With virtio-vga, pci bar are reordered. Bar #2 is used for compatibility
> with stdvga. By default, bar #2 is used by virtio modern io bar.
> This bar is the last one introduce in the virtio pci bar layout and it's
> crushed by the virtio-vga reordering. So virtio-vga and
> modern-pio-notify are incompatible because virtio-vga failed to
> initialize with this option.
> 
> This fix sets the modern io bar to the bar #5 to avoid conflict.
> 
> Signed-off-by: Anthoine Bourgeois <anthoine.bourgeois@gmail.com>

Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

> ---
>  hw/display/virtio-vga.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c
> index 2b4c2aa126..95757a6619 100644
> --- a/hw/display/virtio-vga.c
> +++ b/hw/display/virtio-vga.c
> @@ -114,6 +114,7 @@ static void virtio_vga_base_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
>       */
>      vpci_dev->modern_mem_bar_idx = 2;
>      vpci_dev->msix_bar_idx = 4;
> +    vpci_dev->modern_io_bar_idx = 5;
>  
>      if (!(vpci_dev->flags & VIRTIO_PCI_FLAG_PAGE_PER_VQ)) {
>          /*
> -- 
> 2.20.1
> 

Re: [PATCH v2 2/2] virtio-pci: update virtio pci bar layout documentation

[Gerd Hoffmann]

On Wed, Apr 22, 2020 at 11:54:55PM +0200, Anthoine Bourgeois wrote:
> The modern io bar was never documented.
> 
> Signed-off-by: Anthoine Bourgeois <anthoine.bourgeois@gmail.com>
> ---
>  hw/virtio/virtio-pci.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 4cb784389c..d028c17c24 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1705,6 +1705,7 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp)
>       *
>       *   region 0   --  virtio legacy io bar
>       *   region 1   --  msi-x bar
> +     *   region 2   --  virtio modern io bar (off by default)
>       *   region 4+5 --  virtio modern memory (64bit) bar

Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

>       *
>       */
> -- 
> 2.20.1
> 

Re: [PATCH v2 1/2] virtio-vga: fix virtio-vga bar ordering

[Michael S. Tsirkin]

On Wed, Apr 22, 2020 at 11:54:54PM +0200, Anthoine Bourgeois wrote:
> With virtio-vga, pci bar are reordered. Bar #2 is used for compatibility
> with stdvga. By default, bar #2 is used by virtio modern io bar.
> This bar is the last one introduce in the virtio pci bar layout and it's
> crushed by the virtio-vga reordering. So virtio-vga and
> modern-pio-notify are incompatible because virtio-vga failed to
> initialize with this option.
> 
> This fix sets the modern io bar to the bar #5 to avoid conflict.
> 
> Signed-off-by: Anthoine Bourgeois <anthoine.bourgeois@gmail.com>

Gerd, would you say it's required for 5.0?

> ---
>  hw/display/virtio-vga.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c
> index 2b4c2aa126..95757a6619 100644
> --- a/hw/display/virtio-vga.c
> +++ b/hw/display/virtio-vga.c
> @@ -114,6 +114,7 @@ static void virtio_vga_base_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
>       */
>      vpci_dev->modern_mem_bar_idx = 2;
>      vpci_dev->msix_bar_idx = 4;
> +    vpci_dev->modern_io_bar_idx = 5;
>  
>      if (!(vpci_dev->flags & VIRTIO_PCI_FLAG_PAGE_PER_VQ)) {
>          /*
> -- 
> 2.20.1

Re: [PATCH v2 1/2] virtio-vga: fix virtio-vga bar ordering

[Gerd Hoffmann]

On Thu, Apr 23, 2020 at 08:52:58AM -0400, Michael S. Tsirkin wrote:
> On Wed, Apr 22, 2020 at 11:54:54PM +0200, Anthoine Bourgeois wrote:
> > With virtio-vga, pci bar are reordered. Bar #2 is used for compatibility
> > with stdvga. By default, bar #2 is used by virtio modern io bar.
> > This bar is the last one introduce in the virtio pci bar layout and it's
> > crushed by the virtio-vga reordering. So virtio-vga and
> > modern-pio-notify are incompatible because virtio-vga failed to
> > initialize with this option.
> > 
> > This fix sets the modern io bar to the bar #5 to avoid conflict.
> > 
> > Signed-off-by: Anthoine Bourgeois <anthoine.bourgeois@gmail.com>
> 
> Gerd, would you say it's required for 5.0?

Given that modern-pio-notify is off by default I wouldn't classify this
as release blocker, i.e. I wouldn't delay the release for that.

We are at -rc4 (last rc) already, so I'd tend to say no.  Maybe if we
need -rc5 for other reasons.

cheers,
  Gerd