From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35053) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aHN5x-00064i-Nj for qemu-devel@nongnu.org; Thu, 07 Jan 2016 21:48:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aHN5v-0003gV-EP for qemu-devel@nongnu.org; Thu, 07 Jan 2016 21:48:05 -0500 Received: from mail-pa0-x22c.google.com ([2607:f8b0:400e:c03::22c]:34542) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aHN5v-0003f7-6j for qemu-devel@nongnu.org; Thu, 07 Jan 2016 21:48:03 -0500 Received: by mail-pa0-x22c.google.com with SMTP id uo6so255651423pac.1 for ; Thu, 07 Jan 2016 18:48:01 -0800 (PST) References: <1452104533-8516-1-git-send-email-mark.cave-ayland@ilande.co.uk> <1452104533-8516-5-git-send-email-mark.cave-ayland@ilande.co.uk> From: Alexey Kardashevskiy Message-ID: <568F235B.1010506@ozlabs.ru> Date: Fri, 8 Jan 2016 13:47:55 +1100 MIME-Version: 1.0 In-Reply-To: <1452104533-8516-5-git-send-email-mark.cave-ayland@ilande.co.uk> Content-Type: text/plain; charset=koi8-r; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 4/4] target-ppc: ensure we include the decrementer value during migration 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, quintela@redhat.com, amit.shah@redhat.com, David Gibson On 01/07/2016 05:22 AM, Mark Cave-Ayland wrote: > During local testing with TCG, intermittent errors were found when trying to > migrate Darwin OS images. > > The underlying cause was that Darwin resets the decrementer value to fairly > small values on each interrupt. cpu_ppc_set_tb_clk() sets the default value > of the decrementer to 0xffffffff during initialisation which typically > corresponds to several seconds. Hence when restoring the image, the guest > would effectively "lose" decrementer interrupts during this time causing > confusion in the guest. > > NOTE: there does seem to be some overlap here with the vmstate_ppc_timebase > code, however it doesn't seem to handle multiple CPUs which is why I've gone > for an independent implementation. It does handle multiple CPUs: static int timebase_post_load(void *opaque, int version_id) { ... /* Set new offset to all CPUs */ CPU_FOREACH(cpu) { PowerPCCPU *pcpu = POWERPC_CPU(cpu); pcpu->env.tb_env->tb_offset = tb_off_adj; } It does not transfer DECR though, it transfers the offset instead, one for all CPUs. The easier solution would be just adding this instead of the whole patch: spr_register(env, SPR_DECR, "DECR", SPR_NOACCESS, SPR_NOACCESS, &spr_read_decr, &spr_write_decr, 0x00000000); somewhere in target-ppc/translate_init.c for CPUs you want to support, gen_tbl() seems to be the right place for this. As long as it is just spr_register() and not spr_register_kvm(), I suppose it should not break KVM and pseries. > Signed-off-by: Mark Cave-Ayland > --- > target-ppc/machine.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 49 insertions(+) > > diff --git a/target-ppc/machine.c b/target-ppc/machine.c > index cb56423..5ee6269 100644 > --- a/target-ppc/machine.c > +++ b/target-ppc/machine.c > @@ -499,6 +499,54 @@ static const VMStateDescription vmstate_tlbmas = { > } > }; > > +static bool decr_needed(void *opaque) > +{ > + return true; > +} > + > +static int get_decr_state(QEMUFile *f, void *opaque, size_t size) > +{ > + PowerPCCPU *cpu = opaque; > + CPUPPCState *env = &cpu->env; > + > + cpu_ppc_store_decr(env, qemu_get_be32(f)); > + > + return 0; > +} > + > +static void put_decr_state(QEMUFile *f, void *opaque, size_t size) > +{ > + PowerPCCPU *cpu = opaque; > + CPUPPCState *env = &cpu->env; > + > + qemu_put_be32(f, cpu_ppc_load_decr(env)); > +} > + > +static const VMStateInfo vmstate_info_decr = { > + .name = "decr_state", > + .get = get_decr_state, > + .put = put_decr_state > +}; > + > +static const VMStateDescription vmstate_decr = { > + .name = "cpu/decr", > + .version_id = 0, > + .minimum_version_id = 0, > + .needed = decr_needed, > + .fields = (VMStateField[]) { > + { > + .name = "cpu/decr", > + .version_id = 0, > + .field_exists = NULL, > + .size = 0, > + .info = &vmstate_info_decr, > + .flags = VMS_SINGLE, > + .offset = 0, > + }, > + VMSTATE_END_OF_LIST() > + } > +}; > + > const VMStateDescription vmstate_ppc_cpu = { > .name = "cpu", > .version_id = 5, > @@ -555,6 +603,7 @@ const VMStateDescription vmstate_ppc_cpu = { > &vmstate_tlb6xx, > &vmstate_tlbemb, > &vmstate_tlbmas, > + &vmstate_decr, > NULL > } > }; > -- Alexey