| Message ID | 20171124042325.28012-3-sjitindarsingh@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 24 Nov 2017 15:23:25 +1100 Suraj Jitindar Singh <sjitindarsingh@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 <sjitindarsingh@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;
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;
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(). Fixes: 5dfaa532 ("ppc: fix ppc_set_compat() with KVM PR") Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@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(-)