Re: [Qemu-devel] [RFC PATCH v1 5/6] spapr: Unregister HPT savevm handlers for radix guests

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2653 bytes --]

On Wed, May 17, 2017 at 12:48:21PM +0530, Bharata B Rao wrote:
> On Wed, May 17, 2017 at 04:59:33PM +1000, David Gibson wrote:
> > On Wed, May 17, 2017 at 09:19:21AM +0530, Bharata B Rao wrote:
> > > HPT gets created by default and later when the guest turns out to be
> > > a radix guest, the HPT is destroyed when guest does H_REGISTER_PROC_TBL
> > > hcall.
> > 
> > I don't think that's entirely accurate.  At least in some KVM
> > configurations, we assume radix first, and only allocate the HPT once
> > the guest confirms it is doing hash.
> 
> Right, that statement is true for TCG radix guests, will rephrase
> it.

And may not remain true even for TCG radix guests.  Since for TCG mode
we don't actually need either an HPT or an RPT while the guest is in
real mode, I've suggested postponing HPT allocation to CAS time for
TCG as well; I think that will make for slightly fewer differeneces
between the KVM and TCG paths.

> > > Let HTAB savevm handlers registration and unregistration follow
> > > the same model so that we don't end up having unrequired HTAB savevm
> > > handlers for radix guests.
> > > 
> > > This also ensures that HTAB savevm handlers seemlessly get destroyed and
> > > recreated like HTAB itself when hash guest reboots.
> > > 
> > > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> > > ---
> > >  hw/ppc/spapr.c         | 15 +++++++++++++--
> > >  hw/ppc/spapr_hcall.c   |  1 +
> > >  include/hw/ppc/spapr.h |  2 ++
> > >  3 files changed, 16 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index 521eef1..05abfc1 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -1237,6 +1237,7 @@ static void spapr_reallocate_hpt(sPAPRMachineState *spapr, int shift,
> > >  
> > >      /* Clean up any HPT info from a previous boot */
> > >      spapr_free_hpt(spapr);
> > > +    spapr_htab_savevm_unregister(spapr);
> > 
> > I'd prefer that the unregister be folded into spapr_free_hpt().
> > Basically we want calls that create or remove the HPT and handle
> > everything - allocation/freeing if necessary, informing KVM if
> > necessary, and registering/deregistering the savevm handlers if
> > necesary.
> > 
> > I think that will also remove the need for the trivial
> > spapr_htab_savevm_{un,}register() wrappers.
> 
> Sure, will consolidate this in the next version.
> 
> Regards,
> Bharata.
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]