From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55196) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y1ELS-00036U-Ol for qemu-devel@nongnu.org; Wed, 17 Dec 2014 08:08:59 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y1ELI-0000cc-UE for qemu-devel@nongnu.org; Wed, 17 Dec 2014 08:08:50 -0500 Received: from mail-wi0-x22b.google.com ([2a00:1450:400c:c05::22b]:48815) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y1ELI-0000cJ-Jz for qemu-devel@nongnu.org; Wed, 17 Dec 2014 08:08:40 -0500 Received: by mail-wi0-f171.google.com with SMTP id bs8so15621894wib.16 for ; Wed, 17 Dec 2014 05:08:39 -0800 (PST) Message-ID: <54918054.8070500@gmail.com> Date: Wed, 17 Dec 2014 15:08:36 +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 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 , "Michael S. Tsirkin" , Igor Mammedov On 12/16/2014 01:23 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 Maybe we can add them to Q35 Machines instead? - machine q35,disable_s3=1,disable_s4=1 We have built-in support now. Thanks, Marcel > > Signed-off-by: Amit Shah > --- > hw/acpi/ich9.c | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++ > include/hw/acpi/ich9.h | 4 +++ > 2 files changed, 100 insertions(+) > > diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c > index ea991a3..98828f5 100644 > --- a/hw/acpi/ich9.c > +++ b/hw/acpi/ich9.c > @@ -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, >