Re: [Qemu-devel] [PATCHv3] Add PIIX4 properties to control PM system states.

[Gleb Natapov]

On Wed, Jun 13, 2012 at 04:23:28PM +0300, Gleb Natapov wrote:
> Ping?
> 
Ping 2?

> On Mon, Jun 04, 2012 at 02:31:55PM +0300, Gleb Natapov wrote:
> > This patch adds two things. First it allows QEMU to distinguish between
> > regular powerdown and S4 powerdown. Later separate QMP notification will
> > be added for S4 powerdown. Second it allows S3/S4 states to be disabled
> > from QEMU command line. Some guests known to be broken with regards to
> > power management, but allow to use it anyway. Using new properties
> > management will be able to disable S3/S4 for such guests.
> > 
> > Supported system state are passed to a firmware using new fw_cfg file.
> > The file contains  6 byte array. Each byte represents one system
> > state. If byte at offset X has its MSB set it means that system state
> > X is supported and to enter it guest should use the value from lowest 3
> > bits.
> > 
> > Signed-off-by: Gleb Natapov <address@hidden>
> > ---
> >  hw/acpi.c       |    5 ++++-
> >  hw/acpi.h       |    2 +-
> >  hw/acpi_piix4.c |   20 ++++++++++++++++++--
> >  hw/mips_malta.c |    2 +-
> >  hw/pc.c         |    3 ++-
> >  hw/pc.h         |    4 ++--
> >  hw/pc_piix.c    |    5 +++--
> >  hw/vt82c686.c   |    2 +-
> >  8 files changed, 32 insertions(+), 11 deletions(-)
> > 
> > diff --git a/hw/acpi.c b/hw/acpi.c
> > index 5d521e5..effc7ec 100644
> > --- a/hw/acpi.c
> > +++ b/hw/acpi.c
> > @@ -370,7 +370,7 @@ void acpi_pm1_cnt_init(ACPIREGS *ar)
> >      qemu_register_wakeup_notifier(&ar->wakeup);
> >  }
> >  
> > -void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val)
> > +void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val, char s4)
> >  {
> >      ar->pm1.cnt.cnt = val & ~(ACPI_BITMASK_SLEEP_ENABLE);
> >  
> > @@ -385,6 +385,9 @@ void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val)
> >              qemu_system_suspend_request();
> >              break;
> >          default:
> > +            if (sus_typ == s4) { /* S4 request */
> > +                qemu_system_shutdown_request();
> > +            }
> >              break;
> >          }
> >      }
> > diff --git a/hw/acpi.h b/hw/acpi.h
> > index fe8cdb4..7337f41 100644
> > --- a/hw/acpi.h
> > +++ b/hw/acpi.h
> > @@ -139,7 +139,7 @@ void acpi_pm1_evt_reset(ACPIREGS *ar);
> >  
> >  /* PM1a_CNT: piix and ich9 don't implement PM1b CNT. */
> >  void acpi_pm1_cnt_init(ACPIREGS *ar);
> > -void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val);
> > +void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val, char s4);
> >  void acpi_pm1_cnt_update(ACPIREGS *ar,
> >                           bool sci_enable, bool sci_disable);
> >  void acpi_pm1_cnt_reset(ACPIREGS *ar);
> > diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
> > index 0345490..812e62f 100644
> > --- a/hw/acpi_piix4.c
> > +++ b/hw/acpi_piix4.c
> > @@ -27,6 +27,7 @@
> >  #include "sysemu.h"
> >  #include "range.h"
> >  #include "ioport.h"
> > +#include "fw_cfg.h"
> >  
> >  //#define DEBUG
> >  
> > @@ -71,6 +72,10 @@ typedef struct PIIX4PMState {
> >      struct pci_status pci0_status;
> >      uint32_t pci0_hotplug_enable;
> >      uint32_t pci0_slot_device_present;
> > +
> > +    uint8_t disable_s3;
> > +    uint8_t disable_s4;
> > +    uint8_t s4_val;
> >  } PIIX4PMState;
> >  
> >  static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s);
> > @@ -123,7 +128,7 @@ static void pm_ioport_write(IORange *ioport, uint64_t 
> > addr, unsigned width,
> >          pm_update_sci(s);
> >          break;
> >      case 0x04:
> > -        acpi_pm1_cnt_write(&s->ar, val);
> > +        acpi_pm1_cnt_write(&s->ar, val, s->s4_val);
> >          break;
> >      default:
> >          break;
> > @@ -422,7 +427,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 smi_irq,
> > -                       int kvm_enabled)
> > +                       int kvm_enabled, void *fw_cfg)
> >  {
> >      PCIDevice *dev;
> >      PIIX4PMState *s;
> > @@ -438,11 +443,22 @@ i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, 
> > uint32_t smb_io_base,
> >  
> >      qdev_init_nofail(&dev->qdev);
> >  
> > +    if (fw_cfg) {
> > +        uint8_t suspend[6] = {128, 0, 0, 129, 128, 128};
> > +        suspend[3] = 1 | ((!s->disable_s3) << 7);
> > +        suspend[4] = s->s4_val | ((!s->disable_s4) << 7);
> > +
> > +        fw_cfg_add_file(fw_cfg, "etc/system-states", g_memdup(suspend, 6), 
> > 6);
> > +    }
> > +
> >      return s->smb.smbus;
> >  }
> >  
> >  static Property piix4_pm_properties[] = {
> >      DEFINE_PROP_UINT32("smb_io_base", PIIX4PMState, smb_io_base, 0),
> > +    DEFINE_PROP_UINT8("disable_s3", PIIX4PMState, disable_s3, 0),
> > +    DEFINE_PROP_UINT8("disable_s4", PIIX4PMState, disable_s4, 0),
> > +    DEFINE_PROP_UINT8("s4_val", PIIX4PMState, s4_val, 2),
> >      DEFINE_PROP_END_OF_LIST(),
> >  };
> >  
> > diff --git a/hw/mips_malta.c b/hw/mips_malta.c
> > index 4752bb2..205b4a5 100644
> > --- a/hw/mips_malta.c
> > +++ b/hw/mips_malta.c
> > @@ -954,7 +954,7 @@ void mips_malta_init (ram_addr_t ram_size,
> >      pci_piix4_ide_init(pci_bus, hd, piix4_devfn + 1);
> >      pci_create_simple(pci_bus, piix4_devfn + 2, "piix4-usb-uhci");
> >      smbus = piix4_pm_init(pci_bus, piix4_devfn + 3, 0x1100,
> > -                          isa_get_irq(NULL, 9), NULL, 0);
> > +                          isa_get_irq(NULL, 9), NULL, 0, NULL);
> >      /* TODO: Populate SPD eeprom data.  */
> >      smbus_eeprom_init(smbus, 8, NULL, 0);
> >      pit = pit_init(isa_bus, 0x40, 0, NULL);
> > diff --git a/hw/pc.c b/hw/pc.c
> > index e81a06c..e7d65b4 100644
> > --- a/hw/pc.c
> > +++ b/hw/pc.c
> > @@ -976,7 +976,7 @@ void pc_cpus_init(const char *cpu_model)
> >      }
> >  }
> >  
> > -void pc_memory_init(MemoryRegion *system_memory,
> > +void *pc_memory_init(MemoryRegion *system_memory,
> >                      const char *kernel_filename,
> >                      const char *kernel_cmdline,
> >                      const char *initrd_filename,
> > @@ -1035,6 +1035,7 @@ void pc_memory_init(MemoryRegion *system_memory,
> >      for (i = 0; i < nb_option_roms; i++) {
> >          rom_add_option(option_rom[i].name, option_rom[i].bootindex);
> >      }
> > +    return fw_cfg;
> >  }
> >  
> >  qemu_irq *pc_allocate_cpu_irq(void)
> > diff --git a/hw/pc.h b/hw/pc.h
> > index 74d3369..31ccb6f 100644
> > --- a/hw/pc.h
> > +++ b/hw/pc.h
> > @@ -106,7 +106,7 @@ void pc_register_ferr_irq(qemu_irq irq);
> >  void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
> >  
> >  void pc_cpus_init(const char *cpu_model);
> > -void pc_memory_init(MemoryRegion *system_memory,
> > +void *pc_memory_init(MemoryRegion *system_memory,
> >                      const char *kernel_filename,
> >                      const char *kernel_cmdline,
> >                      const char *initrd_filename,
> > @@ -142,7 +142,7 @@ int acpi_table_add(const char *table_desc);
> >  
> >  i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
> >                         qemu_irq sci_irq, qemu_irq smi_irq,
> > -                       int kvm_enabled);
> > +                       int kvm_enabled, void *fw_cfg);
> >  void piix4_smbus_register_device(SMBusDevice *dev, uint8_t addr);
> >  
> >  /* hpet.c */
> > diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> > index a7aad4b..2848af9 100644
> > --- a/hw/pc_piix.c
> > +++ b/hw/pc_piix.c
> > @@ -150,6 +150,7 @@ static void pc_init1(MemoryRegion *system_memory,
> >      MemoryRegion *ram_memory;
> >      MemoryRegion *pci_memory;
> >      MemoryRegion *rom_memory;
> > +    void *fw_cfg = NULL;
> >  
> >      pc_cpus_init(cpu_model);
> >  
> > @@ -176,7 +177,7 @@ static void pc_init1(MemoryRegion *system_memory,
> >  
> >      /* allocate ram and load rom/bios */
> >      if (!xen_enabled()) {
> > -        pc_memory_init(system_memory,
> > +        fw_cfg = pc_memory_init(system_memory,
> >                         kernel_filename, kernel_cmdline, initrd_filename,
> >                         below_4g_mem_size, above_4g_mem_size,
> >                         pci_enabled ? rom_memory : system_memory, 
> > &ram_memory);
> > @@ -280,7 +281,7 @@ static void pc_init1(MemoryRegion *system_memory,
> >          /* TODO: Populate SPD eeprom data.  */
> >          smbus = piix4_pm_init(pci_bus, piix3_devfn + 3, 0xb100,
> >                                gsi[9], *smi_irq,
> > -                              kvm_enabled());
> > +                              kvm_enabled(), fw_cfg);
> >          smbus_eeprom_init(smbus, 8, NULL, 0);
> >      }
> >  
> > diff --git a/hw/vt82c686.c b/hw/vt82c686.c
> > index 6fb7950..5d7c00c 100644
> > --- a/hw/vt82c686.c
> > +++ b/hw/vt82c686.c
> > @@ -210,7 +210,7 @@ static void pm_ioport_writew(void *opaque, uint32_t 
> > addr, uint32_t val)
> >          pm_update_sci(s);
> >          break;
> >      case 0x04:
> > -        acpi_pm1_cnt_write(&s->ar, val);
> > +        acpi_pm1_cnt_write(&s->ar, val, 0);
> >          break;
> >      default:
> >          break;
> > -- 
> > 1.7.7.3
> > 
> 
> --
>                       Gleb.

--
                        Gleb.