From patchwork Thu Jun 17 10:58:44 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Michael S. Tsirkin" X-Patchwork-Id: 56014 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [199.232.76.165]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 95F48B7D5E for ; Thu, 17 Jun 2010 21:14:11 +1000 (EST) Received: from localhost ([127.0.0.1]:36624 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OPD2u-0005xZ-OX for incoming@patchwork.ozlabs.org; Thu, 17 Jun 2010 07:14:08 -0400 Received: from [140.186.70.92] (port=52761 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OPCsq-00006Y-2R for qemu-devel@nongnu.org; Thu, 17 Jun 2010 07:03:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OPCso-0000pU-8z for qemu-devel@nongnu.org; Thu, 17 Jun 2010 07:03:43 -0400 Received: from mx1.redhat.com ([209.132.183.28]:29513) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OPCsn-0000pP-WB for qemu-devel@nongnu.org; Thu, 17 Jun 2010 07:03:42 -0400 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o5HB3Zb4012779 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 17 Jun 2010 07:03:35 -0400 Received: from redhat.com (vpn2-10-219.ams2.redhat.com [10.36.10.219]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with SMTP id o5HB3VHd015190; Thu, 17 Jun 2010 07:03:32 -0400 Date: Thu, 17 Jun 2010 13:58:44 +0300 From: "Michael S. Tsirkin" To: Isaku Yamahata Message-ID: <20100617105843.GA10383@redhat.com> References: <551d7f5ea27a3bf8fcf69dea059d237493a92a2b.1276755023.git.yamahata@valinux.co.jp> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <551d7f5ea27a3bf8fcf69dea059d237493a92a2b.1276755023.git.yamahata@valinux.co.jp> User-Agent: Mutt/1.5.19 (2009-01-05) X-Scanned-By: MIMEDefang 2.67 on 10.5.11.11 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. Cc: stefano.stabellini@eu.citrix.com, jan.kiszka@siemens.com, allen.m.kay@intel.com, qemu-devel@nongnu.org, blauwirbel@gmail.com, kraxel@redhat.com, jean.guyader@gmail.com Subject: [Qemu-devel] Re: [PATCH 03/10] pci: fix pci_bus_reset() with 64bit BAR and several clean ups. X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On Thu, Jun 17, 2010 at 03:15:45PM +0900, Isaku Yamahata wrote: > fix pci_device_reset() with 64bit BAR. > export pci_bus_reset(), pci_device_reset() and two helper functions > for later use. And several clean ups. > > Signed-off-by: Isaku Yamahata > --- > hw/pci.c | 44 ++++++++++++++++++++++++++++++++++++-------- > hw/pci.h | 5 +++++ > 2 files changed, 41 insertions(+), 8 deletions(-) > > diff --git a/hw/pci.c b/hw/pci.c > index 9ba62eb..87f5e6c 100644 > --- a/hw/pci.c > +++ b/hw/pci.c > @@ -144,28 +144,50 @@ static void pci_update_irq_status(PCIDevice *dev) > } > } > > -static void pci_device_reset(PCIDevice *dev) > +void pci_device_reset_default(PCIDevice *dev) > { > int r; > > dev->irq_state = 0; > pci_update_irq_status(dev); > - dev->config[PCI_COMMAND] &= ~(PCI_COMMAND_IO | PCI_COMMAND_MEMORY | > - PCI_COMMAND_MASTER); > + pci_set_word(dev->config + PCI_COMMAND, > + pci_get_word(dev->config + PCI_COMMAND) & > + ~(PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER)); > dev->config[PCI_CACHE_LINE_SIZE] = 0x0; > dev->config[PCI_INTERRUPT_LINE] = 0x0; > for (r = 0; r < PCI_NUM_REGIONS; ++r) { > - if (!dev->io_regions[r].size) { > + PCIIORegion *region = &dev->io_regions[r]; > + if (!region->size) { > continue; > } > - pci_set_long(dev->config + pci_bar(dev, r), dev->io_regions[r].type); > + > + if (!(region->type & PCI_BASE_ADDRESS_SPACE_IO) && > + region->type & PCI_BASE_ADDRESS_MEM_TYPE_64) { > + pci_set_quad(dev->config + pci_bar(dev, r), region->type); > + } else { > + pci_set_long(dev->config + pci_bar(dev, r), region->type); > + } > } > pci_update_mappings(dev); > } > I applied the first hunk. Looking at it made me notice that we don't clear interrupt disable bit on reset, and we really should as it is read/write. Rather than duplicating code, we should just use wmask. I ended up with this: commit b82d3876099c4f1fd009082f052e3bac7e3062e7 Author: Isaku Yamahata Date: Thu Jun 17 15:15:45 2010 +0900 pci: fix pci_device_reset Clear interrupt disable bit on reset, according to PCI spec. Fix pci_device_reset() with 64bit BAR. Signed-off-by: Isaku Yamahata Signed-off-by: Michael S. Tsirkin diff --git a/hw/pci.c b/hw/pci.c index 7787005..de33745 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -150,15 +150,24 @@ static void pci_device_reset(PCIDevice *dev) dev->irq_state = 0; pci_update_irq_status(dev); - dev->config[PCI_COMMAND] &= ~(PCI_COMMAND_IO | PCI_COMMAND_MEMORY | - PCI_COMMAND_MASTER); + /* Clear all writeable bits */ + pci_set_word(dev->config + PCI_COMMAND, + pci_get_word(dev->config + PCI_COMMAND) & + ~pci_get_word(dev->wmask + PCI_COMMAND)); dev->config[PCI_CACHE_LINE_SIZE] = 0x0; dev->config[PCI_INTERRUPT_LINE] = 0x0; for (r = 0; r < PCI_NUM_REGIONS; ++r) { - if (!dev->io_regions[r].size) { + PCIIORegion *region = &dev->io_regions[r]; + if (!region->size) { continue; } - pci_set_long(dev->config + pci_bar(dev, r), dev->io_regions[r].type); + + if (!(region->type & PCI_BASE_ADDRESS_SPACE_IO) && + region->type & PCI_BASE_ADDRESS_MEM_TYPE_64) { + pci_set_quad(dev->config + pci_bar(dev, r), region->type); + } else { + pci_set_long(dev->config + pci_bar(dev, r), region->type); + } } pci_update_mappings(dev); }