[Qemu-devel] [PATCH] acpi: initialize s4_val used in s4 shutdown

[Qemu-devel] [PATCH] acpi: initialize s4_val used in s4 shutdown

[Bruce Rogers]

While investigating why a 32 bit Windows 2003 guest wasn't able to
successfully perform a shutdown /h, it was discovered that commit
afafe4bbe0cf7d3318e1ac7b40925561f86a6bd4 inadvertently dropped the
initialization of the s4_val used to handle s4 shutdown.
Initialize the value as before.

Signed-off-by: Bruce Rogers <brogers@suse.com>
---
 hw/acpi.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/hw/acpi.c b/hw/acpi.c
index 53e47d5..26b67a8 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -474,6 +474,7 @@ static const MemoryRegionOps acpi_pm_cnt_ops = {
 
 void acpi_pm1_cnt_init(ACPIREGS *ar, MemoryRegion *parent)
 {
+    ar->pm1.cnt.s4_val = 2;
     ar->wakeup.notify = acpi_notify_wakeup;
     qemu_register_wakeup_notifier(&ar->wakeup);
     memory_region_init_io(&ar->pm1.cnt.io, &acpi_pm_cnt_ops, ar, "acpi-cnt", 2);
-- 
1.7.7

Re: [Qemu-devel] [PATCH] acpi: initialize s4_val used in s4 shutdown

[Gerd Hoffmann]

On 04/01/13 18:57, Bruce Rogers wrote:
> While investigating why a 32 bit Windows 2003 guest wasn't able to
> successfully perform a shutdown /h, it was discovered that commit
> afafe4bbe0cf7d3318e1ac7b40925561f86a6bd4 inadvertently dropped the
> initialization of the s4_val used to handle s4 shutdown.
> Initialize the value as before.

> --- a/hw/acpi.c
> +++ b/hw/acpi.c
> @@ -474,6 +474,7 @@ static const MemoryRegionOps acpi_pm_cnt_ops = {
>  
>  void acpi_pm1_cnt_init(ACPIREGS *ar, MemoryRegion *parent)
>  {
> +    ar->pm1.cnt.s4_val = 2;

The '2' used to come from PIIX4PMState->s4_val before commit
afafe4bbe0cf7d3318e1ac7b40925561f86a6bd4, see piix4_pm_properties, this
behavior should be maintained IMHO.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] acpi: initialize s4_val used in s4 shutdown

[Bruce Rogers]

 >>> On 4/2/2013 at 04:03 AM, Gerd Hoffmann <kraxel@redhat.com> wrote: 
> On 04/01/13 18:57, Bruce Rogers wrote:
>> While investigating why a 32 bit Windows 2003 guest wasn't able to
>> successfully perform a shutdown /h, it was discovered that commit
>> afafe4bbe0cf7d3318e1ac7b40925561f86a6bd4 inadvertently dropped the
>> initialization of the s4_val used to handle s4 shutdown.
>> Initialize the value as before.
> 
>> --- a/hw/acpi.c
>> +++ b/hw/acpi.c
>> @@ -474,6 +474,7 @@ static const MemoryRegionOps acpi_pm_cnt_ops = {
>>  
>>  void acpi_pm1_cnt_init(ACPIREGS *ar, MemoryRegion *parent)
>>  {
>> +    ar->pm1.cnt.s4_val = 2;
> 
> The '2' used to come from PIIX4PMState->s4_val before commit
> afafe4bbe0cf7d3318e1ac7b40925561f86a6bd4, see piix4_pm_properties, this
> behavior should be maintained IMHO.

OK, I'll send out another patch with another parameter to acpi_pm1_cnt_init to
use to init the s4 value, pass in PIIX4PMState->s4_val for the piix4 case, and simply
pass a 2 in the ich9 and vt82c686 cases then. It looks like ich9 and vt82c686 are
not fully set up for doing s4 shutdown from what I can see.

Bruce