[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: |
Cédric Le Goater |
Subject: |
Re: [Qemu-ppc] [PATCH 1/3] ppc/hash64: Various fixes in PTE search in the hash table |
Date: |
Mon, 4 Jul 2016 16:26:52 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.1.0 |
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 :/
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
>