From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:56497) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SXBvs-0006xw-Ab for qemu-devel@nongnu.org; Wed, 23 May 2012 09:49:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SXBvl-0005Oe-LR for qemu-devel@nongnu.org; Wed, 23 May 2012 09:48:55 -0400 Received: from mail-qa0-f52.google.com ([209.85.216.52]:60540) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SXBvl-0005OU-Fp for qemu-devel@nongnu.org; Wed, 23 May 2012 09:48:49 -0400 Received: by qabj34 with SMTP id j34so4304821qab.11 for ; Wed, 23 May 2012 06:48:48 -0700 (PDT) Message-ID: <4FBCEABD.2060905@codemonkey.ws> Date: Wed, 23 May 2012 08:48:45 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1337504561-20297-1-git-send-email-gleb@redhat.com> <1337504561-20297-2-git-send-email-gleb@redhat.com> <4FBCE5A5.7050308@codemonkey.ws> <20120523133420.GJ10209@redhat.com> In-Reply-To: <20120523133420.GJ10209@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/2] Add PIIX4 properties to control PM system states. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gleb Natapov Cc: qemu-devel@nongnu.org On 05/23/2012 08:34 AM, Gleb Natapov wrote: > On Wed, May 23, 2012 at 08:27:01AM -0500, Anthony Liguori wrote: >> On 05/20/2012 04:02 AM, 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 >> >> I see nothing wrong in principle here except that you should use a >> PTR property to pass the fw_cfg object to the ACPI PM device. >> > Hmm, need to check that we have fw_cfg handy when the device is created. Yes, the pc.c/pc_piix.c split needs to die... For now, just add another output parameter to pc_memory_init to return fw_cfg. Then you can pass it to piix4_pm_init(). Regards, Anthony Liguori > Or can I add it through global? > >> Regards, >> >> Anthony Liguori >> >>> --- >>> hw/acpi.c | 5 ++++- >>> hw/acpi.h | 2 +- >>> hw/acpi_piix4.c | 16 +++++++++++++++- >>> hw/vt82c686.c | 2 +- >>> 4 files changed, 21 insertions(+), 4 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 585da4e..883314d 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; >>> @@ -425,6 +430,7 @@ i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base, >>> { >>> PCIDevice *dev; >>> PIIX4PMState *s; >>> + uint8_t suspend[6] = {128, 0, 0, 129, 128, 128}; >>> >>> dev = pci_create(bus, devfn, "PIIX4_PM"); >>> qdev_prop_set_uint32(&dev->qdev, "smb_io_base", smb_io_base); >>> @@ -437,11 +443,19 @@ i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base, >>> >>> qdev_init_nofail(&dev->qdev); >>> >>> + suspend[3] = 1 | ((!s->disable_s3)<< 7); >>> + suspend[4] = s->s4_val | ((!s->disable_s4)<< 7); >>> + >>> + fw_cfg_add_file("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/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; > > -- > Gleb.