Re: [Qemu-devel] [Seabios] [PATCH v2] seabios: restore piix pm config registers after resume

[Michael S. Tsirkin]

On Wed, Jan 15, 2014 at 02:20:06PM +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 <address@hidden>

Acked-by: Michael S. Tsirkin <address@hidden>

> ---
> v1 -> v2:
>  Addressed Kevin O'Connor comments
>  - Simplified patch by remembering PM device bdf
> 
> 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.
> 
> 
>  src/fw/pciinit.c | 24 +++++++++++++++++++++---
>  src/hw/pci.h     |  1 +
>  src/resume.c     |  2 ++
>  src/util.h       |  1 +
>  4 files changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
> index a24b8ff..11c92ae 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);
> @@ -295,6 +302,17 @@ static const struct pci_device_id pci_device_tbl[] = {
>      PCI_DEVICE_END,
>  };
>  
> +void pci_resume(void)
> +{
> +    if (!CONFIG_QEMU) {
> +        return;
> +    }
> +
> +    if (PiixPmBDF >= 0) {
> +        piix4_pm_config_setup(PiixPmBDF);
> +    }
> +}
> +
>  static void pci_bios_init_device(struct pci_device *pci)
>  {
>      u16 bdf = pci->bdf;
> diff --git a/src/hw/pci.h b/src/hw/pci.h
> index 9c7351d..17689a8 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_resume(void);
>  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/resume.c b/src/resume.c
> index d69429c..9ad2e4f 100644
> --- a/src/resume.c
> +++ b/src/resume.c
> @@ -101,6 +101,8 @@ s3_resume(void)
>      pic_setup();
>      smm_setup();
>  
> +    pci_resume();
> +
>      s3_resume_vga();
>  
>      make_bios_readonly();
> diff --git a/src/util.h b/src/util.h
> index 1b7d525..3f53f4d 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_resume(void);
>  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);
> -- 
> 1.8.3.1