[Qemu-devel] Re: [PATCH v9 1/8] pci: revise pci command register initialization

[Michael S. Tsirkin]

On Wed, Nov 17, 2010 at 11:03:14AM +0900, Isaku Yamahata wrote:
> On Tue, Nov 16, 2010 at 12:50:19PM +0200, Michael S. Tsirkin wrote:
> > On Tue, Nov 16, 2010 at 05:26:05PM +0900, Isaku Yamahata wrote:
> > > This patch cleans up command register initialization with
> > > comments. It also fixes the initialization of io/memory bit of command 
> > > register.
> > > Those bits for type 1 device is RW.
> > > Those bits for type 0 device is
> > >   RO = 0 if it has no io/memory BAR
> > >   RW if it has io/memory BAR
> > > 
> > > Signed-off-by: Isaku Yamahata <address@hidden>
> > 
> > There's a bug here: you can not assume that device that has
> > no io BAR claims no io transactions.
> 
> Which device are you mentioning?

Look at appendix D in PCI spec. There are many programming
classes like display controllers that claim specific
addresses without using a BAR. We could code it all
up with a huge table. But what we have is much simpler
and works well AFAIK.

> Such device should set the bit by itself, not by pci generic layer.

Since this is PCI spec specified behaviour, I think it's better
to have it in a common place. Devices are sure to get it wrong.


> > Another bug is that migrating from qemu where a bit is writeable to one
> > where it's RO creates a situation where a RW bit becomes RO, or the
> > reverse, which might confuse guests.  So we will need a compatibility
> > flag and set it for old machine types.
> 
> We needs to keep compatibility. Which way do you prefer?
> 
> - don't care: no way
> 
> - introduce global property to indicate compat qemu version or flags
>   something like if (compat version <= 0.13) old behaviour...
>   or if (flags & ...)
> 
> - introduce global-pci property
> 
> - introduce pci bus property
>   Users needs to specify this property for all pci devices.
> 
> - Don't change common code(pci.c), and provide a helper function.
>   Each device which needs new behavior like pcie calls it.
>   Probably each device may provide property to specify compat behavior
> 
> - any other?

- Don't change behaviour at all.

        What is the motivation for the change?  Why do we bother? What we have
        is spec compliant, I think, so it's hard for me to believe pcie *needs*
        the new behaviour.