From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51967) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZcQsD-0005wp-VH for qemu-devel@nongnu.org; Thu, 17 Sep 2015 00:32:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZcQs8-0000qm-TX for qemu-devel@nongnu.org; Thu, 17 Sep 2015 00:32:41 -0400 Received: from mail-pa0-f49.google.com ([209.85.220.49]:35325) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZcQs8-0000qa-OD for qemu-devel@nongnu.org; Thu, 17 Sep 2015 00:32:36 -0400 Received: by pacfv12 with SMTP id fv12so8950975pac.2 for ; Wed, 16 Sep 2015 21:32:35 -0700 (PDT) References: <1442259039-22137-1-git-send-email-mark.cave-ayland@ilande.co.uk> <1442259039-22137-3-git-send-email-mark.cave-ayland@ilande.co.uk> <55F753DE.2080003@ozlabs.ru> <55F88922.7020804@ilande.co.uk> From: Alexey Kardashevskiy Message-ID: <55FA425D.1060905@ozlabs.ru> Date: Thu, 17 Sep 2015 14:32:29 +1000 MIME-Version: 1.0 In-Reply-To: <55F88922.7020804@ilande.co.uk> Content-Type: text/plain; charset=koi8-r; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH 2/2] target-ppc: add CPU IRQ state to PPC VMStateDescription List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Mark Cave-Ayland , qemu-devel@nongnu.org, qemu-ppc@nongnu.org, agraf@suse.de On 09/16/2015 07:09 AM, Mark Cave-Ayland wrote: > 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? Not sure here, looks like a bug actually, cpu_post_load() should call it. But it should only affect TCG migration (which we have not extensively tested :) ). >>> >>> Signed-off-by: Mark Cave-Ayland >>> --- >>> 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? Yes, I suppose. -- Alexey