[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 07/10] serial: fixing vmstate for save/restore
From: |
Juan Quintela |
Subject: |
Re: [Qemu-devel] [PATCH 07/10] serial: fixing vmstate for save/restore |
Date: |
Tue, 09 Sep 2014 15:59:54 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
Paolo Bonzini <address@hidden> wrote:
> From: Pavel Dovgalyuk <address@hidden>
>
> Some fields were added to VMState by this patch to preserve correct
> loading of the serial port controller state.
> Updating FCR value while loading was also modified to disable generating
> an interrupt by loadvm.
>
> Signed-off-by: Pavel Dovgalyuk <address@hidden>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
> hw/char/serial.c | 265
> +++++++++++++++++++++++++++++++++++++++++++++----------
> 1 file changed, 220 insertions(+), 45 deletions(-)
>
> diff --git a/hw/char/serial.c b/hw/char/serial.c
> index 764e184..2b04927 100644
> --- a/hw/char/serial.c
> +++ b/hw/char/serial.c
> @@ -272,6 +272,64 @@ static gboolean serial_xmit(GIOChannel *chan,
> GIOCondition cond, void *opaque)
> }
>
>
> +/* Setter for FCR.
> + is_load flag means, that value is set while loading VM state
> + and interrupt should not be invoked */
> +static void serial_write_fcr(void *opaque, uint32_t val, int is_load)
> +{
> + SerialState *s = opaque;
Both users call it with SerialState *s, not a void *.
is_load can be a bool.
> +static int serial_pre_load(void *opaque)
> +{
> + SerialState *s = (SerialState *)opaque;
Unneeded cast (and not used in the rest of the file)
> + s->thr_ipending = -1;
> + s->poll_msl = -1;
We set both to -1.
> + return 0;
> +}
> +
> static int serial_post_load(void *opaque, int version_id)
> {
> SerialState *s = opaque;
> @@ -597,17 +620,139 @@ static int serial_post_load(void *opaque, int
> version_id)
> if (version_id < 3) {
> s->fcr_vmstate = 0;
> }
> + if (s->thr_ipending == -1) {
> + s->thr_ipending = ((s->iir & UART_IIR_ID) == UART_IIR_THRI);
> + }
If it is still -1 at this point, we "calculate" a value for it.
(I assume it is right, no knowledge of serial port)
But poll_msl is "more" interesting, because we are not "reseting it".
So, we have that if we are migrating from an old version, we would have
poll_msl == -1, and we used to have it to poll_msl == 0.
Should we change it?
I think that putting:
s->poll_msl = 0;
in preload, and
static bool serial_poll_needed(void *opaque)
{
SerialState *s = opaque;
return s->poll_msl != 0;
}
would give exactly the same behaviour for new qemus, and behave better
for older ones?
what do you think?
Later, Juan.
> + s->last_break_enable = (s->lcr >> 6) & 1;
> /* Initialize fcr via setter to perform essential side-effects */
> - serial_ioport_write(s, 0x02, s->fcr_vmstate, 1);
> + serial_write_fcr(s, s->fcr_vmstate, 1);
> serial_update_parameters(s);
> return 0;
> }
>
> +static bool serial_thr_ipending_needed(void *opaque)
> +{
> + SerialState *s = opaque;
> + bool expected_value = ((s->iir & UART_IIR_ID) == UART_IIR_THRI);
> + return s->thr_ipending != expected_value;
> +}
> +
> +const VMStateDescription vmstate_serial_thr_ipending = {
> + .name = "serial/thr_ipending",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_INT32(thr_ipending, SerialState),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static bool serial_tsr_needed(void *opaque)
> +{
> + SerialState *s = (SerialState *)opaque;
> + return s->tsr_retry != 0;
> +}
> +
> +const VMStateDescription vmstate_serial_tsr = {
> + .name = "serial/tsr",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_INT32(tsr_retry, SerialState),
> + VMSTATE_UINT8(thr, SerialState),
> + VMSTATE_UINT8(tsr, SerialState),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static bool serial_recv_fifo_needed(void *opaque)
> +{
> + SerialState *s = (SerialState *)opaque;
> + return !fifo8_is_empty(&s->recv_fifo);
> +
> +}
> +
> +const VMStateDescription vmstate_serial_recv_fifo = {
> + .name = "serial/recv_fifo",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_STRUCT(recv_fifo, SerialState, 1, vmstate_fifo8, Fifo8),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static bool serial_xmit_fifo_needed(void *opaque)
> +{
> + SerialState *s = (SerialState *)opaque;
> + return !fifo8_is_empty(&s->xmit_fifo);
> +}
> +
> +const VMStateDescription vmstate_serial_xmit_fifo = {
> + .name = "serial/xmit_fifo",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_STRUCT(xmit_fifo, SerialState, 1, vmstate_fifo8, Fifo8),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static bool serial_fifo_timeout_timer_needed(void *opaque)
> +{
> + SerialState *s = (SerialState *)opaque;
> + return timer_pending(s->fifo_timeout_timer);
> +}
> +
> +const VMStateDescription vmstate_serial_fifo_timeout_timer = {
> + .name = "serial/fifo_timeout_timer",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_TIMER(fifo_timeout_timer, SerialState),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static bool serial_timeout_ipending_needed(void *opaque)
> +{
> + SerialState *s = (SerialState *)opaque;
> + return s->timeout_ipending != 0;
> +}
> +
> +const VMStateDescription vmstate_serial_timeout_ipending = {
> + .name = "serial/timeout_ipending",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_INT32(timeout_ipending, SerialState),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static bool serial_poll_needed(void *opaque)
> +{
> + SerialState *s = (SerialState *)opaque;
> + return s->poll_msl >= 0;
> +}
> +
> +const VMStateDescription vmstate_serial_poll = {
> + .name = "serial/poll",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_INT32(poll_msl, SerialState),
> + VMSTATE_TIMER(modem_status_poll, SerialState),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> const VMStateDescription vmstate_serial = {
> .name = "serial",
> .version_id = 3,
> .minimum_version_id = 2,
> .pre_save = serial_pre_save,
> + .pre_load = serial_pre_load,
> .post_load = serial_post_load,
> .fields = (VMStateField[]) {
> VMSTATE_UINT16_V(divider, SerialState, 2),
> @@ -621,6 +766,32 @@ const VMStateDescription vmstate_serial = {
> VMSTATE_UINT8(scr, SerialState),
> VMSTATE_UINT8_V(fcr_vmstate, SerialState, 3),
> VMSTATE_END_OF_LIST()
> + },
> + .subsections = (VMStateSubsection[]) {
> + {
> + .vmsd = &vmstate_serial_thr_ipending,
> + .needed = &serial_thr_ipending_needed,
> + } , {
> + .vmsd = &vmstate_serial_tsr,
> + .needed = &serial_tsr_needed,
> + } , {
> + .vmsd = &vmstate_serial_recv_fifo,
> + .needed = &serial_recv_fifo_needed,
> + } , {
> + .vmsd = &vmstate_serial_xmit_fifo,
> + .needed = &serial_xmit_fifo_needed,
> + } , {
> + .vmsd = &vmstate_serial_fifo_timeout_timer,
> + .needed = &serial_fifo_timeout_timer_needed,
> + } , {
> + .vmsd = &vmstate_serial_timeout_ipending,
> + .needed = &serial_timeout_ipending_needed,
> + } , {
> + .vmsd = &vmstate_serial_poll,
> + .needed = &serial_poll_needed,
> + } , {
> + /* empty */
> + }
> }
> };
>
> @@ -642,6 +813,10 @@ static void serial_reset(void *opaque)
> s->char_transmit_time = (get_ticks_per_sec() / 9600) * 10;
> s->poll_msl = 0;
>
> + s->timeout_ipending = 0;
> + timer_del(s->fifo_timeout_timer);
> + timer_del(s->modem_status_poll);
> +
> fifo8_reset(&s->recv_fifo);
> fifo8_reset(&s->xmit_fifo);
- Re: [Qemu-devel] [PATCH 04/10] pcspk: adding vmstate for save/restore, (continued)
[Qemu-devel] [PATCH 06/10] parallel: adding vmstate for save/restore, Paolo Bonzini, 2014/09/09
[Qemu-devel] [PATCH 05/10] fdc: adding vmstate for save/restore, Paolo Bonzini, 2014/09/09
[Qemu-devel] [PATCH 07/10] serial: fixing vmstate for save/restore, Paolo Bonzini, 2014/09/09
- Re: [Qemu-devel] [PATCH 07/10] serial: fixing vmstate for save/restore,
Juan Quintela <=
Re: [Qemu-devel] [PATCH 07/10] serial: fixing vmstate for save/restore, Paolo Bonzini, 2014/09/10
[Qemu-devel] [PATCH 08/10] pckbd: adding new fields to vmstate, Paolo Bonzini, 2014/09/09
[Qemu-devel] [PATCH 10/10] mc146818rtc: add missed field to vmstate, Paolo Bonzini, 2014/09/09
[Qemu-devel] [PATCH 09/10] piix: do not raise irq while loading vmstate, Paolo Bonzini, 2014/09/09