From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:34737) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TbBtM-0003sl-VX for qemu-devel@nongnu.org; Wed, 21 Nov 2012 10:07:15 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TbBtG-0000c9-8p for qemu-devel@nongnu.org; Wed, 21 Nov 2012 10:07:08 -0500 Received: from cantor2.suse.de ([195.135.220.15]:59226 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TbBtF-0000by-V7 for qemu-devel@nongnu.org; Wed, 21 Nov 2012 10:07:02 -0500 Message-ID: <50ACEE12.4000800@suse.de> Date: Wed, 21 Nov 2012 16:06:58 +0100 From: Alexander Graf MIME-Version: 1.0 References: <1353509165-26865-1-git-send-email-borntraeger@de.ibm.com> <1353509165-26865-2-git-send-email-borntraeger@de.ibm.com> <50ACEB95.7090300@suse.de> <50ACEC4F.4050109@de.ibm.com> <50ACECE9.4000004@suse.de> <50ACED48.3050308@de.ibm.com> In-Reply-To: <50ACED48.3050308@de.ibm.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/3] s390/migration: Provide a cpu save for initial life migration work List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Christian Borntraeger Cc: Jan Kiszka , Jens Freimann , Heinz Graalfs , qemu-devel , "Jason J. herne" On 11/21/2012 04:03 PM, Christian Borntraeger wrote: > On 21/11/12 16:02, Alexander Graf wrote: >> On 11/21/2012 03:59 PM, Christian Borntraeger wrote: >>> On 21/11/12 15:56, Alexander Graf wrote: >>>> On 11/21/2012 03:46 PM, Christian Borntraeger wrote: >>>>> This provides a simple cpu load and save function. With the recent >>>>> addition of sync regs we have the crs,acrs, the prefix and the >>>>> PSW already up to date. Lets also save the fpu via pre/post hooks. >>>>> >>>>> This patch also changes the license of machine.c to GPLv2 or later. >>>>> (The old code was just empty glue code, so there is no need >>>>> to go the "contributions after" way). >>>>> >>>>> Signed-off-by: Christian Borntraeger >>>>> --- >>>>> target-s390x/cpu.h | 1 + >>>>> target-s390x/machine.c | 115 ++++++++++++++++++++++++++++++++++++++++++------ >>>>> 2 files changed, 103 insertions(+), 13 deletions(-) >>>>> >>>>> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h >>>>> index 0f9a1f7..ba695dd 100644 >>>>> --- a/target-s390x/cpu.h >>>>> +++ b/target-s390x/cpu.h >>>>> @@ -27,6 +27,7 @@ >>>>> #define ELF_MACHINE EM_S390 >>>>> >>>>> #define CPUArchState struct CPUS390XState >>>>> +#define CPU_SAVE_VERSION 1 >>>>> >>>>> #include "cpu-defs.h" >>>>> #define TARGET_PAGE_BITS 12 >>>>> diff --git a/target-s390x/machine.c b/target-s390x/machine.c >>>>> index 3e79be6..02706fd 100644 >>>>> --- a/target-s390x/machine.c >>>>> +++ b/target-s390x/machine.c >>>>> @@ -2,29 +2,118 @@ >>>>> * QEMU S390x machine definitions >>>>> * >>>>> * Copyright (c) 2009 Alexander Graf >>>>> + * Copyright IBM Corp. 2012 >>>>> * >>>>> - * This library is free software; you can redistribute it and/or >>>>> - * modify it under the terms of the GNU Lesser General Public >>>>> - * License as published by the Free Software Foundation; either >>>>> - * version 2 of the License, or (at your option) any later version. >>>>> - * >>>>> - * This library is distributed in the hope that it will be useful, >>>>> - * but WITHOUT ANY WARRANTY; without even the implied warranty of >>>>> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >>>>> - * Lesser General Public License for more details. >>>>> - * >>>>> - * You should have received a copy of the GNU Lesser General Public >>>>> - * License along with this library; if not, see. >>>>> + * This work is licensed under the terms of the GNU GPL, version 2 or (at your >>>>> + * option) any later version. See the COPYING file in the top-level directory. >>>>> */ >>>>> >>>>> #include "hw/hw.h" >>>>> #include "hw/boards.h" >>>>> +#include "cpu.h" >>>>> +#include "kvm.h" >>>>> + >>>>> +#if defined CONFIG_KVM >>>>> +static void cpu_pre_save(void *opaque) >>>>> +{ >>>>> + CPUS390XState *env = opaque; >>>>> + struct kvm_fpu fpu; >>>>> + int i, r; >>>>> + >>>>> + if (!kvm_enabled()) { >>>>> + return; >>>>> + } >>>>> + >>>>> + r = kvm_vcpu_ioctl(env, KVM_GET_FPU,&fpu); >>>>> + assert(r == 0); >>>>> + for (i = 0; i< 16; i++) { >>>>> + env->fregs[i].ll = fpu.fprs[i]; >>>>> + } >>>>> + env->fpc = fpu.fpc; >>>>> +} >>>>> + >>>>> +static int cpu_post_load(void *opaque, int version_id) >>>>> +{ >>>>> + CPUS390XState *env = opaque; >>>>> + struct kvm_fpu fpu; >>>>> + int i, r; >>>>> + >>>>> + if (!kvm_enabled()) { >>>>> + return 0; >>>>> + } >>>>> + >>>>> + for (i = 0; i< 16; i++) { >>>>> + fpu.fprs[i] = env->fregs[i].ll; >>>>> + } >>>>> + fpu.fpc = env->fpc; >>>>> + >>>>> + r = kvm_vcpu_ioctl(env, KVM_SET_FPU,&fpu); >>>>> + assert(r == 0); >>>>> + >>>>> + return 0; >>>>> +} >>>> The kvm register sync needs to happen in the kvm register sync function :) >>> That would eliminate the whole purpose of sync regs and forces us to have an >>> expensive ioctl on lots of exits (again). I would prefer to sync the registers >>> that we never need in qemu just here. >> That's why the register sync has different stages. > Not the get_register. Which is called on every synchronize_state. Which happen quite often > on s390. Sounds like bad design then :). Maybe we should explicitly tell the register synchronization which register sets to sync, so that we don't waste time getting _all_ the state every time we sync registers? Jan, ideas? Alex