| Message ID | 20140717110233.8352.53806.stgit@PASHA-ISP |
|---|---|
| State | New |
| Headers | show |
Il 17/07/2014 13:02, Pavel Dovgalyuk ha scritto: > - .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), > @@ -613,6 +627,15 @@ 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), Subsection, only migrated if it doesn't match "(s->iir & UART_IIR_ID) == UART_IIR_THRI". > + VMSTATE_INT32_V(last_break_enable, SerialState, 4), Can be reconstructed in the post_load callback from s->lcr. > + VMSTATE_INT32_V(tsr_retry, SerialState, 4), Subsection, only migrated if nonzero. thr/tsr can be in this subsection as well. > + VMSTATE_STRUCT(recv_fifo, SerialState, 4, vmstate_fifo8, Fifo8), > + VMSTATE_STRUCT(xmit_fifo, SerialState, 4, vmstate_fifo8, Fifo8), Two subsections, only transmitted if nonempty. > + VMSTATE_TIMER_V(fifo_timeout_timer, SerialState, 4), Subsection, only transmitted if pending. > + VMSTATE_INT32_V(timeout_ipending, SerialState, 4), Subsection, transmitted only if nonzero. > + VMSTATE_INT32_V(poll_msl, SerialState, 4), > + VMSTATE_TIMER_V(modem_status_poll, SerialState, 4), Both in a subsection, only migrated if poll_msl is not -1. Paolo > VMSTATE_END_OF_LIST()
> -----Original Message----- > From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini > Sent: Monday, July 28, 2014 1:59 PM > To: Pavel Dovgalyuk; qemu-devel@nongnu.org > Cc: peter.maydell@linaro.org; peter.crosthwaite@xilinx.com; mark.burton@greensocs.com; > real@ispras.ru; batuzovk@ispras.ru; fred.konrad@greensocs.com > Subject: Re: [RFC PATCH v2 06/49] serial: fixing vmstate for save/restore > > Il 17/07/2014 13:02, Pavel Dovgalyuk ha scritto: > > - .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), > > @@ -613,6 +627,15 @@ 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), > > Subsection, only migrated if it doesn't match "(s->iir & UART_IIR_ID) == > UART_IIR_THRI". thr_ipending is set here: if (s->lsr & UART_LSR_THRE) { s->lsr |= UART_LSR_TEMT; s->thr_ipending = 1; serial_update_irq(s); } serial_update_irq has several if-else branches. One of them sets tmp_iir = UART_IIR_THRI, as you said. Couldn't be possible, that this branch will not be reached, because one of the previous ones will be executed? Pavel Dovgalyuk
Il 30/07/2014 09:01, Pavel Dovgaluk ha scritto: > >> -----Original Message----- >> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini >> Sent: Monday, July 28, 2014 1:59 PM >> To: Pavel Dovgalyuk; qemu-devel@nongnu.org >> Cc: peter.maydell@linaro.org; peter.crosthwaite@xilinx.com; mark.burton@greensocs.com; >> real@ispras.ru; batuzovk@ispras.ru; fred.konrad@greensocs.com >> Subject: Re: [RFC PATCH v2 06/49] serial: fixing vmstate for save/restore >> >> Il 17/07/2014 13:02, Pavel Dovgalyuk ha scritto: >>> - .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), >>> @@ -613,6 +627,15 @@ 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), >> >> Subsection, only migrated if it doesn't match "(s->iir & UART_IIR_ID) == >> UART_IIR_THRI". > > thr_ipending is set here: > if (s->lsr & UART_LSR_THRE) { > s->lsr |= UART_LSR_TEMT; > s->thr_ipending = 1; > serial_update_irq(s); > } > > serial_update_irq has several if-else branches. One of them sets tmp_iir = UART_IIR_THRI, as you said. > Couldn't be possible, that this branch will not be reached, because one of the previous ones will be executed? Yes, what I wrote is equivalent to the following: int serial_pre_load(void *opaque) { SerialState *s = opaque; s->thr_ipending = -1; return 0; } int serial_post_load(void *opaque) { SerialState *s = opaque; ... if (s->thr_ipending == -1) { s->thr_ipending = ((s->iir & UART_IIR_ID) == UART_IIR_THRI); } ... } bool thr_ipending_needed(void *opaque) { SerialState *s = opaque; bool expected_value = ((s->iir & UART_IIR_ID) == UART_IIR_THRI); return s->thr_pending != expected_value; } The case you mention is exactly the one that will cause thr_ipending to be migrated. Paolo
diff --git a/hw/char/serial.c b/hw/char/serial.c index 54180a9..1969723 100644 --- a/hw/char/serial.c +++ b/hw/char/serial.c @@ -267,6 +267,61 @@ 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; + val = val & 0xFF; + + if (s->fcr == val) + return; + + /* 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) { + timer_del(s->fifo_timeout_timer); + s->timeout_ipending=0; + fifo8_reset(&s->recv_fifo); + } + + if (val & UART_FCR_XFR) { + fifo8_reset(&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); + } +} + static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val, unsigned size) { @@ -320,50 +375,7 @@ static void serial_ioport_write(void *opaque, hwaddr addr, uint64_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) { - timer_del(s->fifo_timeout_timer); - s->timeout_ipending=0; - fifo8_reset(&s->recv_fifo); - } - - if (val & UART_FCR_XFR) { - fifo8_reset(&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: { @@ -591,20 +603,22 @@ 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, 1); + serial_write_fcr(s, s->fcr_vmstate, 1); serial_update_parameters(s); return 0; } 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), @@ -613,6 +627,15 @@ 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_fifo8, Fifo8), + VMSTATE_STRUCT(xmit_fifo, SerialState, 4, vmstate_fifo8, Fifo8), + VMSTATE_TIMER_V(fifo_timeout_timer, SerialState, 4), + VMSTATE_INT32_V(timeout_ipending, SerialState, 4), + VMSTATE_INT32_V(poll_msl, SerialState, 4), + VMSTATE_TIMER_V(modem_status_poll, SerialState, 4), VMSTATE_END_OF_LIST() } };
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 <pavel.dovgaluk@ispras.ru> --- hw/char/serial.c | 115 ++++++++++++++++++++++++++++++++---------------------- 1 files changed, 69 insertions(+), 46 deletions(-)