On Wed, 2013-12-18 at 19:20 +0200, Michael S. Tsirkin wrote:
On Wed, Dec 18, 2013 at 06:49:24PM +0200, Marcel Apfelbaum wrote:
On Wed, 2013-12-18 at 18:33 +0200, Michael S. Tsirkin wrote:
On Wed, Dec 18, 2013 at 06:27:12PM +0200, Marcel Apfelbaum wrote:
On Wed, 2013-12-18 at 17:22 +0200, Michael S. Tsirkin wrote:
On Wed, Dec 18, 2013 at 03:22:59PM +0100, Paolo Bonzini wrote:
Il 11/12/2013 10:21, Gal Hammer ha scritto: > Fix a bug that was introduced in commit c046e8c4. QEMU fails to > resume from suspend mode (S3). > > Signed-off-by: Gal Hammer ghammer@redhat.com > --- > hw/acpi/piix4.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c > index 93849c8..5c736a4 100644 > --- a/hw/acpi/piix4.c > +++ b/hw/acpi/piix4.c > @@ -376,7 +376,6 @@ static void piix4_reset(void *opaque) > pci_conf[0x5b] = 0; > > pci_conf[0x40] = 0x01; /* PM io base read only bit */ > - pci_conf[0x80] = 0; > > if (s->kvm_enabled) { > /* Mark SMM as already inited (until KVM supports SMM). */
Note this is not the APIC base address, that one is 80h on the ISA bridge (function 0). You're changing the behavior for 80h on the power management function, which is function 3. The register is "PMBA—POWER MANAGEMENT BASE ADDRESS" and it is indeed initialized by SeaBIOS in piix4_pm_setup (src/fw/pciinit.c).
Michael, perhaps a part of pci_setup (same file) should run on S3 resume?
Paolo
Seems reasonable: either seabios or guest OS must do it, and guest does not seem to.
I was looking into this today, but it seems that we have a problem. We cannot run pci_setup() in init section: .data.varinit.seabios/src/hw/pci.h.66 is VARVERIFY32INIT but used from ['.text.runtime.seabios/src/resume.c.150', '.text.pci_setup']
Any thoughts how to get around this? Thanks, Marcel
We defintely don't want to do full pci enumeration. Just pci_bios_init_platform or even less.
The problem still remains, we have to use pci_bios_init_device that in turn uses the PCIDevices list.
Thanks, Marcel
It does but it does not have to. We can use a chunk out of pci_probe_devices to initialize struct pci_device on stack.
Basically something like the below (warning: completely untested, sorry).
I tested and it works fine for both windows and linux guests! Also I agree this is the right thing to do after s3. Do you want to submit the patch? (or I can do it, no problem)
Thanks, Marcel
Seems much easier and more robust than building up the list of devices in memory.
Signed-off-by: Michael S. Tsirkin mst@redhat.com
diff --git a/src/hw/pci.h b/src/hw/pci.h index 9c7351d..a64f7c5 100644 --- a/src/hw/pci.h +++ b/src/hw/pci.h @@ -66,6 +66,7 @@ extern u64 pcimem64_start, pcimem64_end; extern struct hlist_head PCIDevices; extern int MaxPCIBus; int pci_probe_host(void); +void pci_probe_device(int bdf, struct pci_device *dev); void pci_probe_devices(void); static inline u32 pci_classprog(struct pci_device *pci) { return (pci->class << 8) | pci->prog_if; diff --git a/src/util.h b/src/util.h index e6a6cb5..a8c71a8 100644 --- a/src/util.h +++ b/src/util.h @@ -28,6 +28,7 @@ void boot_add_cbfs(void *data, const char *desc, int prio); void interactive_bootmenu(void); void bcv_prepboot(void); struct pci_device; +void pci_bios_init_device(struct pci_device *pci); int bootprio_find_pci_device(struct pci_device *pci); int bootprio_find_scsi_device(struct pci_device *pci, int target, int lun); int bootprio_find_ata_device(struct pci_device *pci, int chanid, int slave); diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c index 34279a4..a35b58d 100644 --- a/src/fw/pciinit.c +++ b/src/fw/pciinit.c @@ -293,7 +293,7 @@ static const struct pci_device_id pci_device_tbl[] = { PCI_DEVICE_END, };
-static void pci_bios_init_device(struct pci_device *pci) +void pci_bios_init_device(struct pci_device *pci) { u16 bdf = pci->bdf; dprintf(1, "PCI: init bdf=%02x:%02x.%x id=%04x:%04x\n" diff --git a/src/hw/pci.c b/src/hw/pci.c index 6c9aa81..d22804f 100644 --- a/src/hw/pci.c +++ b/src/hw/pci.c @@ -105,6 +105,24 @@ pci_probe_host(void) return 0; }
+void +pci_probe_device(int bdf, struct pci_device *dev) +{
- dev->bdf = bdf;
- u32 vendev = pci_config_readl(bdf, PCI_VENDOR_ID);
- dev->vendor = vendev & 0xffff;
- dev->device = vendev >> 16;
- u32 classrev = pci_config_readl(bdf, PCI_CLASS_REVISION);
- dev->class = classrev >> 16;
- dev->prog_if = classrev >> 8;
- dev->revision = classrev & 0xff;
- dev->header_type = pci_config_readb(bdf, PCI_HEADER_TYPE);
- u8 v = dev->header_type & 0x7f;
- if (v == PCI_HEADER_TYPE_BRIDGE || v == PCI_HEADER_TYPE_CARDBUS) {
u8 secbus = pci_config_readb(bdf, PCI_SECONDARY_BUS);dev->secondary_bus = secbus;- }
+} // Find all PCI devices and populate PCIDevices linked list. void pci_probe_devices(void) @@ -145,21 +163,12 @@ pci_probe_devices(void) }
// Populate pci_device info.
dev->bdf = bdf;
pci_probe_device(bdf, dev); dev->parent = parent; dev->rootbus = rootbus;
u32 vendev = pci_config_readl(bdf, PCI_VENDOR_ID);dev->vendor = vendev & 0xffff;dev->device = vendev >> 16;u32 classrev = pci_config_readl(bdf, PCI_CLASS_REVISION);dev->class = classrev >> 16;dev->prog_if = classrev >> 8;dev->revision = classrev & 0xff;dev->header_type = pci_config_readb(bdf, PCI_HEADER_TYPE); u8 v = dev->header_type & 0x7f; if (v == PCI_HEADER_TYPE_BRIDGE || v == PCI_HEADER_TYPE_CARDBUS) {u8 secbus = pci_config_readb(bdf, PCI_SECONDARY_BUS);dev->secondary_bus = secbus;
u8 secbus = dev->secondary_bus; if (secbus > bus && !busdevs[secbus]) busdevs[secbus] = dev; if (secbus > MaxPCIBus)diff --git a/src/resume.c b/src/resume.c index fc2fee9..f6c8b3b 100644 --- a/src/resume.c +++ b/src/resume.c @@ -101,6 +101,14 @@ s3_resume(void) pic_setup(); smm_setup();
int bdf;
foreachbdf(bdf, 0) {
struct pci_device pci;pci_probe_device(bdf, &pci);pci_bios_init_device(&pci);}
s3_resume_vga();
make_bios_readonly();