[Qemu-devel] [PATCH 0/4] ppc: fixes for large page and VRMA support

[Qemu-devel] [PATCH 0/4] ppc: fixes for large page and VRMA support

[Cédric Le Goater]

Here is a little serie with API cleanups and fixes for large page and
VRMA. Previous patches which added the support did not take into
account the segment page size attribute.

Cédric Le Goater (4):
  ppc: simplify ppc_hash64_hpte_page_shift_noslb()
  ppc: fix large page support
  ppc: simplify ppc_hash64_pteg_search()
  ppc: fix VRMA support

 hw/ppc/spapr_hcall.c    |  4 +--
 target-ppc/mmu-hash64.c | 93 ++++++++++++++++++++++++-------------------------
 target-ppc/mmu-hash64.h |  3 +-
 3 files changed, 48 insertions(+), 52 deletions(-)

-- 
2.1.4

[Qemu-devel] [PATCH 1/4] ppc: simplify ppc_hash64_hpte_page_shift_noslb()

[Cédric Le Goater]

The segment page shift parameter is never used. Let's remove it.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/spapr_hcall.c    | 4 ++--
 target-ppc/mmu-hash64.c | 6 +-----
 target-ppc/mmu-hash64.h | 3 +--
 3 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index e011ed4b664b..73af112e1d36 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -83,12 +83,12 @@ 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 apshift;
     target_ulong raddr;
     target_ulong index;
     uint64_t token;
 
-    apshift = ppc_hash64_hpte_page_shift_noslb(cpu, pteh, ptel, &spshift);
+    apshift = ppc_hash64_hpte_page_shift_noslb(cpu, pteh, ptel);
     if (!apshift) {
         /* Bad page size encoding */
         return H_PARAMETER;
diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
index fa26ad2e875b..7d056c1e3b4a 100644
--- a/target-ppc/mmu-hash64.c
+++ b/target-ppc/mmu-hash64.c
@@ -610,14 +610,12 @@ static unsigned hpte_page_shift(const struct ppc_one_seg_page_size *sps,
 }
 
 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;
     }
 
@@ -635,12 +633,10 @@ unsigned ppc_hash64_hpte_page_shift_noslb(PowerPCCPU *cpu,
 
         shift = hpte_page_shift(sps, pte0, pte1);
         if (shift) {
-            *seg_page_shift = sps->page_shift;
             return shift;
         }
     }
 
-    *seg_page_shift = 0;
     return 0;
 }
 
diff --git a/target-ppc/mmu-hash64.h b/target-ppc/mmu-hash64.h
index 13ad060cfefb..f625de03da44 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
 
 /*
-- 
2.1.4

Re: [Qemu-devel] [PATCH 1/4] ppc: simplify ppc_hash64_hpte_page_shift_noslb()

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3291 bytes --]

On Fri, Jul 01, 2016 at 09:10:10AM +0200, Cédric Le Goater wrote:
> The segment page shift parameter is never used. Let's remove it.

I think I did have a use case for this in mind when I made it, but I
can't remember what it was now.  Oh well, we can always add it back
when I remember.  I'll apply this to ppc-for-2.7.

> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  hw/ppc/spapr_hcall.c    | 4 ++--
>  target-ppc/mmu-hash64.c | 6 +-----
>  target-ppc/mmu-hash64.h | 3 +--
>  3 files changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index e011ed4b664b..73af112e1d36 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -83,12 +83,12 @@ 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 apshift;
>      target_ulong raddr;
>      target_ulong index;
>      uint64_t token;
>  
> -    apshift = ppc_hash64_hpte_page_shift_noslb(cpu, pteh, ptel, &spshift);
> +    apshift = ppc_hash64_hpte_page_shift_noslb(cpu, pteh, ptel);
>      if (!apshift) {
>          /* Bad page size encoding */
>          return H_PARAMETER;
> diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
> index fa26ad2e875b..7d056c1e3b4a 100644
> --- a/target-ppc/mmu-hash64.c
> +++ b/target-ppc/mmu-hash64.c
> @@ -610,14 +610,12 @@ static unsigned hpte_page_shift(const struct ppc_one_seg_page_size *sps,
>  }
>  
>  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;
>      }
>  
> @@ -635,12 +633,10 @@ unsigned ppc_hash64_hpte_page_shift_noslb(PowerPCCPU *cpu,
>  
>          shift = hpte_page_shift(sps, pte0, pte1);
>          if (shift) {
> -            *seg_page_shift = sps->page_shift;
>              return shift;
>          }
>      }
>  
> -    *seg_page_shift = 0;
>      return 0;
>  }
>  
> diff --git a/target-ppc/mmu-hash64.h b/target-ppc/mmu-hash64.h
> index 13ad060cfefb..f625de03da44 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
>  
>  /*

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH 1/4] ppc: simplify ppc_hash64_hpte_page_shift_noslb()

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3702 bytes --]

On Mon, Jul 04, 2016 at 02:46:51PM +1000, David Gibson wrote:
> On Fri, Jul 01, 2016 at 09:10:10AM +0200, Cédric Le Goater wrote:
> > The segment page shift parameter is never used. Let's remove it.
> 
> I think I did have a use case for this in mind when I made it, but I
> can't remember what it was now.  Oh well, we can always add it back
> when I remember.  I'll apply this to ppc-for-2.7.

Actually.. no I won't.  There are some problems in the later patches
in this series, and to fix this correctly we're going to need that
slb_pshift return value after all.

> 
> > 
> > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > ---
> >  hw/ppc/spapr_hcall.c    | 4 ++--
> >  target-ppc/mmu-hash64.c | 6 +-----
> >  target-ppc/mmu-hash64.h | 3 +--
> >  3 files changed, 4 insertions(+), 9 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> > index e011ed4b664b..73af112e1d36 100644
> > --- a/hw/ppc/spapr_hcall.c
> > +++ b/hw/ppc/spapr_hcall.c
> > @@ -83,12 +83,12 @@ 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 apshift;
> >      target_ulong raddr;
> >      target_ulong index;
> >      uint64_t token;
> >  
> > -    apshift = ppc_hash64_hpte_page_shift_noslb(cpu, pteh, ptel, &spshift);
> > +    apshift = ppc_hash64_hpte_page_shift_noslb(cpu, pteh, ptel);
> >      if (!apshift) {
> >          /* Bad page size encoding */
> >          return H_PARAMETER;
> > diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
> > index fa26ad2e875b..7d056c1e3b4a 100644
> > --- a/target-ppc/mmu-hash64.c
> > +++ b/target-ppc/mmu-hash64.c
> > @@ -610,14 +610,12 @@ static unsigned hpte_page_shift(const struct ppc_one_seg_page_size *sps,
> >  }
> >  
> >  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;
> >      }
> >  
> > @@ -635,12 +633,10 @@ unsigned ppc_hash64_hpte_page_shift_noslb(PowerPCCPU *cpu,
> >  
> >          shift = hpte_page_shift(sps, pte0, pte1);
> >          if (shift) {
> > -            *seg_page_shift = sps->page_shift;
> >              return shift;
> >          }
> >      }
> >  
> > -    *seg_page_shift = 0;
> >      return 0;
> >  }
> >  
> > diff --git a/target-ppc/mmu-hash64.h b/target-ppc/mmu-hash64.h
> > index 13ad060cfefb..f625de03da44 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
> >  
> >  /*
> 



-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[Qemu-devel] [PATCH 2/4] ppc: fix large page support

[Cédric Le Goater]

A regression was introduced by commit 53df75a59bcf ('ppc: Fix 64K
pages support in full emulation'). ppc_hash64_hpte_page_shift_noslb()
should be used to compute the page size.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 target-ppc/mmu-hash64.c | 24 +-----------------------
 1 file changed, 1 insertion(+), 23 deletions(-)

diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
index 7d056c1e3b4a..fdaff9e874ba 100644
--- a/target-ppc/mmu-hash64.c
+++ b/target-ppc/mmu-hash64.c
@@ -450,28 +450,6 @@ 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)
-{
-    switch (slb_pshift) {
-    case 12:
-        return 12;
-    case 16:
-        if ((pte1 & 0xf000) == 0x1000) {
-            return 16;
-        }
-        return 0;
-    case 24:
-        if ((pte1 & 0xff000) == 0) {
-            return 24;
-        }
-        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)
@@ -494,7 +472,7 @@ static hwaddr ppc_hash64_pteg_search(PowerPCCPU *cpu, hwaddr hash,
         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);
+            uint32_t pshift = ppc_hash64_hpte_page_shift_noslb(cpu, pte0, pte1);
             if (pshift == 0) {
                 continue;
             }
-- 
2.1.4

[Qemu-devel] [PATCH 3/4] ppc: simplify ppc_hash64_pteg_search()

[Cédric Le Goater]

The page shift parameter is never used. Let's remove it.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 target-ppc/mmu-hash64.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
index fdaff9e874ba..7ef45ee53bf5 100644
--- a/target-ppc/mmu-hash64.c
+++ b/target-ppc/mmu-hash64.c
@@ -451,8 +451,8 @@ void ppc_hash64_stop_access(PowerPCCPU *cpu, uint64_t token)
 }
 
 static hwaddr ppc_hash64_pteg_search(PowerPCCPU *cpu, hwaddr hash,
-                                     uint32_t slb_pshift, bool secondary,
-                                     target_ulong ptem, ppc_hash_pte64_t *pte)
+                                     bool secondary, target_ulong ptem,
+                                     ppc_hash_pte64_t *pte)
 {
     CPUPPCState *env = &cpu->env;
     int i;
@@ -532,8 +532,7 @@ 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, 0, ptem, pte);
 
     if (pte_offset == -1) {
         /* Secondary PTEG lookup */
@@ -543,8 +542,7 @@ static hwaddr ppc_hash64_htab_lookup(PowerPCCPU *cpu,
                 " 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, 1, ptem, pte);
     }
 
     return pte_offset;
-- 
2.1.4

[Qemu-devel] [PATCH 4/4] ppc: fix VRMA support

[Cédric Le Goater]

commit 08109fd4360d ('ppc: Add proper real mode translation support')
introduced VRMA support for which SLB entries need to be created. But
it did not take into account the changes in ppc_slb_t and missed the
setting of the segment page size attribute.

However, gcc spotted it :

target-ppc/mmu-hash64.c: In function 'ppc_hash64_get_phys_page_debug':
target-ppc/mmu-hash64.c:936:16: error: '*((void *)&slb+16)' may be used uninitialized in this function [-Werror=maybe-uninitialized]

     pte_offset = ppc_hash64_htab_lookup(cpu, &slb, addr, &pte);

This adds an extra routine to built the slb and compute the segment
page size.

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

 I am not sure how to handle errors. Could there be one ? If so,
 should we generate a POWERPC_EXCP_MCHECK ?

 target-ppc/mmu-hash64.c | 53 ++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 39 insertions(+), 14 deletions(-)

diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
index 7ef45ee53bf5..117f198a9a2e 100644
--- a/target-ppc/mmu-hash64.c
+++ b/target-ppc/mmu-hash64.c
@@ -684,6 +684,43 @@ static int64_t ppc_hash64_get_rmls(CPUPPCState *env)
     }
 }
 
+static int ppc_hash64_make_vrma_slb(CPUPPCState *env, ppc_slb_t *slb)
+{
+    uint32_t vrmasd;
+    const struct ppc_one_seg_page_size *sps = NULL;
+    target_ulong esid, vsid;
+    int i;
+
+    vsid = SLB_VSID_VRMA;
+    vrmasd = (env->spr[SPR_LPCR] & LPCR_VRMASD) >> LPCR_VRMASD_SHIFT;
+    vsid |= (vrmasd << 4) & (SLB_VSID_L | SLB_VSID_LP);
+    esid = SLB_ESID_V;
+
+    for (i = 0; i < PPC_PAGE_SIZES_MAX_SZ; i++) {
+        const struct ppc_one_seg_page_size *sps1 = &env->sps.sps[i];
+
+        if (!sps1->page_shift) {
+            break;
+        }
+
+        if ((vsid & SLB_VSID_LLP_MASK) == sps1->slb_enc) {
+            sps = sps1;
+            break;
+        }
+    }
+
+    if (!sps) {
+        error_report("Bad page size encoding esid 0x"TARGET_FMT_lx
+                     " vsid 0x"TARGET_FMT_lx, esid, vsid);
+        return -1;
+    }
+
+    slb->vsid = vsid;
+    slb->esid = esid;
+    slb->sps = sps;
+    return 0;
+}
+
 int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
                                 int rwx, int mmu_idx)
 {
@@ -722,13 +759,7 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
         } else {
             /* Otherwise, check VPM for RMA vs VRMA */
             if (env->spr[SPR_LPCR] & LPCR_VPM0) {
-                uint32_t vrmasd;
-                /* VRMA, we make up an SLB entry */
-                slb.vsid = SLB_VSID_VRMA;
-                vrmasd = (env->spr[SPR_LPCR] & LPCR_VRMASD) >>
-                    LPCR_VRMASD_SHIFT;
-                slb.vsid |= (vrmasd << 4) & (SLB_VSID_L | SLB_VSID_LP);
-                slb.esid = SLB_ESID_V;
+                ppc_hash64_make_vrma_slb(env, &slb);
                 goto skip_slb;
             }
             /* RMA. Check bounds in RMLS */
@@ -893,13 +924,7 @@ hwaddr ppc_hash64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr)
 
         /* Otherwise, check VPM for RMA vs VRMA */
         if (env->spr[SPR_LPCR] & LPCR_VPM0) {
-            uint32_t vrmasd;
-
-            /* VRMA, we make up an SLB entry */
-            slb.vsid = SLB_VSID_VRMA;
-            vrmasd = (env->spr[SPR_LPCR] & LPCR_VRMASD) >> LPCR_VRMASD_SHIFT;
-            slb.vsid |= (vrmasd << 4) & (SLB_VSID_L | SLB_VSID_LP);
-            slb.esid = SLB_ESID_V;
+            ppc_hash64_make_vrma_slb(env, &slb);
             goto skip_slb;
         }
         /* RMA. Check bounds in RMLS */
-- 
2.1.4

Re: [Qemu-devel] [PATCH 0/4] ppc: fixes for large page and VRMA support

[Benjamin Herrenschmidt]

On Fri, 2016-07-01 at 09:10 +0200, Cédric Le Goater wrote:
> Here is a little serie with API cleanups and fixes for large page and
> VRMA. Previous patches which added the support did not take into
> account the segment page size attribute.

I've done a slightly different patch that subsumes 1...3 and is I think
now architecturally correct ;-) (unlike my original code).

Additionaly we now avoid walking all the SLB sizes so things should be
a bit faster too.

The VRMA patch still applies, I'll include it in what I post.

Cheers,
Ben.

Re: [Qemu-devel] [PATCH 0/4] ppc: fixes for large page and VRMA support

[Cédric Le Goater]

On 07/04/2016 08:11 AM, Benjamin Herrenschmidt wrote:
> On Fri, 2016-07-01 at 09:10 +0200, Cédric Le Goater wrote:
>> Here is a little serie with API cleanups and fixes for large page and
>> VRMA. Previous patches which added the support did not take into
>> account the segment page size attribute.
> 
> I've done a slightly different patch that subsumes 1...3 and is I think 
> now architecturally correct ;-) (unlike my original code).
>
> Additionaly we noavoid walking all the SLB sizes so things should be
> a bit faster too.

good that part felt like it needed some optimization.  

> The VRMA patch still applies, I'll include it in what I post.

I will give it a try.

Thanks,

C.