[Qemu-devel] [PATCH V3 0/2] PL061 device state cleaning

[Qemu-devel] [PATCH V3 0/2] PL061 device state cleaning

[Wei Huang]

Current QEMU doesn't reset PL061 device state. As a result the device
keeps a stale state after guest reboots. This causes VM fail to recceive
GPIO commands (e.g. virsh shutdown/reoot commands) after reboot. This
patchset adds reset support for PL061 devices. The second patch removes
an unused field.

V3: 
 * hook up dc->reset to existing reset function. And stop calling reset
   in initfn()
 * add Reviewed-by
V2:
 * derived from V1 after further debug. Now it resets device state
   instead of using pulse for GPIO IRQ.
V1:
 * initial patch

-Wei


Wei Huang (2):
  ARM: PL061: Clear PL061 device state after reset
  ARM: PL061: Cleaning field of PL061 device state

 hw/gpio/pl061.c | 37 +++++++++++++++++++++++++++++--------
 1 file changed, 29 insertions(+), 8 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH V3 1/2] ARM: PL061: Clear PL061 device state after reset

[Wei Huang]

Current QEMU doesn't clear PL061 state after reset. This causes a
weird issue with guest reboot via GPIO. Here is the device state
with two reboot requests:

  (PL061State fields)           data   old_in_data   istate
VM boot                         0      0             0
After 1st ACPI reboot request   8      8             8
After VM PL061 driver ACK       8      8             0
After VM reboot                 8      8             0
------------------------------------------------------------
2nd ACPI reboot request         8

In the second reboot request above, because the old_in_data field is 8,
QEMU decides that there is a pending edge IRQ already (see
pl061_update()) in input; so it doesn't raise up IRQ again. As a result
the second reboot request is lost. The correct way is to clear PL061
device state after reset.

The default reset state is found from the documents listed below. Per
Peter's suggestion that QEMU automatically calls reset function after
device initialization, this patch removes calling pl061_reset() from
pl061_initfn().

Reference:
[1] PL061 Technical Reference Manual
[2] Stellaris LM3S8962 Microcontroller Data Sheet
[3] Stellaris LM3S5P31 Microcontroller Data Sheet

Signed-off-by: Wei Huang <wei@redhat.com>
---
 hw/gpio/pl061.c | 31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c
index e5a696e..f9773b8 100644
--- a/hw/gpio/pl061.c
+++ b/hw/gpio/pl061.c
@@ -282,10 +282,32 @@ static void pl061_write(void *opaque, hwaddr offset,
     pl061_update(s);
 }
 
-static void pl061_reset(PL061State *s)
+static void pl061_reset(DeviceState *dev)
 {
-  s->locked = 1;
-  s->cr = 0xff;
+    PL061State *s = PL061(dev);
+
+    /* reset values from PL061 TRM, Stellaris LM3S5P31 & LM3S8962 Data Sheet */
+    s->data = 0;
+    s->old_out_data = 0;
+    s->old_in_data = 0;
+    s->dir = 0;
+    s->isense = 0;
+    s->ibe = 0;
+    s->iev = 0;
+    s->im = 0;
+    s->istate = 0;
+    s->afsel = 0;
+    s->dr2r = 0xff;
+    s->dr4r = 0;
+    s->dr8r = 0;
+    s->odr = 0;
+    s->pur = 0;
+    s->pdr = 0;
+    s->slr = 0;
+    s->den = 0;
+    s->locked = 1;
+    s->cr = 0xff;
+    s->amsel = 0;
 }
 
 static void pl061_set_irq(void * opaque, int irq, int level)
@@ -318,7 +340,7 @@ static int pl061_initfn(SysBusDevice *sbd)
     sysbus_init_irq(sbd, &s->irq);
     qdev_init_gpio_in(dev, pl061_set_irq, 8);
     qdev_init_gpio_out(dev, s->out, 8);
-    pl061_reset(s);
+
     return 0;
 }
 
@@ -343,6 +365,7 @@ static void pl061_class_init(ObjectClass *klass, void *data)
 
     k->init = pl061_initfn;
     dc->vmsd = &vmstate_pl061;
+    dc->reset = &pl061_reset;
 }
 
 static const TypeInfo pl061_info = {
-- 
1.8.3.1

[Qemu-devel] [PATCH V3 2/2] ARM: PL061: Cleaning field of PL061 device state

[Wei Huang]

This patch removes the float_high field of PL061State, which doesn't
seem to be used anywhere. Because this changes the device state, the
version ID is also bumped up for the reason of compatiblity.

Signed-off-by: Wei Huang <wei@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/gpio/pl061.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c
index f9773b8..5ece8b0 100644
--- a/hw/gpio/pl061.c
+++ b/hw/gpio/pl061.c
@@ -56,7 +56,6 @@ typedef struct PL061State {
     uint32_t slr;
     uint32_t den;
     uint32_t cr;
-    uint32_t float_high;
     uint32_t amsel;
     qemu_irq irq;
     qemu_irq out[8];
@@ -65,8 +64,8 @@ typedef struct PL061State {
 
 static const VMStateDescription vmstate_pl061 = {
     .name = "pl061",
-    .version_id = 3,
-    .minimum_version_id = 3,
+    .version_id = 4,
+    .minimum_version_id = 4,
     .fields = (VMStateField[]) {
         VMSTATE_UINT32(locked, PL061State),
         VMSTATE_UINT32(data, PL061State),
@@ -88,7 +87,6 @@ static const VMStateDescription vmstate_pl061 = {
         VMSTATE_UINT32(slr, PL061State),
         VMSTATE_UINT32(den, PL061State),
         VMSTATE_UINT32(cr, PL061State),
-        VMSTATE_UINT32(float_high, PL061State),
         VMSTATE_UINT32_V(amsel, PL061State, 2),
         VMSTATE_END_OF_LIST()
     }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH V3 0/2] PL061 device state cleaning

[Peter Maydell]

On 17 February 2016 at 17:19, Wei Huang <wei@redhat.com> wrote:
> Current QEMU doesn't reset PL061 device state. As a result the device
> keeps a stale state after guest reboots. This causes VM fail to recceive
> GPIO commands (e.g. virsh shutdown/reoot commands) after reboot. This
> patchset adds reset support for PL061 devices. The second patch removes
> an unused field.



Applied to target-arm.next, thanks.

-- PMM