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

[David Gibson]

On Thu, Apr 26, 2018 at 09:20:11AM +0200, Cédric Le Goater wrote:
> On 04/26/2018 08:46 AM, David Gibson wrote:
> > 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 <address@hidden>
> >>> ---
> >>>  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.
> 
> yes. I will send a fix.

Not sure if this is still on your radar, but if it is, don't bother.
I've already made a fix (along with various others) and will be
posting soonish.

-- 
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

signature.asc
Description: PGP signature