All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 1/1] ich9: add disable_s3, disable_s4, s4_val properties
@ 2015-01-12 12:00 Amit Shah
  2015-01-12 12:01 ` Marcel Apfelbaum
  0 siblings, 1 reply; 2+ messages in thread
From: Amit Shah @ 2015-01-12 12:00 UTC (permalink / raw)
  To: qemu list
  Cc: Marcel Apfelbaum, Paolo Bonzini, Igor Mammedov, Amit Shah,
	Michael S. Tsirkin

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 <amit.shah@redhat.com>

---
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,
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [Qemu-devel] [PATCH v2 1/1] ich9: add disable_s3, disable_s4, s4_val properties
  2015-01-12 12:00 [Qemu-devel] [PATCH v2 1/1] ich9: add disable_s3, disable_s4, s4_val properties Amit Shah
@ 2015-01-12 12:01 ` Marcel Apfelbaum
  0 siblings, 0 replies; 2+ messages in thread
From: Marcel Apfelbaum @ 2015-01-12 12:01 UTC (permalink / raw)
  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 <amit.shah@redhat.com>
>
> ---
> 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 <marcel@redhat.com>

Thanks,
Marcel

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2015-01-12 12:02 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-12 12:00 [Qemu-devel] [PATCH v2 1/1] ich9: add disable_s3, disable_s4, s4_val properties Amit Shah
2015-01-12 12:01 ` Marcel Apfelbaum

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.