From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48254) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YAdh5-00026e-GJ for qemu-devel@nongnu.org; Mon, 12 Jan 2015 07:02:04 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YAdh0-0007Af-D9 for qemu-devel@nongnu.org; Mon, 12 Jan 2015 07:02:03 -0500 Received: from mx1.redhat.com ([209.132.183.28]:48524) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YAdh0-0007AY-5k for qemu-devel@nongnu.org; Mon, 12 Jan 2015 07:01:58 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t0CC1vLV010883 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Mon, 12 Jan 2015 07:01:57 -0500 Message-ID: <54B3B7B2.9090000@redhat.com> Date: Mon, 12 Jan 2015 14:01:54 +0200 From: Marcel Apfelbaum MIME-Version: 1.0 References: In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 1/1] ich9: add disable_s3, disable_s4, s4_val properties List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Amit Shah , qemu list Cc: Paolo Bonzini , Igor Mammedov , "Michael S. Tsirkin" On 01/12/2015 02:00 PM, Amit Shah wrote: > PIIX4 has disable_s3 and disable_s4 properties to enable or disable PM > functions. Add such properties to the ICH9 chipset as well for the Q35 > machine type. > > S3 / S4 are not guaranteed to always work (needs work in the guest as > well as QEMU for things to work properly), and disabling advertising of > these features ensures guests don't go into zombie state if something > isn't working right. > > The defaults are kept the same as in PIIX4: both S3 and S4 are enabled > by default. > > These can be disabled via the cmdline: > > ... -global ICH9-LPC.disable_s3=1 -global ICH9-LPC.disable_s4=1 > > Note: some guests can fake hibernation by writing a hibernate image and > doing a shutdown instead of S4 if S4 isn't available; there's nothing we > can do guests to stop doing this, and this patch can't affect that > functionality. > > Signed-off-by: Amit Shah > > --- > v2: * Use s4_val in ich9_pm_init(); else a reboot will end up using > the default value insted of the set value. (Marcel) > * Fix commit msg, add FYI note for guests faking s4. > --- > hw/acpi/ich9.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++++- > include/hw/acpi/ich9.h | 4 +++ > 2 files changed, 101 insertions(+), 1 deletion(-) > > diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c > index ea991a3..e4195ea 100644 > --- a/hw/acpi/ich9.c > +++ b/hw/acpi/ich9.c > @@ -219,7 +219,7 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm, > > acpi_pm_tmr_init(&pm->acpi_regs, ich9_pm_update_sci_fn, &pm->io); > acpi_pm1_evt_init(&pm->acpi_regs, ich9_pm_update_sci_fn, &pm->io); > - acpi_pm1_cnt_init(&pm->acpi_regs, &pm->io, 2); > + acpi_pm1_cnt_init(&pm->acpi_regs, &pm->io, pm->s4_val); > > acpi_gpe_init(&pm->acpi_regs, ICH9_PMIO_GPE0_LEN); > memory_region_init_io(&pm->io_gpe, OBJECT(lpc_pci), &ich9_gpe_ops, pm, > @@ -269,10 +269,94 @@ static void ich9_pm_set_memory_hotplug_support(Object *obj, bool value, > s->pm.acpi_memory_hotplug.is_enabled = value; > } > > +static void ich9_pm_get_disable_s3(Object *obj, Visitor *v, > + void *opaque, const char *name, > + Error **errp) > +{ > + ICH9LPCPMRegs *pm = opaque; > + uint8_t value = pm->disable_s3; > + > + visit_type_uint8(v, &value, name, errp); > +} > + > +static void ich9_pm_set_disable_s3(Object *obj, Visitor *v, > + void *opaque, const char *name, > + Error **errp) > +{ > + ICH9LPCPMRegs *pm = opaque; > + Error *local_err = NULL; > + uint8_t value; > + > + visit_type_uint8(v, &value, name, &local_err); > + if (local_err) { > + goto out; > + } > + pm->disable_s3 = value; > +out: > + error_propagate(errp, local_err); > +} > + > +static void ich9_pm_get_disable_s4(Object *obj, Visitor *v, > + void *opaque, const char *name, > + Error **errp) > +{ > + ICH9LPCPMRegs *pm = opaque; > + uint8_t value = pm->disable_s4; > + > + visit_type_uint8(v, &value, name, errp); > +} > + > +static void ich9_pm_set_disable_s4(Object *obj, Visitor *v, > + void *opaque, const char *name, > + Error **errp) > +{ > + ICH9LPCPMRegs *pm = opaque; > + Error *local_err = NULL; > + uint8_t value; > + > + visit_type_uint8(v, &value, name, &local_err); > + if (local_err) { > + goto out; > + } > + pm->disable_s4 = value; > +out: > + error_propagate(errp, local_err); > +} > + > +static void ich9_pm_get_s4_val(Object *obj, Visitor *v, > + void *opaque, const char *name, > + Error **errp) > +{ > + ICH9LPCPMRegs *pm = opaque; > + uint8_t value = pm->s4_val; > + > + visit_type_uint8(v, &value, name, errp); > +} > + > +static void ich9_pm_set_s4_val(Object *obj, Visitor *v, > + void *opaque, const char *name, > + Error **errp) > +{ > + ICH9LPCPMRegs *pm = opaque; > + Error *local_err = NULL; > + uint8_t value; > + > + visit_type_uint8(v, &value, name, &local_err); > + if (local_err) { > + goto out; > + } > + pm->s4_val = value; > +out: > + error_propagate(errp, local_err); > +} > + > void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm, Error **errp) > { > static const uint32_t gpe0_len = ICH9_PMIO_GPE0_LEN; > pm->acpi_memory_hotplug.is_enabled = true; > + pm->disable_s3 = 0; > + pm->disable_s4 = 0; > + pm->s4_val = 2; > > object_property_add_uint32_ptr(obj, ACPI_PM_PROP_PM_IO_BASE, > &pm->pm_io_base, errp); > @@ -285,6 +369,18 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm, Error **errp) > ich9_pm_get_memory_hotplug_support, > ich9_pm_set_memory_hotplug_support, > NULL); > + object_property_add(obj, ACPI_PM_PROP_S3_DISABLED, "uint8", > + ich9_pm_get_disable_s3, > + ich9_pm_set_disable_s3, > + NULL, pm, NULL); > + object_property_add(obj, ACPI_PM_PROP_S4_DISABLED, "uint8", > + ich9_pm_get_disable_s4, > + ich9_pm_set_disable_s4, > + NULL, pm, NULL); > + object_property_add(obj, ACPI_PM_PROP_S4_VAL, "uint8", > + ich9_pm_get_s4_val, > + ich9_pm_set_s4_val, > + NULL, pm, NULL); > } > > void ich9_pm_device_plug_cb(ICH9LPCPMRegs *pm, DeviceState *dev, Error **errp) > diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h > index fe975e6..12d7a7a 100644 > --- a/include/hw/acpi/ich9.h > +++ b/include/hw/acpi/ich9.h > @@ -49,6 +49,10 @@ typedef struct ICH9LPCPMRegs { > AcpiCpuHotplug gpe_cpu; > > MemHotplugState acpi_memory_hotplug; > + > + uint8_t disable_s3; > + uint8_t disable_s4; > + uint8_t s4_val; > } ICH9LPCPMRegs; > > void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm, > Reviewed-by: Marcel Apfelbaum Thanks, Marcel