[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] Fix serial interface vmstate
From: |
Juan Quintela |
Subject: |
Re: [Qemu-devel] [PATCH] Fix serial interface vmstate |
Date: |
Tue, 21 Jun 2011 15:31:03 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) |
"Pavel Dovgaluk" <address@hidden> wrote:
> This patch fixes save/restore of serial interface's state.
> It includes changing of fcr setter function (it now does not invoke
> an interrupt while loading vmstate), and saving/restoring all
> fields that describe the state of serial interface (including timers).
>
> Signed-off-by: Pavel Dovgalyuk <address@hidden>
I think we can do this with a new subsection.
> ---
> hw/serial.c | 133 ++++++++++++++++++++++++++++++++++++++--------------------
> 1 files changed, 87 insertions(+), 46 deletions(-)
>
> diff --git a/hw/serial.c b/hw/serial.c
> index 0ee61dd..936e048 100644
> --- a/hw/serial.c
> +++ b/hw/serial.c
> @@ -362,6 +362,62 @@ static void serial_xmit(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;
> +
> + val = val & 0xFF;
> +
> + if (s->fcr == val)
> + return;
This looks like a test. if this is true, we don't need to restore the
other values, so we shouldbe safe, right?
> + /* Did the enable/disable flag change? If so, make sure FIFOs get
> flushed */
> + if ((val ^ s->fcr) & UART_FCR_FE)
> + val |= UART_FCR_XFR | UART_FCR_RFR;
> +
> + /* FIFO clear */
> +
> + if (val & UART_FCR_RFR) {
> + qemu_del_timer(s->fifo_timeout_timer);
> + s->timeout_ipending=0;
> + fifo_clear(s,RECV_FIFO);
> + }
> +
> + if (val & UART_FCR_XFR) {
> + fifo_clear(s,XMIT_FIFO);
> + }
> +
> + if (val & UART_FCR_FE) {
> + s->iir |= UART_IIR_FE;
> + /* Set RECV_FIFO trigger Level */
> + switch (val & 0xC0) {
> + case UART_FCR_ITL_1:
> + s->recv_fifo.itl = 1;
> + break;
> + case UART_FCR_ITL_2:
> + s->recv_fifo.itl = 4;
> + break;
> + case UART_FCR_ITL_3:
> + s->recv_fifo.itl = 8;
> + break;
> + case UART_FCR_ITL_4:
> + s->recv_fifo.itl = 14;
> + break;
> + }
> + } else
> + s->iir &= ~UART_IIR_FE;
> +
> + /* Set fcr - or at least the bits in it that are supposed to "stick" */
> + s->fcr = val & 0xC9;
> + if (!is_load) {
> + serial_update_irq(s);
> + }
we can put the serial_update_irq() at caller site. Function is only
called twice, on place need to call serial_update_irq() and the other not.
> +}
> +
> +
> static void serial_ioport_write(void *opaque, uint32_t addr, uint32_t val)
> {
> SerialState *s = opaque;
> @@ -414,50 +470,7 @@ static void serial_ioport_write(void *opaque, uint32_t
> addr, uint32_t val)
> }
> break;
> case 2:
> - val = val & 0xFF;
> -
> - if (s->fcr == val)
> - break;
> -
> - /* Did the enable/disable flag change? If so, make sure FIFOs get
> flushed */
> - if ((val ^ s->fcr) & UART_FCR_FE)
> - val |= UART_FCR_XFR | UART_FCR_RFR;
> -
> - /* FIFO clear */
> -
> - if (val & UART_FCR_RFR) {
> - qemu_del_timer(s->fifo_timeout_timer);
> - s->timeout_ipending=0;
> - fifo_clear(s,RECV_FIFO);
> - }
> -
> - if (val & UART_FCR_XFR) {
> - fifo_clear(s,XMIT_FIFO);
> - }
> -
> - if (val & UART_FCR_FE) {
> - s->iir |= UART_IIR_FE;
> - /* Set RECV_FIFO trigger Level */
> - switch (val & 0xC0) {
> - case UART_FCR_ITL_1:
> - s->recv_fifo.itl = 1;
> - break;
> - case UART_FCR_ITL_2:
> - s->recv_fifo.itl = 4;
> - break;
> - case UART_FCR_ITL_3:
> - s->recv_fifo.itl = 8;
> - break;
> - case UART_FCR_ITL_4:
> - s->recv_fifo.itl = 14;
> - break;
> - }
> - } else
> - s->iir &= ~UART_IIR_FE;
> -
> - /* Set fcr - or at least the bits in it that are supposed to "stick"
> */
> - s->fcr = val & 0xC9;
> - serial_update_irq(s);
> + serial_write_fcr(s, val, 0);
> break;
> case 3:
> {
> @@ -673,20 +686,38 @@ static int serial_post_load(void *opaque, int
> version_id)
> s->fcr_vmstate = 0;
> }
> /* Initialize fcr via setter to perform essential side-effects */
> - serial_ioport_write(s, 0x02, s->fcr_vmstate);
> + serial_write_fcr(s, s->fcr_vmstate, 1);
> serial_update_parameters(s);
> return 0;
> }
>
> +
> +static const VMStateDescription vmstate_fifo = {
> + .name = "serial FIFO",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .fields = (VMStateField []) {
> + VMSTATE_BUFFER(data, SerialFIFO),
> + VMSTATE_UINT8(count, SerialFIFO),
> + VMSTATE_UINT8(itl, SerialFIFO),
> + VMSTATE_UINT8(tail, SerialFIFO),
> + VMSTATE_UINT8(head, SerialFIFO),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +
> static const VMStateDescription vmstate_serial = {
> .name = "serial",
> - .version_id = 3,
> + .version_id = 4,
> .minimum_version_id = 2,
> .pre_save = serial_pre_save,
> .post_load = serial_post_load,
> .fields = (VMStateField []) {
> VMSTATE_UINT16_V(divider, SerialState, 2),
> VMSTATE_UINT8(rbr, SerialState),
> + VMSTATE_UINT8_V(thr, SerialState, 4),
> + VMSTATE_UINT8_V(tsr, SerialState, 4),
> VMSTATE_UINT8(ier, SerialState),
> VMSTATE_UINT8(iir, SerialState),
> VMSTATE_UINT8(lcr, SerialState),
> @@ -695,6 +726,16 @@ static const VMStateDescription vmstate_serial = {
> VMSTATE_UINT8(msr, SerialState),
> VMSTATE_UINT8(scr, SerialState),
> VMSTATE_UINT8_V(fcr_vmstate, SerialState, 3),
> + VMSTATE_INT32_V(thr_ipending, SerialState, 4),
> + VMSTATE_INT32_V(last_break_enable, SerialState, 4),
> + VMSTATE_INT32_V(tsr_retry, SerialState, 4),
> + VMSTATE_STRUCT(recv_fifo, SerialState, 4, vmstate_fifo, SerialFIFO),
> + VMSTATE_STRUCT(xmit_fifo, SerialState, 4, vmstate_fifo, SerialFIFO),
> + VMSTATE_TIMER_V(fifo_timeout_timer, SerialState, 4),
> + VMSTATE_INT32_V(timeout_ipending, SerialState, 4),
> + VMSTATE_TIMER_V(transmit_timer, SerialState, 4),
> + VMSTATE_INT32_V(poll_msl, SerialState, 4),
> + VMSTATE_TIMER_V(modem_status_poll, SerialState, 4),
> VMSTATE_END_OF_LIST()
> }
> };
Anyways, I think that it is better to split the change in two patches.
One that refactor the common code in another function. And the other
that adds the VMSTATE bits, I can add the subsection part if you want.
Later, Juan.
- [Qemu-devel] [PATCH] Fix serial interface vmstate, Pavel Dovgaluk, 2011/06/21
- Re: [Qemu-devel] [PATCH] Fix serial interface vmstate,
Juan Quintela <=
- Re: [Qemu-devel] [PATCH] Fix serial interface vmstate, Pavel Dovgaluk, 2011/06/22
- Message not available
- Re: [Qemu-devel] [PATCH] Fix serial interface vmstate, Jan Kiszka, 2011/06/22
- Re: [Qemu-devel] [PATCH] Fix serial interface vmstate, Pavel Dovgaluk, 2011/06/22
- Re: [Qemu-devel] [PATCH] Fix serial interface vmstate, Jan Kiszka, 2011/06/22
- Re: [Qemu-devel] [PATCH] Fix serial interface vmstate, Pavel Dovgaluk, 2011/06/22
- Re: [Qemu-devel] [PATCH] Fix serial interface vmstate, Jan Kiszka, 2011/06/22
- Re: [Qemu-devel] [PATCH] Fix serial interface vmstate, Pavel Dovgaluk, 2011/06/22
- Re: [Qemu-devel] [PATCH] Fix serial interface vmstate, Jan Kiszka, 2011/06/22
- Message not available
- Re: [Qemu-devel] [PATCH] Fix serial interface vmstate, Andreas Färber, 2011/06/23