On Mon, May 22, 2017 at 04:30:50PM +1000, Suraj Jitindar Singh wrote:
> On Fri, 2017-05-19 at 11:10 +0530, Bharata B Rao wrote:
> > Fix migration of radix guests by ensuring that we issue
> > KVM_PPC_CONFIGURE_V3_MMU for radix case post migration.
> >
> > Reported-by: Nageswara R Sastry <rnsas...@linux.vnet.ibm.com>
> > Signed-off-by: Bharata B Rao <bhar...@linux.vnet.ibm.com>
> > ---
> > hw/ppc/spapr.c | 12 ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index daf335c..8f20f14 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -1400,6 +1400,18 @@ static int spapr_post_load(void *opaque, int
> > version_id)
> > err = spapr_rtc_import_offset(&spapr->rtc, spapr-
> > >rtc_offset);
> > }
>
> This will break migration for tcg radix guests.
>
> Given that there is essentially nothing special we need to do on
> incoming tcg migration, I suggest we make it:
>
> if (spapr->patb_entry && kvm_enabled()) {
> [snip]
> }
>
> >
> > + if (spapr->patb_entry) {
> > + PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
> > + if (kvmppc_has_cap_mmu_radix() && kvm_enabled()) {
>
> Why not make this a bit more generic? Something like:
>
> err = kvmppc_configure_v3_mmu(cpu, !!(spapr->patb_entry & PATBE1_GR),
> !!(cpu->env.spr[SPR_LPCR] & LPCR_GTSE), spapr->patb_entry);
> if (err) {
> error_report("Process table config unsupported by the host");
> return -EINVAL;
> }
>
> return err;
>
> While I don't think it's possible currently, there is nothing
> inherently incorrect about having a non-zero process table entry for a
> hash guest. And this saves a future update.
How about having this logic in spapr_post_load() ?
if (spapr->patb_entry) {
/* Can be Hash(in future?) or Radix guest (current) */
PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
bool radix = !!(spapr->patb_entry & PATBE1_GR);
bool gtse = !!(cpu->env.spr[SPR_LPCR] & LPCR_GTSE);
if (radix) {
/* Radix guest, configure MMU */
/* kvmppc_configure_v3_mmu() is NOP for TCG */
err = kvmppc_configure_v3_mmu(cpu, radix, gtse, spapr->patb_entry);
if (err) {
error_report("Process table config unsupported by the host");
return -EINVAL;
}
} else {
/* Can be Hash guest (in future ?), nothing to do */
}
} else {
/* Hash guest (current), nothing to do */
}
Regards,
Bharata.
