On Thu, Nov 10, 2016 at 05:55:36PM -0600, Michael Roth wrote: > Quoting David Gibson (2016-11-09 19:59:37) > > On Tue, Nov 08, 2016 at 04:51:10PM +1100, Alexey Kardashevskiy wrote: > > > On 08/11/16 16:19, David Gibson wrote: > > > > On Fri, Nov 04, 2016 at 04:58:47PM +1100, Alexey Kardashevskiy wrote: > > > >> On 30/10/16 22:12, David Gibson wrote: > > > >>> Server-class POWER CPUs can be put into several compatibility modes. > > > >>> These > > > >>> can be specified on the command line, or negotiated by the guest > > > >>> during > > > >>> boot. > > > >>> > > > >>> Currently we don't migrate the compatibility mode, which means after a > > > >>> migration the guest will revert to running with whatever compatibility > > > >>> mode (or none) specified on the command line. > > > >>> > > > >>> With the limited range of CPUs currently used, this doesn't usually > > > >>> cause > > > >>> a problem, but it could. Fix this by adding the compatibility mode > > > >>> (if > > > >>> set) to the migration stream. > > > >>> > > > >>> Signed-off-by: David Gibson <da...@gibson.dropbear.id.au> > > > >>> --- > > > >>> target-ppc/machine.c | 34 ++++++++++++++++++++++++++++++++++ > > > >>> 1 file changed, 34 insertions(+) > > > >>> > > > >>> diff --git a/target-ppc/machine.c b/target-ppc/machine.c > > > >>> index 4820f22..5d87ff6 100644 > > > >>> --- a/target-ppc/machine.c > > > >>> +++ b/target-ppc/machine.c > > > >>> @@ -9,6 +9,7 @@ > > > >>> #include "mmu-hash64.h" > > > >>> #include "migration/cpu.h" > > > >>> #include "exec/exec-all.h" > > > >>> +#include "qapi/error.h" > > > >>> > > > >>> static int cpu_load_old(QEMUFile *f, void *opaque, int version_id) > > > >>> { > > > >>> @@ -176,6 +177,20 @@ static int cpu_post_load(void *opaque, int > > > >>> version_id) > > > >>> * software has to take care of running QEMU in a compatible > > > >>> mode. > > > >>> */ > > > >>> env->spr[SPR_PVR] = env->spr_cb[SPR_PVR].default_value; > > > >>> + > > > >>> +#if defined(TARGET_PPC64) > > > >>> + if (cpu->compat_pvr) { > > > >>> + Error *local_err = NULL; > > > >>> + > > > >>> + ppc_set_compat(cpu, cpu->compat_pvr, &local_err); > > > >>> + if (local_err) { > > > >>> + error_report_err(local_err); > > > >>> + error_free(local_err); > > > >>> + return -1; > > > >>> + } > > > >>> + } > > > >>> +#endif > > > >>> + > > > >>> env->lr = env->spr[SPR_LR]; > > > >>> env->ctr = env->spr[SPR_CTR]; > > > >>> cpu_write_xer(env, env->spr[SPR_XER]); > > > >>> @@ -528,6 +543,24 @@ static const VMStateDescription vmstate_tlbmas = > > > >>> { > > > >>> } > > > >>> }; > > > >>> > > > >>> +static bool compat_needed(void *opaque) > > > >>> +{ > > > >>> + PowerPCCPU *cpu = opaque; > > > >>> + > > > >>> + return cpu->compat_pvr != 0; > > > >> > > > >> > > > >> Finally got to trying how this affects migration :) > > > >> > > > >> This breaks migration to QEMU <=2.7, and it should not at least when > > > >> both > > > >> source and destination are running with -cpu host,compat=power7. > > > > > > > > IIUC, we don't generally try to maintain backwards migration, even for > > > > old machine types. > > > > > > > > > I thought the opposite - we generally try to maintain it, this is pretty > > > much why we use these subsections in cases like this; otherwise you could > > > just add a new field and bump the vmstate_ppc_cpu.version. > > > > Hm, I guess that's true. We do at least try to allow backwards > > migration, we just don't insist on it. The example here partially > > suceeds - it will allow backwards migration for CPUs in raw mode. > > What if we changed the condition to cpu->compat_pvr != cpu->max_compat? > Assuming each end sets compat=powerX (which seems like a reasonable > requirement for migration), it seems like in most cases the guest would end > up negotiating max_compat anyway. It's only in cases where it doesn't that we > end up in an ambiguous state.
So, since I've already given up on backwards migration in other cases, I've changed this in my next version to simply (cpu->vhyp != NULL), so the compat mode (including raw) is always transferred on pseries. > Newer QEMU would guard against this by migrating it's compat_pvr > accordingly (which will probably become more important with p9 guests > potentially running older kernels), but otherwise it seems like we could > maintain backwards migration in most cases. > > Although with the proposed version bump for vmstate_spapr_pci I guess > the discussion is somewhat moot unless we decide we rely want to > maintain backward migration... Not enough that it's worth the considerable extra difficulty it entails, I think. > > > > > I'll look at just bumping the version instead. > > > > > > > > > > > > > > > >> > > > >> > > > >>> +} > > > >>> + > > > >>> +static const VMStateDescription vmstate_compat = { > > > >>> + .name = "cpu/compat", > > > >>> + .version_id = 1, > > > >>> + .minimum_version_id = 1, > > > >>> + .needed = compat_needed, > > > >>> + .fields = (VMStateField[]) { > > > >>> + VMSTATE_UINT32(compat_pvr, PowerPCCPU), > > > >>> + VMSTATE_END_OF_LIST() > > > >>> + } > > > >>> +}; > > > >>> + > > > >>> const VMStateDescription vmstate_ppc_cpu = { > > > >>> .name = "cpu", > > > >>> .version_id = 5, > > > >>> @@ -580,6 +613,7 @@ const VMStateDescription vmstate_ppc_cpu = { > > > >>> &vmstate_tlb6xx, > > > >>> &vmstate_tlbemb, > > > >>> &vmstate_tlbmas, > > > >>> + &vmstate_compat, > > > >>> NULL > > > >>> } > > > >>> }; > > > >>> > > > >> > > > >> > > > > > > > > > > > > > > > > > > > -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature