Re: [Qemu-stable] [PATCH] virtio-pci: enable bus master for old guests

[Michael S. Tsirkin]

On Thu, Sep 11, 2014 at 06:45:33PM +0200, Greg Kurz wrote:
> From: Michael S. Tsirkin <address@hidden>
> 
> commit cc943c36faa192cd4b32af8fe5edb31894017d35
>     pci: Use bus master address space for delivering MSI/MSI-X messages
> breaks virtio-net for rhel6.[56] x86 guests because they don't
> enable bus mastering for virtio PCI devices. For the same reason,
> rhel6.[56] ppc64 guests cannot boot on a virtio-blk disk anymore.
> 
> Old guests forgot to enable bus mastering, enable it automatically on
> DRIVER (ppc64 guests may never reach DRIVER_OK). We also need to prevent
> these old guests to disable bus mastering (this happens when the driver
> probes the device). And we must also re-enable bus mastering after migration.
> 
> Cc: address@hidden
> Reported-by: Greg Kurz <address@hidden>
> Signed-off-by: Michael S. Tsirkin <address@hidden>
> [ old guest detection on DRIVER,
>   squashed patch from Michael S. Tsirkin to re-enable bus mastering,
>   Greg Kurz <address@hidden> ]
> Tested-by: Cedric Le Goater <address@hidden>
> Signed-off-by: Greg Kurz <address@hidden>

I think this is too fragile. Let's try to figure out a strategy
that does not depend on VIRTIO_PCI_FLAG_BUS_MASTER_BUG.

> ---
>  hw/virtio/virtio-pci.c |   18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index ddb5da1..f981841 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -317,9 +317,11 @@ static void virtio_ioport_write(void *opaque, uint32_t 
> addr, uint32_t val)
>          /* Linux before 2.6.34 sets the device as OK without enabling
>             the PCI device bus master bit. In this case we need to disable
>             some safety checks. */
> -        if ((val & VIRTIO_CONFIG_S_DRIVER_OK) &&
> +        if ((val & VIRTIO_CONFIG_S_DRIVER) &&
>              !(proxy->pci_dev.config[PCI_COMMAND] & PCI_COMMAND_MASTER)) {
>              proxy->flags |= VIRTIO_PCI_FLAG_BUS_MASTER_BUG;
> +            
> memory_region_set_enabled(&proxy->pci_dev.bus_master_enable_region,
> +                                      true);
>          }
>          break;
>      case VIRTIO_MSI_CONFIG_VECTOR:
> @@ -473,10 +475,14 @@ static void virtio_write_config(PCIDevice *pci_dev, 
> uint32_t address,
>      pci_default_write_config(pci_dev, address, val, len);
>  
>      if (range_covers_byte(address, len, PCI_COMMAND) &&
> -        !(pci_dev->config[PCI_COMMAND] & PCI_COMMAND_MASTER) &&
> -        !(proxy->flags & VIRTIO_PCI_FLAG_BUS_MASTER_BUG)) {
> -        virtio_pci_stop_ioeventfd(proxy);
> -        virtio_set_status(vdev, vdev->status & ~VIRTIO_CONFIG_S_DRIVER_OK);
> +        !(pci_dev->config[PCI_COMMAND] & PCI_COMMAND_MASTER)) {
> +        if (proxy->flags & VIRTIO_PCI_FLAG_BUS_MASTER_BUG) {
> +            
> memory_region_set_enabled(&proxy->pci_dev.bus_master_enable_region,
> +                                      true);
> +        } else {
> +            virtio_pci_stop_ioeventfd(proxy);
> +            virtio_set_status(vdev, vdev->status & 
> ~VIRTIO_CONFIG_S_DRIVER_OK);
> +        }
>      }
>  }
>  
> @@ -890,6 +896,8 @@ static void virtio_pci_vmstate_change(DeviceState *d, 
> bool running)
>          if ((vdev->status & VIRTIO_CONFIG_S_DRIVER_OK) &&
>              !(proxy->pci_dev.config[PCI_COMMAND] & PCI_COMMAND_MASTER)) {
>              proxy->flags |= VIRTIO_PCI_FLAG_BUS_MASTER_BUG;
> +            
> memory_region_set_enabled(&proxy->pci_dev.bus_master_enable_region,
> +                                      true);
>          }
>          virtio_pci_start_ioeventfd(proxy);
>      } else {