On Mon, Jan 13, 2014 at 11:31:20AM -0500, Kevin O'Connor wrote: > On Thu, Jan 02, 2014 at 07:00:45PM +0200, Marcel Apfelbaum wrote: > > On resume, the OS queries the power management event that > > caused it. In order to complete this task, it executes some > > reads to the piix pm io space. This all happens before the > > OS has a chance to restore the PCI config space for devices, > > so it is bios's responsibility to make sure the pm IO space > > is configured correctly. (During suspend, the piix pm > > configuration space is lost). > > > > Note: For 'ordinary' pci devices the config space is > > saved by the OS on sleep and restored on resume. > > > > Signed-off-by: Marcel Apfelbaum <[email protected]> > > --- > > This patch is based on Michael S. Tsirkin's patch: > > [SeaBIOS] [PATCH] seabios: call pci_init_device on resume > > If Michael agrees with it, I'll add him to signed-off section. > > > > Notes: > > - Without this patch the OS gets stuck because it tries repeatedly > > to read/write to pm io space, but the > > memory region is not enabled, so -1 is returned. > > - After resume the OS does not actually use the pm base address > > configured by the bios to get the IO ports, but uses > > the value from the ACPI FADT table actually. However, as a side effect > > of the configuration, the pm-io space is enabled by Qemu and > > the OS can continue the the boot sequence. > > - Bioses used for hardware like coreboot have the same init > > sequence for piix, see enable_pm from > > src/southbridge/intel/i82371eb/early_pm.c. > > Thanks. SeaBIOS isn't responsible for PCI setup on CSM/coreboot, so > the patch must check for CONFIG_QEMU. > > Also, I think we can simplify this a bit - how about the patch below > (untested)? > > -Kevin > > > diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c > index a24b8ff..1c90059 100644 > --- a/src/fw/pciinit.c > +++ b/src/fw/pciinit.c > @@ -230,10 +230,8 @@ static void apple_macio_setup(struct pci_device *pci, > void *arg) > pci_set_io_region_addr(pci, 0, 0x80800000, 0); > } > > -/* PIIX4 Power Management device (for ACPI) */ > -static void piix4_pm_setup(struct pci_device *pci, void *arg) > +static void piix4_pm_config_setup(u16 bdf) > { > - u16 bdf = pci->bdf; > // acpi sci is hardwired to 9 > pci_config_writeb(bdf, PCI_INTERRUPT_LINE, 9); > > @@ -241,6 +239,15 @@ static void piix4_pm_setup(struct pci_device *pci, void > *arg) > pci_config_writeb(bdf, 0x80, 0x01); /* enable PM io space */ > pci_config_writel(bdf, 0x90, PORT_SMB_BASE | 1); > pci_config_writeb(bdf, 0xd2, 0x09); /* enable SMBus io space */ > +} > + > +static int PiixPMBDF = -1; > + > +/* PIIX4 Power Management device (for ACPI) */ > +static void piix4_pm_setup(struct pci_device *pci, void *arg) > +{ > + PiixPMBDF = pci->bdf; > + piix4_pm_config_setup(pci->bdf); > > acpi_pm1a_cnt = PORT_ACPI_PM_BASE + 0x04; > pmtimer_setup(PORT_ACPI_PM_BASE + 0x08); > @@ -858,3 +865,12 @@ pci_setup(void) > > pci_enable_default_vga(); > } > + > +void > +pci_resume(void) > +{ > + if (!CONFIG_QEMU) > + return; > + if (PiixPMBDF >= 0)
so this does nothing unless piix4_pm_setup run but if it did why do we need to resume anything? > + piix4_pm_config_setup(PiixPMBDF); > +} > diff --git a/src/resume.c b/src/resume.c > index d69429c..0c0e60e 100644 > --- a/src/resume.c > +++ b/src/resume.c > @@ -100,6 +100,7 @@ s3_resume(void) > > pic_setup(); > smm_setup(); > + pci_resume(); > > s3_resume_vga(); > > diff --git a/src/util.h b/src/util.h > index 1b7d525..b8acbfa 100644 > --- a/src/util.h > +++ b/src/util.h > @@ -99,6 +99,7 @@ void mtrr_setup(void); > // fw/pciinit.c > extern const u8 pci_irqs[4]; > void pci_setup(void); > +void pci_resume(void); > > // fw/pirtable.c > extern struct pir_header *PirAddr; _______________________________________________ SeaBIOS mailing list [email protected] http://www.seabios.org/mailman/listinfo/seabios
