Message ID | 20140911164533.24109.16197.stgit@bahia.local |
---|---|
State | New |
Headers | show |
On Thu, Sep 11, 2014 at 06:45:33PM +0200, Greg Kurz wrote: > From: Michael S. Tsirkin <mst@redhat.com> > > 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: qemu-stable@nongnu.org > Reported-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > [ old guest detection on DRIVER, > squashed patch from Michael S. Tsirkin to re-enable bus mastering, > Greg Kurz <gkurz@linux.vnet.ibm.com> ] > Tested-by: Cedric Le Goater <clg@fr.ibm.com> > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> 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 {
On Thu, Sep 11, 2014 at 08:47:01PM +0300, Michael S. Tsirkin wrote: > On Thu, Sep 11, 2014 at 06:45:33PM +0200, Greg Kurz wrote: > > From: Michael S. Tsirkin <mst@redhat.com> > > > > 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: qemu-stable@nongnu.org > > Reported-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > [ old guest detection on DRIVER, > > squashed patch from Michael S. Tsirkin to re-enable bus mastering, > > Greg Kurz <gkurz@linux.vnet.ibm.com> ] > > Tested-by: Cedric Le Goater <clg@fr.ibm.com> > > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > > 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. OTOH it does work, so I think I'll apply something similar (this one is slightly broken wrt migration) and work on simplifications as a follow-up. > > --- > > 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 {
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 {