Re: [Qemu-devel] [RFC for-2.13 05/12] target/ppc: Remove fallback 64k pagesize information

[David Gibson]

On Tue, Mar 27, 2018 at 03:54:55PM +0200, Greg Kurz wrote:
> On Tue, 27 Mar 2018 15:37:34 +1100
> David Gibson <address@hidden> wrote:
> 
> > CPU definitions for cpus with the 64-bit hash MMU can include a table of
> > available pagesizes.  If this isn't supplied ppc_cpu_instance_init() will
> > fill it in a fallback table based on the POWERPC_MMU_64K bit in mmu_model.
> > 
> > However, it turns out all the cpus which support 64K pages already include
> > an explicit table of page sizes, so there's no point to the fallback table
> > including 64k pages.
> > 
> 
> I was thinking that 64k pages came with POWER5+. At least, this is mentioned
> in several places:
> 
> https://www.ibm.com/support/knowledgecenter/ssw_aix_72/com.ibm.aix.performance/supported_page_sizes_processor_type.htm
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a4a0f2524acc2c602cadd8e743be19d86f3a746b

Ok, I didn't know that.  However, that was already wrong - we weren't
setting the MMU_64K bit for POWER5+.

> And we do support POWER5+ with TCG and KVM PR.

Well, theoretically.  I doubt it's been tested in years, and I
strongly suspect it won't actually work.

> Shouldn't we include an explicit
> table of pages sizes there as well ?

Yeah, but I think it makes more sense to fix that later.  Or, more
likely, not, since no-one actually cares about POWER5.

> 
> > That removes the only place which tests POWERPC_MMU_64K, so we can remove
> > it.  Which in turn allows some logic to be removed from
> > kvm_fixup_page_sizes().
> > 
> > Signed-off-by: David Gibson <address@hidden>
> > ---
> >  target/ppc/cpu-qom.h        |  4 ----
> >  target/ppc/kvm.c            |  7 -------
> >  target/ppc/translate_init.c | 20 ++------------------
> >  3 files changed, 2 insertions(+), 29 deletions(-)
> > 
> > diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
> > index deaa46a14b..9bbb05cf62 100644
> > --- a/target/ppc/cpu-qom.h
> > +++ b/target/ppc/cpu-qom.h
> > @@ -70,7 +70,6 @@ enum powerpc_mmu_t {
> >  #define POWERPC_MMU_64       0x00010000
> >  #define POWERPC_MMU_1TSEG    0x00020000
> >  #define POWERPC_MMU_AMR      0x00040000
> > -#define POWERPC_MMU_64K      0x00080000
> >  #define POWERPC_MMU_V3       0x00100000 /* ISA V3.00 MMU Support */
> >      /* 64 bits PowerPC MMU                                     */
> >      POWERPC_MMU_64B        = POWERPC_MMU_64 | 0x00000001,
> > @@ -78,15 +77,12 @@ enum powerpc_mmu_t {
> >      POWERPC_MMU_2_03       = POWERPC_MMU_64 | 0x00000002,
> >      /* Architecture 2.06 variant                               */
> >      POWERPC_MMU_2_06       = POWERPC_MMU_64 | POWERPC_MMU_1TSEG
> > -                             | POWERPC_MMU_64K
> >                               | POWERPC_MMU_AMR | 0x00000003,
> >      /* Architecture 2.07 variant                               */
> >      POWERPC_MMU_2_07       = POWERPC_MMU_64 | POWERPC_MMU_1TSEG
> > -                             | POWERPC_MMU_64K
> >                               | POWERPC_MMU_AMR | 0x00000004,
> >      /* Architecture 3.00 variant                               */
> >      POWERPC_MMU_3_00       = POWERPC_MMU_64 | POWERPC_MMU_1TSEG
> > -                             | POWERPC_MMU_64K
> >                               | POWERPC_MMU_AMR | POWERPC_MMU_V3
> >                               | 0x00000005,
> >  };
> > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> > index 79a436a384..6160356a4a 100644
> > --- a/target/ppc/kvm.c
> > +++ b/target/ppc/kvm.c
> > @@ -425,7 +425,6 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu)
> >      static bool has_smmu_info;
> >      CPUPPCState *env = &cpu->env;
> >      int iq, ik, jq, jk;
> > -    bool has_64k_pages = false;
> >  
> >      /* We only handle page sizes for 64-bit server guests for now */
> >      if (!(env->mmu_model & POWERPC_MMU_64)) {
> > @@ -471,9 +470,6 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu)
> >                                       ksps->enc[jk].page_shift)) {
> >                  continue;
> >              }
> > -            if (ksps->enc[jk].page_shift == 16) {
> > -                has_64k_pages = true;
> > -            }
> >              qsps->enc[jq].page_shift = ksps->enc[jk].page_shift;
> >              qsps->enc[jq].pte_enc = ksps->enc[jk].pte_enc;
> >              if (++jq >= PPC_PAGE_SIZES_MAX_SZ) {
> > @@ -488,9 +484,6 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu)
> >      if (!(smmu_info.flags & KVM_PPC_1T_SEGMENTS)) {
> >          env->mmu_model &= ~POWERPC_MMU_1TSEG;
> >      }
> > -    if (!has_64k_pages) {
> > -        env->mmu_model &= ~POWERPC_MMU_64K;
> > -    }
> >  }
> >  
> >  bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path)
> > diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
> > index 29bd6f3654..99be6fcd68 100644
> > --- a/target/ppc/translate_init.c
> > +++ b/target/ppc/translate_init.c
> > @@ -10469,7 +10469,7 @@ static void ppc_cpu_instance_init(Object *obj)
> >          env->sps = *pcc->sps;
> >      } else if (env->mmu_model & POWERPC_MMU_64) {
> >          /* Use default sets of page sizes. We don't support MPSS */
> > -        static const struct ppc_segment_page_sizes defsps_4k = {
> > +        static const struct ppc_segment_page_sizes defsps = {
> >              .sps = {
> >                  { .page_shift = 12, /* 4K */
> >                    .slb_enc = 0,
> > @@ -10481,23 +10481,7 @@ static void ppc_cpu_instance_init(Object *obj)
> >                  },
> >              },
> >          };
> > -        static const struct ppc_segment_page_sizes defsps_64k = {
> > -            .sps = {
> > -                { .page_shift = 12, /* 4K */
> > -                  .slb_enc = 0,
> > -                  .enc = { { .page_shift = 12, .pte_enc = 0 } }
> > -                },
> > -                { .page_shift = 16, /* 64K */
> > -                  .slb_enc = 0x110,
> > -                  .enc = { { .page_shift = 16, .pte_enc = 1 } }
> > -                },
> > -                { .page_shift = 24, /* 16M */
> > -                  .slb_enc = 0x100,
> > -                  .enc = { { .page_shift = 24, .pte_enc = 0 } }
> > -                },
> > -            },
> > -        };
> > -        env->sps = (env->mmu_model & POWERPC_MMU_64K) ? defsps_64k : 
> > defsps_4k;
> > +        env->sps = defsps;
> >      }
> >  #endif /* defined(TARGET_PPC64) */
> >  }
> 

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature