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

[Cédric Le Goater]

On 05/01/2018 08:39 AM, David Gibson wrote:
> 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.

As you reverted the changes in the ppc-2.13 branch, I postponed the
changes. Go ahead. 

Thanks,

C.