Re: [Qemu-devel] [PATCH, RFC] pci: allow PCI devices to fix address space

["Michael S. Tsirkin" <mst@redhat.com>]

On Sun, Jun 12, 2011 at 04:20:10PM +0200, Andreas Färber wrote:
> 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 <hpoussin@reactos.org>
> >>>>>>
> >>>>>>v1:
> >>>>>>* Rebased.
> >>>>>>
> >>>>>>Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> >>>>>>Cc: Michael S. Tsirkin <mst@redhat.com>
> >>>>>>Signed-off-by: Andreas Färber <andreas.faerber@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)
> 
> Andreas

I'm just asking what exactly happens with the patch?
- After first boot, firmware writes something
  into BARs, then triggers a reset and then
  avoids writing there?
- After first boot, firmware writes something
  into BARs, but does not enable memory/io/bus master?
- Something else?

-- 
MST