[v2,02/10] ppc: Remove stub of PPC970 HID4 implementation
diff mbox series

Message ID 20200107044827.471355-3-david@gibson.dropbear.id.au
State New
Headers show
Series
  • target/ppc: Correct some errors with real mode handling
Related show

Commit Message

David Gibson Jan. 7, 2020, 4:48 a.m. UTC
The PowerPC 970 CPU was a cut-down POWER4, which had hypervisor capability.
However, it can be (and often was) strapped into "Apple mode", where the
hypervisor capabilities were disabled (essentially putting it always in
hypervisor mode).

That's actually the only mode of the 970 we support in qemu, and we're
unlikely to change that any time soon.  However, we do have a partial
implementation of the 970's HID4 register which affects things only
relevant for hypervisor mode.

That stub is also really ugly, since it attempts to duplicate the effects
of HID4 by re-encoding it into the LPCR register used in newer CPUs, but
in a really confusing way.

Just get rid of it.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 target/ppc/mmu-hash64.c         | 28 +---------------------------
 target/ppc/translate_init.inc.c | 17 ++++++-----------
 2 files changed, 7 insertions(+), 38 deletions(-)

Comments

Cédric Le Goater Jan. 7, 2020, 12:51 p.m. UTC | #1
On 1/7/20 5:48 AM, David Gibson wrote:
> The PowerPC 970 CPU was a cut-down POWER4, which had hypervisor capability.
> However, it can be (and often was) strapped into "Apple mode", where the
> hypervisor capabilities were disabled (essentially putting it always in
> hypervisor mode).
> 
> That's actually the only mode of the 970 we support in qemu, and we're
> unlikely to change that any time soon.  However, we do have a partial
> implementation of the 970's HID4 register which affects things only
> relevant for hypervisor mode.
> 
> That stub is also really ugly, since it attempts to duplicate the effects
> of HID4 by re-encoding it into the LPCR register used in newer CPUs, but
> in a really confusing way.
> 
> Just get rid of it.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>


Reviewed-by: Cédric Le Goater <clg@kaod.org>


> ---
>  target/ppc/mmu-hash64.c         | 28 +---------------------------
>  target/ppc/translate_init.inc.c | 17 ++++++-----------
>  2 files changed, 7 insertions(+), 38 deletions(-)
> 
> diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> index da8966ccf5..a881876647 100644
> --- a/target/ppc/mmu-hash64.c
> +++ b/target/ppc/mmu-hash64.c
> @@ -1091,33 +1091,6 @@ void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val)
>  
>      /* Filter out bits */
>      switch (env->mmu_model) {
> -    case POWERPC_MMU_64B: /* 970 */
> -        if (val & 0x40) {
> -            lpcr |= LPCR_LPES0;
> -        }
> -        if (val & 0x8000000000000000ull) {
> -            lpcr |= LPCR_LPES1;
> -        }
> -        if (val & 0x20) {
> -            lpcr |= (0x4ull << LPCR_RMLS_SHIFT);
> -        }
> -        if (val & 0x4000000000000000ull) {
> -            lpcr |= (0x2ull << LPCR_RMLS_SHIFT);
> -        }
> -        if (val & 0x2000000000000000ull) {
> -            lpcr |= (0x1ull << LPCR_RMLS_SHIFT);
> -        }
> -        env->spr[SPR_RMOR] = ((lpcr >> 41) & 0xffffull) << 26;
> -
> -        /*
> -         * XXX We could also write LPID from HID4 here
> -         * but since we don't tag any translation on it
> -         * it doesn't actually matter
> -         *
> -         * XXX For proper emulation of 970 we also need
> -         * to dig HRMOR out of HID5
> -         */
> -        break;
>      case POWERPC_MMU_2_03: /* P5p */
>          lpcr = val & (LPCR_RMLS | LPCR_ILE |
>                        LPCR_LPES0 | LPCR_LPES1 |
> @@ -1154,6 +1127,7 @@ void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val)
>          }
>          break;
>      default:
> +        g_assert_not_reached();
>          ;
>      }
>      env->spr[SPR_LPCR] = lpcr;
> diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
> index d33d65dff7..436d0d5a51 100644
> --- a/target/ppc/translate_init.inc.c
> +++ b/target/ppc/translate_init.inc.c
> @@ -7884,25 +7884,20 @@ static void spr_write_lpcr(DisasContext *ctx, int sprn, int gprn)
>  {
>      gen_helper_store_lpcr(cpu_env, cpu_gpr[gprn]);
>  }
> -
> -static void spr_write_970_hid4(DisasContext *ctx, int sprn, int gprn)
> -{
> -#if defined(TARGET_PPC64)
> -    spr_write_generic(ctx, sprn, gprn);
> -    gen_helper_store_lpcr(cpu_env, cpu_gpr[gprn]);
> -#endif
> -}
> -
>  #endif /* !defined(CONFIG_USER_ONLY) */
>  
>  static void gen_spr_970_lpar(CPUPPCState *env)
>  {
>  #if !defined(CONFIG_USER_ONLY)
>      /* Logical partitionning */
> -    /* PPC970: HID4 is effectively the LPCR */
> +    /* PPC970: HID4 covers things later controlled by the LPCR and
> +     * RMOR in later CPUs, but with a different encoding.  We only
> +     * support the 970 in "Apple mode" which has all hypervisor
> +     * facilities disabled by strapping, so we can basically just
> +     * ignore it */
>      spr_register(env, SPR_970_HID4, "HID4",
>                   SPR_NOACCESS, SPR_NOACCESS,
> -                 &spr_read_generic, &spr_write_970_hid4,
> +                 &spr_read_generic, &spr_write_generic,
>                   0x00000000);
>  #endif
>  }
>
Greg Kurz Jan. 7, 2020, 5:32 p.m. UTC | #2
On Tue,  7 Jan 2020 15:48:19 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> The PowerPC 970 CPU was a cut-down POWER4, which had hypervisor capability.
> However, it can be (and often was) strapped into "Apple mode", where the
> hypervisor capabilities were disabled (essentially putting it always in
> hypervisor mode).
> 
> That's actually the only mode of the 970 we support in qemu, and we're
> unlikely to change that any time soon.  However, we do have a partial
> implementation of the 970's HID4 register which affects things only
> relevant for hypervisor mode.
> 
> That stub is also really ugly, since it attempts to duplicate the effects
> of HID4 by re-encoding it into the LPCR register used in newer CPUs, but
> in a really confusing way.
> 
> Just get rid of it.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  target/ppc/mmu-hash64.c         | 28 +---------------------------
>  target/ppc/translate_init.inc.c | 17 ++++++-----------
>  2 files changed, 7 insertions(+), 38 deletions(-)
> 
> diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> index da8966ccf5..a881876647 100644
> --- a/target/ppc/mmu-hash64.c
> +++ b/target/ppc/mmu-hash64.c
> @@ -1091,33 +1091,6 @@ void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val)
>  
>      /* Filter out bits */
>      switch (env->mmu_model) {
> -    case POWERPC_MMU_64B: /* 970 */
> -        if (val & 0x40) {
> -            lpcr |= LPCR_LPES0;
> -        }
> -        if (val & 0x8000000000000000ull) {
> -            lpcr |= LPCR_LPES1;
> -        }
> -        if (val & 0x20) {
> -            lpcr |= (0x4ull << LPCR_RMLS_SHIFT);
> -        }
> -        if (val & 0x4000000000000000ull) {
> -            lpcr |= (0x2ull << LPCR_RMLS_SHIFT);
> -        }
> -        if (val & 0x2000000000000000ull) {
> -            lpcr |= (0x1ull << LPCR_RMLS_SHIFT);
> -        }
> -        env->spr[SPR_RMOR] = ((lpcr >> 41) & 0xffffull) << 26;
> -
> -        /*
> -         * XXX We could also write LPID from HID4 here
> -         * but since we don't tag any translation on it
> -         * it doesn't actually matter
> -         *
> -         * XXX For proper emulation of 970 we also need
> -         * to dig HRMOR out of HID5
> -         */
> -        break;
>      case POWERPC_MMU_2_03: /* P5p */
>          lpcr = val & (LPCR_RMLS | LPCR_ILE |
>                        LPCR_LPES0 | LPCR_LPES1 |
> @@ -1154,6 +1127,7 @@ void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val)
>          }
>          break;
>      default:
> +        g_assert_not_reached();
>          ;
>      }
>      env->spr[SPR_LPCR] = lpcr;
> diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
> index d33d65dff7..436d0d5a51 100644
> --- a/target/ppc/translate_init.inc.c
> +++ b/target/ppc/translate_init.inc.c
> @@ -7884,25 +7884,20 @@ static void spr_write_lpcr(DisasContext *ctx, int sprn, int gprn)
>  {
>      gen_helper_store_lpcr(cpu_env, cpu_gpr[gprn]);
>  }
> -
> -static void spr_write_970_hid4(DisasContext *ctx, int sprn, int gprn)
> -{
> -#if defined(TARGET_PPC64)
> -    spr_write_generic(ctx, sprn, gprn);
> -    gen_helper_store_lpcr(cpu_env, cpu_gpr[gprn]);
> -#endif
> -}
> -
>  #endif /* !defined(CONFIG_USER_ONLY) */
>  
>  static void gen_spr_970_lpar(CPUPPCState *env)
>  {
>  #if !defined(CONFIG_USER_ONLY)
>      /* Logical partitionning */
> -    /* PPC970: HID4 is effectively the LPCR */
> +    /* PPC970: HID4 covers things later controlled by the LPCR and
> +     * RMOR in later CPUs, but with a different encoding.  We only
> +     * support the 970 in "Apple mode" which has all hypervisor
> +     * facilities disabled by strapping, so we can basically just
> +     * ignore it */
>      spr_register(env, SPR_970_HID4, "HID4",
>                   SPR_NOACCESS, SPR_NOACCESS,
> -                 &spr_read_generic, &spr_write_970_hid4,
> +                 &spr_read_generic, &spr_write_generic,
>                   0x00000000);
>  #endif
>  }
Greg Kurz Jan. 7, 2020, 5:36 p.m. UTC | #3
On Tue, 7 Jan 2020 18:32:15 +0100
Greg Kurz <groug@kaod.org> wrote:

> On Tue,  7 Jan 2020 15:48:19 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > The PowerPC 970 CPU was a cut-down POWER4, which had hypervisor capability.
> > However, it can be (and often was) strapped into "Apple mode", where the
> > hypervisor capabilities were disabled (essentially putting it always in
> > hypervisor mode).

Isn't it supervisor mode instead of hypervisor mode ?

> > 
> > That's actually the only mode of the 970 we support in qemu, and we're
> > unlikely to change that any time soon.  However, we do have a partial
> > implementation of the 970's HID4 register which affects things only
> > relevant for hypervisor mode.
> > 
> > That stub is also really ugly, since it attempts to duplicate the effects
> > of HID4 by re-encoding it into the LPCR register used in newer CPUs, but
> > in a really confusing way.
> > 
> > Just get rid of it.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> 
> Reviewed-by: Greg Kurz <groug@kaod.org>
> 
> >  target/ppc/mmu-hash64.c         | 28 +---------------------------
> >  target/ppc/translate_init.inc.c | 17 ++++++-----------
> >  2 files changed, 7 insertions(+), 38 deletions(-)
> > 
> > diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> > index da8966ccf5..a881876647 100644
> > --- a/target/ppc/mmu-hash64.c
> > +++ b/target/ppc/mmu-hash64.c
> > @@ -1091,33 +1091,6 @@ void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val)
> >  
> >      /* Filter out bits */
> >      switch (env->mmu_model) {
> > -    case POWERPC_MMU_64B: /* 970 */
> > -        if (val & 0x40) {
> > -            lpcr |= LPCR_LPES0;
> > -        }
> > -        if (val & 0x8000000000000000ull) {
> > -            lpcr |= LPCR_LPES1;
> > -        }
> > -        if (val & 0x20) {
> > -            lpcr |= (0x4ull << LPCR_RMLS_SHIFT);
> > -        }
> > -        if (val & 0x4000000000000000ull) {
> > -            lpcr |= (0x2ull << LPCR_RMLS_SHIFT);
> > -        }
> > -        if (val & 0x2000000000000000ull) {
> > -            lpcr |= (0x1ull << LPCR_RMLS_SHIFT);
> > -        }
> > -        env->spr[SPR_RMOR] = ((lpcr >> 41) & 0xffffull) << 26;
> > -
> > -        /*
> > -         * XXX We could also write LPID from HID4 here
> > -         * but since we don't tag any translation on it
> > -         * it doesn't actually matter
> > -         *
> > -         * XXX For proper emulation of 970 we also need
> > -         * to dig HRMOR out of HID5
> > -         */
> > -        break;
> >      case POWERPC_MMU_2_03: /* P5p */
> >          lpcr = val & (LPCR_RMLS | LPCR_ILE |
> >                        LPCR_LPES0 | LPCR_LPES1 |
> > @@ -1154,6 +1127,7 @@ void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val)
> >          }
> >          break;
> >      default:
> > +        g_assert_not_reached();
> >          ;
> >      }
> >      env->spr[SPR_LPCR] = lpcr;
> > diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
> > index d33d65dff7..436d0d5a51 100644
> > --- a/target/ppc/translate_init.inc.c
> > +++ b/target/ppc/translate_init.inc.c
> > @@ -7884,25 +7884,20 @@ static void spr_write_lpcr(DisasContext *ctx, int sprn, int gprn)
> >  {
> >      gen_helper_store_lpcr(cpu_env, cpu_gpr[gprn]);
> >  }
> > -
> > -static void spr_write_970_hid4(DisasContext *ctx, int sprn, int gprn)
> > -{
> > -#if defined(TARGET_PPC64)
> > -    spr_write_generic(ctx, sprn, gprn);
> > -    gen_helper_store_lpcr(cpu_env, cpu_gpr[gprn]);
> > -#endif
> > -}
> > -
> >  #endif /* !defined(CONFIG_USER_ONLY) */
> >  
> >  static void gen_spr_970_lpar(CPUPPCState *env)
> >  {
> >  #if !defined(CONFIG_USER_ONLY)
> >      /* Logical partitionning */
> > -    /* PPC970: HID4 is effectively the LPCR */
> > +    /* PPC970: HID4 covers things later controlled by the LPCR and
> > +     * RMOR in later CPUs, but with a different encoding.  We only
> > +     * support the 970 in "Apple mode" which has all hypervisor
> > +     * facilities disabled by strapping, so we can basically just
> > +     * ignore it */
> >      spr_register(env, SPR_970_HID4, "HID4",
> >                   SPR_NOACCESS, SPR_NOACCESS,
> > -                 &spr_read_generic, &spr_write_970_hid4,
> > +                 &spr_read_generic, &spr_write_generic,
> >                   0x00000000);
> >  #endif
> >  }
> 
>
BALATON Zoltan Jan. 7, 2020, 6:05 p.m. UTC | #4
On Tue, 7 Jan 2020, Greg Kurz wrote:
> On Tue, 7 Jan 2020 18:32:15 +0100
> Greg Kurz <groug@kaod.org> wrote:
>
>> On Tue,  7 Jan 2020 15:48:19 +1100
>> David Gibson <david@gibson.dropbear.id.au> wrote:
>>
>>> The PowerPC 970 CPU was a cut-down POWER4, which had hypervisor capability.
>>> However, it can be (and often was) strapped into "Apple mode", where the
>>> hypervisor capabilities were disabled (essentially putting it always in
>>> hypervisor mode).
>
> Isn't it supervisor mode instead of hypervisor mode ?

By the way, do you know if this strapping is hardware or software based? 
So is it the firmware that disables it on Apple hardware or is it some CPU 
pin connected somewhere on the motherboard or it's within the CPU and 
cannot be changed? I wonder if it's theoretically possible to re-enable it 
on an Apple G5 or we would likely never see a PowerPC 970 with HV enabled?

Regards,
BALATON Zoltan
David Gibson Jan. 8, 2020, 1:08 a.m. UTC | #5
On Tue, Jan 07, 2020 at 06:36:38PM +0100, Greg Kurz wrote:
> On Tue, 7 Jan 2020 18:32:15 +0100
> Greg Kurz <groug@kaod.org> wrote:
> 
> > On Tue,  7 Jan 2020 15:48:19 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> > 
> > > The PowerPC 970 CPU was a cut-down POWER4, which had hypervisor capability.
> > > However, it can be (and often was) strapped into "Apple mode", where the
> > > hypervisor capabilities were disabled (essentially putting it always in
> > > hypervisor mode).
> 
> Isn't it supervisor mode instead of hypervisor mode ?

No; hypervisor is correct.  If the cpu was always in supervisor mode,
the boot OS couldn't access the hypervisor privileged registers that
are needed for basic setup (e.g. SDR1).  "Apple mode" means the cpu
doesn't have a supervisor mode that _isn't_ hypervisor privileged and
hence, can't run guests.

> 
> > > 
> > > That's actually the only mode of the 970 we support in qemu, and we're
> > > unlikely to change that any time soon.  However, we do have a partial
> > > implementation of the 970's HID4 register which affects things only
> > > relevant for hypervisor mode.
> > > 
> > > That stub is also really ugly, since it attempts to duplicate the effects
> > > of HID4 by re-encoding it into the LPCR register used in newer CPUs, but
> > > in a really confusing way.
> > > 
> > > Just get rid of it.
> > > 
> > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > > ---
> > 
> > Reviewed-by: Greg Kurz <groug@kaod.org>
> > 
> > >  target/ppc/mmu-hash64.c         | 28 +---------------------------
> > >  target/ppc/translate_init.inc.c | 17 ++++++-----------
> > >  2 files changed, 7 insertions(+), 38 deletions(-)
> > > 
> > > diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> > > index da8966ccf5..a881876647 100644
> > > --- a/target/ppc/mmu-hash64.c
> > > +++ b/target/ppc/mmu-hash64.c
> > > @@ -1091,33 +1091,6 @@ void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val)
> > >  
> > >      /* Filter out bits */
> > >      switch (env->mmu_model) {
> > > -    case POWERPC_MMU_64B: /* 970 */
> > > -        if (val & 0x40) {
> > > -            lpcr |= LPCR_LPES0;
> > > -        }
> > > -        if (val & 0x8000000000000000ull) {
> > > -            lpcr |= LPCR_LPES1;
> > > -        }
> > > -        if (val & 0x20) {
> > > -            lpcr |= (0x4ull << LPCR_RMLS_SHIFT);
> > > -        }
> > > -        if (val & 0x4000000000000000ull) {
> > > -            lpcr |= (0x2ull << LPCR_RMLS_SHIFT);
> > > -        }
> > > -        if (val & 0x2000000000000000ull) {
> > > -            lpcr |= (0x1ull << LPCR_RMLS_SHIFT);
> > > -        }
> > > -        env->spr[SPR_RMOR] = ((lpcr >> 41) & 0xffffull) << 26;
> > > -
> > > -        /*
> > > -         * XXX We could also write LPID from HID4 here
> > > -         * but since we don't tag any translation on it
> > > -         * it doesn't actually matter
> > > -         *
> > > -         * XXX For proper emulation of 970 we also need
> > > -         * to dig HRMOR out of HID5
> > > -         */
> > > -        break;
> > >      case POWERPC_MMU_2_03: /* P5p */
> > >          lpcr = val & (LPCR_RMLS | LPCR_ILE |
> > >                        LPCR_LPES0 | LPCR_LPES1 |
> > > @@ -1154,6 +1127,7 @@ void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val)
> > >          }
> > >          break;
> > >      default:
> > > +        g_assert_not_reached();
> > >          ;
> > >      }
> > >      env->spr[SPR_LPCR] = lpcr;
> > > diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
> > > index d33d65dff7..436d0d5a51 100644
> > > --- a/target/ppc/translate_init.inc.c
> > > +++ b/target/ppc/translate_init.inc.c
> > > @@ -7884,25 +7884,20 @@ static void spr_write_lpcr(DisasContext *ctx, int sprn, int gprn)
> > >  {
> > >      gen_helper_store_lpcr(cpu_env, cpu_gpr[gprn]);
> > >  }
> > > -
> > > -static void spr_write_970_hid4(DisasContext *ctx, int sprn, int gprn)
> > > -{
> > > -#if defined(TARGET_PPC64)
> > > -    spr_write_generic(ctx, sprn, gprn);
> > > -    gen_helper_store_lpcr(cpu_env, cpu_gpr[gprn]);
> > > -#endif
> > > -}
> > > -
> > >  #endif /* !defined(CONFIG_USER_ONLY) */
> > >  
> > >  static void gen_spr_970_lpar(CPUPPCState *env)
> > >  {
> > >  #if !defined(CONFIG_USER_ONLY)
> > >      /* Logical partitionning */
> > > -    /* PPC970: HID4 is effectively the LPCR */
> > > +    /* PPC970: HID4 covers things later controlled by the LPCR and
> > > +     * RMOR in later CPUs, but with a different encoding.  We only
> > > +     * support the 970 in "Apple mode" which has all hypervisor
> > > +     * facilities disabled by strapping, so we can basically just
> > > +     * ignore it */
> > >      spr_register(env, SPR_970_HID4, "HID4",
> > >                   SPR_NOACCESS, SPR_NOACCESS,
> > > -                 &spr_read_generic, &spr_write_970_hid4,
> > > +                 &spr_read_generic, &spr_write_generic,
> > >                   0x00000000);
> > >  #endif
> > >  }
> > 
> > 
>
David Gibson Jan. 8, 2020, 1:09 a.m. UTC | #6
On Tue, Jan 07, 2020 at 07:05:41PM +0100, BALATON Zoltan wrote:
> On Tue, 7 Jan 2020, Greg Kurz wrote:
> > On Tue, 7 Jan 2020 18:32:15 +0100
> > Greg Kurz <groug@kaod.org> wrote:
> > 
> > > On Tue,  7 Jan 2020 15:48:19 +1100
> > > David Gibson <david@gibson.dropbear.id.au> wrote:
> > > 
> > > > The PowerPC 970 CPU was a cut-down POWER4, which had hypervisor capability.
> > > > However, it can be (and often was) strapped into "Apple mode", where the
> > > > hypervisor capabilities were disabled (essentially putting it always in
> > > > hypervisor mode).
> > 
> > Isn't it supervisor mode instead of hypervisor mode ?
> 
> By the way, do you know if this strapping is hardware or software based? So
> is it the firmware that disables it on Apple hardware or is it some CPU pin
> connected somewhere on the motherboard or it's within the CPU and cannot be
> changed? I wonder if it's theoretically possible to re-enable it on an Apple
> G5 or we would likely never see a PowerPC 970 with HV enabled?

I don't know, sorry.
Paul Mackerras Jan. 8, 2020, 2:17 a.m. UTC | #7
On Tue, Jan 07, 2020 at 06:36:38PM +0100, Greg Kurz wrote:
> On Tue, 7 Jan 2020 18:32:15 +0100
> Greg Kurz <groug@kaod.org> wrote:
> 
> > On Tue,  7 Jan 2020 15:48:19 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> > 
> > > The PowerPC 970 CPU was a cut-down POWER4, which had hypervisor capability.
> > > However, it can be (and often was) strapped into "Apple mode", where the
> > > hypervisor capabilities were disabled (essentially putting it always in
> > > hypervisor mode).
> 
> Isn't it supervisor mode instead of hypervisor mode ?

No, it's hypervisor mode.  MSR[HV] always reads as 1.

Paul.
Greg Kurz Jan. 8, 2020, 8:11 a.m. UTC | #8
On Wed, 8 Jan 2020 12:08:50 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Tue, Jan 07, 2020 at 06:36:38PM +0100, Greg Kurz wrote:
> > On Tue, 7 Jan 2020 18:32:15 +0100
> > Greg Kurz <groug@kaod.org> wrote:
> > 
> > > On Tue,  7 Jan 2020 15:48:19 +1100
> > > David Gibson <david@gibson.dropbear.id.au> wrote:
> > > 
> > > > The PowerPC 970 CPU was a cut-down POWER4, which had hypervisor capability.
> > > > However, it can be (and often was) strapped into "Apple mode", where the
> > > > hypervisor capabilities were disabled (essentially putting it always in
> > > > hypervisor mode).
> > 
> > Isn't it supervisor mode instead of hypervisor mode ?
> 
> No; hypervisor is correct.  If the cpu was always in supervisor mode,
> the boot OS couldn't access the hypervisor privileged registers that
> are needed for basic setup (e.g. SDR1).  "Apple mode" means the cpu
> doesn't have a supervisor mode that _isn't_ hypervisor privileged and
> hence, can't run guests.
> 

Ok, thanks for the clarification.

> > 
> > > > 
> > > > That's actually the only mode of the 970 we support in qemu, and we're
> > > > unlikely to change that any time soon.  However, we do have a partial
> > > > implementation of the 970's HID4 register which affects things only
> > > > relevant for hypervisor mode.
> > > > 
> > > > That stub is also really ugly, since it attempts to duplicate the effects
> > > > of HID4 by re-encoding it into the LPCR register used in newer CPUs, but
> > > > in a really confusing way.
> > > > 
> > > > Just get rid of it.
> > > > 
> > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > > > ---
> > > 
> > > Reviewed-by: Greg Kurz <groug@kaod.org>
> > > 
> > > >  target/ppc/mmu-hash64.c         | 28 +---------------------------
> > > >  target/ppc/translate_init.inc.c | 17 ++++++-----------
> > > >  2 files changed, 7 insertions(+), 38 deletions(-)
> > > > 
> > > > diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> > > > index da8966ccf5..a881876647 100644
> > > > --- a/target/ppc/mmu-hash64.c
> > > > +++ b/target/ppc/mmu-hash64.c
> > > > @@ -1091,33 +1091,6 @@ void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val)
> > > >  
> > > >      /* Filter out bits */
> > > >      switch (env->mmu_model) {
> > > > -    case POWERPC_MMU_64B: /* 970 */
> > > > -        if (val & 0x40) {
> > > > -            lpcr |= LPCR_LPES0;
> > > > -        }
> > > > -        if (val & 0x8000000000000000ull) {
> > > > -            lpcr |= LPCR_LPES1;
> > > > -        }
> > > > -        if (val & 0x20) {
> > > > -            lpcr |= (0x4ull << LPCR_RMLS_SHIFT);
> > > > -        }
> > > > -        if (val & 0x4000000000000000ull) {
> > > > -            lpcr |= (0x2ull << LPCR_RMLS_SHIFT);
> > > > -        }
> > > > -        if (val & 0x2000000000000000ull) {
> > > > -            lpcr |= (0x1ull << LPCR_RMLS_SHIFT);
> > > > -        }
> > > > -        env->spr[SPR_RMOR] = ((lpcr >> 41) & 0xffffull) << 26;
> > > > -
> > > > -        /*
> > > > -         * XXX We could also write LPID from HID4 here
> > > > -         * but since we don't tag any translation on it
> > > > -         * it doesn't actually matter
> > > > -         *
> > > > -         * XXX For proper emulation of 970 we also need
> > > > -         * to dig HRMOR out of HID5
> > > > -         */
> > > > -        break;
> > > >      case POWERPC_MMU_2_03: /* P5p */
> > > >          lpcr = val & (LPCR_RMLS | LPCR_ILE |
> > > >                        LPCR_LPES0 | LPCR_LPES1 |
> > > > @@ -1154,6 +1127,7 @@ void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val)
> > > >          }
> > > >          break;
> > > >      default:
> > > > +        g_assert_not_reached();
> > > >          ;
> > > >      }
> > > >      env->spr[SPR_LPCR] = lpcr;
> > > > diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
> > > > index d33d65dff7..436d0d5a51 100644
> > > > --- a/target/ppc/translate_init.inc.c
> > > > +++ b/target/ppc/translate_init.inc.c
> > > > @@ -7884,25 +7884,20 @@ static void spr_write_lpcr(DisasContext *ctx, int sprn, int gprn)
> > > >  {
> > > >      gen_helper_store_lpcr(cpu_env, cpu_gpr[gprn]);
> > > >  }
> > > > -
> > > > -static void spr_write_970_hid4(DisasContext *ctx, int sprn, int gprn)
> > > > -{
> > > > -#if defined(TARGET_PPC64)
> > > > -    spr_write_generic(ctx, sprn, gprn);
> > > > -    gen_helper_store_lpcr(cpu_env, cpu_gpr[gprn]);
> > > > -#endif
> > > > -}
> > > > -
> > > >  #endif /* !defined(CONFIG_USER_ONLY) */
> > > >  
> > > >  static void gen_spr_970_lpar(CPUPPCState *env)
> > > >  {
> > > >  #if !defined(CONFIG_USER_ONLY)
> > > >      /* Logical partitionning */
> > > > -    /* PPC970: HID4 is effectively the LPCR */
> > > > +    /* PPC970: HID4 covers things later controlled by the LPCR and
> > > > +     * RMOR in later CPUs, but with a different encoding.  We only
> > > > +     * support the 970 in "Apple mode" which has all hypervisor
> > > > +     * facilities disabled by strapping, so we can basically just
> > > > +     * ignore it */
> > > >      spr_register(env, SPR_970_HID4, "HID4",
> > > >                   SPR_NOACCESS, SPR_NOACCESS,
> > > > -                 &spr_read_generic, &spr_write_970_hid4,
> > > > +                 &spr_read_generic, &spr_write_generic,
> > > >                   0x00000000);
> > > >  #endif
> > > >  }
> > > 
> > > 
> > 
>
Thomas Huth Jan. 8, 2020, 8:29 a.m. UTC | #9
On 07/01/2020 19.05, BALATON Zoltan wrote:
> On Tue, 7 Jan 2020, Greg Kurz wrote:
>> On Tue, 7 Jan 2020 18:32:15 +0100
>> Greg Kurz <groug@kaod.org> wrote:
>>
>>> On Tue,  7 Jan 2020 15:48:19 +1100
>>> David Gibson <david@gibson.dropbear.id.au> wrote:
>>>
>>>> The PowerPC 970 CPU was a cut-down POWER4, which had hypervisor
>>>> capability.
>>>> However, it can be (and often was) strapped into "Apple mode", where
>>>> the
>>>> hypervisor capabilities were disabled (essentially putting it always in
>>>> hypervisor mode).
>>
>> Isn't it supervisor mode instead of hypervisor mode ?
> 
> By the way, do you know if this strapping is hardware or software based?
> So is it the firmware that disables it on Apple hardware or is it some
> CPU pin connected somewhere on the motherboard or it's within the CPU
> and cannot be changed? I wonder if it's theoretically possible to
> re-enable it on an Apple G5 or we would likely never see a PowerPC 970
> with HV enabled?

I don't know how Apple disabled it, but you can buy a used Terrasoft YDL
PowerStation if you want to see it in action.

 Thomas
Greg Kurz Jan. 8, 2020, 1:35 p.m. UTC | #10
On Tue,  7 Jan 2020 15:48:19 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> The PowerPC 970 CPU was a cut-down POWER4, which had hypervisor capability.
> However, it can be (and often was) strapped into "Apple mode", where the
> hypervisor capabilities were disabled (essentially putting it always in
> hypervisor mode).
> 
> That's actually the only mode of the 970 we support in qemu, and we're
> unlikely to change that any time soon.  However, we do have a partial
> implementation of the 970's HID4 register which affects things only
> relevant for hypervisor mode.
> 

FWIW, it is currently _possible_ to boot an RHEL 6.1 guest with a 970mp
CPU under TCG (newer guests like RHEL 7.5 don't work because the kernel
seems to have instructions not supported on 970). Of course this no longer
works with this patch, but I guess we don't really care for such an old
setup, do we ?

> That stub is also really ugly, since it attempts to duplicate the effects
> of HID4 by re-encoding it into the LPCR register used in newer CPUs, but
> in a really confusing way.
> 
> Just get rid of it.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---

Just one cosmetic nit I hadn't spotted before...

>  target/ppc/mmu-hash64.c         | 28 +---------------------------
>  target/ppc/translate_init.inc.c | 17 ++++++-----------
>  2 files changed, 7 insertions(+), 38 deletions(-)
> 
> diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> index da8966ccf5..a881876647 100644
> --- a/target/ppc/mmu-hash64.c
> +++ b/target/ppc/mmu-hash64.c
> @@ -1091,33 +1091,6 @@ void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val)
>  
>      /* Filter out bits */
>      switch (env->mmu_model) {
> -    case POWERPC_MMU_64B: /* 970 */
> -        if (val & 0x40) {
> -            lpcr |= LPCR_LPES0;
> -        }
> -        if (val & 0x8000000000000000ull) {
> -            lpcr |= LPCR_LPES1;
> -        }
> -        if (val & 0x20) {
> -            lpcr |= (0x4ull << LPCR_RMLS_SHIFT);
> -        }
> -        if (val & 0x4000000000000000ull) {
> -            lpcr |= (0x2ull << LPCR_RMLS_SHIFT);
> -        }
> -        if (val & 0x2000000000000000ull) {
> -            lpcr |= (0x1ull << LPCR_RMLS_SHIFT);
> -        }
> -        env->spr[SPR_RMOR] = ((lpcr >> 41) & 0xffffull) << 26;
> -
> -        /*
> -         * XXX We could also write LPID from HID4 here
> -         * but since we don't tag any translation on it
> -         * it doesn't actually matter
> -         *
> -         * XXX For proper emulation of 970 we also need
> -         * to dig HRMOR out of HID5
> -         */
> -        break;
>      case POWERPC_MMU_2_03: /* P5p */
>          lpcr = val & (LPCR_RMLS | LPCR_ILE |
>                        LPCR_LPES0 | LPCR_LPES1 |
> @@ -1154,6 +1127,7 @@ void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val)
>          }
>          break;
>      default:
> +        g_assert_not_reached();
>          ;

... remove the line with the semi-colon ?
 
>      }
>      env->spr[SPR_LPCR] = lpcr;
> diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
> index d33d65dff7..436d0d5a51 100644
> --- a/target/ppc/translate_init.inc.c
> +++ b/target/ppc/translate_init.inc.c
> @@ -7884,25 +7884,20 @@ static void spr_write_lpcr(DisasContext *ctx, int sprn, int gprn)
>  {
>      gen_helper_store_lpcr(cpu_env, cpu_gpr[gprn]);
>  }
> -
> -static void spr_write_970_hid4(DisasContext *ctx, int sprn, int gprn)
> -{
> -#if defined(TARGET_PPC64)
> -    spr_write_generic(ctx, sprn, gprn);
> -    gen_helper_store_lpcr(cpu_env, cpu_gpr[gprn]);
> -#endif
> -}
> -
>  #endif /* !defined(CONFIG_USER_ONLY) */
>  
>  static void gen_spr_970_lpar(CPUPPCState *env)
>  {
>  #if !defined(CONFIG_USER_ONLY)
>      /* Logical partitionning */
> -    /* PPC970: HID4 is effectively the LPCR */
> +    /* PPC970: HID4 covers things later controlled by the LPCR and
> +     * RMOR in later CPUs, but with a different encoding.  We only
> +     * support the 970 in "Apple mode" which has all hypervisor
> +     * facilities disabled by strapping, so we can basically just
> +     * ignore it */
>      spr_register(env, SPR_970_HID4, "HID4",
>                   SPR_NOACCESS, SPR_NOACCESS,
> -                 &spr_read_generic, &spr_write_970_hid4,
> +                 &spr_read_generic, &spr_write_generic,
>                   0x00000000);
>  #endif
>  }

Patch
diff mbox series

diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
index da8966ccf5..a881876647 100644
--- a/target/ppc/mmu-hash64.c
+++ b/target/ppc/mmu-hash64.c
@@ -1091,33 +1091,6 @@  void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val)
 
     /* Filter out bits */
     switch (env->mmu_model) {
-    case POWERPC_MMU_64B: /* 970 */
-        if (val & 0x40) {
-            lpcr |= LPCR_LPES0;
-        }
-        if (val & 0x8000000000000000ull) {
-            lpcr |= LPCR_LPES1;
-        }
-        if (val & 0x20) {
-            lpcr |= (0x4ull << LPCR_RMLS_SHIFT);
-        }
-        if (val & 0x4000000000000000ull) {
-            lpcr |= (0x2ull << LPCR_RMLS_SHIFT);
-        }
-        if (val & 0x2000000000000000ull) {
-            lpcr |= (0x1ull << LPCR_RMLS_SHIFT);
-        }
-        env->spr[SPR_RMOR] = ((lpcr >> 41) & 0xffffull) << 26;
-
-        /*
-         * XXX We could also write LPID from HID4 here
-         * but since we don't tag any translation on it
-         * it doesn't actually matter
-         *
-         * XXX For proper emulation of 970 we also need
-         * to dig HRMOR out of HID5
-         */
-        break;
     case POWERPC_MMU_2_03: /* P5p */
         lpcr = val & (LPCR_RMLS | LPCR_ILE |
                       LPCR_LPES0 | LPCR_LPES1 |
@@ -1154,6 +1127,7 @@  void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val)
         }
         break;
     default:
+        g_assert_not_reached();
         ;
     }
     env->spr[SPR_LPCR] = lpcr;
diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
index d33d65dff7..436d0d5a51 100644
--- a/target/ppc/translate_init.inc.c
+++ b/target/ppc/translate_init.inc.c
@@ -7884,25 +7884,20 @@  static void spr_write_lpcr(DisasContext *ctx, int sprn, int gprn)
 {
     gen_helper_store_lpcr(cpu_env, cpu_gpr[gprn]);
 }
-
-static void spr_write_970_hid4(DisasContext *ctx, int sprn, int gprn)
-{
-#if defined(TARGET_PPC64)
-    spr_write_generic(ctx, sprn, gprn);
-    gen_helper_store_lpcr(cpu_env, cpu_gpr[gprn]);
-#endif
-}
-
 #endif /* !defined(CONFIG_USER_ONLY) */
 
 static void gen_spr_970_lpar(CPUPPCState *env)
 {
 #if !defined(CONFIG_USER_ONLY)
     /* Logical partitionning */
-    /* PPC970: HID4 is effectively the LPCR */
+    /* PPC970: HID4 covers things later controlled by the LPCR and
+     * RMOR in later CPUs, but with a different encoding.  We only
+     * support the 970 in "Apple mode" which has all hypervisor
+     * facilities disabled by strapping, so we can basically just
+     * ignore it */
     spr_register(env, SPR_970_HID4, "HID4",
                  SPR_NOACCESS, SPR_NOACCESS,
-                 &spr_read_generic, &spr_write_970_hid4,
+                 &spr_read_generic, &spr_write_generic,
                  0x00000000);
 #endif
 }