Message ID | 1410459530-29832-1-git-send-email-mst@redhat.com |
---|---|
State | New |
Headers | show |
On Thu, 11 Sep 2014 21:20:03 +0300 "Michael S. Tsirkin" <mst@redhat.com> wrote: > 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 (guests use some devices before DRIVER_OK). > > Reported-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > Sorry, v4 was broken - forgot to commit before git format-patch. > Here's the correct version. > Still not tested - will do next week. > Testing reports appreciated meanwhile. > Nice patch indeed ! :) Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com> Tested-by: Greg Kurz <gkurz@linux.vnet.ibm.com> Cc'ing qemu-stable as Michael said he would queue the initial bus master patch for 2.1.2. > hw/virtio/virtio-pci.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index ddb5da1..a827cd4 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -314,6 +314,16 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val) > msix_unuse_all_vectors(&proxy->pci_dev); > } > > + /* Linux before 2.6.34 drives the device without enabling > + the PCI device bus master bit. Enable it automatically > + for the guest. This is a PCI spec violation but so is > + initiating DMA with bus master bit clear. */ > + if (val == (VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER)) { > + pci_default_write_config(&proxy->pci_dev, PCI_COMMAND, > + proxy->pci_dev.config[PCI_COMMAND] | > + PCI_COMMAND_MASTER, 1); > + } > + > /* 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. */
On Thu, Sep 11, 2014 at 08:42:06PM +0200, Greg Kurz wrote: > On Thu, 11 Sep 2014 21:20:03 +0300 > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > 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 (guests use some devices before DRIVER_OK). > > > > Reported-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > --- > > Sorry, v4 was broken - forgot to commit before git format-patch. > > Here's the correct version. > > Still not tested - will do next week. > > Testing reports appreciated meanwhile. > > > > Nice patch indeed ! :) > > Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > Tested-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > > Cc'ing qemu-stable as Michael said he would queue the initial > bus master patch for 2.1.2. In my opinion it's best to keep the initial patch off stable. > > hw/virtio/virtio-pci.c | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > > index ddb5da1..a827cd4 100644 > > --- a/hw/virtio/virtio-pci.c > > +++ b/hw/virtio/virtio-pci.c > > @@ -314,6 +314,16 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val) > > msix_unuse_all_vectors(&proxy->pci_dev); > > } > > > > + /* Linux before 2.6.34 drives the device without enabling > > + the PCI device bus master bit. Enable it automatically > > + for the guest. This is a PCI spec violation but so is > > + initiating DMA with bus master bit clear. */ > > + if (val == (VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER)) { > > + pci_default_write_config(&proxy->pci_dev, PCI_COMMAND, > > + proxy->pci_dev.config[PCI_COMMAND] | > > + PCI_COMMAND_MASTER, 1); > > + } > > + > > /* 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. */ > > > > -- > Gregory Kurz kurzgreg@fr.ibm.com > gkurz@linux.vnet.ibm.com > Software Engineer @ IBM/Meiosys http://www.ibm.com > Tel +33 (0)562 165 496 > > "Anarchy is about taking complete responsibility for yourself." > Alan Moore.
On 09/12/2014 02:20 AM, Michael S. Tsirkin wrote: > 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 (guests use some devices before DRIVER_OK). > > Reported-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > Sorry, v4 was broken - forgot to commit before git format-patch. > Here's the correct version. > Still not tested - will do next week. > Testing reports appreciated meanwhile. > > hw/virtio/virtio-pci.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index ddb5da1..a827cd4 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -314,6 +314,16 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val) > msix_unuse_all_vectors(&proxy->pci_dev); > } > > + /* Linux before 2.6.34 drives the device without enabling > + the PCI device bus master bit. Enable it automatically > + for the guest. This is a PCI spec violation but so is > + initiating DMA with bus master bit clear. */ > + if (val == (VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER)) { > + pci_default_write_config(&proxy->pci_dev, PCI_COMMAND, > + proxy->pci_dev.config[PCI_COMMAND] | > + PCI_COMMAND_MASTER, 1); > + } > + > /* 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. */ So with this patch, we can also remove the detecting and handling of VIRTIO_PCI_FLAG_BUS_MATER_BUG?
On Mon, Sep 15, 2014 at 05:48:17PM +0800, Jason Wang wrote: > On 09/12/2014 02:20 AM, Michael S. Tsirkin wrote: > > 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 (guests use some devices before DRIVER_OK). > > > > Reported-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > --- > > Sorry, v4 was broken - forgot to commit before git format-patch. > > Here's the correct version. > > Still not tested - will do next week. > > Testing reports appreciated meanwhile. > > > > hw/virtio/virtio-pci.c | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > > index ddb5da1..a827cd4 100644 > > --- a/hw/virtio/virtio-pci.c > > +++ b/hw/virtio/virtio-pci.c > > @@ -314,6 +314,16 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val) > > msix_unuse_all_vectors(&proxy->pci_dev); > > } > > > > + /* Linux before 2.6.34 drives the device without enabling > > + the PCI device bus master bit. Enable it automatically > > + for the guest. This is a PCI spec violation but so is > > + initiating DMA with bus master bit clear. */ > > + if (val == (VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER)) { > > + pci_default_write_config(&proxy->pci_dev, PCI_COMMAND, > > + proxy->pci_dev.config[PCI_COMMAND] | > > + PCI_COMMAND_MASTER, 1); > > + } > > + > > /* 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. */ > > So with this patch, we can also remove the detecting and handling of > VIRTIO_PCI_FLAG_BUS_MATER_BUG? Yes - I sent a follow-up patch to do that.
On Mon, Sep 15, 2014 at 12:59:44PM +0300, Michael S. Tsirkin wrote: > On Mon, Sep 15, 2014 at 05:48:17PM +0800, Jason Wang wrote: > > On 09/12/2014 02:20 AM, Michael S. Tsirkin wrote: > > > 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 (guests use some devices before DRIVER_OK). > > > > > > Reported-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > --- > > > Sorry, v4 was broken - forgot to commit before git format-patch. > > > Here's the correct version. > > > Still not tested - will do next week. > > > Testing reports appreciated meanwhile. > > > > > > hw/virtio/virtio-pci.c | 10 ++++++++++ > > > 1 file changed, 10 insertions(+) > > > > > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > > > index ddb5da1..a827cd4 100644 > > > --- a/hw/virtio/virtio-pci.c > > > +++ b/hw/virtio/virtio-pci.c > > > @@ -314,6 +314,16 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val) > > > msix_unuse_all_vectors(&proxy->pci_dev); > > > } > > > > > > + /* Linux before 2.6.34 drives the device without enabling > > > + the PCI device bus master bit. Enable it automatically > > > + for the guest. This is a PCI spec violation but so is > > > + initiating DMA with bus master bit clear. */ > > > + if (val == (VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER)) { > > > + pci_default_write_config(&proxy->pci_dev, PCI_COMMAND, > > > + proxy->pci_dev.config[PCI_COMMAND] | > > > + PCI_COMMAND_MASTER, 1); > > > + } > > > + > > > /* 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. */ > > > > So with this patch, we can also remove the detecting and handling of > > VIRTIO_PCI_FLAG_BUS_MATER_BUG? > > Yes - I sent a follow-up patch to do that. I will check if this patch fixed this bug (it seems WIN8r2 doesn't set BUS_MASTER bit correctly): https://bugzilla.redhat.com/show_bug.cgi?id=1090237
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index ddb5da1..a827cd4 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -314,6 +314,16 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val) msix_unuse_all_vectors(&proxy->pci_dev); } + /* Linux before 2.6.34 drives the device without enabling + the PCI device bus master bit. Enable it automatically + for the guest. This is a PCI spec violation but so is + initiating DMA with bus master bit clear. */ + if (val == (VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER)) { + pci_default_write_config(&proxy->pci_dev, PCI_COMMAND, + proxy->pci_dev.config[PCI_COMMAND] | + PCI_COMMAND_MASTER, 1); + } + /* 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. */
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 (guests use some devices before DRIVER_OK). Reported-by: Greg Kurz <gkurz@linux.vnet.ibm.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- Sorry, v4 was broken - forgot to commit before git format-patch. Here's the correct version. Still not tested - will do next week. Testing reports appreciated meanwhile. hw/virtio/virtio-pci.c | 10 ++++++++++ 1 file changed, 10 insertions(+)