Re: [Qemu-devel] [Qemu-ppc] [PATCH for-2.13 03/10] target/ppc: Remove unnecessary initialization of LPCR_UPRT

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

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

On Wed, Apr 25, 2018 at 11:52:18AM +0200, Cédric Le Goater wrote:
> On 04/17/2018 09:17 AM, David Gibson wrote:
> > In cpu_ppc_set_papr() the UPRT and GTSE bits of the LPCR are initialized
> > based on on ppc64_radix_guest().  Which seems reasonable, except that
> > ppc64_radix_guest() is based on spapr->patb_entry which is only set up
> > in spapr_machine_reset, called much later than cpu_ppc_set_papr().
> > 
> > So the initialization here is pointless.  The base cpu initialization
> > already sets a value that's good enough until the guest uses an hcall to
> > configure it's preferred MMU mode.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  target/ppc/translate_init.c | 16 ----------------
> >  1 file changed, 16 deletions(-)
> > 
> > diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
> > index bb79d23b50..14f346f441 100644
> > --- a/target/ppc/translate_init.c
> > +++ b/target/ppc/translate_init.c
> > @@ -8897,22 +8897,6 @@ void cpu_ppc_set_papr(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp)
> >      lpcr->default_value &= ~LPCR_RMLS;
> >      lpcr->default_value |= 1ull << LPCR_RMLS_SHIFT;
> >  
> > -    if (env->mmu_model == POWERPC_MMU_3_00) {
> > -        /* By default we choose legacy mode and switch to new hash or radix
> > -         * when a register process table hcall is made. So disable process
> > -         * tables and guest translation shootdown by default
> > -         *
> > -         * Hot-plugged CPUs inherit from the guest radix setting under
> > -         * KVM but not under TCG. Update the default LPCR to keep new
> > -         * CPUs in sync when radix is enabled.
> > -         */
> 
> 
> This is breaking CPU hotplug under TCG. Should we reintroduce the same 
> settings in spapr_cpu_reset() now ?

Sod.  Yeah, this code is important for the hotplug case.

But, no, I don't think it should go into reset, I think it should go
into the rtas start-cpu path; it only makes sense for secondary CPUs.

-- 
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: 833 bytes --]