From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48773) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XS6no-0005er-SV for qemu-devel@nongnu.org; Thu, 11 Sep 2014 12:01:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XS6ng-0003tC-HK for qemu-devel@nongnu.org; Thu, 11 Sep 2014 12:00:56 -0400 Received: from e06smtp11.uk.ibm.com ([195.75.94.107]:49394) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XS6ng-0003sv-9E for qemu-devel@nongnu.org; Thu, 11 Sep 2014 12:00:48 -0400 Received: from /spool/local by e06smtp11.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 11 Sep 2014 17:00:46 +0100 Date: Thu, 11 Sep 2014 18:00:39 +0200 From: Greg Kurz Message-ID: <20140911180039.2c3df0d2@bahia.local> In-Reply-To: <1410370197-15976-1-git-send-email-mst@redhat.com> References: <1410370197-15976-1-git-send-email-mst@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] virtio-pci: enable bus master for old guests List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: qemu-devel@nongnu.org, Anthony Liguori , qemu-stable@nongnu.org On Wed, 10 Sep 2014 20:30:47 +0300 "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 > > Old guests forgot to enable bus mastering, enable it > automatically when driver discovers device. > > Cc: qemu-stable@nongnu.org > Reported-by: Greg Kurz > Signed-off-by: Michael S. Tsirkin > --- > > OK, this should have better luck. Unfortunately not... it is even worse than V1 as both x86 and ppc64 guests fail. I've added some debug messages in virtio_ioport_write() and virtio_write_config() to track: - VIRTIO_PCI_STATUS changes - bus master disablement This is what I get for x86 with a virtio-net device: virtio_write_config: PCI_COMMAND = 0x3 virtio_ioport_write: VIRTIO_PCI_STATUS = 0x0 virtio_ioport_write: VIRTIO_PCI_STATUS = 0x1 ==> v2 patch enables BM virtio_ioport_write: VIRTIO_PCI_STATUS = 0x3 virtio_write_config: PCI_COMMAND = 0x3 ==> guest disables BM virtio_ioport_write: VIRTIO_PCI_STATUS = 0x7 virtio_ioport_write: detected bus master bug ==> v1 patch would have enabled BM here and for ppc64 when booting from a virtio-blk disk: virtio_write_config: PCI_COMMAND = 0x3 virtio_ioport_write: VIRTIO_PCI_STATUS = 0x0 virtio_ioport_write: VIRTIO_PCI_STATUS = 0x1 ==> v2 patch enables BM virtio_ioport_write: VIRTIO_PCI_STATUS = 0x3 virtio_write_config: PCI_COMMAND = 0x3 ==> guest disables BM I think we should: - detect the "old driver" when reaching DRIVER as it works for both architectures - always re-enable BM when an "old driver" disables it That is basically your v1 plus its follow-up patch: https://lists.nongnu.org/archive/html/qemu-devel/2014-09/msg01918.html plus: --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -317,7 +317,7 @@ 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, I could verify it fixes the issues. I will send a patch shortly. Cheers. -- Greg > This also makes it possible to simplify code, > will do that in a follow-up patch. > > > hw/virtio/virtio-pci.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index ddb5da1..a29d94f 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -303,6 +303,14 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val) > virtio_pci_stop_ioeventfd(proxy); > } > > + /* Linux before 2.6.34 uses the device without enabling > + the PCI device bus master bit. As a work-around, enable it > + automatically when driver detects the device. */ > + if (val == VIRTIO_CONFIG_S_ACKNOWLEDGE) { > + memory_region_set_enabled(&proxy->pci_dev.bus_master_enable_region, > + true); > + } > + > virtio_set_status(vdev, val & 0xFF); > > if (val & VIRTIO_CONFIG_S_DRIVER_OK) {