Re: [Qemu-devel] [PATCH v3 2/6] suspend: switch acpi s3 to new infrastructure.

[Gleb Natapov <gleb@redhat.com>]

On Wed, Feb 08, 2012 at 12:00:15PM +0100, Gerd Hoffmann wrote:
> This patch switches pc s3 suspend over to the new infrastructure.
> The cmos_s3 qemu_irq is killed, the new notifier is used instead.
> The xen hack goes away with that too, the hypercall can simply be
> done in a notifier function now.
> 
> This patch also makes the guest actually stay suspended instead
> of leaving suspend instantly, so it is useful for more than just
> testing whenever the suspend/resume cycle actually works.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/acpi.c        |   11 +----------
>  hw/acpi.h        |    2 --
>  hw/acpi_piix4.c  |    3 +--
>  hw/mc146818rtc.c |   12 ++++++++++++
>  hw/mips_malta.c  |    2 +-
>  hw/pc.c          |   11 -----------
>  hw/pc.h          |    3 +--
>  hw/pc_piix.c     |    8 +-------
>  hw/vt82c686.c    |    1 -
>  xen-all.c        |   11 ++++++-----
>  10 files changed, 23 insertions(+), 41 deletions(-)
> 
> diff --git a/hw/acpi.c b/hw/acpi.c
> index 79b179b..67dcd43 100644
> --- a/hw/acpi.c
> +++ b/hw/acpi.c
> @@ -330,11 +330,6 @@ void acpi_pm_tmr_reset(ACPIPMTimer *tmr)
>  }
>  
>  /* ACPI PM1aCNT */
> -void acpi_pm1_cnt_init(ACPIPM1CNT *pm1_cnt, qemu_irq cmos_s3)
> -{
> -    pm1_cnt->cmos_s3 = cmos_s3;
> -}
> -
>  void acpi_pm1_cnt_write(ACPIPM1EVT *pm1a, ACPIPM1CNT *pm1_cnt, uint16_t val)
>  {
>      pm1_cnt->cnt = val & ~(ACPI_BITMASK_SLEEP_ENABLE);
> @@ -351,8 +346,7 @@ void acpi_pm1_cnt_write(ACPIPM1EVT *pm1a, ACPIPM1CNT *pm1_cnt, uint16_t val)
>                 Pretend that resume was caused by power button */
>              pm1a->sts |=
>                  (ACPI_BITMASK_WAKE_STATUS | ACPI_BITMASK_POWER_BUTTON_STATUS);
Here we should report real reason for a wakeup (if it can be reported in
mp1sts that is).

> -            qemu_system_reset_request();
> -            qemu_irq_raise(pm1_cnt->cmos_s3);
> +            qemu_system_suspend_request();
>          default:
>              break;
>          }
> @@ -373,9 +367,6 @@ void acpi_pm1_cnt_update(ACPIPM1CNT *pm1_cnt,
>  void acpi_pm1_cnt_reset(ACPIPM1CNT *pm1_cnt)
>  {
>      pm1_cnt->cnt = 0;
> -    if (pm1_cnt->cmos_s3) {
> -        qemu_irq_lower(pm1_cnt->cmos_s3);
> -    }
>  }
>  
>  /* ACPI GPE */
> diff --git a/hw/acpi.h b/hw/acpi.h
> index c141e65..e0c7dbe 100644
> --- a/hw/acpi.h
> +++ b/hw/acpi.h
> @@ -115,8 +115,6 @@ void acpi_pm1_evt_reset(ACPIPM1EVT *pm1);
>  /* PM1a_CNT: piix and ich9 don't implement PM1b CNT. */
>  struct ACPIPM1CNT {
>      uint16_t cnt;
> -
> -    qemu_irq cmos_s3;
>  };
>  typedef struct ACPIPM1CNT ACPIPM1CNT;
>  
> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
> index 21484ae..bc77dc5 100644
> --- a/hw/acpi_piix4.c
> +++ b/hw/acpi_piix4.c
> @@ -376,7 +376,7 @@ static int piix4_pm_initfn(PCIDevice *dev)
>  }
>  
>  i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
> -                       qemu_irq sci_irq, qemu_irq cmos_s3, qemu_irq smi_irq,
> +                       qemu_irq sci_irq, qemu_irq smi_irq,
>                         int kvm_enabled)
>  {
>      PCIDevice *dev;
> @@ -387,7 +387,6 @@ i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
>  
>      s = DO_UPCAST(PIIX4PMState, dev, dev);
>      s->irq = sci_irq;
> -    acpi_pm1_cnt_init(&s->pm1_cnt, cmos_s3);
>      s->smi_irq = smi_irq;
>      s->kvm_enabled = kvm_enabled;
>  
> diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
> index 4a43225..314ed52 100644
> --- a/hw/mc146818rtc.c
> +++ b/hw/mc146818rtc.c
> @@ -102,6 +102,7 @@ typedef struct RTCState {
>      QEMUTimer *second_timer2;
>      Notifier clock_reset_notifier;
>      LostTickPolicy lost_tick_policy;
> +    Notifier suspend_notifier;
>  } RTCState;
>  
>  static void rtc_set_time(RTCState *s);
> @@ -596,6 +597,14 @@ static void rtc_notify_clock_reset(Notifier *notifier, void *data)
>  #endif
>  }
>  
> +/* set CMOS shutdown status register (index 0xF) as S3_resume(0xFE)
> +   BIOS will read it and start S3 resume at POST Entry */
> +static void rtc_notify_suspend(Notifier *notifier, void *data)
> +{
> +    RTCState *s = container_of(notifier, RTCState, suspend_notifier);
> +    rtc_set_memory(&s->dev, 0xF, 0xFE);
> +}
> +
>  static void rtc_reset(void *opaque)
>  {
>      RTCState *s = opaque;
> @@ -676,6 +685,9 @@ static int rtc_initfn(ISADevice *dev)
>      s->clock_reset_notifier.notify = rtc_notify_clock_reset;
>      qemu_register_clock_reset_notifier(rtc_clock, &s->clock_reset_notifier);
>  
> +    s->suspend_notifier.notify = rtc_notify_suspend;
> +    qemu_register_suspend_notifier(&s->suspend_notifier);
> +
>      s->next_second_time =
>          qemu_get_clock_ns(rtc_clock) + (get_ticks_per_sec() * 99) / 100;
>      qemu_mod_timer(s->second_timer2, s->next_second_time);
> diff --git a/hw/mips_malta.c b/hw/mips_malta.c
> index d232630..fe02c93 100644
> --- a/hw/mips_malta.c
> +++ b/hw/mips_malta.c
> @@ -966,7 +966,7 @@ void mips_malta_init (ram_addr_t ram_size,
>      pci_piix4_ide_init(pci_bus, hd, piix4_devfn + 1);
>      usb_uhci_piix4_init(pci_bus, piix4_devfn + 2);
>      smbus = piix4_pm_init(pci_bus, piix4_devfn + 3, 0x1100,
> -                          isa_get_irq(NULL, 9), NULL, NULL, 0);
> +                          isa_get_irq(NULL, 9), NULL, 0);
>      /* TODO: Populate SPD eeprom data.  */
>      smbus_eeprom_init(smbus, 8, NULL, 0);
>      pit = pit_init(isa_bus, 0x40, 0);
> diff --git a/hw/pc.c b/hw/pc.c
> index 7f3aa65..7b93aee 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -915,17 +915,6 @@ static DeviceState *apic_init(void *env, uint8_t apic_id)
>      return dev;
>  }
>  
> -/* set CMOS shutdown status register (index 0xF) as S3_resume(0xFE)
> -   BIOS will read it and start S3 resume at POST Entry */
> -void pc_cmos_set_s3_resume(void *opaque, int irq, int level)
> -{
> -    ISADevice *s = opaque;
> -
> -    if (level) {
> -        rtc_set_memory(s, 0xF, 0xFE);
> -    }
> -}
> -
>  void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
>  {
>      CPUState *s = opaque;
> diff --git a/hw/pc.h b/hw/pc.h
> index c666ec9..571c915 100644
> --- a/hw/pc.h
> +++ b/hw/pc.h
> @@ -128,7 +128,6 @@ void i8042_setup_a20_line(ISADevice *dev, qemu_irq *a20_out);
>  extern int fd_bootchk;
>  
>  void pc_register_ferr_irq(qemu_irq irq);
> -void pc_cmos_set_s3_resume(void *opaque, int irq, int level);
>  void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
>  
>  void pc_cpus_init(const char *cpu_model);
> @@ -167,7 +166,7 @@ int acpi_table_add(const char *table_desc);
>  /* acpi_piix.c */
>  
>  i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
> -                       qemu_irq sci_irq, qemu_irq cmos_s3, qemu_irq smi_irq,
> +                       qemu_irq sci_irq, qemu_irq smi_irq,
>                         int kvm_enabled);
>  void piix4_smbus_register_device(SMBusDevice *dev, uint8_t addr);
>  
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index c06f1b5..918f5ac 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -139,7 +139,6 @@ static void pc_init1(MemoryRegion *system_memory,
>      qemu_irq *cpu_irq;
>      qemu_irq *gsi;
>      qemu_irq *i8259;
> -    qemu_irq *cmos_s3;
>      qemu_irq *smi_irq;
>      GSIState *gsi_state;
>      DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
> @@ -291,15 +290,10 @@ static void pc_init1(MemoryRegion *system_memory,
>      if (pci_enabled && acpi_enabled) {
>          i2c_bus *smbus;
>  
> -        if (!xen_enabled()) {
> -            cmos_s3 = qemu_allocate_irqs(pc_cmos_set_s3_resume, rtc_state, 1);
> -        } else {
> -            cmos_s3 = qemu_allocate_irqs(xen_cmos_set_s3_resume, rtc_state, 1);
> -        }
>          smi_irq = qemu_allocate_irqs(pc_acpi_smi_interrupt, first_cpu, 1);
>          /* TODO: Populate SPD eeprom data.  */
>          smbus = piix4_pm_init(pci_bus, piix3_devfn + 3, 0xb100,
> -                              gsi[9], *cmos_s3, *smi_irq,
> +                              gsi[9], *smi_irq,
>                                kvm_enabled());
>          smbus_eeprom_init(smbus, 8, NULL, 0);
>      }
> diff --git a/hw/vt82c686.c b/hw/vt82c686.c
> index aa0954f..8beb6c5 100644
> --- a/hw/vt82c686.c
> +++ b/hw/vt82c686.c
> @@ -446,7 +446,6 @@ static int vt82c686b_pm_initfn(PCIDevice *dev)
>      apm_init(&s->apm, NULL, s);
>  
>      acpi_pm_tmr_init(&s->tmr, pm_tmr_timer);
> -    acpi_pm1_cnt_init(&s->pm1_cnt, NULL);
>  
>      pm_smbus_init(&s->dev.qdev, &s->smb);
>  
> diff --git a/xen-all.c b/xen-all.c
> index fd39168..b0ed1ed 100644
> --- a/xen-all.c
> +++ b/xen-all.c
> @@ -89,6 +89,7 @@ typedef struct XenIOState {
>      const XenPhysmap *log_for_dirtybit;
>  
>      Notifier exit;
> +    Notifier suspend;
>  } XenIOState;
>  
>  /* Xen specific function for piix pci */
> @@ -121,12 +122,9 @@ void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int len)
>      }
>  }
>  
> -void xen_cmos_set_s3_resume(void *opaque, int irq, int level)
> +static void xen_suspend_notifier(Notifier *notifier, void *data)
>  {
> -    pc_cmos_set_s3_resume(opaque, irq, level);
> -    if (level) {
> -        xc_set_hvm_param(xen_xc, xen_domid, HVM_PARAM_ACPI_S_STATE, 3);
> -    }
> +    xc_set_hvm_param(xen_xc, xen_domid, HVM_PARAM_ACPI_S_STATE, 3);
>  }
>  
>  /* Xen Interrupt Controller */
> @@ -936,6 +934,9 @@ int xen_hvm_init(void)
>      state->exit.notify = xen_exit_notifier;
>      qemu_add_exit_notifier(&state->exit);
>  
> +    state->suspend.notify = xen_suspend_notifier;
> +    qemu_register_suspend_notifier(&state->suspend);
> +
>      xc_get_hvm_param(xen_xc, xen_domid, HVM_PARAM_IOREQ_PFN, &ioreq_pfn);
>      DPRINTF("shared page at pfn %lx\n", ioreq_pfn);
>      state->shared_page = xc_map_foreign_range(xen_xc, xen_domid, XC_PAGE_SIZE,
> -- 
> 1.7.1
> 
> 

--
			Gleb.