Re: [Qemu-devel] [RFC PATCH v2 06/49] serial: fixing vmstate for save/restore

[Paolo Bonzini]

Il 30/07/2014 09:01, Pavel Dovgaluk ha scritto:
> 
>> -----Original Message-----
>> From: Paolo Bonzini [mailto:address@hidden On Behalf Of Paolo Bonzini
>> Sent: Monday, July 28, 2014 1:59 PM
>> To: Pavel Dovgalyuk; address@hidden
>> Cc: address@hidden; address@hidden; address@hidden;
>> address@hidden; address@hidden; address@hidden
>> 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