From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=38864 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PVIDs-0005XI-OK for qemu-devel@nongnu.org; Wed, 22 Dec 2010 01:31:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PVIDo-0005XM-7W for qemu-devel@nongnu.org; Wed, 22 Dec 2010 01:30:52 -0500 Received: from smtp5-g21.free.fr ([212.27.42.5]:41220) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PVIDn-0005S6-Dk for qemu-devel@nongnu.org; Wed, 22 Dec 2010 01:30:48 -0500 Message-ID: <4D119B09.30103@reactos.org> Date: Wed, 22 Dec 2010 07:30:33 +0100 From: =?UTF-8?B?SGVydsOpIFBvdXNzaW5lYXU=?= MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH, RFC] pci: allow PCI devices to fix address space References: <1292973623-2455-1-git-send-email-andreas.faerber@web.de> <20101222025014.GA20337@valinux.co.jp> In-Reply-To: <20101222025014.GA20337@valinux.co.jp> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Isaku Yamahata Cc: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= , =?UTF-8?B?SGVydsOpIFBvdXNzaW5lYXU=?= , qemu-devel@nongnu.org, "Michael S. Tsirkin" Isaku Yamahata a =C3=A9crit : > On Wed, Dec 22, 2010 at 12:20:23AM +0100, Andreas F=C3=A4rber wrote: > =20 >> From: Herv=C3=A9 Poussineau >> >> v1: >> * Rebased. >> >> Signed-off-by: Herv=C3=A9 Poussineau >> Cc: Michael S. Tsirkin >> Signed-off-by: Andreas F=C3=A4rber >> --- >> =20 >> Hello Michael, >> =20 >> Could you please take a look at this? I'm out of my field here. >> =20 >> The intention of the first part appears to be to save (val & ~mask), >> whereas the inline helper would've returned (val & mask). >> =20 > > Such behavior is intended. > The returned value is just discarded in this case. > test-and-clear means > clear the bits > return if those cleared bits were really set. > > > =20 >> The second part makes existing code conditional on that value. >> =20 > > What issue are you addressing? > Although the spec doesn't says about the default value of BAR registers > after reset, the current code assumes that almost all the pci devices c= lear > those registers. > Anyway after cold/warm reset firmware sets up BARs, so it doesn't matte= r. > You, however, seem to want to keep BARs over resets. > > thanks, > > > =20 As you have seen, the intend here is to be able to keep BARs over resets. It is required for some really specific devices, like a PCI to ISA=20 bridge, where MMIO is always at the same address. In that case, the device keeps PCI_COMMAND_MEMORY and/or PCI_COMMAND_IO=20 flags as read-only. Herv=C3=A9 >> =20 >> Regards, >> Andreas >> =20 >> hw/pci.c | 22 ++++++++++++++-------- >> 1 files changed, 14 insertions(+), 8 deletions(-) >> >> diff --git a/hw/pci.c b/hw/pci.c >> index ef00d20..4db4b1f 100644 >> --- a/hw/pci.c >> +++ b/hw/pci.c >> @@ -140,6 +140,7 @@ static void pci_update_irq_status(PCIDevice *dev) >> static void pci_device_reset(PCIDevice *dev) >> { >> int r; >> + uint16_t cmd; >> /* TODO: call the below unconditionally once all pci devices >> * are qdevified */ >> if (dev->qdev.info) { >> @@ -149,9 +150,10 @@ static void pci_device_reset(PCIDevice *dev) >> dev->irq_state =3D 0; >> pci_update_irq_status(dev); >> /* Clear all writeable bits */ >> - pci_word_test_and_clear_mask(dev->config + PCI_COMMAND, >> - pci_get_word(dev->wmask + PCI_COMMAN= D) | >> - pci_get_word(dev->w1cmask + PCI_COMM= AND)); >> + cmd =3D pci_get_word(dev->config + PCI_COMMAND); >> + cmd &=3D ~(pci_get_word(dev->wmask + PCI_COMMAND) | >> + pci_get_word(dev->w1cmask + PCI_COMMAND)); >> + pci_set_word(dev->config + PCI_COMMAND, cmd); >> pci_word_test_and_clear_mask(dev->config + PCI_STATUS, >> pci_get_word(dev->wmask + PCI_STATUS= ) | >> pci_get_word(dev->w1cmask + PCI_STAT= US)); >> @@ -163,11 +165,15 @@ static void pci_device_reset(PCIDevice *dev) >> continue; >> } >> =20 >> - 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)= ; >> + if ((region->type =3D=3D PCI_BASE_ADDRESS_SPACE_IO && !(cmd &= PCI_COMMAND_MEMORY)) || >> + (region->type =3D=3D PCI_BASE_ADDRESS_SPACE_MEMORY && !(c= md & PCI_COMMAND_IO))) { >> + /* Reset region address, as it it is not read-only */ >> + 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->t= ype); >> + } else { >> + pci_set_long(dev->config + pci_bar(dev, r), region->t= ype); >> + } >> } >> } >> pci_update_mappings(dev); >> --=20 >> 1.7.3.4 >> >> >> =20 > > =20