On Fri, 24 Nov 2017 15:23:25 +1100 Suraj Jitindar Singh <sjitindarsi...@gmail.com> wrote:
> cpu->compat_pvr is used to store the current compat mode of the cpu. > > On the receiving side during incoming migration we check compatibility > with the compat mode by calling ppc_set_compat(). However we fail to set > the compat mode with the hypervisor since the "new" compat mode doesn't > differ from the current (due to a "cpu->compat_pvr != compat_pvr" check). > This means that kvm runs the vcpus without a compat mode, which is the > incorrect behaviour. The implication being that a compatibility mode > will never be in effect after migration. > > To fix this so that the compat mode is correctly set with the > hypervisor, store the desired compat mode and reset cpu->compat_pvr to > zero before calling ppc_set_compat(). > So we'd twist the code in cpu_post_load() because of some check done in ppc_set_compat(), which is actually a workaround because PR KVM doesn't support KVM_REG_PPC_ARCH_COMPAT... ouch :-\ David, This was discussed last summer in http://patchwork.ozlabs.org/patch/782039/ and you wrote: > Well, qemu expects to be able to set ARCH_COMPAT for a pseries guest, > if that guest is going into a compatibility mode (which it usually > does these days). We don't want userspace to have to be constantly > checking which KVM implementation its working against, so it makes > sense for PR to implement the call, even if it's a no-op because it > can't really implement the PCR fully. I understand your point but implementing ARCH_COMPAT in PR KVM will probably never happen... and as you say, it might be only a no-op. So I guess we can "implement" this behaviour in kvmppc_set_compat() with kvmppc_is_pr() and a fat comment, and drop the compat_pvr check this patch is trying to negate. > Fixes: 5dfaa532 ("ppc: fix ppc_set_compat() with KVM PR") > > Signed-off-by: Suraj Jitindar Singh <sjitindarsi...@gmail.com> > > --- > > It is worth noting that another option was to add a force flag to the > ppc_set_compat() function, but the enacted solution seemed cleaner with > fewer code changes. > --- > target/ppc/machine.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/target/ppc/machine.c b/target/ppc/machine.c > index 384caee800..24117e8f31 100644 > --- a/target/ppc/machine.c > +++ b/target/ppc/machine.c > @@ -237,9 +237,11 @@ static int cpu_post_load(void *opaque, int version_id) > > #if defined(TARGET_PPC64) > if (cpu->compat_pvr) { > + uint32_t compat_pvr = cpu->compat_pvr; > Error *local_err = NULL; > > - ppc_set_compat(cpu, cpu->compat_pvr, &local_err); > + cpu->compat_pvr = 0; > + ppc_set_compat(cpu, compat_pvr, &local_err); > if (local_err) { > error_report_err(local_err); > return -1;