[Qemu-devel] Re: [PATCH] [Also for STABLE-0.12] Don't check for bus master for old guests

[Michael S. Tsirkin]

On Tue, Mar 16, 2010 at 09:36:14PM +0100, Alexander Graf wrote:
> Older Linux guests don't activate the bus master enable bit. So for those we
> can just try to be clever and track if they set the DEVICE_OK bit even though
> bus mastering is still disabled.
> 
> Under that condition we can disable the windows safety check. With that logic
> in place both guests should work just fine. Without PCI hotplug breaks
> virtio-net in Linux < 2.6.34 guests.
> 
> Signed-off-by: Alexander Graf <address@hidden>
> CC: Michael S. Tsirkin <address@hidden>

Acked-by: Michael S. Tsirkin <address@hidden>

> ---
> 
> v1 -> v2:
> 
>   - Fix state loading
>   - Reduce if nesting
> ---
>  hw/virtio-pci.c |   24 +++++++++++++++++++++++-
>  1 files changed, 23 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index 3594152..dbbaf42 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -76,6 +76,10 @@
>   * 12 is historical, and due to x86 page size. */
>  #define VIRTIO_PCI_QUEUE_ADDR_SHIFT    12
>  
> +/* We can catch some guest bugs inside here so we continue supporting older
> +   guests. */
> +#define VIRTIO_PCI_BUG_BUS_MASTER    (1 << 0)
> +
>  /* QEMU doesn't strictly need write barriers since everything runs in
>   * lock-step.  We'll leave the calls to wmb() in though to make it obvious 
> for
>   * KVM or if kqemu gets SMP support.
> @@ -87,6 +91,7 @@
>  typedef struct {
>      PCIDevice pci_dev;
>      VirtIODevice *vdev;
> +    uint32_t bugs;
>      uint32_t addr;
>      uint32_t class_code;
>      uint32_t nvectors;
> @@ -138,6 +143,13 @@ static int virtio_pci_load_config(void * opaque, 
> QEMUFile *f)
>      if (proxy->vdev->config_vector != VIRTIO_NO_VECTOR) {
>          return msix_vector_use(&proxy->pci_dev, proxy->vdev->config_vector);
>      }
> +
> +    /* Try to find out if the guest has bus master disabled, but is
> +       in ready state. Then we have a buggy guest OS. */
> +    if ((proxy->vdev->status & VIRTIO_CONFIG_S_DRIVER_OK) &&
> +        !(proxy->pci_dev.config[PCI_COMMAND] & PCI_COMMAND_MASTER)) {
> +        proxy->bugs |= VIRTIO_PCI_BUG_BUS_MASTER;
> +    }
>      return 0;
>  }
>  
> @@ -162,6 +174,7 @@ static void virtio_pci_reset(DeviceState *d)
>      VirtIOPCIProxy *proxy = container_of(d, VirtIOPCIProxy, pci_dev.qdev);
>      virtio_reset(proxy->vdev);
>      msix_reset(&proxy->pci_dev);
> +    proxy->bugs = 0;
>  }
>  
>  static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
> @@ -205,6 +218,14 @@ static void virtio_ioport_write(void *opaque, uint32_t 
> addr, uint32_t val)
>              virtio_reset(proxy->vdev);
>              msix_unuse_all_vectors(&proxy->pci_dev);
>          }
> +
> +        /* 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) &&
> +            !(proxy->pci_dev.config[PCI_COMMAND] & PCI_COMMAND_MASTER)) {
> +            proxy->bugs |= VIRTIO_PCI_BUG_BUS_MASTER;
> +        }
>          break;
>      case VIRTIO_MSI_CONFIG_VECTOR:
>          msix_vector_unuse(&proxy->pci_dev, vdev->config_vector);
> @@ -371,7 +392,8 @@ static void virtio_write_config(PCIDevice *pci_dev, 
> uint32_t address,
>      VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
>  
>      if (PCI_COMMAND == address) {
> -        if (!(val & PCI_COMMAND_MASTER)) {
> +        if (!(val & PCI_COMMAND_MASTER) &&
> +            !(proxy->bugs & VIRTIO_PCI_BUG_BUS_MASTER)) {
>              proxy->vdev->status &= ~VIRTIO_CONFIG_S_DRIVER_OK;
>          }
>      }
> -- 
> 1.6.0.2