Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH v2 4/4] spapr: Fix migration of Radix guests

[Bharata B Rao <bharata@linux.vnet.ibm.com>]

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 <rnsastry@linux.vnet.ibm.com>
> > Signed-off-by: Bharata B Rao <bharata@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.