On Sun, Jun 12, 2011 at 08:28:09PM +0200, Hervé Poussineau wrote: > Andreas Färber a écrit : > > > >Am 12.06.2011 um 14:40 schrieb Hervé Poussineau: > > > >>Andreas Färber a écrit : > >>>Am 22.12.2010 um 07:49 schrieb Michael S. Tsirkin: > >>> > >>>>On Wed, Dec 22, 2010 at 07:30:33AM +0100, Hervé Poussineau wrote: > >>>>>Isaku Yamahata a écrit : > >>>>>>On Wed, Dec 22, 2010 at 12:20:23AM +0100, Andreas Färber wrote: > >>>>>>>From: Hervé Poussineau <hpous...@reactos.org> > >>>>>>> > >>>>>>>v1: > >>>>>>>* Rebased. > >>>>>>> > >>>>>>>Signed-off-by: Hervé Poussineau <hpous...@reactos.org> > >>>>>>>Cc: Michael S. Tsirkin <m...@redhat.com> > >>>>>>>Signed-off-by: Andreas Färber <andreas.faer...@web.de> > >>>>>>>--- > >>>>>>>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 & ~mask), > >>>>>>>whereas the inline helper would've returned (val & mask). > >>>>>> > >>>>>>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. > >>>>>> > >>>> > >>>>What about this first chunk? Is it necessary. > >>>> > >>>>>>>The second part makes existing code conditional on that value. > >>>>>> > >>>>>>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. > >>>>>> > >>>>>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. > >>>> > >>>>Aha. Are the BARs still writeable? If not maybe that's the > >>>>right thing > >>>>to check? If yes maybe the device simply should have a reset > >>>>handler to rewrite them? > >>> > >>>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? > >> > >>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] > > > >That I can confirm. > > > >>I think that firmware doesn't initialize BARs for this device. > >>However, I don't know what can be the consequences of not doing > >>it. > > > >Re-testing this after a rebase, I don't get any graphics at all > >without this patch. Must've done something wrong earlier... > > > >So, how to tell if the BARs are "still writable"? (mst's question) > > > I don't know if the BARs are hardwired to some value, or if they > have default values. > I've no real information on this aspect, so you might take a look at > the datasheet: > http://www.datasheetcatalog.org/datasheets2/10/1064845_1.pdf
This doesn't say anything about BARs. > Regarding PCI command register, it is explicitly said page 33 that > "Bus Master Enable", "Memory Space Enable" and "I/O Space Enable" > are hardwired to 1. > > Regards, > > Hervé Right. It would seem that the only way this can be safe is if the device has no BARs (only legacy ranges) or if the BARs are initializaed to a safe value somehow. -- MST
