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

[Bharata B Rao]

On Thu, May 11, 2017 at 11:02:20AM +1000, David Gibson wrote:
> On Mon, May 08, 2017 at 02:36:17PM +0530, Bharata B Rao wrote:
> > Currently HTAB savevm handlers get registered by default and migration
> > of radix guest will fail.
> > 
> > - Ensure that HTAB savevm handlers are not registered for radix case.
> > - Ensure that we issue KVM_PPC_CONFIGURE_V3_MMU for radix case post
> >   migration.
> > 
> > TODO: Right now I have delayed the HTAB savevm handler registration
> > to CAS call where we know if the guest is radix or hash. Another approach
> > is to let the HTAB handlers to be registered by default (as it is being
> > done currently, but unregister them from CAS when we discover radix
> > capability).
> 
> Option 2 there sounds messy.  I also suspect it could break if you try
> to migrate an (eventually) radix guest before it's done CAS.
> 
> Strictly speaking only registering at CAS time will break old hash
> guests that don't do CAS at all.  However such guests are really,
> really ancient, and I suspect we don't work with them already.
> 
> You do, however, need to deregister (and allow the choice to be made
> again) on guest reset.  On KVM we can only (for now) support either
> hash or radix guests.  Under TCG, however, we could run a radix guest
> then reboot to a hash guest or vice versa.

Took care of this in v1.

> 
> 
> > 
> > Reported-by: Nageswara R Sastry <address@hidden>
> > Signed-off-by: Bharata B Rao <address@hidden>
> > ---
> >  hw/ppc/spapr.c         | 18 +++++++++++++++---
> >  hw/ppc/spapr_hcall.c   |  5 +++++
> >  include/hw/ppc/spapr.h |  3 +++
> >  3 files changed, 23 insertions(+), 3 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index e2dc77c..e14f55c 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -1436,6 +1436,14 @@ static int spapr_post_load(void *opaque, int 
> > version_id)
> >          err = spapr_rtc_import_offset(&spapr->rtc, spapr->rtc_offset);
> >      }
> >  
> > +    if (spapr->patb_entry && (spapr->patb_flags & SPAPR_PROC_TABLE_RADIX) 
> > &&
> 
> patb_entry already tells you whether the guest is radix or not
> (PATBE1_GR), you shouldn't need extra flags.
> 
> > +        kvmppc_has_cap_mmu_radix() && kvm_enabled()) {
> 
> You should also fail the migration if you have an incoming radix
> guest, but the your new KVM host can't do radix.  Or the reverse, for
> that matter.

I have checks in v1 to fail migration of radix guest to a host that doesn't
support radix. But I don't see how we can detect and fail the migration
of hash guests into hosts that don't support hash from here (i,e., from
spapr_post_load). The hash guest's migration stream would have htab
savevm entries and the target will fail as it knows not about htab
savevm entries.

Regards,
Bharata.