* [Qemu-devel] [PATCH 1/2] ARM: PL061: Clear PL061 device state after reset
@ 2016-02-01 17:20 Wei Huang
2016-02-01 17:20 ` [Qemu-devel] [PATCH 2/2] ARM: PL061: Misc cleaning fields for PL061 device state Wei Huang
0 siblings, 1 reply; 5+ messages in thread
From: Wei Huang @ 2016-02-01 17:20 UTC (permalink / raw
To: qemu-devel
Cc: qemu-trivial, peter.maydell, imammedo, shannon.zhao,
zhaoshenglong
Current QEMU doesn't clear PL061 state after reset. This causes a
weird issue with guest reboot via GPIO. Here is the device state
description 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 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.
NOTE: The reset state is found from the following documentation:
- PL061 Technical Reference Manual
- Stellaris LM3S8962 Microcontroller Data Sheet
- Stellaris LM3S5P31 Microcontroller Data Sheet
Signed-off-by: Wei Huang <wei@redhat.com>
---
hw/gpio/pl061.c | 32 ++++++++++++++++++++++++++++++--
1 file changed, 30 insertions(+), 2 deletions(-)
diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c
index e5a696e..342a70d 100644
--- a/hw/gpio/pl061.c
+++ b/hw/gpio/pl061.c
@@ -284,8 +284,35 @@ static void pl061_write(void *opaque, hwaddr offset,
static void pl061_reset(PL061State *s)
{
- s->locked = 1;
- s->cr = 0xff;
+ /* 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_state_reset(DeviceState *dev)
+{
+ PL061State *s = PL061(dev);
+
+ pl061_reset(s);
}
static void pl061_set_irq(void * opaque, int irq, int level)
@@ -343,6 +370,7 @@ static void pl061_class_init(ObjectClass *klass, void *data)
k->init = pl061_initfn;
dc->vmsd = &vmstate_pl061;
+ dc->reset = &pl061_state_reset;
}
static const TypeInfo pl061_info = {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH 2/2] ARM: PL061: Misc cleaning fields for PL061 device state
2016-02-01 17:20 [Qemu-devel] [PATCH 1/2] ARM: PL061: Clear PL061 device state after reset Wei Huang
@ 2016-02-01 17:20 ` Wei Huang
2016-02-01 18:01 ` Peter Maydell
0 siblings, 1 reply; 5+ messages in thread
From: Wei Huang @ 2016-02-01 17:20 UTC (permalink / raw
To: qemu-devel
Cc: qemu-trivial, peter.maydell, imammedo, shannon.zhao,
zhaoshenglong
This patch removes float_high field of PL061State, which doesn't seem
to be used anywhere.
Signed-off-by: Wei Huang <wei@redhat.com>
---
hw/gpio/pl061.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c
index 342a70d..2c08e88 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];
@@ -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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] ARM: PL061: Misc cleaning fields for PL061 device state
2016-02-01 17:20 ` [Qemu-devel] [PATCH 2/2] ARM: PL061: Misc cleaning fields for PL061 device state Wei Huang
@ 2016-02-01 18:01 ` Peter Maydell
2016-02-02 7:03 ` Michael Tokarev
0 siblings, 1 reply; 5+ messages in thread
From: Peter Maydell @ 2016-02-01 18:01 UTC (permalink / raw
To: Wei Huang
Cc: QEMU Trivial, Igor Mammedov, Shannon Zhao, QEMU Developers,
Shannon Zhao
On 1 February 2016 at 17:20, Wei Huang <wei@redhat.com> wrote:
> This patch removes float_high field of PL061State, which doesn't seem
> to be used anywhere.
>
> Signed-off-by: Wei Huang <wei@redhat.com>
> ---
> hw/gpio/pl061.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c
> index 342a70d..2c08e88 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];
> @@ -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()
This would be a migration compatibility break, so at a minimum
you need to bump the vmstate struct versions.
thanks
-- PMM
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] ARM: PL061: Misc cleaning fields for PL061 device state
2016-02-01 18:01 ` Peter Maydell
@ 2016-02-02 7:03 ` Michael Tokarev
2016-02-02 15:16 ` Wei Huang
0 siblings, 1 reply; 5+ messages in thread
From: Michael Tokarev @ 2016-02-02 7:03 UTC (permalink / raw
To: Peter Maydell, Wei Huang
Cc: QEMU Trivial, Igor Mammedov, Shannon Zhao, QEMU Developers,
Shannon Zhao
01.02.2016 21:01, Peter Maydell wrote:
> On 1 February 2016 at 17:20, Wei Huang <wei@redhat.com> wrote:
>> This patch removes float_high field of PL061State, which doesn't seem
>> to be used anywhere.
[]
>> @@ -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()
>
> This would be a migration compatibility break, so at a minimum
> you need to bump the vmstate struct versions.
Is it worth the effort to remove this field if it causes
compatibility break? Maybe keep it around, it doesn't hurt?
At the very least, we may rename it to "unused_float_high",
or something, to indicate it is a known-unused?
Thanks,
/mjt
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] ARM: PL061: Misc cleaning fields for PL061 device state
2016-02-02 7:03 ` Michael Tokarev
@ 2016-02-02 15:16 ` Wei Huang
0 siblings, 0 replies; 5+ messages in thread
From: Wei Huang @ 2016-02-02 15:16 UTC (permalink / raw
To: Michael Tokarev, Peter Maydell
Cc: QEMU Trivial, Igor Mammedov, Shannon Zhao, QEMU Developers,
Shannon Zhao
On 02/02/2016 01:03 AM, Michael Tokarev wrote:
> 01.02.2016 21:01, Peter Maydell wrote:
>> On 1 February 2016 at 17:20, Wei Huang <wei@redhat.com> wrote:
>>> This patch removes float_high field of PL061State, which doesn't seem
>>> to be used anywhere.
> []
>>> @@ -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()
>>
>> This would be a migration compatibility break, so at a minimum
>> you need to bump the vmstate struct versions.
>
> Is it worth the effort to remove this field if it causes
> compatibility break? Maybe keep it around, it doesn't hurt?
It doesn't hurt. So either way is fine. I just happened to find it while
reviewing the code.
> At the very least, we may rename it to "unused_float_high",
> or something, to indicate it is a known-unused?
I don't think renaming solves any problem. Either we keep this variable
as it is or remove it.
>
> Thanks,
>
> /mjt
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-02-02 15:16 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-01 17:20 [Qemu-devel] [PATCH 1/2] ARM: PL061: Clear PL061 device state after reset Wei Huang
2016-02-01 17:20 ` [Qemu-devel] [PATCH 2/2] ARM: PL061: Misc cleaning fields for PL061 device state Wei Huang
2016-02-01 18:01 ` Peter Maydell
2016-02-02 7:03 ` Michael Tokarev
2016-02-02 15:16 ` Wei Huang
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.