[Qemu-devel] [PATCH v2 0/4] Fix pxa2xx/strongarm missing vmstate

[Qemu-devel] [PATCH v2 0/4] Fix pxa2xx/strongarm missing vmstate

[Peter Maydell]

This patchseries fixes some cases for pxa2xx and strongarm
where we'd defined a vmstate struct but forgotten to actually
register it (caught by new warnings in clang 3.4).

As a prerequisite (suggested by Peter Crosthwaite) we clean
up the handling of GPSR/GPCR reads (which are unpredictable)
so we can avoid having bogus device state in the vmstate.

Peter Maydell (4):
  hw/arm/strongarm: Fix handling of GPSR/GPCR reads
  hw/arm/strongarm: Wire up missing GPIO and PPC vmstate
  hw/arm/pxa2xx_gpio: Fix handling of GPSR/GPCR reads
  hw/arm/pxa2xx_gpio: Correct and register vmstate

 hw/arm/pxa2xx_gpio.c | 17 ++++++++---------
 hw/arm/strongarm.c   | 18 ++++++++++--------
 2 files changed, 18 insertions(+), 17 deletions(-)

-- 
1.9.2

[Qemu-devel] [PATCH v2 1/4] hw/arm/strongarm: Fix handling of GPSR/GPCR reads

[Peter Maydell]

The StrongARM GPIO GPSR and GPCR registers are write-only, with reads being
undefined behaviour. Instead of having GPCR return 31337 and GPSR return
the value last written, make both log the guest error and return 0.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/strongarm.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/hw/arm/strongarm.c b/hw/arm/strongarm.c
index 0da9015..cc2d7f2 100644
--- a/hw/arm/strongarm.c
+++ b/hw/arm/strongarm.c
@@ -480,7 +480,6 @@ struct StrongARMGPIOInfo {
     uint32_t rising;
     uint32_t falling;
     uint32_t status;
-    uint32_t gpsr;
     uint32_t gafr;
 
     uint32_t prev_level;
@@ -544,14 +543,14 @@ static uint64_t strongarm_gpio_read(void *opaque, hwaddr offset,
         return s->dir;
 
     case GPSR:        /* GPIO Pin-Output Set registers */
-        DPRINTF("%s: Read from a write-only register 0x" TARGET_FMT_plx "\n",
-                        __func__, offset);
-        return s->gpsr;    /* Return last written value.  */
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "strongarm GPIO: read from write only register GPSR\n");
+        return 0;
 
     case GPCR:        /* GPIO Pin-Output Clear registers */
-        DPRINTF("%s: Read from a write-only register 0x" TARGET_FMT_plx "\n",
-                        __func__, offset);
-        return 31337;        /* Specified as unpredictable in the docs.  */
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "strongarm GPIO: read from write only register GPCR\n");
+        return 0;
 
     case GRER:        /* GPIO Rising-Edge Detect Enable registers */
         return s->rising;
@@ -590,7 +589,6 @@ static void strongarm_gpio_write(void *opaque, hwaddr offset,
     case GPSR:        /* GPIO Pin-Output Set registers */
         s->olevel |= value;
         strongarm_gpio_handler_update(s);
-        s->gpsr = value;
         break;
 
     case GPCR:        /* GPIO Pin-Output Clear registers */
-- 
1.9.2

[Qemu-devel] [PATCH v2 2/4] hw/arm/strongarm: Wire up missing GPIO and PPC vmstate

[Peter Maydell]

The VMStateDescription structs for the GPIO and PPC devices were
accidentally never wired up. Add missing state fields and register
them via dc->vmsd.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/strongarm.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/arm/strongarm.c b/hw/arm/strongarm.c
index cc2d7f2..9e2a0d4 100644
--- a/hw/arm/strongarm.c
+++ b/hw/arm/strongarm.c
@@ -674,6 +674,7 @@ static const VMStateDescription vmstate_strongarm_gpio_regs = {
         VMSTATE_UINT32(falling, StrongARMGPIOInfo),
         VMSTATE_UINT32(status, StrongARMGPIOInfo),
         VMSTATE_UINT32(gafr, StrongARMGPIOInfo),
+        VMSTATE_UINT32(prev_level, StrongARMGPIOInfo),
         VMSTATE_END_OF_LIST(),
     },
 };
@@ -685,6 +686,7 @@ static void strongarm_gpio_class_init(ObjectClass *klass, void *data)
 
     k->init = strongarm_gpio_initfn;
     dc->desc = "StrongARM GPIO controller";
+    dc->vmsd = &vmstate_strongarm_gpio_regs;
 }
 
 static const TypeInfo strongarm_gpio_info = {
@@ -844,6 +846,7 @@ static const VMStateDescription vmstate_strongarm_ppc_regs = {
         VMSTATE_UINT32(ppar, StrongARMPPCInfo),
         VMSTATE_UINT32(psdr, StrongARMPPCInfo),
         VMSTATE_UINT32(ppfr, StrongARMPPCInfo),
+        VMSTATE_UINT32(prev_level, StrongARMPPCInfo),
         VMSTATE_END_OF_LIST(),
     },
 };
@@ -855,6 +858,7 @@ static void strongarm_ppc_class_init(ObjectClass *klass, void *data)
 
     k->init = strongarm_ppc_init;
     dc->desc = "StrongARM PPC controller";
+    dc->vmsd = &vmstate_strongarm_ppc_regs;
 }
 
 static const TypeInfo strongarm_ppc_info = {
-- 
1.9.2

[Qemu-devel] [PATCH v2 3/4] hw/arm/pxa2xx_gpio: Fix handling of GPSR/GPCR reads

[Peter Maydell]

The PXA2xx GPIO GPSR and GPCR registers are write-only, with reads being
undefined behaviour. Instead of having GPCR return 31337 and GPSR return
the value last written, make both log the guest error and return 0.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/pxa2xx_gpio.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/hw/arm/pxa2xx_gpio.c b/hw/arm/pxa2xx_gpio.c
index 7f75f05..cd506df 100644
--- a/hw/arm/pxa2xx_gpio.c
+++ b/hw/arm/pxa2xx_gpio.c
@@ -36,7 +36,6 @@ struct PXA2xxGPIOInfo {
     uint32_t rising[PXA2XX_GPIO_BANKS];
     uint32_t falling[PXA2XX_GPIO_BANKS];
     uint32_t status[PXA2XX_GPIO_BANKS];
-    uint32_t gpsr[PXA2XX_GPIO_BANKS];
     uint32_t gafr[PXA2XX_GPIO_BANKS * 2];
 
     uint32_t prev_level[PXA2XX_GPIO_BANKS];
@@ -162,14 +161,14 @@ static uint64_t pxa2xx_gpio_read(void *opaque, hwaddr offset,
         return s->dir[bank];
 
     case GPSR:		/* GPIO Pin-Output Set registers */
-        printf("%s: Read from a write-only register " REG_FMT "\n",
-                        __FUNCTION__, offset);
-        return s->gpsr[bank];	/* Return last written value.  */
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "pxa2xx GPIO: read from write only register GPSR\n");
+        return 0;
 
     case GPCR:		/* GPIO Pin-Output Clear registers */
-        printf("%s: Read from a write-only register " REG_FMT "\n",
-                        __FUNCTION__, offset);
-        return 31337;		/* Specified as unpredictable in the docs.  */
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "pxa2xx GPIO: read from write only register GPCR\n");
+        return 0;
 
     case GRER:		/* GPIO Rising-Edge Detect Enable registers */
         return s->rising[bank];
@@ -217,7 +216,6 @@ static void pxa2xx_gpio_write(void *opaque, hwaddr offset,
     case GPSR:		/* GPIO Pin-Output Set registers */
         s->olevel[bank] |= value;
         pxa2xx_gpio_handler_update(s);
-        s->gpsr[bank] = value;
         break;
 
     case GPCR:		/* GPIO Pin-Output Clear registers */
-- 
1.9.2

[Qemu-devel] [PATCH v2 4/4] hw/arm/pxa2xx_gpio: Correct and register vmstate

[Peter Maydell]

The pxa2xx-gpio device has a VMStateDescription, but it was accidentally
never actually registered, and it wasn't quite correct. Remove the
'lines' field (this is a device property, not mutable state), add the
missing 'prev_level' field, and set dc->vmsd so it actually gets used.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/pxa2xx_gpio.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/arm/pxa2xx_gpio.c b/hw/arm/pxa2xx_gpio.c
index cd506df..354ccf1 100644
--- a/hw/arm/pxa2xx_gpio.c
+++ b/hw/arm/pxa2xx_gpio.c
@@ -312,7 +312,6 @@ static const VMStateDescription vmstate_pxa2xx_gpio_regs = {
     .version_id = 1,
     .minimum_version_id = 1,
     .fields = (VMStateField[]) {
-        VMSTATE_INT32(lines, PXA2xxGPIOInfo),
         VMSTATE_UINT32_ARRAY(ilevel, PXA2xxGPIOInfo, PXA2XX_GPIO_BANKS),
         VMSTATE_UINT32_ARRAY(olevel, PXA2xxGPIOInfo, PXA2XX_GPIO_BANKS),
         VMSTATE_UINT32_ARRAY(dir, PXA2xxGPIOInfo, PXA2XX_GPIO_BANKS),
@@ -320,6 +319,7 @@ static const VMStateDescription vmstate_pxa2xx_gpio_regs = {
         VMSTATE_UINT32_ARRAY(falling, PXA2xxGPIOInfo, PXA2XX_GPIO_BANKS),
         VMSTATE_UINT32_ARRAY(status, PXA2xxGPIOInfo, PXA2XX_GPIO_BANKS),
         VMSTATE_UINT32_ARRAY(gafr, PXA2xxGPIOInfo, PXA2XX_GPIO_BANKS * 2),
+        VMSTATE_UINT32_ARRAY(prev_level, PXA2xxGPIOInfo, PXA2XX_GPIO_BANKS),
         VMSTATE_END_OF_LIST(),
     },
 };
@@ -338,6 +338,7 @@ static void pxa2xx_gpio_class_init(ObjectClass *klass, void *data)
     k->init = pxa2xx_gpio_initfn;
     dc->desc = "PXA2xx GPIO controller";
     dc->props = pxa2xx_gpio_properties;
+    dc->vmsd = &vmstate_pxa2xx_gpio_regs;
 }
 
 static const TypeInfo pxa2xx_gpio_info = {
-- 
1.9.2

Re: [Qemu-devel] [PATCH v2 0/4] Fix pxa2xx/strongarm missing vmstate

[Peter Crosthwaite]

On Thu, Jun 26, 2014 at 9:42 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> This patchseries fixes some cases for pxa2xx and strongarm
> where we'd defined a vmstate struct but forgotten to actually
> register it (caught by new warnings in clang 3.4).
>
> As a prerequisite (suggested by Peter Crosthwaite) we clean
> up the handling of GPSR/GPCR reads (which are unpredictable)
> so we can avoid having bogus device state in the vmstate.
>
> Peter Maydell (4):
>   hw/arm/strongarm: Fix handling of GPSR/GPCR reads
>   hw/arm/strongarm: Wire up missing GPIO and PPC vmstate
>   hw/arm/pxa2xx_gpio: Fix handling of GPSR/GPCR reads
>   hw/arm/pxa2xx_gpio: Correct and register vmstate
>

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

>  hw/arm/pxa2xx_gpio.c | 17 ++++++++---------
>  hw/arm/strongarm.c   | 18 ++++++++++--------
>  2 files changed, 18 insertions(+), 17 deletions(-)
>
> --
> 1.9.2
>
>