From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:37055) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QVqgS-00068n-Jl for qemu-devel@nongnu.org; Sun, 12 Jun 2011 15:50:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QVqgR-0008Ed-6q for qemu-devel@nongnu.org; Sun, 12 Jun 2011 15:50:56 -0400 Received: from smtp5-g21.free.fr ([212.27.42.5]:41309) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QVqgQ-0008EL-Gi for qemu-devel@nongnu.org; Sun, 12 Jun 2011 15:50:55 -0400 Message-ID: <4DF51895.4030302@reactos.org> Date: Sun, 12 Jun 2011 21:50:45 +0200 From: =?ISO-8859-1?Q?Herv=E9_Poussineau?= MIME-Version: 1.0 References: <1292973623-2455-1-git-send-email-andreas.faerber@web.de> <20101222025014.GA20337@valinux.co.jp> <4D119B09.30103@reactos.org> <20101222064921.GE7814@redhat.com> <193FFA10-B7C4-4928-ADDE-C874C56BE70C@web.de> <4DF4B3C3.7000907@reactos.org> <20110612193327.GA20531@redhat.com> In-Reply-To: <20110612193327.GA20531@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH, RFC] pci: allow PCI devices to fix address space List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: Isaku Yamahata , =?ISO-8859-1?Q?Andreas_F=E4rber?= , =?ISO-8859-1?Q?Herv=E9_Poussineau?= , qemu-devel Developers Michael S. Tsirkin a =E9crit : > On Sun, Jun 12, 2011 at 02:40:35PM +0200, Herv=E9 Poussineau wrote: > =20 >> Andreas, >> >> Andreas F=E4rber a =E9crit : >> =20 >>> Herv=E9, >>> >>> Am 22.12.2010 um 07:49 schrieb Michael S. Tsirkin: >>> >>> =20 >>>> On Wed, Dec 22, 2010 at 07:30:33AM +0100, Herv=E9 Poussineau wrote: >>>> =20 >>>>> Isaku Yamahata a =E9crit : >>>>> =20 >>>>>> On Wed, Dec 22, 2010 at 12:20:23AM +0100, Andreas F=E4rber wrote: >>>>>> =20 >>>>>>> From: Herv=E9 Poussineau >>>>>>> >>>>>>> v1: >>>>>>> * Rebased. >>>>>>> >>>>>>> Signed-off-by: Herv=E9 Poussineau >>>>>>> Cc: Michael S. Tsirkin >>>>>>> Signed-off-by: Andreas F=E4rber >>>>>>> --- >>>>>>> Hello Michael, >>>>>>> Could you please take a look at this? I'm out of my field here. >>>>>>> The intention of the first part appears to be to save (val & ~mas= k), >>>>>>> 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 >>>> What about this first chunk? Is it necessary. >>>> >>>> =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 clear >>>>>> those registers. >>>>>> Anyway after cold/warm reset firmware sets up BARs, so it >>>>>> doesn't matter. >>>>>> 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 >>>>> bridge, where MMIO is always at the same address. >>>>> In that case, the device keeps PCI_COMMAND_MEMORY and/or >>>>> PCI_COMMAND_IO flags as read-only. >>>>> >>>>> Herv=E9 >>>>> =20 >>>> Aha. Are the BARs still writeable? If not maybe that's the right th= ing >>>> to check? If yes maybe the device simply should have a reset >>>> handler to rewrite them? >>>> =20 >>> I haven't noticed a follow-up patch of yours. >>> >>> Since I don't know what to do here, I'll have to take this out of >>> the PReP queue for now. >>> Without this patch, I get to at least the second bootloader icon, >>> the PCI graphics still work. What particular symptoms did you >>> observe wrt the i82378 that we can reproduce? >>> >>> Thanks, >>> Andreas >>> >>> =20 >> Try do do info qtree with and without this patch, and check the >> i82378 device >> With this patch, I have: >> bar 0: mem at 0x80000000 [0x8000ffff] >> bar 1: mem at 0xc0000000 [0xc0ffffff] >> >> Without it, I have: >> bar 0: mem at 0xffffffffffffffff [0xfffe] >> bar 1: mem at 0xffffffffffffffff [0xfffffe] >> >> I think that firmware doesn't initialize BARs for this device. >> =20 > > Interesting. So what set the BARs to these values (0x80000000 > and 0xc0000000) Currently, those default values are stored in qdev properties of device, and device fills in BARs addresses in its init function +static int pci_i82378_init(PCIDevice *pci_dev) ... + /* Make addresses read only */ + pci_set_word(pci_dev->wmask + PCI_COMMAND, + PCI_COMMAND_SPECIAL); + pci_set_long(pci_conf + PCI_BASE_ADDRESS_0 + 0 * 4, pci->isa_io_base= ); + pci_set_long(pci_conf + PCI_BASE_ADDRESS_0 + 1 * 4, pci->isa_mem_bas= e); Regards Herv=E9