On Wed, Nov 17, 2010 at 02:02:00PM +0200, Michael S. Tsirkin wrote: > > > 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.
AER wants SERR bit to be writable and you requested it as below. I thought, you wanted me to revise PCI_COMMAND and PCI_STATUS initialization. If I misunderstood, can you please elaborate on it? If you accept the following PCI_COMMAND line, I'm fine with dropping this clean up patch. http://lists.nongnu.org/archive/html/qemu-devel/2010-11/msg00131.html > > +void pcie_aer_init(PCIDevice *dev, uint16_t offset) > > +{ > > + PCIExpressDevice *exp; > > + > > + pci_word_test_and_set_mask(dev->wmask + PCI_COMMAND, PCI_COMMAND_SERR); > > + pci_word_test_and_set_mask(dev->w1cmask + PCI_STATUS, > > + PCI_STATUS_SIG_SYSTEM_ERROR); > > + > > I would say we should just set these for all devices. > But if we do my concern is that guest might write 1 to this register, > then we migrate to an old guest and that one can not clear this bit. > Thoughts? Let's add a flag so old machine types can disable this? > > > Also - what about other bits in the status register? -- yamahata
