diff mbox series

[for-2.13,13/13] target/ppc: Fold slb_nr into PPCHash64Options

Message ID 20180405021437.16761-14-david@gibson.dropbear.id.au
State New
Headers show
Series target/ppc: Assorted cpu cleanups (esp. hash64 MMU) | expand

Commit Message

David Gibson April 5, 2018, 2:14 a.m. UTC
The env->slb_nr field gives the size of the SLB (Segment Lookaside Buffer).
This is another static-after-initialization parameter of the specific
version of the 64-bit hash MMU in the CPU.  So, this patch folds the field
into PPCHash64Options with the other hash MMU options.

This is a bit more complicated that the things previously put in there,
because slb_nr was foolishly included in the migration stream.  So we need
some of the usual dance to handle backwards compatible migration.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/pnv.c                |  2 +-
 hw/ppc/spapr.c              | 11 ++++++++---
 target/ppc/cpu.h            |  3 ++-
 target/ppc/kvm.c            |  2 +-
 target/ppc/machine.c        | 23 ++++++++++++++++++++---
 target/ppc/mmu-hash64.c     | 15 +++++++++------
 target/ppc/mmu-hash64.h     |  1 +
 target/ppc/translate_init.c | 15 ---------------
 8 files changed, 42 insertions(+), 30 deletions(-)

Comments

Greg Kurz April 5, 2018, 1:12 p.m. UTC | #1
On Thu,  5 Apr 2018 12:14:37 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> The env->slb_nr field gives the size of the SLB (Segment Lookaside Buffer).
> This is another static-after-initialization parameter of the specific
> version of the 64-bit hash MMU in the CPU.  So, this patch folds the field
> into PPCHash64Options with the other hash MMU options.
> 
> This is a bit more complicated that the things previously put in there,
> because slb_nr was foolishly included in the migration stream.  So we need
> some of the usual dance to handle backwards compatible migration.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/ppc/pnv.c                |  2 +-
>  hw/ppc/spapr.c              | 11 ++++++++---
>  target/ppc/cpu.h            |  3 ++-
>  target/ppc/kvm.c            |  2 +-
>  target/ppc/machine.c        | 23 ++++++++++++++++++++---
>  target/ppc/mmu-hash64.c     | 15 +++++++++------
>  target/ppc/mmu-hash64.h     |  1 +
>  target/ppc/translate_init.c | 15 ---------------
>  8 files changed, 42 insertions(+), 30 deletions(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 5905be3f71..53f672afa8 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -180,7 +180,7 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
>  
>      _FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq)));
>      _FDT((fdt_setprop_cell(fdt, offset, "clock-frequency", cpufreq)));
> -    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", env->slb_nr)));
> +    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", cpu->hash64_opts->slb_size)));
>      _FDT((fdt_setprop_string(fdt, offset, "status", "okay")));
>      _FDT((fdt_setprop(fdt, offset, "64-bit", NULL, 0)));
>  
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 60bc8417b6..6021631722 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -547,8 +547,8 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
>  
>      _FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq)));
>      _FDT((fdt_setprop_cell(fdt, offset, "clock-frequency", cpufreq)));
> -    _FDT((fdt_setprop_cell(fdt, offset, "slb-size", env->slb_nr)));
> -    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", env->slb_nr)));
> +    _FDT((fdt_setprop_cell(fdt, offset, "slb-size", cpu->hash64_opts->slb_size)));
> +    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", cpu->hash64_opts->slb_size)));
>      _FDT((fdt_setprop_string(fdt, offset, "status", "okay")));
>      _FDT((fdt_setprop(fdt, offset, "64-bit", NULL, 0)));
>  
> @@ -4000,7 +4000,12 @@ DEFINE_SPAPR_MACHINE(2_13, "2.13", true);
>   * pseries-2.12
>   */
>  #define SPAPR_COMPAT_2_12                                              \
> -    HW_COMPAT_2_12

This hunk doesn't apply on master, nor on your ppc-for-2.13 branch...

It looks like a patch to introduce the 2.13 machine type is missing.

FWIW, Connie has already queued a patch to do so for s390x, that also
introduces HW_COMPAT_2_12.

https://github.com/cohuck/qemu/commit/b54cde7350b6681b4349b904e0f9a8a8d58c0951

Maybe the HW_COMPAT_ macros should be added in a standalone patch ?

Cc'ing Connie for insights.

> +    HW_COMPAT_2_12                                                     \
> +    {                                                                  \
> +        .driver = TYPE_POWERPC_CPU,                                    \
> +            .property = "pre-2.13-migration",                          \
> +            .value    = "on",                                          \

indentation ?

Also, this property must be added to the TYPE_POWERPC_CPU class.

--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c
@@ -10471,6 +10471,8 @@ static Property ppc_cpu_properties[] = {
     DEFINE_PROP_BOOL("pre-2.8-migration", PowerPCCPU, pre_2_8_migration, false),
     DEFINE_PROP_BOOL("pre-2.10-migration", PowerPCCPU, pre_2_10_migration,
                      false),
+    DEFINE_PROP_BOOL("pre-2.13-migration", PowerPCCPU, pre_2_13_migration,
+                     false),
     DEFINE_PROP_END_OF_LIST(),
 };

Appart from that, the patch looks good, so:

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

With all the above points addressed, I could successfully migrate from an
older QEMU and back, so:

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

> +    },
>  
>  static void spapr_machine_2_12_instance_options(MachineState *machine)
>  {
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index c0c44fb91d..8c9e03f54d 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1025,7 +1025,6 @@ struct CPUPPCState {
>  #if defined(TARGET_PPC64)
>      /* PowerPC 64 SLB area */
>      ppc_slb_t slb[MAX_SLB_ENTRIES];
> -    int32_t slb_nr;
>      /* tcg TLB needs flush (deferred slb inval instruction typically) */
>  #endif
>      /* segment registers */
> @@ -1216,6 +1215,8 @@ struct PowerPCCPU {
>      uint64_t mig_insns_flags2;
>      uint32_t mig_nb_BATs;
>      bool pre_2_10_migration;
> +    bool pre_2_13_migration;
> +    int32_t mig_slb_nr;
>  };
>  
>  static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env)
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index b329cd8173..1bd38c6a90 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -484,7 +484,7 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu)
>              break;
>          }
>      }
> -    env->slb_nr = smmu_info.slb_size;
> +    cpu->hash64_opts->slb_size = smmu_info.slb_size;
>      if (!(smmu_info.flags & KVM_PPC_1T_SEGMENTS)) {
>          cpu->hash64_opts->flags &= ~PPC_HASH64_1TSEG;
>      }
> diff --git a/target/ppc/machine.c b/target/ppc/machine.c
> index 0634cdb295..3d6434a006 100644
> --- a/target/ppc/machine.c
> +++ b/target/ppc/machine.c
> @@ -18,6 +18,9 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
>      unsigned int i, j;
>      target_ulong sdr1;
>      uint32_t fpscr;
> +#if defined(TARGET_PPC64)
> +    int32_t slb_nr;
> +#endif
>      target_ulong xer;
>  
>      for (i = 0; i < 32; i++)
> @@ -49,7 +52,7 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
>      qemu_get_sbe32s(f, &env->access_type);
>  #if defined(TARGET_PPC64)
>      qemu_get_betls(f, &env->spr[SPR_ASR]);
> -    qemu_get_sbe32s(f, &env->slb_nr);
> +    qemu_get_sbe32s(f, &slb_nr);
>  #endif
>      qemu_get_betls(f, &sdr1);
>      for (i = 0; i < 32; i++)
> @@ -146,6 +149,15 @@ static bool cpu_pre_2_8_migration(void *opaque, int version_id)
>      return cpu->pre_2_8_migration;
>  }
>  
> +#if defined(TARGET_PPC64)
> +static bool cpu_pre_2_13_migration(void *opaque, int version_id)
> +{
> +    PowerPCCPU *cpu = opaque;
> +
> +    return cpu->pre_2_13_migration;
> +}
> +#endif
> +
>  static int cpu_pre_save(void *opaque)
>  {
>      PowerPCCPU *cpu = opaque;
> @@ -203,6 +215,11 @@ static int cpu_pre_save(void *opaque)
>          cpu->mig_insns_flags2 = env->insns_flags2 & insns_compat_mask2;
>          cpu->mig_nb_BATs = env->nb_BATs;
>      }
> +    if (cpu->pre_2_13_migration) {
> +        if (cpu->hash64_opts) {
> +            cpu->mig_slb_nr = cpu->hash64_opts->slb_size;
> +        }
> +    }
>  
>      return 0;
>  }
> @@ -478,7 +495,7 @@ static int slb_post_load(void *opaque, int version_id)
>  
>      /* We've pulled in the raw esid and vsid values from the migration
>       * stream, but we need to recompute the page size pointers */
> -    for (i = 0; i < env->slb_nr; i++) {
> +    for (i = 0; i < cpu->hash64_opts->slb_size; i++) {
>          if (ppc_store_slb(cpu, i, env->slb[i].esid, env->slb[i].vsid) < 0) {
>              /* Migration source had bad values in its SLB */
>              return -1;
> @@ -495,7 +512,7 @@ static const VMStateDescription vmstate_slb = {
>      .needed = slb_needed,
>      .post_load = slb_post_load,
>      .fields = (VMStateField[]) {
> -        VMSTATE_INT32_EQUAL(env.slb_nr, PowerPCCPU, NULL),
> +        VMSTATE_INT32_TEST(mig_slb_nr, PowerPCCPU, cpu_pre_2_13_migration),
>          VMSTATE_SLB_ARRAY(env.slb, PowerPCCPU, MAX_SLB_ENTRIES),
>          VMSTATE_END_OF_LIST()
>      }
> diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> index a5570c8774..7e0adecfd9 100644
> --- a/target/ppc/mmu-hash64.c
> +++ b/target/ppc/mmu-hash64.c
> @@ -52,7 +52,7 @@ static ppc_slb_t *slb_lookup(PowerPCCPU *cpu, target_ulong eaddr)
>      esid_256M = (eaddr & SEGMENT_MASK_256M) | SLB_ESID_V;
>      esid_1T = (eaddr & SEGMENT_MASK_1T) | SLB_ESID_V;
>  
> -    for (n = 0; n < env->slb_nr; n++) {
> +    for (n = 0; n < cpu->hash64_opts->slb_size; n++) {
>          ppc_slb_t *slb = &env->slb[n];
>  
>          LOG_SLB("%s: slot %d %016" PRIx64 " %016"
> @@ -80,7 +80,7 @@ void dump_slb(FILE *f, fprintf_function cpu_fprintf, PowerPCCPU *cpu)
>      cpu_synchronize_state(CPU(cpu));
>  
>      cpu_fprintf(f, "SLB\tESID\t\t\tVSID\n");
> -    for (i = 0; i < env->slb_nr; i++) {
> +    for (i = 0; i < cpu->hash64_opts->slb_size; i++) {
>          slbe = env->slb[i].esid;
>          slbv = env->slb[i].vsid;
>          if (slbe == 0 && slbv == 0) {
> @@ -93,10 +93,11 @@ void dump_slb(FILE *f, fprintf_function cpu_fprintf, PowerPCCPU *cpu)
>  
>  void helper_slbia(CPUPPCState *env)
>  {
> +    PowerPCCPU *cpu = ppc_env_get_cpu(env);
>      int n;
>  
>      /* XXX: Warning: slbia never invalidates the first segment */
> -    for (n = 1; n < env->slb_nr; n++) {
> +    for (n = 1; n < cpu->hash64_opts->slb_size; n++) {
>          ppc_slb_t *slb = &env->slb[n];
>  
>          if (slb->esid & SLB_ESID_V) {
> @@ -151,7 +152,7 @@ int ppc_store_slb(PowerPCCPU *cpu, target_ulong slot,
>      const PPCHash64SegmentPageSizes *sps = NULL;
>      int i;
>  
> -    if (slot >= env->slb_nr) {
> +    if (slot >= cpu->hash64_opts->slb_size) {
>          return -1; /* Bad slot number */
>      }
>      if (esid & ~(SLB_ESID_ESID | SLB_ESID_V)) {
> @@ -202,7 +203,7 @@ static int ppc_load_slb_esid(PowerPCCPU *cpu, target_ulong rb,
>      int slot = rb & 0xfff;
>      ppc_slb_t *slb = &env->slb[slot];
>  
> -    if (slot >= env->slb_nr) {
> +    if (slot >= cpu->hash64_opts->slb_size) {
>          return -1;
>      }
>  
> @@ -217,7 +218,7 @@ static int ppc_load_slb_vsid(PowerPCCPU *cpu, target_ulong rb,
>      int slot = rb & 0xfff;
>      ppc_slb_t *slb = &env->slb[slot];
>  
> -    if (slot >= env->slb_nr) {
> +    if (slot >= cpu->hash64_opts->slb_size) {
>          return -1;
>      }
>  
> @@ -1115,6 +1116,7 @@ void ppc_hash64_finalize(PowerPCCPU *cpu)
>  
>  const PPCHash64Options ppc_hash64_opts_basic = {
>      .flags = 0,
> +    .slb_size = 64,
>      .sps = {
>          { .page_shift = 12, /* 4K */
>            .slb_enc = 0,
> @@ -1129,6 +1131,7 @@ const PPCHash64Options ppc_hash64_opts_basic = {
>  
>  const PPCHash64Options ppc_hash64_opts_POWER7 = {
>      .flags = PPC_HASH64_1TSEG | PPC_HASH64_AMR | PPC_HASH64_CI_LARGEPAGE,
> +    .slb_size = 32,
>      .sps = {
>          {
>              .page_shift = 12, /* 4K */
> diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
> index f1babb0afc..d5fc03441d 100644
> --- a/target/ppc/mmu-hash64.h
> +++ b/target/ppc/mmu-hash64.h
> @@ -157,6 +157,7 @@ struct PPCHash64Options {
>  #define PPC_HASH64_AMR          0x00002
>  #define PPC_HASH64_CI_LARGEPAGE 0x00004
>      unsigned flags;
> +    unsigned slb_size;
>      PPCHash64SegmentPageSizes sps[PPC_PAGE_SIZES_MAX_SZ];
>  };
>  
> diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
> index a925cf5cd3..2f63acd310 100644
> --- a/target/ppc/translate_init.c
> +++ b/target/ppc/translate_init.c
> @@ -8195,9 +8195,6 @@ static void init_proc_970(CPUPPCState *env)
>      gen_spr_970_dbg(env);
>  
>      /* env variables */
> -#if !defined(CONFIG_USER_ONLY)
> -    env->slb_nr = 64;
> -#endif
>      env->dcache_line_size = 128;
>      env->icache_line_size = 128;
>  
> @@ -8272,9 +8269,6 @@ static void init_proc_power5plus(CPUPPCState *env)
>      gen_spr_power5p_ear(env);
>  
>      /* env variables */
> -#if !defined(CONFIG_USER_ONLY)
> -    env->slb_nr = 64;
> -#endif
>      env->dcache_line_size = 128;
>      env->icache_line_size = 128;
>  
> @@ -8389,9 +8383,6 @@ static void init_proc_POWER7(CPUPPCState *env)
>      gen_spr_power7_book4(env);
>  
>      /* env variables */
> -#if !defined(CONFIG_USER_ONLY)
> -    env->slb_nr = 32;
> -#endif
>      env->dcache_line_size = 128;
>      env->icache_line_size = 128;
>  
> @@ -8543,9 +8534,6 @@ static void init_proc_POWER8(CPUPPCState *env)
>      gen_spr_power8_rpr(env);
>  
>      /* env variables */
> -#if !defined(CONFIG_USER_ONLY)
> -    env->slb_nr = 32;
> -#endif
>      env->dcache_line_size = 128;
>      env->icache_line_size = 128;
>  
> @@ -8743,9 +8731,6 @@ static void init_proc_POWER9(CPUPPCState *env)
>                          KVM_REG_PPC_PSSCR, 0);
>  
>      /* env variables */
> -#if !defined(CONFIG_USER_ONLY)
> -    env->slb_nr = 32;
> -#endif
>      env->dcache_line_size = 128;
>      env->icache_line_size = 128;
>
Cornelia Huck April 5, 2018, 1:27 p.m. UTC | #2
On Thu, 5 Apr 2018 15:12:55 +0200
Greg Kurz <groug@kaod.org> wrote:

> On Thu,  5 Apr 2018 12:14:37 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:

> > @@ -4000,7 +4000,12 @@ DEFINE_SPAPR_MACHINE(2_13, "2.13", true);
> >   * pseries-2.12
> >   */
> >  #define SPAPR_COMPAT_2_12                                              \
> > -    HW_COMPAT_2_12  
> 
> This hunk doesn't apply on master, nor on your ppc-for-2.13 branch...
> 
> It looks like a patch to introduce the 2.13 machine type is missing.
> 
> FWIW, Connie has already queued a patch to do so for s390x, that also
> introduces HW_COMPAT_2_12.
> 
> https://github.com/cohuck/qemu/commit/b54cde7350b6681b4349b904e0f9a8a8d58c0951
> 
> Maybe the HW_COMPAT_ macros should be added in a standalone patch ?
> 
> Cc'ing Connie for insights.
> 
> > +    HW_COMPAT_2_12                                                     \
> > +    {                                                                  \
> > +        .driver = TYPE_POWERPC_CPU,                                    \
> > +            .property = "pre-2.13-migration",                          \
> > +            .value    = "on",                                          \  

I think the usual procedure is

- every arch that uses compat machines queues a patch that creates the
  new compat machine(s) and adds an empty HW_COMPAT_<version>
- whoever has their queue pulled first wins wrt hw_compat

So, I'm happy with anyone adding the empty HW_COMPAT_2_12 -- it needn't
be me :)

[We could also introduce the 2.13 machines for all architectures in one
sweep, but I think that would be generating needless churn for arch
maintainers.]
David Gibson April 6, 2018, 12:49 a.m. UTC | #3
On Thu, Apr 05, 2018 at 03:12:55PM +0200, Greg Kurz wrote:
> On Thu,  5 Apr 2018 12:14:37 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > The env->slb_nr field gives the size of the SLB (Segment Lookaside Buffer).
> > This is another static-after-initialization parameter of the specific
> > version of the 64-bit hash MMU in the CPU.  So, this patch folds the field
> > into PPCHash64Options with the other hash MMU options.
> > 
> > This is a bit more complicated that the things previously put in there,
> > because slb_nr was foolishly included in the migration stream.  So we need
> > some of the usual dance to handle backwards compatible migration.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  hw/ppc/pnv.c                |  2 +-
> >  hw/ppc/spapr.c              | 11 ++++++++---
> >  target/ppc/cpu.h            |  3 ++-
> >  target/ppc/kvm.c            |  2 +-
> >  target/ppc/machine.c        | 23 ++++++++++++++++++++---
> >  target/ppc/mmu-hash64.c     | 15 +++++++++------
> >  target/ppc/mmu-hash64.h     |  1 +
> >  target/ppc/translate_init.c | 15 ---------------
> >  8 files changed, 42 insertions(+), 30 deletions(-)
> > 
> > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> > index 5905be3f71..53f672afa8 100644
> > --- a/hw/ppc/pnv.c
> > +++ b/hw/ppc/pnv.c
> > @@ -180,7 +180,7 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
> >  
> >      _FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq)));
> >      _FDT((fdt_setprop_cell(fdt, offset, "clock-frequency", cpufreq)));
> > -    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", env->slb_nr)));
> > +    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", cpu->hash64_opts->slb_size)));
> >      _FDT((fdt_setprop_string(fdt, offset, "status", "okay")));
> >      _FDT((fdt_setprop(fdt, offset, "64-bit", NULL, 0)));
> >  
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 60bc8417b6..6021631722 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -547,8 +547,8 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
> >  
> >      _FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq)));
> >      _FDT((fdt_setprop_cell(fdt, offset, "clock-frequency", cpufreq)));
> > -    _FDT((fdt_setprop_cell(fdt, offset, "slb-size", env->slb_nr)));
> > -    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", env->slb_nr)));
> > +    _FDT((fdt_setprop_cell(fdt, offset, "slb-size", cpu->hash64_opts->slb_size)));
> > +    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", cpu->hash64_opts->slb_size)));
> >      _FDT((fdt_setprop_string(fdt, offset, "status", "okay")));
> >      _FDT((fdt_setprop(fdt, offset, "64-bit", NULL, 0)));
> >  
> > @@ -4000,7 +4000,12 @@ DEFINE_SPAPR_MACHINE(2_13, "2.13", true);
> >   * pseries-2.12
> >   */
> >  #define SPAPR_COMPAT_2_12                                              \
> > -    HW_COMPAT_2_12
> 
> This hunk doesn't apply on master, nor on your ppc-for-2.13 branch...

Uh.. I think you need to pull the ppc-for-2.13 brnch again, then..

> It looks like a patch to introduce the 2.13 machine type is missing.

..since I added a patch to do exactly that.

> FWIW, Connie has already queued a patch to do so for s390x, that also
> introduces HW_COMPAT_2_12.
> 
> https://github.com/cohuck/qemu/commit/b54cde7350b6681b4349b904e0f9a8a8d58c0951
> 
> Maybe the HW_COMPAT_ macros should be added in a standalone patch ?
> 
> Cc'ing Connie for insights.
> 
> > +    HW_COMPAT_2_12                                                     \
> > +    {                                                                  \
> > +        .driver = TYPE_POWERPC_CPU,                                    \
> > +            .property = "pre-2.13-migration",                          \
> > +            .value    = "on",                                          \
> 
> indentation ?

Oops, adjusted.

> 
> Also, this property must be added to the TYPE_POWERPC_CPU class.
> 
> --- a/target/ppc/translate_init.c
> +++ b/target/ppc/translate_init.c
> @@ -10471,6 +10471,8 @@ static Property ppc_cpu_properties[] = {
>      DEFINE_PROP_BOOL("pre-2.8-migration", PowerPCCPU, pre_2_8_migration, false),
>      DEFINE_PROP_BOOL("pre-2.10-migration", PowerPCCPU, pre_2_10_migration,
>                       false),
> +    DEFINE_PROP_BOOL("pre-2.13-migration", PowerPCCPU, pre_2_13_migration,
> +                     false),
>      DEFINE_PROP_END_OF_LIST(),
>  };

Oops again, adjusted.

> Appart from that, the patch looks good, so:
> 
> Reviewed-by: Greg Kurz <groug@kaod.org>
> 
> With all the above points addressed, I could successfully migrate from an
> older QEMU and back, so:
> 
> Tested-by: Greg Kurz <groug@kaod.org>
> 
> > +    },
> >  
> >  static void spapr_machine_2_12_instance_options(MachineState *machine)
> >  {
> > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> > index c0c44fb91d..8c9e03f54d 100644
> > --- a/target/ppc/cpu.h
> > +++ b/target/ppc/cpu.h
> > @@ -1025,7 +1025,6 @@ struct CPUPPCState {
> >  #if defined(TARGET_PPC64)
> >      /* PowerPC 64 SLB area */
> >      ppc_slb_t slb[MAX_SLB_ENTRIES];
> > -    int32_t slb_nr;
> >      /* tcg TLB needs flush (deferred slb inval instruction typically) */
> >  #endif
> >      /* segment registers */
> > @@ -1216,6 +1215,8 @@ struct PowerPCCPU {
> >      uint64_t mig_insns_flags2;
> >      uint32_t mig_nb_BATs;
> >      bool pre_2_10_migration;
> > +    bool pre_2_13_migration;
> > +    int32_t mig_slb_nr;
> >  };
> >  
> >  static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env)
> > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> > index b329cd8173..1bd38c6a90 100644
> > --- a/target/ppc/kvm.c
> > +++ b/target/ppc/kvm.c
> > @@ -484,7 +484,7 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu)
> >              break;
> >          }
> >      }
> > -    env->slb_nr = smmu_info.slb_size;
> > +    cpu->hash64_opts->slb_size = smmu_info.slb_size;
> >      if (!(smmu_info.flags & KVM_PPC_1T_SEGMENTS)) {
> >          cpu->hash64_opts->flags &= ~PPC_HASH64_1TSEG;
> >      }
> > diff --git a/target/ppc/machine.c b/target/ppc/machine.c
> > index 0634cdb295..3d6434a006 100644
> > --- a/target/ppc/machine.c
> > +++ b/target/ppc/machine.c
> > @@ -18,6 +18,9 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
> >      unsigned int i, j;
> >      target_ulong sdr1;
> >      uint32_t fpscr;
> > +#if defined(TARGET_PPC64)
> > +    int32_t slb_nr;
> > +#endif
> >      target_ulong xer;
> >  
> >      for (i = 0; i < 32; i++)
> > @@ -49,7 +52,7 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
> >      qemu_get_sbe32s(f, &env->access_type);
> >  #if defined(TARGET_PPC64)
> >      qemu_get_betls(f, &env->spr[SPR_ASR]);
> > -    qemu_get_sbe32s(f, &env->slb_nr);
> > +    qemu_get_sbe32s(f, &slb_nr);
> >  #endif
> >      qemu_get_betls(f, &sdr1);
> >      for (i = 0; i < 32; i++)
> > @@ -146,6 +149,15 @@ static bool cpu_pre_2_8_migration(void *opaque, int version_id)
> >      return cpu->pre_2_8_migration;
> >  }
> >  
> > +#if defined(TARGET_PPC64)
> > +static bool cpu_pre_2_13_migration(void *opaque, int version_id)
> > +{
> > +    PowerPCCPU *cpu = opaque;
> > +
> > +    return cpu->pre_2_13_migration;
> > +}
> > +#endif
> > +
> >  static int cpu_pre_save(void *opaque)
> >  {
> >      PowerPCCPU *cpu = opaque;
> > @@ -203,6 +215,11 @@ static int cpu_pre_save(void *opaque)
> >          cpu->mig_insns_flags2 = env->insns_flags2 & insns_compat_mask2;
> >          cpu->mig_nb_BATs = env->nb_BATs;
> >      }
> > +    if (cpu->pre_2_13_migration) {
> > +        if (cpu->hash64_opts) {
> > +            cpu->mig_slb_nr = cpu->hash64_opts->slb_size;
> > +        }
> > +    }
> >  
> >      return 0;
> >  }
> > @@ -478,7 +495,7 @@ static int slb_post_load(void *opaque, int version_id)
> >  
> >      /* We've pulled in the raw esid and vsid values from the migration
> >       * stream, but we need to recompute the page size pointers */
> > -    for (i = 0; i < env->slb_nr; i++) {
> > +    for (i = 0; i < cpu->hash64_opts->slb_size; i++) {
> >          if (ppc_store_slb(cpu, i, env->slb[i].esid, env->slb[i].vsid) < 0) {
> >              /* Migration source had bad values in its SLB */
> >              return -1;
> > @@ -495,7 +512,7 @@ static const VMStateDescription vmstate_slb = {
> >      .needed = slb_needed,
> >      .post_load = slb_post_load,
> >      .fields = (VMStateField[]) {
> > -        VMSTATE_INT32_EQUAL(env.slb_nr, PowerPCCPU, NULL),
> > +        VMSTATE_INT32_TEST(mig_slb_nr, PowerPCCPU, cpu_pre_2_13_migration),
> >          VMSTATE_SLB_ARRAY(env.slb, PowerPCCPU, MAX_SLB_ENTRIES),
> >          VMSTATE_END_OF_LIST()
> >      }
> > diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> > index a5570c8774..7e0adecfd9 100644
> > --- a/target/ppc/mmu-hash64.c
> > +++ b/target/ppc/mmu-hash64.c
> > @@ -52,7 +52,7 @@ static ppc_slb_t *slb_lookup(PowerPCCPU *cpu, target_ulong eaddr)
> >      esid_256M = (eaddr & SEGMENT_MASK_256M) | SLB_ESID_V;
> >      esid_1T = (eaddr & SEGMENT_MASK_1T) | SLB_ESID_V;
> >  
> > -    for (n = 0; n < env->slb_nr; n++) {
> > +    for (n = 0; n < cpu->hash64_opts->slb_size; n++) {
> >          ppc_slb_t *slb = &env->slb[n];
> >  
> >          LOG_SLB("%s: slot %d %016" PRIx64 " %016"
> > @@ -80,7 +80,7 @@ void dump_slb(FILE *f, fprintf_function cpu_fprintf, PowerPCCPU *cpu)
> >      cpu_synchronize_state(CPU(cpu));
> >  
> >      cpu_fprintf(f, "SLB\tESID\t\t\tVSID\n");
> > -    for (i = 0; i < env->slb_nr; i++) {
> > +    for (i = 0; i < cpu->hash64_opts->slb_size; i++) {
> >          slbe = env->slb[i].esid;
> >          slbv = env->slb[i].vsid;
> >          if (slbe == 0 && slbv == 0) {
> > @@ -93,10 +93,11 @@ void dump_slb(FILE *f, fprintf_function cpu_fprintf, PowerPCCPU *cpu)
> >  
> >  void helper_slbia(CPUPPCState *env)
> >  {
> > +    PowerPCCPU *cpu = ppc_env_get_cpu(env);
> >      int n;
> >  
> >      /* XXX: Warning: slbia never invalidates the first segment */
> > -    for (n = 1; n < env->slb_nr; n++) {
> > +    for (n = 1; n < cpu->hash64_opts->slb_size; n++) {
> >          ppc_slb_t *slb = &env->slb[n];
> >  
> >          if (slb->esid & SLB_ESID_V) {
> > @@ -151,7 +152,7 @@ int ppc_store_slb(PowerPCCPU *cpu, target_ulong slot,
> >      const PPCHash64SegmentPageSizes *sps = NULL;
> >      int i;
> >  
> > -    if (slot >= env->slb_nr) {
> > +    if (slot >= cpu->hash64_opts->slb_size) {
> >          return -1; /* Bad slot number */
> >      }
> >      if (esid & ~(SLB_ESID_ESID | SLB_ESID_V)) {
> > @@ -202,7 +203,7 @@ static int ppc_load_slb_esid(PowerPCCPU *cpu, target_ulong rb,
> >      int slot = rb & 0xfff;
> >      ppc_slb_t *slb = &env->slb[slot];
> >  
> > -    if (slot >= env->slb_nr) {
> > +    if (slot >= cpu->hash64_opts->slb_size) {
> >          return -1;
> >      }
> >  
> > @@ -217,7 +218,7 @@ static int ppc_load_slb_vsid(PowerPCCPU *cpu, target_ulong rb,
> >      int slot = rb & 0xfff;
> >      ppc_slb_t *slb = &env->slb[slot];
> >  
> > -    if (slot >= env->slb_nr) {
> > +    if (slot >= cpu->hash64_opts->slb_size) {
> >          return -1;
> >      }
> >  
> > @@ -1115,6 +1116,7 @@ void ppc_hash64_finalize(PowerPCCPU *cpu)
> >  
> >  const PPCHash64Options ppc_hash64_opts_basic = {
> >      .flags = 0,
> > +    .slb_size = 64,
> >      .sps = {
> >          { .page_shift = 12, /* 4K */
> >            .slb_enc = 0,
> > @@ -1129,6 +1131,7 @@ const PPCHash64Options ppc_hash64_opts_basic = {
> >  
> >  const PPCHash64Options ppc_hash64_opts_POWER7 = {
> >      .flags = PPC_HASH64_1TSEG | PPC_HASH64_AMR | PPC_HASH64_CI_LARGEPAGE,
> > +    .slb_size = 32,
> >      .sps = {
> >          {
> >              .page_shift = 12, /* 4K */
> > diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
> > index f1babb0afc..d5fc03441d 100644
> > --- a/target/ppc/mmu-hash64.h
> > +++ b/target/ppc/mmu-hash64.h
> > @@ -157,6 +157,7 @@ struct PPCHash64Options {
> >  #define PPC_HASH64_AMR          0x00002
> >  #define PPC_HASH64_CI_LARGEPAGE 0x00004
> >      unsigned flags;
> > +    unsigned slb_size;
> >      PPCHash64SegmentPageSizes sps[PPC_PAGE_SIZES_MAX_SZ];
> >  };
> >  
> > diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
> > index a925cf5cd3..2f63acd310 100644
> > --- a/target/ppc/translate_init.c
> > +++ b/target/ppc/translate_init.c
> > @@ -8195,9 +8195,6 @@ static void init_proc_970(CPUPPCState *env)
> >      gen_spr_970_dbg(env);
> >  
> >      /* env variables */
> > -#if !defined(CONFIG_USER_ONLY)
> > -    env->slb_nr = 64;
> > -#endif
> >      env->dcache_line_size = 128;
> >      env->icache_line_size = 128;
> >  
> > @@ -8272,9 +8269,6 @@ static void init_proc_power5plus(CPUPPCState *env)
> >      gen_spr_power5p_ear(env);
> >  
> >      /* env variables */
> > -#if !defined(CONFIG_USER_ONLY)
> > -    env->slb_nr = 64;
> > -#endif
> >      env->dcache_line_size = 128;
> >      env->icache_line_size = 128;
> >  
> > @@ -8389,9 +8383,6 @@ static void init_proc_POWER7(CPUPPCState *env)
> >      gen_spr_power7_book4(env);
> >  
> >      /* env variables */
> > -#if !defined(CONFIG_USER_ONLY)
> > -    env->slb_nr = 32;
> > -#endif
> >      env->dcache_line_size = 128;
> >      env->icache_line_size = 128;
> >  
> > @@ -8543,9 +8534,6 @@ static void init_proc_POWER8(CPUPPCState *env)
> >      gen_spr_power8_rpr(env);
> >  
> >      /* env variables */
> > -#if !defined(CONFIG_USER_ONLY)
> > -    env->slb_nr = 32;
> > -#endif
> >      env->dcache_line_size = 128;
> >      env->icache_line_size = 128;
> >  
> > @@ -8743,9 +8731,6 @@ static void init_proc_POWER9(CPUPPCState *env)
> >                          KVM_REG_PPC_PSSCR, 0);
> >  
> >      /* env variables */
> > -#if !defined(CONFIG_USER_ONLY)
> > -    env->slb_nr = 32;
> > -#endif
> >      env->dcache_line_size = 128;
> >      env->icache_line_size = 128;
> >  
>
David Gibson April 6, 2018, 1:09 a.m. UTC | #4
On Thu, Apr 05, 2018 at 03:27:34PM +0200, Cornelia Huck wrote:
> On Thu, 5 Apr 2018 15:12:55 +0200
> Greg Kurz <groug@kaod.org> wrote:
> 
> > On Thu,  5 Apr 2018 12:14:37 +1000
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > > @@ -4000,7 +4000,12 @@ DEFINE_SPAPR_MACHINE(2_13, "2.13", true);
> > >   * pseries-2.12
> > >   */
> > >  #define SPAPR_COMPAT_2_12                                              \
> > > -    HW_COMPAT_2_12  
> > 
> > This hunk doesn't apply on master, nor on your ppc-for-2.13 branch...
> > 
> > It looks like a patch to introduce the 2.13 machine type is missing.
> > 
> > FWIW, Connie has already queued a patch to do so for s390x, that also
> > introduces HW_COMPAT_2_12.
> > 
> > https://github.com/cohuck/qemu/commit/b54cde7350b6681b4349b904e0f9a8a8d58c0951
> > 
> > Maybe the HW_COMPAT_ macros should be added in a standalone patch ?
> > 
> > Cc'ing Connie for insights.
> > 
> > > +    HW_COMPAT_2_12                                                     \
> > > +    {                                                                  \
> > > +        .driver = TYPE_POWERPC_CPU,                                    \
> > > +            .property = "pre-2.13-migration",                          \
> > > +            .value    = "on",                                          \  
> 
> I think the usual procedure is
> 
> - every arch that uses compat machines queues a patch that creates the
>   new compat machine(s) and adds an empty HW_COMPAT_<version>
> - whoever has their queue pulled first wins wrt hw_compat

That's my understanding as well.  It's an easy conflict to resolve.

> So, I'm happy with anyone adding the empty HW_COMPAT_2_12 -- it needn't
> be me :)

Likewise.  I'm planning to keep it in my tree for the time being, so
as not to rely on external patches, but when the 2.13 tree opens, who
wins the race is mostly chance, and that's fine.

> [We could also introduce the 2.13 machines for all architectures in one
> sweep, but I think that would be generating needless churn for arch
> maintainers.]
>
diff mbox series

Patch

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 5905be3f71..53f672afa8 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -180,7 +180,7 @@  static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
 
     _FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq)));
     _FDT((fdt_setprop_cell(fdt, offset, "clock-frequency", cpufreq)));
-    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", env->slb_nr)));
+    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", cpu->hash64_opts->slb_size)));
     _FDT((fdt_setprop_string(fdt, offset, "status", "okay")));
     _FDT((fdt_setprop(fdt, offset, "64-bit", NULL, 0)));
 
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 60bc8417b6..6021631722 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -547,8 +547,8 @@  static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
 
     _FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq)));
     _FDT((fdt_setprop_cell(fdt, offset, "clock-frequency", cpufreq)));
-    _FDT((fdt_setprop_cell(fdt, offset, "slb-size", env->slb_nr)));
-    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", env->slb_nr)));
+    _FDT((fdt_setprop_cell(fdt, offset, "slb-size", cpu->hash64_opts->slb_size)));
+    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", cpu->hash64_opts->slb_size)));
     _FDT((fdt_setprop_string(fdt, offset, "status", "okay")));
     _FDT((fdt_setprop(fdt, offset, "64-bit", NULL, 0)));
 
@@ -4000,7 +4000,12 @@  DEFINE_SPAPR_MACHINE(2_13, "2.13", true);
  * pseries-2.12
  */
 #define SPAPR_COMPAT_2_12                                              \
-    HW_COMPAT_2_12
+    HW_COMPAT_2_12                                                     \
+    {                                                                  \
+        .driver = TYPE_POWERPC_CPU,                                    \
+            .property = "pre-2.13-migration",                          \
+            .value    = "on",                                          \
+    },
 
 static void spapr_machine_2_12_instance_options(MachineState *machine)
 {
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index c0c44fb91d..8c9e03f54d 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1025,7 +1025,6 @@  struct CPUPPCState {
 #if defined(TARGET_PPC64)
     /* PowerPC 64 SLB area */
     ppc_slb_t slb[MAX_SLB_ENTRIES];
-    int32_t slb_nr;
     /* tcg TLB needs flush (deferred slb inval instruction typically) */
 #endif
     /* segment registers */
@@ -1216,6 +1215,8 @@  struct PowerPCCPU {
     uint64_t mig_insns_flags2;
     uint32_t mig_nb_BATs;
     bool pre_2_10_migration;
+    bool pre_2_13_migration;
+    int32_t mig_slb_nr;
 };
 
 static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env)
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index b329cd8173..1bd38c6a90 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -484,7 +484,7 @@  static void kvm_fixup_page_sizes(PowerPCCPU *cpu)
             break;
         }
     }
-    env->slb_nr = smmu_info.slb_size;
+    cpu->hash64_opts->slb_size = smmu_info.slb_size;
     if (!(smmu_info.flags & KVM_PPC_1T_SEGMENTS)) {
         cpu->hash64_opts->flags &= ~PPC_HASH64_1TSEG;
     }
diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index 0634cdb295..3d6434a006 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -18,6 +18,9 @@  static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
     unsigned int i, j;
     target_ulong sdr1;
     uint32_t fpscr;
+#if defined(TARGET_PPC64)
+    int32_t slb_nr;
+#endif
     target_ulong xer;
 
     for (i = 0; i < 32; i++)
@@ -49,7 +52,7 @@  static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
     qemu_get_sbe32s(f, &env->access_type);
 #if defined(TARGET_PPC64)
     qemu_get_betls(f, &env->spr[SPR_ASR]);
-    qemu_get_sbe32s(f, &env->slb_nr);
+    qemu_get_sbe32s(f, &slb_nr);
 #endif
     qemu_get_betls(f, &sdr1);
     for (i = 0; i < 32; i++)
@@ -146,6 +149,15 @@  static bool cpu_pre_2_8_migration(void *opaque, int version_id)
     return cpu->pre_2_8_migration;
 }
 
+#if defined(TARGET_PPC64)
+static bool cpu_pre_2_13_migration(void *opaque, int version_id)
+{
+    PowerPCCPU *cpu = opaque;
+
+    return cpu->pre_2_13_migration;
+}
+#endif
+
 static int cpu_pre_save(void *opaque)
 {
     PowerPCCPU *cpu = opaque;
@@ -203,6 +215,11 @@  static int cpu_pre_save(void *opaque)
         cpu->mig_insns_flags2 = env->insns_flags2 & insns_compat_mask2;
         cpu->mig_nb_BATs = env->nb_BATs;
     }
+    if (cpu->pre_2_13_migration) {
+        if (cpu->hash64_opts) {
+            cpu->mig_slb_nr = cpu->hash64_opts->slb_size;
+        }
+    }
 
     return 0;
 }
@@ -478,7 +495,7 @@  static int slb_post_load(void *opaque, int version_id)
 
     /* We've pulled in the raw esid and vsid values from the migration
      * stream, but we need to recompute the page size pointers */
-    for (i = 0; i < env->slb_nr; i++) {
+    for (i = 0; i < cpu->hash64_opts->slb_size; i++) {
         if (ppc_store_slb(cpu, i, env->slb[i].esid, env->slb[i].vsid) < 0) {
             /* Migration source had bad values in its SLB */
             return -1;
@@ -495,7 +512,7 @@  static const VMStateDescription vmstate_slb = {
     .needed = slb_needed,
     .post_load = slb_post_load,
     .fields = (VMStateField[]) {
-        VMSTATE_INT32_EQUAL(env.slb_nr, PowerPCCPU, NULL),
+        VMSTATE_INT32_TEST(mig_slb_nr, PowerPCCPU, cpu_pre_2_13_migration),
         VMSTATE_SLB_ARRAY(env.slb, PowerPCCPU, MAX_SLB_ENTRIES),
         VMSTATE_END_OF_LIST()
     }
diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
index a5570c8774..7e0adecfd9 100644
--- a/target/ppc/mmu-hash64.c
+++ b/target/ppc/mmu-hash64.c
@@ -52,7 +52,7 @@  static ppc_slb_t *slb_lookup(PowerPCCPU *cpu, target_ulong eaddr)
     esid_256M = (eaddr & SEGMENT_MASK_256M) | SLB_ESID_V;
     esid_1T = (eaddr & SEGMENT_MASK_1T) | SLB_ESID_V;
 
-    for (n = 0; n < env->slb_nr; n++) {
+    for (n = 0; n < cpu->hash64_opts->slb_size; n++) {
         ppc_slb_t *slb = &env->slb[n];
 
         LOG_SLB("%s: slot %d %016" PRIx64 " %016"
@@ -80,7 +80,7 @@  void dump_slb(FILE *f, fprintf_function cpu_fprintf, PowerPCCPU *cpu)
     cpu_synchronize_state(CPU(cpu));
 
     cpu_fprintf(f, "SLB\tESID\t\t\tVSID\n");
-    for (i = 0; i < env->slb_nr; i++) {
+    for (i = 0; i < cpu->hash64_opts->slb_size; i++) {
         slbe = env->slb[i].esid;
         slbv = env->slb[i].vsid;
         if (slbe == 0 && slbv == 0) {
@@ -93,10 +93,11 @@  void dump_slb(FILE *f, fprintf_function cpu_fprintf, PowerPCCPU *cpu)
 
 void helper_slbia(CPUPPCState *env)
 {
+    PowerPCCPU *cpu = ppc_env_get_cpu(env);
     int n;
 
     /* XXX: Warning: slbia never invalidates the first segment */
-    for (n = 1; n < env->slb_nr; n++) {
+    for (n = 1; n < cpu->hash64_opts->slb_size; n++) {
         ppc_slb_t *slb = &env->slb[n];
 
         if (slb->esid & SLB_ESID_V) {
@@ -151,7 +152,7 @@  int ppc_store_slb(PowerPCCPU *cpu, target_ulong slot,
     const PPCHash64SegmentPageSizes *sps = NULL;
     int i;
 
-    if (slot >= env->slb_nr) {
+    if (slot >= cpu->hash64_opts->slb_size) {
         return -1; /* Bad slot number */
     }
     if (esid & ~(SLB_ESID_ESID | SLB_ESID_V)) {
@@ -202,7 +203,7 @@  static int ppc_load_slb_esid(PowerPCCPU *cpu, target_ulong rb,
     int slot = rb & 0xfff;
     ppc_slb_t *slb = &env->slb[slot];
 
-    if (slot >= env->slb_nr) {
+    if (slot >= cpu->hash64_opts->slb_size) {
         return -1;
     }
 
@@ -217,7 +218,7 @@  static int ppc_load_slb_vsid(PowerPCCPU *cpu, target_ulong rb,
     int slot = rb & 0xfff;
     ppc_slb_t *slb = &env->slb[slot];
 
-    if (slot >= env->slb_nr) {
+    if (slot >= cpu->hash64_opts->slb_size) {
         return -1;
     }
 
@@ -1115,6 +1116,7 @@  void ppc_hash64_finalize(PowerPCCPU *cpu)
 
 const PPCHash64Options ppc_hash64_opts_basic = {
     .flags = 0,
+    .slb_size = 64,
     .sps = {
         { .page_shift = 12, /* 4K */
           .slb_enc = 0,
@@ -1129,6 +1131,7 @@  const PPCHash64Options ppc_hash64_opts_basic = {
 
 const PPCHash64Options ppc_hash64_opts_POWER7 = {
     .flags = PPC_HASH64_1TSEG | PPC_HASH64_AMR | PPC_HASH64_CI_LARGEPAGE,
+    .slb_size = 32,
     .sps = {
         {
             .page_shift = 12, /* 4K */
diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
index f1babb0afc..d5fc03441d 100644
--- a/target/ppc/mmu-hash64.h
+++ b/target/ppc/mmu-hash64.h
@@ -157,6 +157,7 @@  struct PPCHash64Options {
 #define PPC_HASH64_AMR          0x00002
 #define PPC_HASH64_CI_LARGEPAGE 0x00004
     unsigned flags;
+    unsigned slb_size;
     PPCHash64SegmentPageSizes sps[PPC_PAGE_SIZES_MAX_SZ];
 };
 
diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
index a925cf5cd3..2f63acd310 100644
--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c
@@ -8195,9 +8195,6 @@  static void init_proc_970(CPUPPCState *env)
     gen_spr_970_dbg(env);
 
     /* env variables */
-#if !defined(CONFIG_USER_ONLY)
-    env->slb_nr = 64;
-#endif
     env->dcache_line_size = 128;
     env->icache_line_size = 128;
 
@@ -8272,9 +8269,6 @@  static void init_proc_power5plus(CPUPPCState *env)
     gen_spr_power5p_ear(env);
 
     /* env variables */
-#if !defined(CONFIG_USER_ONLY)
-    env->slb_nr = 64;
-#endif
     env->dcache_line_size = 128;
     env->icache_line_size = 128;
 
@@ -8389,9 +8383,6 @@  static void init_proc_POWER7(CPUPPCState *env)
     gen_spr_power7_book4(env);
 
     /* env variables */
-#if !defined(CONFIG_USER_ONLY)
-    env->slb_nr = 32;
-#endif
     env->dcache_line_size = 128;
     env->icache_line_size = 128;
 
@@ -8543,9 +8534,6 @@  static void init_proc_POWER8(CPUPPCState *env)
     gen_spr_power8_rpr(env);
 
     /* env variables */
-#if !defined(CONFIG_USER_ONLY)
-    env->slb_nr = 32;
-#endif
     env->dcache_line_size = 128;
     env->icache_line_size = 128;
 
@@ -8743,9 +8731,6 @@  static void init_proc_POWER9(CPUPPCState *env)
                         KVM_REG_PPC_PSSCR, 0);
 
     /* env variables */
-#if !defined(CONFIG_USER_ONLY)
-    env->slb_nr = 32;
-#endif
     env->dcache_line_size = 128;
     env->icache_line_size = 128;