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.
- */
- if (ppc64_radix_guest(cpu)) {
- lpcr->default_value |= LPCR_UPRT | LPCR_GTSE;
- } else {
- lpcr->default_value &= ~(LPCR_UPRT | LPCR_GTSE);
- }
- }
-
/* Only enable Power-saving mode Exit Cause exceptions on the boot
* CPU. The RTAS command start-cpu will enable them on secondaries.
*/
--
2.14.3
On Tue, 17 Apr 2018 17:17:15 +1000
David Gibson <david@gibson.dropbear.id.au> 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>
> ---
Reviewed-by: Greg Kurz <groug@kaod.org>
> 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.
> - */
> - if (ppc64_radix_guest(cpu)) {
> - lpcr->default_value |= LPCR_UPRT | LPCR_GTSE;
> - } else {
> - lpcr->default_value &= ~(LPCR_UPRT | LPCR_GTSE);
> - }
> - }
> -
> /* Only enable Power-saving mode Exit Cause exceptions on the boot
> * CPU. The RTAS command start-cpu will enable them on secondaries.
> */
On Fri, Apr 20, 2018 at 01:34:14PM +0200, Greg Kurz wrote: > On Tue, 17 Apr 2018 17:17:15 +1000 > David Gibson <david@gibson.dropbear.id.au> 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> > > --- > > Reviewed-by: Greg Kurz <groug@kaod.org> Merged to ppc-for-2.13. -- 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
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 ?
Thanks,
C.
> - if (ppc64_radix_guest(cpu)) {
> - lpcr->default_value |= LPCR_UPRT | LPCR_GTSE;
> - } else {
> - lpcr->default_value &= ~(LPCR_UPRT | LPCR_GTSE);
> - }
> - }
> -
> /* Only enable Power-saving mode Exit Cause exceptions on the boot
> * CPU. The RTAS command start-cpu will enable them on secondaries.
> */
>
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
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 <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.
yes. I will send a fix.
Thanks,
C.
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 <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.
>
> 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
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 <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.
>>
>> 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.
© 2016 - 2026 Red Hat, Inc.