[PATCH] acpi_piix4: remove bad save/restore of cpus_sts

[PATCH] acpi_piix4: remove bad save/restore of cpus_sts

[Avi Kivity]

Introduced by a mismerge and segfaults immediately.  Still need proper
save/restore of this field.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 hw/acpi.h       |    1 -
 hw/acpi_piix4.c |    1 -
 2 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/hw/acpi.h b/hw/acpi.h
index 96f1d5f..c141e65 100644
--- a/hw/acpi.h
+++ b/hw/acpi.h
@@ -133,7 +133,6 @@ struct ACPIGPE {
 
     uint8_t *sts;
     uint8_t *en;
-    uint8_t *cpu_sts;
 };
 typedef struct ACPIGPE ACPIGPE;
 
diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 9dc6f43..b582302 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -235,7 +235,6 @@ static const VMStateDescription vmstate_gpe = {
     .fields      = (VMStateField []) {
         VMSTATE_GPE_ARRAY(sts, ACPIGPE),
         VMSTATE_GPE_ARRAY(en, ACPIGPE),
-        VMSTATE_UINT8_ARRAY(cpus_sts, struct gpe_regs, 32),
         VMSTATE_END_OF_LIST()
     }
 };
-- 
1.7.4.3

Re: [PATCH] acpi_piix4: remove bad save/restore of cpus_sts

[Isaku Yamahata]

This patch would fix the segfaults. But I suppose the followings
are necessary.

- PIIX4PMState::gpe_cpu needs to be saved/loaded somewhere

- gpe_writeb() needs to handle PROC_BASE ... PROC_BASE+31
  like gpe_readb(). To be honest, I don't see why gpe_readb/writeb()
  are used for PROC_BASE...PROC_BASE + 31

- enable/disable_processor() manipulates gpe->sts pointer.
  it should be
  gpe->sts[0] |= PIIX4_CPU_HOTPLUG_STATUS

thanks,  

On Mon, Apr 18, 2011 at 11:44:16AM +0300, Avi Kivity wrote:
> Introduced by a mismerge and segfaults immediately.  Still need proper
> save/restore of this field.
> 
> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
>  hw/acpi.h       |    1 -
>  hw/acpi_piix4.c |    1 -
>  2 files changed, 0 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/acpi.h b/hw/acpi.h
> index 96f1d5f..c141e65 100644
> --- a/hw/acpi.h
> +++ b/hw/acpi.h
> @@ -133,7 +133,6 @@ struct ACPIGPE {
>  
>      uint8_t *sts;
>      uint8_t *en;
> -    uint8_t *cpu_sts;
>  };
>  typedef struct ACPIGPE ACPIGPE;
>  
> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
> index 9dc6f43..b582302 100644
> --- a/hw/acpi_piix4.c
> +++ b/hw/acpi_piix4.c
> @@ -235,7 +235,6 @@ static const VMStateDescription vmstate_gpe = {
>      .fields      = (VMStateField []) {
>          VMSTATE_GPE_ARRAY(sts, ACPIGPE),
>          VMSTATE_GPE_ARRAY(en, ACPIGPE),
> -        VMSTATE_UINT8_ARRAY(cpus_sts, struct gpe_regs, 32),
>          VMSTATE_END_OF_LIST()
>      }
>  };
> -- 
> 1.7.4.3
> 

-- 
yamahata

Re: [PATCH] acpi_piix4: remove bad save/restore of cpus_sts

[Isaku Yamahata]

On Mon, Apr 18, 2011 at 10:56:58PM +0900, Isaku Yamahata wrote:
> - enable/disable_processor() manipulates gpe->sts pointer.
>   it should be
>   gpe->sts[0] |= PIIX4_CPU_HOTPLUG_STATUS

The last one is wrong. enable/diable_processor() looks correct.

-- 
yamahata

Re: [PATCH] acpi_piix4: remove bad save/restore of cpus_sts

[Avi Kivity]

On 04/18/2011 04:56 PM, Isaku Yamahata wrote:
> This patch would fix the segfaults. But I suppose the followings
> are necessary.
>
> - PIIX4PMState::gpe_cpu needs to be saved/loaded somewhere

Yes.  Juan?

> - gpe_writeb() needs to handle PROC_BASE ... PROC_BASE+31
>    like gpe_readb(). To be honest, I don't see why gpe_readb/writeb()
>    are used for PROC_BASE...PROC_BASE + 31
>

Even before the merge, we didn't handle a write to this address.  
Perhaps it's read-only? (that should explain no save/restore).

-- 
error compiling committee.c: too many arguments to function