Re: [Qemu-devel] [RFC PATCH 2/2] target-ppc: add CPU IRQ state to PPC VMStateDescription

[Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>]

On 15/09/15 00:10, Alexey Kardashevskiy wrote:

> On 09/15/2015 05:30 AM, Mark Cave-Ayland wrote:
>> Commit a90db15 "target-ppc: Convert ppc cpu savevm to VMStateDescription"
>> appears to drop the internal CPU IRQ state from the migration stream.
>> Whilst
>> testing migration on g3beige/mac99 machines, test images would
>> randomly fail to
>> resume unless a key was pressed on the VGA console.
>>
>> Further investigation suggests that internal CPU IRQ state isn't being
>> preserved and so interrupts asserted at the time of migration are
>> lost. Adding
>> the pending_interrupts and irq_input_state fields back into the migration
>> stream appears to fix the problem here during local tests.
>  
> On spapr, interrupt state migrates with XICS interrupt controller and it
> resets the CPU bits you are adding to the migration descriptor. I'd
> expect openpic (this one is used for mac99?) to do the same.

Interesting. I wrote the patch that converted openpic to
VMStateDescription at the end of last year, and my understanding from
the feedback was that ideally interrupt state should be maintained so
that no post_load function was required. I guess spapr is very different
from the basic Mac machines though.

Also I see that you also removed the reference to cpu_write_xer() which
appears to set some related internal state variables. Is this now not
necessary either?

>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>>   target-ppc/machine.c |    2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/target-ppc/machine.c b/target-ppc/machine.c
>> index bd99844..968a7d6 100644
>> --- a/target-ppc/machine.c
>> +++ b/target-ppc/machine.c
>> @@ -528,6 +528,8 @@ const VMStateDescription vmstate_ppc_cpu = {
>>
>>           /* Internal state */
>>           VMSTATE_UINTTL(env.hflags_nmsr, PowerPCCPU),
>> +        VMSTATE_UINT32(env.pending_interrupts, PowerPCCPU),
>> +        VMSTATE_UINT32(env.irq_input_state, PowerPCCPU),
> 
> This update requires a "version" increment for vmstate_ppc_cpu and
> VMSTATE_UINT32_V instead of VMSTATE_UINT32.

So this means you're happy with the basic patch if I go ahead and make
the version changes too?


ATB,

Mark.