From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36669) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YSPXX-0004aD-GD for qemu-devel@nongnu.org; Mon, 02 Mar 2015 07:33:43 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YSPXU-0000rk-6V for qemu-devel@nongnu.org; Mon, 02 Mar 2015 07:33:39 -0500 Received: from mx1.redhat.com ([209.132.183.28]:49538) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YSPXT-0000rZ-UF for qemu-devel@nongnu.org; Mon, 02 Mar 2015 07:33:36 -0500 Date: Mon, 2 Mar 2015 13:33:31 +0100 From: "Michael S. Tsirkin" Message-ID: <20150302123331.GB29257@redhat.com> References: <1424687012-18524-1-git-send-email-kraxel@redhat.com> <1424687012-18524-5-git-send-email-kraxel@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1424687012-18524-5-git-send-email-kraxel@redhat.com> Subject: Re: [Qemu-devel] [RfC PATCH 04/15] virtio-pci: make modern bar 64bit prefetchable List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: qemu-devel@nongnu.org, Anthony Liguori On Mon, Feb 23, 2015 at 11:23:20AM +0100, Gerd Hoffmann wrote: > Modern bar is made prefetchable. In case it is configured to use one of > the bars 0, 2, or 4 (which by default is the case) it is also configured > as 64bit region. > > Signed-off-by: Gerd Hoffmann Fine, but I can't apply this without 3/15. > --- > hw/virtio/virtio-pci.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index f97baf2..96b3692 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -1263,7 +1263,8 @@ static void virtio_pci_device_plugged(DeviceState *d) > pci_set_word(config + PCI_DEVICE_ID, > 0x1040 + virtio_bus_get_vdev_id(bus)); > pci_config_set_revision(config, 1); > - if (proxy->msix_bar == proxy->modern_mem_bar) { > + if ((proxy->msix_bar == proxy->modern_mem_bar) || > + (proxy->msix_bar == proxy->modern_mem_bar+1)) { > proxy->msix_bar = (proxy->msix_bar + 2) % 6; > } > } I'd like to avoid extra (), and see space around +. E.g. if (proxy->msix_bar == proxy->modern_mem_bar || proxy->msix_bar == proxy->modern_mem_bar + 1) we can generally rely on C to DTRT wrt priority of arithmentic operations and comparisons. > @@ -1301,6 +1302,7 @@ static void virtio_pci_device_plugged(DeviceState *d) > VIRTIO_PCI_QUEUE_MAX), > .notify_off_multiplier = cpu_to_le32(QEMU_VIRTIO_PCI_QUEUE_MEM_MULT), > }; > + uint32_t modern_mem_attr; > > static const MemoryRegionOps common_ops = { > .read = virtio_pci_common_read, > @@ -1374,9 +1376,13 @@ static void virtio_pci_device_plugged(DeviceState *d) > QEMU_VIRTIO_PCI_QUEUE_MEM_MULT * > VIRTIO_PCI_QUEUE_MAX); > memory_region_add_subregion(&proxy->modern_bar, 0x3000, &proxy->notify); > + modern_mem_attr = (PCI_BASE_ADDRESS_SPACE_MEMORY | > + PCI_BASE_ADDRESS_MEM_PREFETCH); no need for () to the right of =. > + if (!(proxy->modern_mem_bar % 2)) { > + modern_mem_attr |= PCI_BASE_ADDRESS_MEM_TYPE_64; > + } That's pretty unexpected. This kind of hack seems to show why it's not a good idea to let device play with layouts. > pci_register_bar(&proxy->pci_dev, proxy->modern_mem_bar, > - PCI_BASE_ADDRESS_SPACE_MEMORY, > - &proxy->modern_bar); > + modern_mem_attr, &proxy->modern_bar); > } > > if (proxy->nvectors && > -- > 1.8.3.1