Patchwork virtio-pci: fix bus master bug setting on load

login
register
mail settings
Submitter Alex Williamson
Date June 17, 2010, 3:15 p.m.
Message ID <20100617151502.9916.3597.stgit@localhost.localdomain>
Download mbox | patch
Permalink /patch/56057/
State New
Headers show

Comments

Alex Williamson - June 17, 2010, 3:15 p.m.
The comment suggests we're checking for the driver in the ready
state and bus master disabled, but the code is checking that it's
not in the ready state.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Found-by: Amit Shah <amit.shah@redhat.com>
---

 hw/virtio-pci.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Michael S. Tsirkin - June 17, 2010, 3:24 p.m.
On Thu, Jun 17, 2010 at 09:15:02AM -0600, Alex Williamson wrote:
> The comment suggests we're checking for the driver in the ready
> state and bus master disabled, but the code is checking that it's
> not in the ready state.
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> Found-by: Amit Shah <amit.shah@redhat.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
> 
>  hw/virtio-pci.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index e101fa0..7a86a81 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -155,7 +155,7 @@ static int virtio_pci_load_config(void * opaque, QEMUFile *f)
>  
>      /* 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) &&
> +    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;
>      }
Amit Shah - June 17, 2010, 3:57 p.m.
On (Thu) Jun 17 2010 [09:15:02], Alex Williamson wrote:
> The comment suggests we're checking for the driver in the ready
> state and bus master disabled, but the code is checking that it's
> not in the ready state.
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> Found-by: Amit Shah <amit.shah@redhat.com>
> ---

Acked-by: Amit Shah <amit.shah@redhat.com>


		Amit
Alexander Graf - June 17, 2010, 4:01 p.m.
Alex Williamson wrote:
> The comment suggests we're checking for the driver in the ready
> state and bus master disabled, but the code is checking that it's
> not in the ready state.
>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> Found-by: Amit Shah <amit.shah@redhat.com>
> ---
>
>  hw/virtio-pci.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index e101fa0..7a86a81 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -155,7 +155,7 @@ static int virtio_pci_load_config(void * opaque, QEMUFile *f)
>  
>      /* 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) &&
> +    if ((proxy->vdev->status & VIRTIO_CONFIG_S_DRIVER_OK) &&
>   

Phew - that's an evil one. Thanks for the catch!

Acked-by: Alexander Graf <agraf@suse.de>

Alex
Juan Quintela - June 17, 2010, 4:40 p.m.
Alex Williamson <alex.williamson@redhat.com> wrote:
> The comment suggests we're checking for the driver in the ready
> state and bus master disabled, but the code is checking that it's
> not in the ready state.
>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> Found-by: Amit Shah <amit.shah@redhat.com>

Acked-by: Juan Quintela <quintela@redhat.com>

> ---
>
>  hw/virtio-pci.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index e101fa0..7a86a81 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -155,7 +155,7 @@ static int virtio_pci_load_config(void * opaque, QEMUFile *f)
>  
>      /* 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) &&
> +    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;
>      }
Anthony Liguori - June 23, 2010, 1:48 a.m.
On 06/17/2010 10:15 AM, Alex Williamson wrote:
> The comment suggests we're checking for the driver in the ready
> state and bus master disabled, but the code is checking that it's
> not in the ready state.
>
> Signed-off-by: Alex Williamson<alex.williamson@redhat.com>
> Found-by: Amit Shah<amit.shah@redhat.com>
>    

Applied.  Thanks.

Regards,

Anthony Liguori
> ---
>
>   hw/virtio-pci.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index e101fa0..7a86a81 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -155,7 +155,7 @@ static int virtio_pci_load_config(void * opaque, QEMUFile *f)
>
>       /* 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)&&
> +    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;
>       }
>
>
>
>

Patch

diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index e101fa0..7a86a81 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -155,7 +155,7 @@  static int virtio_pci_load_config(void * opaque, QEMUFile *f)
 
     /* 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) &&
+    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;
     }