On Thu, Dec 19, 2013 at 01:49:02PM +0200, Michael S. Tsirkin wrote:
PIIX spec says that most its registers are reset on suspend. However, QEMU didn't reset most of them properly. Now that it started doing that, resume is broken. Fix it up by probing system devices on bus 0 and re-initializing them.
Thanks. This patch runs part of the "pciinit" code and not all of it. What's the reasoning behind which code to run and which code not to run? Re-running parts of pciinit on resume is a change in behaviour and I'm concerned about unintended consequences. Is this definitely what should be setup for PCI on resume - nothing more should be done and nothing less?
Also, we were just about to release SeaBIOS v1.7.4. I don't think this behaviour change would be good to add so late in the release cycle. (As a side note, the patch would not be correct for coreboot and CSM users.) That means either delaying the release, or implementing this change as part of a future stable release.
Finally, if the issue is just on the PIIX PM device, would a more targeted change be more appropriate? For example, something like:
void qemu_piix_resume_fixup(void) { if (!CONFIG_QEMU) return; int bdf; foreachbdf(bdf, 0) { u32 vendev = pci_config_readl(bdf, PCI_VENDOR_ID); u16 vendor = vendev & 0xffff, device = vendev >> 16; if (vendor == PCI_VENDOR_ID_INTEL && device == PCI_DEVICE_ID_INTEL_82371AB_3) { pci_config_writeb(bdf, 0x80, 0x01); ... return; } } }
-Kevin