qemu-ppc
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-ppc] [PATCH 1/3] ppc/hash64: Various fixes in PTE search in th


From: Benjamin Herrenschmidt
Subject: Re: [Qemu-ppc] [PATCH 1/3] ppc/hash64: Various fixes in PTE search in the hash table
Date: Tue, 05 Jul 2016 07:18:54 +1000

On Mon, 2016-07-04 at 16:26 +0200, Cédric Le Goater wrote:
> On 07/04/2016 09:44 AM, Benjamin Herrenschmidt wrote:
> > We need to properly ignore different base size encodings,
> > since Linux might end up mixing them up. This also has the
> > advantage of speeding things up by no longer searching the
> > whole SPS array but only the sub-array for the SLB size.
> > 
> > This includes some fixes by Cédric Le Goater <address@hidden>
> > 
> > Signed-off-by: Benjamin Herrenschmidt <address@hidden>
> 
> 
> Ben,
> 
> I can not apply these pathes on f3a161e3e2aa, David's branch ppc-for-
> 2.7 :/

Ah, David, you added "add proper real mode translation support" which I
rewrote in my series so it collides... can you drop it ?

Ben.

> C.
> 
> > ---
> >  hw/ppc/spapr_hcall.c    |   8 +--
> >  target-ppc/mmu-hash64.c | 172 +++++++++++++++++++-----------------
> > ------------
> >  target-ppc/mmu-hash64.h |   5 +-
> >  3 files changed, 75 insertions(+), 110 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> > index e011ed4..fe05078 100644
> > --- a/hw/ppc/spapr_hcall.c
> > +++ b/hw/ppc/spapr_hcall.c
> > @@ -83,18 +83,18 @@ static target_ulong h_enter(PowerPCCPU *cpu,
> > sPAPRMachineState *spapr,
> >      target_ulong pte_index = args[1];
> >      target_ulong pteh = args[2];
> >      target_ulong ptel = args[3];
> > -    unsigned apshift, spshift;
> > +    unsigned pshift;
> >      target_ulong raddr;
> >      target_ulong index;
> >      uint64_t token;
> >  
> > -    apshift = ppc_hash64_hpte_page_shift_noslb(cpu, pteh, ptel,
> > &spshift);
> > -    if (!apshift) {
> > +    pshift = ppc_hash64_hpte_page_shift_noslb(cpu, pteh, ptel);
> > +    if (!pshift) {
> >          /* Bad page size encoding */
> >          return H_PARAMETER;
> >      }
> >  
> > -    raddr = (ptel & HPTE64_R_RPN) & ~((1ULL << apshift) - 1);
> > +    raddr = (ptel & HPTE64_R_RPN) & ~((1ULL << pshift) - 1);
> >  
> >      if (is_ram_address(spapr, raddr)) {
> >          /* Regular RAM - should have WIMG=0010 */
> > diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
> > index 3b1357a..8a39295 100644
> > --- a/target-ppc/mmu-hash64.c
> > +++ b/target-ppc/mmu-hash64.c
> > @@ -450,37 +450,51 @@ void ppc_hash64_stop_access(PowerPCCPU *cpu,
> > uint64_t token)
> >      }
> >  }
> >  
> > -/* Returns the effective page shift or 0. MPSS isn't supported yet
> > so
> > - * this will always be the slb_pshift or 0
> > - */
> > -static uint32_t ppc_hash64_pte_size_decode(uint64_t pte1, uint32_t
> > slb_pshift)
> > +
> > +static unsigned hpte_decode_psize(const struct
> > ppc_one_seg_page_size *sps,
> > +                                  uint64_t pte0, uint64_t pte1)
> >  {
> > -    switch (slb_pshift) {
> > -    case 12:
> > -        return 12;
> > -    case 16:
> > -        if ((pte1 & 0xf000) == 0x1000) {
> > -            return 16;
> > +    int i;
> > +
> > +    /* A 4K PTE is only valid on a 4K segment */
> > +    if (!(pte0 & HPTE64_V_LARGE)) {
> > +        return sps->page_shift == 12 ? 12 : 0;
> > +    }
> > +
> > +    /* Compare the PTE with the various encodings for the
> > +     * segment base page size.
> > +     */
> > +    for (i = 0; i < PPC_PAGE_SIZES_MAX_SZ; i++) {
> > +        const struct ppc_one_page_size *ps = &sps->enc[i];
> > +        uint64_t mask;
> > +
> > +        if (!ps->page_shift) {
> > +            break;
> >          }
> > -        return 0;
> > -    case 24:
> > -        if ((pte1 & 0xff000) == 0) {
> > -            return 24;
> > +
> > +        if (ps->page_shift == 12) {
> > +            /* L bit is set so this can't be a 4kiB page */
> > +            continue;
> > +        }
> > +
> > +        mask = ((1ULL << ps->page_shift) - 1) & HPTE64_R_RPN;
> > +
> > +        if ((pte1 & mask) == (ps->pte_enc << HPTE64_R_RPN_SHIFT))
> > {
> > +            return ps->page_shift;
> >          }
> > -        return 0;
> >      }
> >      return 0;
> >  }
> >  
> > -static hwaddr ppc_hash64_pteg_search(PowerPCCPU *cpu, hwaddr hash,
> > -                                     uint32_t slb_pshift, bool
> > secondary,
> > -                                     target_ulong ptem,
> > ppc_hash_pte64_t *pte)
> > +static target_long ppc_hash64_pteg_search(PowerPCCPU *cpu, hwaddr
> > hash,
> > +                                          ppc_slb_t *slb,
> > target_ulong ptem,
> > +                                          ppc_hash_pte64_t *pte,
> > +                                          unsigned *pshift)
> >  {
> >      CPUPPCState *env = &cpu->env;
> > -    int i;
> > +    target_ulong pte_index, pte0, pte1;
> >      uint64_t token;
> > -    target_ulong pte0, pte1;
> > -    target_ulong pte_index;
> > +    int i;
> >  
> >      pte_index = (hash & env->htab_mask) * HPTES_PER_GROUP;
> >      token = ppc_hash64_start_access(cpu, pte_index);
> > @@ -489,18 +503,25 @@ static hwaddr
> > ppc_hash64_pteg_search(PowerPCCPU *cpu, hwaddr hash,
> >      }
> >      for (i = 0; i < HPTES_PER_GROUP; i++) {
> >          pte0 = ppc_hash64_load_hpte0(cpu, token, i);
> > -        pte1 = ppc_hash64_load_hpte1(cpu, token, i);
> >  
> > -        if ((pte0 & HPTE64_V_VALID)
> > -            && (secondary == !!(pte0 & HPTE64_V_SECONDARY))
> > -            && HPTE64_V_COMPARE(pte0, ptem)) {
> > -            uint32_t pshift = ppc_hash64_pte_size_decode(pte1,
> > slb_pshift);
> > -            if (pshift == 0) {
> > +        /* This compares V, B, H (secondary) and the AVPN */
> > +        if (HPTE64_V_COMPARE(pte0, ptem)) {
> > +
> > +            /* Load the rest of the PTE */
> > +            pte1 = ppc_hash64_load_hpte1(cpu, token, i);
> > +
> > +            /* Decode the actual page size */
> > +            *pshift = hpte_decode_psize(slb->sps, pte0, pte1);
> > +
> > +            /* If there is no match, ignore the PTE, it could
> > simply be
> > +             * for a different segment size encoding and the
> > architecture
> > +             * specifies we should not match. Linux will
> > potentially leave
> > +             * behind PTEs for the wrong base page size when
> > demoting
> > +             * segments.
> > +             */
> > +            if ((*pshift) == 0) {
> >                  continue;
> >              }
> > -            /* We don't do anything with pshift yet as qemu TLB
> > only deals
> > -             * with 4K pages anyway
> > -             */
> >              pte->pte0 = pte0;
> >              pte->pte1 = pte1;
> >              ppc_hash64_stop_access(cpu, token);
> > @@ -508,6 +529,7 @@ static hwaddr ppc_hash64_pteg_search(PowerPCCPU
> > *cpu, hwaddr hash,
> >          }
> >      }
> >      ppc_hash64_stop_access(cpu, token);
> > +
> >      /*
> >       * We didn't find a valid entry.
> >       */
> > @@ -516,7 +538,7 @@ static hwaddr ppc_hash64_pteg_search(PowerPCCPU
> > *cpu, hwaddr hash,
> >  
> >  static hwaddr ppc_hash64_htab_lookup(PowerPCCPU *cpu,
> >                                       ppc_slb_t *slb, target_ulong
> > eaddr,
> > -                                     ppc_hash_pte64_t *pte)
> > +                                     ppc_hash_pte64_t *pte,
> > unsigned *pshift)
> >  {
> >      CPUPPCState *env = &cpu->env;
> >      hwaddr pte_offset;
> > @@ -541,6 +563,7 @@ static hwaddr ppc_hash64_htab_lookup(PowerPCCPU
> > *cpu,
> >          hash = vsid ^ (epn >> slb->sps->page_shift);
> >      }
> >      ptem = (slb->vsid & SLB_VSID_PTEM) | ((epn >> 16) &
> > HPTE64_V_AVPN);
> > +    ptem |= HPTE64_V_VALID;
> >  
> >      /* Page address translation */
> >      qemu_log_mask(CPU_LOG_MMU,
> > @@ -554,70 +577,33 @@ static hwaddr
> > ppc_hash64_htab_lookup(PowerPCCPU *cpu,
> >              " vsid=" TARGET_FMT_lx " ptem=" TARGET_FMT_lx
> >              " hash=" TARGET_FMT_plx "\n",
> >              env->htab_base, env->htab_mask, vsid, ptem,  hash);
> > -    pte_offset = ppc_hash64_pteg_search(cpu, hash, slb->sps-
> > >page_shift,
> > -                                        0, ptem, pte);
> > +    pte_offset = ppc_hash64_pteg_search(cpu, hash, slb,
> > +                                        ptem, pte, pshift);
> >  
> >      if (pte_offset == -1) {
> >          /* Secondary PTEG lookup */
> > +        ptem |= HPTE64_V_SECONDARY;
> >          qemu_log_mask(CPU_LOG_MMU,
> >                  "1 htab=" TARGET_FMT_plx "/" TARGET_FMT_plx
> >                  " vsid=" TARGET_FMT_lx " api=" TARGET_FMT_lx
> >                  " hash=" TARGET_FMT_plx "\n", env->htab_base,
> >                  env->htab_mask, vsid, ptem, ~hash);
> >  
> > -        pte_offset = ppc_hash64_pteg_search(cpu, ~hash, slb->sps-
> > >page_shift, 1,
> > -                                            ptem, pte);
> > +        pte_offset = ppc_hash64_pteg_search(cpu, ~hash, slb,
> > +                                            ptem, pte, pshift);
> >      }
> >  
> >      return pte_offset;
> >  }
> >  
> > -static unsigned hpte_page_shift(const struct ppc_one_seg_page_size
> > *sps,
> > -    uint64_t pte0, uint64_t pte1)
> > -{
> > -    int i;
> > -
> > -    if (!(pte0 & HPTE64_V_LARGE)) {
> > -        if (sps->page_shift != 12) {
> > -            /* 4kiB page in a non 4kiB segment */
> > -            return 0;
> > -        }
> > -        /* Normal 4kiB page */
> > -        return 12;
> > -    }
> > -
> > -    for (i = 0; i < PPC_PAGE_SIZES_MAX_SZ; i++) {
> > -        const struct ppc_one_page_size *ps = &sps->enc[i];
> > -        uint64_t mask;
> > -
> > -        if (!ps->page_shift) {
> > -            break;
> > -        }
> > -
> > -        if (ps->page_shift == 12) {
> > -            /* L bit is set so this can't be a 4kiB page */
> > -            continue;
> > -        }
> > -
> > -        mask = ((1ULL << ps->page_shift) - 1) & HPTE64_R_RPN;
> > -
> > -        if ((pte1 & mask) == (ps->pte_enc << HPTE64_R_RPN_SHIFT))
> > {
> > -            return ps->page_shift;
> > -        }
> > -    }
> > -
> > -    return 0; /* Bad page size encoding */
> > -}
> > -
> > +/* This is used by the H_ENTER implementation in hw/ppc/spapr.c */
> >  unsigned ppc_hash64_hpte_page_shift_noslb(PowerPCCPU *cpu,
> > -                                          uint64_t pte0, uint64_t
> > pte1,
> > -                                          unsigned
> > *seg_page_shift)
> > +                                          uint64_t pte0, uint64_t
> > pte1)
> >  {
> >      CPUPPCState *env = &cpu->env;
> >      int i;
> >  
> >      if (!(pte0 & HPTE64_V_LARGE)) {
> > -        *seg_page_shift = 12;
> >          return 12;
> >      }
> >  
> > @@ -633,14 +619,11 @@ unsigned
> > ppc_hash64_hpte_page_shift_noslb(PowerPCCPU *cpu,
> >              break;
> >          }
> >  
> > -        shift = hpte_page_shift(sps, pte0, pte1);
> > +        shift = hpte_decode_psize(sps, pte0, pte1);
> >          if (shift) {
> > -            *seg_page_shift = sps->page_shift;
> >              return shift;
> >          }
> >      }
> > -
> > -    *seg_page_shift = 0;
> >      return 0;
> >  }
> >  
> > @@ -691,7 +674,7 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU
> > *cpu, vaddr eaddr,
> >      CPUState *cs = CPU(cpu);
> >      CPUPPCState *env = &cpu->env;
> >      ppc_slb_t *slb;
> > -    unsigned apshift;
> > +    unsigned pshift;
> >      hwaddr pte_offset;
> >      ppc_hash_pte64_t pte;
> >      int pp_prot, amr_prot, prot;
> > @@ -734,7 +717,7 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU
> > *cpu, vaddr eaddr,
> >      }
> >  
> >      /* 4. Locate the PTE in the hash table */
> > -    pte_offset = ppc_hash64_htab_lookup(cpu, slb, eaddr, &pte);
> > +    pte_offset = ppc_hash64_htab_lookup(cpu, slb, eaddr, &pte,
> > &pshift);
> >      if (pte_offset == -1) {
> >          dsisr = 0x40000000;
> >          if (rwx == 2) {
> > @@ -750,18 +733,6 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU
> > *cpu, vaddr eaddr,
> >      qemu_log_mask(CPU_LOG_MMU,
> >                  "found PTE at offset %08" HWADDR_PRIx "\n",
> > pte_offset);
> >  
> > -    /* Validate page size encoding */
> > -    apshift = hpte_page_shift(slb->sps, pte.pte0, pte.pte1);
> > -    if (!apshift) {
> > -        error_report("Bad page size encoding in HPTE 0x%"PRIx64" -
> > 0x%"PRIx64
> > -                     " @ 0x%"HWADDR_PRIx, pte.pte0, pte.pte1,
> > pte_offset);
> > -        /* Not entirely sure what the right action here, but
> > machine
> > -         * check seems reasonable */
> > -        cs->exception_index = POWERPC_EXCP_MCHECK;
> > -        env->error_code = 0;
> > -        return 1;
> > -    }
> > -
> >      /* 5. Check access permissions */
> >  
> >      pp_prot = ppc_hash64_pte_prot(cpu, slb, pte);
> > @@ -809,10 +780,10 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU
> > *cpu, vaddr eaddr,
> >  
> >      /* 7. Determine the real address from the PTE */
> >  
> > -    raddr = deposit64(pte.pte1 & HPTE64_R_RPN, 0, apshift, eaddr);
> > +    raddr = deposit64(pte.pte1 & HPTE64_R_RPN, 0, pshift, eaddr);
> >  
> >      tlb_set_page(cs, eaddr & TARGET_PAGE_MASK, raddr &
> > TARGET_PAGE_MASK,
> > -                 prot, mmu_idx, 1ULL << apshift);
> > +                 prot, mmu_idx, 1ULL << pshift);
> >  
> >      return 0;
> >  }
> > @@ -823,7 +794,7 @@ hwaddr
> > ppc_hash64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr)
> >      ppc_slb_t *slb;
> >      hwaddr pte_offset;
> >      ppc_hash_pte64_t pte;
> > -    unsigned apshift;
> > +    unsigned pshift;
> >  
> >      if (msr_dr == 0) {
> >          /* In real mode the top 4 effective address bits are
> > ignored */
> > @@ -835,17 +806,12 @@ hwaddr
> > ppc_hash64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr)
> >          return -1;
> >      }
> >  
> > -    pte_offset = ppc_hash64_htab_lookup(cpu, slb, addr, &pte);
> > +    pte_offset = ppc_hash64_htab_lookup(cpu, slb, addr, &pte,
> > &pshift);
> >      if (pte_offset == -1) {
> >          return -1;
> >      }
> >  
> > -    apshift = hpte_page_shift(slb->sps, pte.pte0, pte.pte1);
> > -    if (!apshift) {
> > -        return -1;
> > -    }
> > -
> > -    return deposit64(pte.pte1 & HPTE64_R_RPN, 0, apshift, addr)
> > +    return deposit64(pte.pte1 & HPTE64_R_RPN, 0, pshift, addr)
> >          & TARGET_PAGE_MASK;
> >  }
> >  
> > diff --git a/target-ppc/mmu-hash64.h b/target-ppc/mmu-hash64.h
> > index 6423b9f..154a306 100644
> > --- a/target-ppc/mmu-hash64.h
> > +++ b/target-ppc/mmu-hash64.h
> > @@ -17,8 +17,7 @@ void ppc_hash64_tlb_flush_hpte(PowerPCCPU *cpu,
> >                                 target_ulong pte_index,
> >                                 target_ulong pte0, target_ulong
> > pte1);
> >  unsigned ppc_hash64_hpte_page_shift_noslb(PowerPCCPU *cpu,
> > -                                          uint64_t pte0, uint64_t
> > pte1,
> > -                                          unsigned
> > *seg_page_shift);
> > +                                          uint64_t pte0, uint64_t
> > pte1);
> >  #endif
> >  
> >  /*
> > @@ -63,7 +62,7 @@ unsigned
> > ppc_hash64_hpte_page_shift_noslb(PowerPCCPU *cpu,
> >  #define HPTE64_V_AVPN_SHIFT     7
> >  #define HPTE64_V_AVPN           0x3fffffffffffff80ULL
> >  #define HPTE64_V_AVPN_VAL(x)    (((x) & HPTE64_V_AVPN) >>
> > HPTE64_V_AVPN_SHIFT)
> > -#define HPTE64_V_COMPARE(x, y)  (!(((x) ^ (y)) &
> > 0xffffffffffffff80ULL))
> > +#define HPTE64_V_COMPARE(x, y)  (!(((x) ^ (y)) &
> > 0xffffffffffffff83ULL))
> >  #define HPTE64_V_LARGE          0x0000000000000004ULL
> >  #define HPTE64_V_SECONDARY      0x0000000000000002ULL
> >  #define HPTE64_V_VALID          0x0000000000000001ULL
> > 

reply via email to

[Prev in Thread] Current Thread [Next in Thread]