On 1 February 2016 at 20:49, Wei Huang <w...@redhat.com> wrote:
> 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 <w...@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;
> +}
These reset values are all OK...
> +
> +static void pl061_state_reset(DeviceState *dev)
> +{
> + PL061State *s = PL061(dev);
> +
> + pl061_reset(s);
> }
...but you don't need to have this wrapper function.
You can just do the reset in a function called pl061_reset()
with the function signature we need for dc->reset.
The only place that currently calls the existing pl061_reset()
is the device's init function, and you can delete that call
because the Device framework automatically calls the dc->reset
function after device initialization.
thanks
-- PMM
