From patchwork Wed Aug 3 15:16:35 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Michael S. Tsirkin" X-Patchwork-Id: 108269 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [140.186.70.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 30CB7B71D5 for ; Thu, 4 Aug 2011 01:16:14 +1000 (EST) Received: from localhost ([::1]:56059 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QodB5-0002hs-JX for incoming@patchwork.ozlabs.org; Wed, 03 Aug 2011 11:16:11 -0400 Received: from eggs.gnu.org ([140.186.70.92]:52272) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QodB0-0002eP-Dp for qemu-devel@nongnu.org; Wed, 03 Aug 2011 11:16:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QodAz-0005wd-8H for qemu-devel@nongnu.org; Wed, 03 Aug 2011 11:16:06 -0400 Received: from mx1.redhat.com ([209.132.183.28]:18520) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QodAy-0005vQ-Tq for qemu-devel@nongnu.org; Wed, 03 Aug 2011 11:16:05 -0400 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p73FFxFD007463 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 3 Aug 2011 11:15:59 -0400 Received: from redhat.com (vpn-203-160.tlv.redhat.com [10.35.203.160]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with SMTP id p73FFuhi003543; Wed, 3 Aug 2011 11:15:57 -0400 Date: Wed, 3 Aug 2011 18:16:35 +0300 From: "Michael S. Tsirkin" To: Avi Kivity Message-ID: <20110803151635.GA15804@redhat.com> References: <1312135082-31985-1-git-send-email-avi@redhat.com> <1312135082-31985-21-git-send-email-avi@redhat.com> <20110801082600.GD5439@redhat.com> <4E367370.6070100@redhat.com> <20110801102322.GF5439@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20110801102322.GF5439@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Scanned-By: MIMEDefang 2.67 on 10.5.11.12 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.132.183.28 Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org Subject: Re: [Qemu-devel] [PATCH 20/39] virtio-pci: convert to memory API X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On Mon, Aug 01, 2011 at 01:23:22PM +0300, Michael S. Tsirkin wrote: > On Mon, Aug 01, 2011 at 12:35:44PM +0300, Avi Kivity wrote: > > On 08/01/2011 11:26 AM, Michael S. Tsirkin wrote: > > >> > > >> static void virtio_write_config(PCIDevice *pci_dev, uint32_t address, > > >> uint32_t val, int len) > > >> { > > >> VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev); > > >> + VirtIODevice *vdev = proxy->vdev; > > >> > > >> if (PCI_COMMAND == address) { > > >> if (!(val& PCI_COMMAND_MASTER)) { > > >> @@ -525,6 +503,9 @@ static void virtio_write_config(PCIDevice *pci_dev, uint32_t address, > > >> } > > >> } > > >> } > > >> + if (address == PCI_BASE_ADDRESS_0&& vdev->config_len) { > > >> + vdev->get_config(vdev, vdev->config); > > >> + } > > >> > > >> pci_default_write_config(pci_dev, address, val, len); > > >> msix_write_config(pci_dev, address, val, len); > > > > > >I'm not really sure why did we get the config on map, > > >specifically - Anthony, do you know? > > >But if we want to do that, memory space enable might > > >be a better place. Or maybe we just want a callback on > > >map. > > > > > > Just because a memory region becomes visible to the cpu is no reason > > to have a callback. From the device perspective, it can't tell that > > it happened. > > Well, the reason we have this logic here, I think, is > to make sure it runs before the guest accesses > the configuration with a write access. > > I'm not sure why we don't do this in the init > callback - Anthony? So the following should do this. Anthony, could you ack please? Avi, this is on top of the memory API branch. But since it's not exactly the same, maybe this should go in *before* the memory API patches, to make it easier to bisect etc? You decide. diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c index 92ba3c9..cb0e0a9 100644 --- a/hw/virtio-pci.c +++ b/hw/virtio-pci.c @@ -492,7 +492,6 @@ static void virtio_write_config(PCIDevice *pci_dev, uint32_t address, uint32_t val, int len) { VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev); - VirtIODevice *vdev = proxy->vdev; if (PCI_COMMAND == address) { if (!(val & PCI_COMMAND_MASTER)) { @@ -503,9 +502,6 @@ static void virtio_write_config(PCIDevice *pci_dev, uint32_t address, } } } - if (address == PCI_BASE_ADDRESS_0 && vdev->config_len) { - vdev->get_config(vdev, vdev->config); - } pci_default_write_config(pci_dev, address, val, len); msix_write_config(pci_dev, address, val, len); @@ -672,6 +668,7 @@ void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev) proxy->host_features |= 0x1 << VIRTIO_F_NOTIFY_ON_EMPTY; proxy->host_features |= 0x1 << VIRTIO_F_BAD_FEATURE; proxy->host_features = vdev->get_features(vdev, proxy->host_features); + vdev->get_config(vdev, vdev->config); } static int virtio_blk_init_pci(PCIDevice *pci_dev)