[Qemu-devel] [PATCH 0/3] PPC64 Linux bringup patches

[Qemu-devel] [PATCH 0/3] PPC64 Linux bringup patches

[Alexander Graf]

I spent my weekend on trying to get Linux running on qemu-system-ppc64 and
partly succeeded :-).

This patchset extends MMU support far enough for Linux to be happy and
works around one segmentation fault I encountered during the process.

Now the kernel panic()s on setting up nvram, which is a different field
to look at next.

Alexander Graf (3):
  PPC: Circumvent overflow in mtcrf
  PPC64: Implement slbmte
  PPC64: Implement large pages

 target-ppc/cpu.h       |    4 +-
 target-ppc/helper.c    |  135 ++++++++++++++++++++++++++++++++----------------
 target-ppc/op_helper.c |    6 ++-
 target-ppc/translate.c |   25 +++++++--
 4 files changed, 118 insertions(+), 52 deletions(-)

[Qemu-devel] [PATCH 1/3] PPC: Circumvent overflow in mtcrf

[Alexander Graf]

I had a segmentation fault in mtcrf, where ffs() returned 8 and the
code then accessed cpu_crf[7 - 8].

In order to circumvent this, I just put in an & 7 to the ffs result,
so we'll never run negative. This is probably not correct, but makes
things work for me so far.

Signed-off-by: Alexander Graf <alex@csgraf.de>
---
 target-ppc/translate.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 2a06e4c..2e7420f 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -3937,7 +3937,7 @@ GEN_HANDLER(mtcrf, 0x1F, 0x10, 0x04, 0x00000801, PPC_MISC)
     crm = CRM(ctx->opcode);
     if (likely((ctx->opcode & 0x00100000) || (crm ^ (crm - 1)) == 0)) {
         TCGv_i32 temp = tcg_temp_new_i32();
-        crn = ffs(crm);
+        crn = ffs(crm) & 7;
         tcg_gen_trunc_tl_i32(temp, cpu_gpr[rS(ctx->opcode)]);
         tcg_gen_shri_i32(cpu_crf[7 - crn], temp, crn * 4);
         tcg_gen_andi_i32(cpu_crf[7 - crn], cpu_crf[7 - crn], 0xf);
-- 
1.6.0.2

[Qemu-devel] [PATCH 2/3] PPC64: Implement slbmte

[Alexander Graf]

In order to modify SLB entries on recent PPC64 machines, the slbmte
instruction is used.

This patch implements the slbmte instruction and makes the "bridge"
mode code use the slb set functions, so we can move the SLB into
the CPU struct later.

This is required for Linux to run on PPC64.

Signed-off-by: Alexander Graf <alex@csgraf.de>
---
 target-ppc/cpu.h       |    3 +-
 target-ppc/helper.c    |   57 ++++++++++++++++++++++++++++++++++++-----------
 target-ppc/op_helper.c |    6 +++-
 target-ppc/translate.c |   23 ++++++++++++++++---
 4 files changed, 68 insertions(+), 21 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 80ee76c..67a0731 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -714,7 +714,8 @@ void ppc_store_sdr1 (CPUPPCState *env, target_ulong value);
 #if defined(TARGET_PPC64)
 void ppc_store_asr (CPUPPCState *env, target_ulong value);
 target_ulong ppc_load_slb (CPUPPCState *env, int slb_nr);
-void ppc_store_slb (CPUPPCState *env, int slb_nr, target_ulong rs);
+target_ulong ppc_load_sr (CPUPPCState *env, int sr_nr);
+void ppc_store_slb (CPUPPCState *env, target_ulong rb, target_ulong rs);
 #endif /* defined(TARGET_PPC64) */
 void ppc_store_sr (CPUPPCState *env, int srnum, target_ulong value);
 #endif /* !defined(CONFIG_USER_ONLY) */
diff --git a/target-ppc/helper.c b/target-ppc/helper.c
index e02dcb0..9461daf 100644
--- a/target-ppc/helper.c
+++ b/target-ppc/helper.c
@@ -821,27 +821,34 @@ target_ulong ppc_load_slb (CPUPPCState *env, int slb_nr)
     return rt;
 }
 
-void ppc_store_slb (CPUPPCState *env, int slb_nr, target_ulong rs)
+void ppc_store_slb (CPUPPCState *env, target_ulong rb, target_ulong rs)
 {
     target_phys_addr_t sr_base;
     uint64_t tmp64;
     uint32_t tmp;
 
+    uint64_t vsid;
+    uint64_t esid;
+    int flags, valid, slb_nr;
+
+    vsid = rs >> 12;
+    flags = ((rs >> 8) & 0xf);
+
+    esid = rb >> 28;
+    valid = (rb & (1 << 27));
+    slb_nr = rb & 0xfff;
+
+    tmp64 = (esid << 28) | valid | (vsid >> 24);
+    tmp = (vsid << 8) | (flags << 3);
+
+    /* Write SLB entry to memory */
     sr_base = env->spr[SPR_ASR];
     sr_base += 12 * slb_nr;
-    /* Copy Rs bits 37:63 to SLB 62:88 */
-    tmp = rs << 8;
-    tmp64 = (rs >> 24) & 0x7;
-    /* Copy Rs bits 33:36 to SLB 89:92 */
-    tmp |= ((rs >> 27) & 0xF) << 4;
-    /* Set the valid bit */
-    tmp64 |= 1 << 27;
-    /* Set ESID */
-    tmp64 |= (uint32_t)slb_nr << 28;
-    LOG_SLB("%s: %d " ADDRX " => " PADDRX " %016" PRIx64
+
+    LOG_SLB("%s: %d " ADDRX " - " ADDRX " => " PADDRX " %016" PRIx64
                 " %08" PRIx32 "\n", __func__,
-                slb_nr, rs, sr_base, tmp64, tmp);
-    /* Write SLB entry to memory */
+                slb_nr, rb, rs, sr_base, tmp64, tmp);
+
     stq_phys(sr_base, tmp64);
     stl_phys(sr_base + 8, tmp);
 }
@@ -1945,11 +1952,33 @@ void ppc_store_sdr1 (CPUPPCState *env, target_ulong value)
     }
 }
 
+target_ulong ppc_load_sr (CPUPPCState *env, int slb_nr)
+{
+    // XXX
+    return 0;
+}
+
 void ppc_store_sr (CPUPPCState *env, int srnum, target_ulong value)
 {
     LOG_MMU("%s: reg=%d " ADDRX " " ADDRX "\n",
                 __func__, srnum, value, env->sr[srnum]);
-    if (env->sr[srnum] != value) {
+    if (env->mmu_model & POWERPC_MMU_64) {
+        uint64_t rb = 0, rs = 0;
+
+        /* ESID = srnum */
+        rb |= ((uint32_t)srnum & 0xf) << 28;
+        /* Set the valid bit */
+        rb |= 1 << 27;
+        /* Index = ESID */
+        rb |= (uint32_t)srnum;
+
+        /* VSID = VSID */
+        rs |= (value & 0xfffffff) << 12;
+        /* flags = flags */
+        rs |= ((value >> 27) & 0xf) << 9;
+
+        ppc_store_slb(env, rb, rs);
+    } else if (env->sr[srnum] != value) {
         env->sr[srnum] = value;
 #if !defined(FLUSH_ALL_TLBS) && 0
         {
diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
index 3afd217..527f8de 100644
--- a/target-ppc/op_helper.c
+++ b/target-ppc/op_helper.c
@@ -3752,6 +3752,8 @@ void tlb_fill (target_ulong addr, int is_write, int mmu_idx, void *retaddr)
 /* Segment registers load and store */
 target_ulong helper_load_sr (target_ulong sr_num)
 {
+    if (env->mmu_model & POWERPC_MMU_64)
+        return ppc_load_sr(env, sr_num);
     return env->sr[sr_num];
 }
 
@@ -3767,9 +3769,9 @@ target_ulong helper_load_slb (target_ulong slb_nr)
     return ppc_load_slb(env, slb_nr);
 }
 
-void helper_store_slb (target_ulong slb_nr, target_ulong rs)
+void helper_store_slb (target_ulong rb, target_ulong rs)
 {
-    ppc_store_slb(env, slb_nr, rs);
+    ppc_store_slb(env, rb, rs);
 }
 
 void helper_slbia (void)
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 2e7420f..d544c8a 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -4291,7 +4291,7 @@ GEN_HANDLER2(mfsr_64b, "mfsr", 0x1F, 0x13, 0x12, 0x0010F801, PPC_SEGMENT_64B)
         return;
     }
     t0 = tcg_const_tl(SR(ctx->opcode));
-    gen_helper_load_slb(cpu_gpr[rD(ctx->opcode)], t0);
+    gen_helper_load_sr(cpu_gpr[rD(ctx->opcode)], t0);
     tcg_temp_free(t0);
 #endif
 }
@@ -4311,7 +4311,7 @@ GEN_HANDLER2(mfsrin_64b, "mfsrin", 0x1F, 0x13, 0x14, 0x001F0001,
     t0 = tcg_temp_new();
     tcg_gen_shri_tl(t0, cpu_gpr[rB(ctx->opcode)], 28);
     tcg_gen_andi_tl(t0, t0, 0xF);
-    gen_helper_load_slb(cpu_gpr[rD(ctx->opcode)], t0);
+    gen_helper_load_sr(cpu_gpr[rD(ctx->opcode)], t0);
     tcg_temp_free(t0);
 #endif
 }
@@ -4328,7 +4328,7 @@ GEN_HANDLER2(mtsr_64b, "mtsr", 0x1F, 0x12, 0x06, 0x0010F801, PPC_SEGMENT_64B)
         return;
     }
     t0 = tcg_const_tl(SR(ctx->opcode));
-    gen_helper_store_slb(t0, cpu_gpr[rS(ctx->opcode)]);
+    gen_helper_store_sr(t0, cpu_gpr[rS(ctx->opcode)]);
     tcg_temp_free(t0);
 #endif
 }
@@ -4348,10 +4348,25 @@ GEN_HANDLER2(mtsrin_64b, "mtsrin", 0x1F, 0x12, 0x07, 0x001F0001,
     t0 = tcg_temp_new();
     tcg_gen_shri_tl(t0, cpu_gpr[rB(ctx->opcode)], 28);
     tcg_gen_andi_tl(t0, t0, 0xF);
-    gen_helper_store_slb(t0, cpu_gpr[rS(ctx->opcode)]);
+    gen_helper_store_sr(t0, cpu_gpr[rS(ctx->opcode)]);
     tcg_temp_free(t0);
 #endif
 }
+
+/* slbmte */
+GEN_HANDLER2(slbmte, "slbmte", 0x1F, 0x12, 0x0C, 0x00000000, PPC_SEGMENT_64B)
+{
+#if defined(CONFIG_USER_ONLY)
+    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
+#else
+    if (unlikely(!ctx->mem_idx)) {
+        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
+        return;
+    }
+    gen_helper_store_slb(cpu_gpr[rB(ctx->opcode)], cpu_gpr[rS(ctx->opcode)]);
+#endif
+}
+
 #endif /* defined(TARGET_PPC64) */
 
 /***                      Lookaside buffer management                      ***/
-- 
1.6.0.2

[Qemu-devel] [PATCH 3/3] PPC64: Implement large pages

[Alexander Graf]

The current SLB/PTE code does not support large pages, which are
required by Linux, as it boots up with the kernel regions up as large.

This patch implements large page support, so we can run Linux.

Signed-off-by: Alexander Graf <alex@csgraf.de>
---
 target-ppc/cpu.h    |    1 +
 target-ppc/helper.c |   78 +++++++++++++++++++++++++++++++-------------------
 2 files changed, 49 insertions(+), 30 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 67a0731..cce744c 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -675,6 +675,7 @@ struct CPUPPCState {
 typedef struct mmu_ctx_t mmu_ctx_t;
 struct mmu_ctx_t {
     target_phys_addr_t raddr;      /* Real address              */
+    target_phys_addr_t eaddr;      /* Effective address         */
     int prot;                      /* Protection bits           */
     target_phys_addr_t pg_addr[2]; /* PTE tables base addresses */
     target_ulong ptem;             /* Virtual segment ID | API  */
diff --git a/target-ppc/helper.c b/target-ppc/helper.c
index 9461daf..0fa87dc 100644
--- a/target-ppc/helper.c
+++ b/target-ppc/helper.c
@@ -582,7 +582,8 @@ static always_inline int get_bat (CPUState *env, mmu_ctx_t *ctx,
 
 /* PTE table lookup */
 static always_inline int _find_pte (mmu_ctx_t *ctx, int is_64b, int h,
-                                    int rw, int type)
+                                    int rw, int type,
+                                    int target_page_bits)
 {
     target_ulong base, pte0, pte1;
     int i, good = -1;
@@ -594,7 +595,14 @@ static always_inline int _find_pte (mmu_ctx_t *ctx, int is_64b, int h,
 #if defined(TARGET_PPC64)
         if (is_64b) {
             pte0 = ldq_phys(base + (i * 16));
-            pte1 =  ldq_phys(base + (i * 16) + 8);
+            pte1 = ldq_phys(base + (i * 16) + 8);
+
+            /* We have a TLB that saves 4K pages, so let's
+             * split a huge page to 4k chunks */
+            if (target_page_bits != TARGET_PAGE_BITS)
+                pte1 |= (ctx->eaddr & (( 1 << target_page_bits ) - 1))
+                        & TARGET_PAGE_MASK;
+
             r = pte64_check(ctx, pte0, pte1, h, rw, type);
             LOG_MMU("Load pte from " ADDRX " => " ADDRX " " ADDRX
                         " %d %d %d " ADDRX "\n",
@@ -658,27 +666,30 @@ static always_inline int _find_pte (mmu_ctx_t *ctx, int is_64b, int h,
     return ret;
 }
 
-static always_inline int find_pte32 (mmu_ctx_t *ctx, int h, int rw, int type)
+static always_inline int find_pte32 (mmu_ctx_t *ctx, int h, int rw,
+                                     int type, int target_page_bits)
 {
-    return _find_pte(ctx, 0, h, rw, type);
+    return _find_pte(ctx, 0, h, rw, type, target_page_bits);
 }
 
 #if defined(TARGET_PPC64)
-static always_inline int find_pte64 (mmu_ctx_t *ctx, int h, int rw, int type)
+static always_inline int find_pte64 (mmu_ctx_t *ctx, int h, int rw,
+                                     int type, int target_page_bits)
 {
-    return _find_pte(ctx, 1, h, rw, type);
+    return _find_pte(ctx, 1, h, rw, type, target_page_bits);
 }
 #endif
 
 static always_inline int find_pte (CPUState *env, mmu_ctx_t *ctx,
-                                   int h, int rw, int type)
+                                   int h, int rw, int type,
+                                   int target_page_bits)
 {
 #if defined(TARGET_PPC64)
     if (env->mmu_model & POWERPC_MMU_64)
-        return find_pte64(ctx, h, rw, type);
+        return find_pte64(ctx, h, rw, type, target_page_bits);
 #endif
 
-    return find_pte32(ctx, h, rw, type);
+    return find_pte32(ctx, h, rw, type, target_page_bits);
 }
 
 #if defined(TARGET_PPC64)
@@ -694,7 +705,8 @@ static always_inline void slb_invalidate (uint64_t *slb64)
 
 static always_inline int slb_lookup (CPUPPCState *env, target_ulong eaddr,
                                      target_ulong *vsid,
-                                     target_ulong *page_mask, int *attr)
+                                     target_ulong *page_mask, int *attr,
+                                     int *target_page_bits)
 {
     target_phys_addr_t sr_base;
     target_ulong mask;
@@ -714,19 +726,16 @@ static always_inline int slb_lookup (CPUPPCState *env, target_ulong eaddr,
                     PRIx32 "\n", __func__, n, sr_base, tmp64, tmp);
         if (slb_is_valid(tmp64)) {
             /* SLB entry is valid */
-            switch (tmp64 & 0x0000000006000000ULL) {
-            case 0x0000000000000000ULL:
-                /* 256 MB segment */
-                mask = 0xFFFFFFFFF0000000ULL;
-                break;
-            case 0x0000000002000000ULL:
-                /* 1 TB segment */
+            if (tmp & 0x8) {
+                /* 1 TB Segment */
                 mask = 0xFFFF000000000000ULL;
-                break;
-            case 0x0000000004000000ULL:
-            case 0x0000000006000000ULL:
-                /* Reserved => segment is invalid */
-                continue;
+                if (target_page_bits)
+                    *target_page_bits = 24; // XXX 16M pages?
+            } else {
+                /* 256MB Segment */
+                mask = 0xFFFFFFFFF0000000ULL;
+                if (target_page_bits)
+                    *target_page_bits = TARGET_PAGE_BITS;
             }
             if ((eaddr & mask) == (tmp64 & mask)) {
                 /* SLB match */
@@ -777,7 +786,7 @@ void ppc_slb_invalidate_one (CPUPPCState *env, uint64_t T0)
     int attr;
     int n;
 
-    n = slb_lookup(env, T0, &vsid, &page_mask, &attr);
+    n = slb_lookup(env, T0, &vsid, &page_mask, &attr, NULL);
     if (n >= 0) {
         sr_base = env->spr[SPR_ASR];
         sr_base += 12 * n;
@@ -871,20 +880,22 @@ static always_inline int get_segment (CPUState *env, mmu_ctx_t *ctx,
 #if defined(TARGET_PPC64)
     int attr;
 #endif
-    int ds, vsid_sh, sdr_sh, pr;
+    int ds, vsid_sh, sdr_sh, pr, target_page_bits;
     int ret, ret2;
 
     pr = msr_pr;
 #if defined(TARGET_PPC64)
     if (env->mmu_model & POWERPC_MMU_64) {
         LOG_MMU("Check SLBs\n");
-        ret = slb_lookup(env, eaddr, &vsid, &page_mask, &attr);
+        ret = slb_lookup(env, eaddr, &vsid, &page_mask, &attr,
+                         &target_page_bits);
         if (ret < 0)
             return ret;
         ctx->key = ((attr & 0x40) && (pr != 0)) ||
             ((attr & 0x80) && (pr == 0)) ? 1 : 0;
         ds = 0;
-        ctx->nx = attr & 0x20 ? 1 : 0;
+        ctx->nx = attr & 0x10 ? 1 : 0;
+        ctx->eaddr = eaddr;
         vsid_mask = 0x00003FFFFFFFFF80ULL;
         vsid_sh = 7;
         sdr_sh = 18;
@@ -903,6 +914,7 @@ static always_inline int get_segment (CPUState *env, mmu_ctx_t *ctx,
         vsid_sh = 6;
         sdr_sh = 16;
         sdr_mask = 0xFFC0;
+        target_page_bits = TARGET_PAGE_BITS;
         LOG_MMU("Check segment v=" ADDRX " %d " ADDRX
                     " nip=" ADDRX " lr=" ADDRX " ir=%d dr=%d pr=%d %d t=%d\n",
                     eaddr, (int)(eaddr >> 28), sr, env->nip,
@@ -918,7 +930,7 @@ static always_inline int get_segment (CPUState *env, mmu_ctx_t *ctx,
             /* Page address translation */
             /* Primary table address */
             sdr = env->sdr1;
-            pgidx = (eaddr & page_mask) >> TARGET_PAGE_BITS;
+            pgidx = (eaddr & page_mask) >> target_page_bits;
 #if defined(TARGET_PPC64)
             if (env->mmu_model & POWERPC_MMU_64) {
                 htab_mask = 0x0FFFFFFF >> (28 - (sdr & 0x1F));
@@ -944,7 +956,12 @@ static always_inline int get_segment (CPUState *env, mmu_ctx_t *ctx,
 #if defined(TARGET_PPC64)
             if (env->mmu_model & POWERPC_MMU_64) {
                 /* Only 5 bits of the page index are used in the AVPN */
-                ctx->ptem = (vsid << 12) | ((pgidx >> 4) & 0x0F80);
+                if (target_page_bits > 23) {
+                    ctx->ptem = (vsid << 12) |
+                                ((pgidx << (target_page_bits - 16)) & 0xF80);
+                } else {
+                    ctx->ptem = (vsid << 12) | ((pgidx >> 4) & 0x0F80);
+                }
             } else
 #endif
             {
@@ -962,7 +979,7 @@ static always_inline int get_segment (CPUState *env, mmu_ctx_t *ctx,
                             " pg_addr=" PADDRX "\n",
                             sdr, vsid, pgidx, hash, ctx->pg_addr[0]);
                 /* Primary table lookup */
-                ret = find_pte(env, ctx, 0, rw, type);
+                ret = find_pte(env, ctx, 0, rw, type, target_page_bits);
                 if (ret < 0) {
                     /* Secondary table lookup */
                     if (eaddr != 0xEFFFFFFF)
@@ -970,7 +987,8 @@ static always_inline int get_segment (CPUState *env, mmu_ctx_t *ctx,
                                 "api=" ADDRX " hash=" PADDRX
                                 " pg_addr=" PADDRX "\n",
                                 sdr, vsid, pgidx, hash, ctx->pg_addr[1]);
-                    ret2 = find_pte(env, ctx, 1, rw, type);
+                    ret2 = find_pte(env, ctx, 1, rw, type,
+                                    target_page_bits);
                     if (ret2 != -1)
                         ret = ret2;
                 }
-- 
1.6.0.2

[Qemu-devel] Re: [PATCH 2/3] PPC64: Implement slbmte

[Blue Swirl]

On 3/2/09, Alexander Graf <agraf@suse.de> wrote:
> In order to modify SLB entries on recent PPC64 machines, the slbmte
>  instruction is used.
>
>  This patch implements the slbmte instruction and makes the "bridge"
>  mode code use the slb set functions, so we can move the SLB into
>  the CPU struct later.
>
>  This is required for Linux to run on PPC64.
>
>  Signed-off-by: Alexander Graf <alex@csgraf.de>

>   void ppc_store_sr (CPUPPCState *env, int srnum, target_ulong value)
>   {
>      LOG_MMU("%s: reg=%d " ADDRX " " ADDRX "\n",
>                  __func__, srnum, value, env->sr[srnum]);
>  -    if (env->sr[srnum] != value) {
>  +    if (env->mmu_model & POWERPC_MMU_64) {

>   target_ulong helper_load_sr (target_ulong sr_num)
>   {
>  +    if (env->mmu_model & POWERPC_MMU_64)
>  +        return ppc_load_sr(env, sr_num);
>      return env->sr[sr_num];
>   }

It would be faster to move the MMU model checks to translation time.

[Qemu-devel] Re: [PATCH 2/3] PPC64: Implement slbmte

[Alexander Graf]

On 02.03.2009, at 19:58, Blue Swirl wrote:

> On 3/2/09, Alexander Graf <agraf@suse.de> wrote:
>> In order to modify SLB entries on recent PPC64 machines, the slbmte
>> instruction is used.
>>
>> This patch implements the slbmte instruction and makes the "bridge"
>> mode code use the slb set functions, so we can move the SLB into
>> the CPU struct later.
>>
>> This is required for Linux to run on PPC64.
>>
>> Signed-off-by: Alexander Graf <alex@csgraf.de>
>
>>  void ppc_store_sr (CPUPPCState *env, int srnum, target_ulong value)
>>  {
>>     LOG_MMU("%s: reg=%d " ADDRX " " ADDRX "\n",
>>                 __func__, srnum, value, env->sr[srnum]);
>> -    if (env->sr[srnum] != value) {
>> +    if (env->mmu_model & POWERPC_MMU_64) {
>
>>  target_ulong helper_load_sr (target_ulong sr_num)
>>  {
>> +    if (env->mmu_model & POWERPC_MMU_64)
>> +        return ppc_load_sr(env, sr_num);
>>     return env->sr[sr_num];
>>  }
>
> It would be faster to move the MMU model checks to translation time.

The problem is that we don't know which -cpu the user will give us  
when we run qemu-system-ppc64.
Of course we could just make qemu-system-ppc64 not take any non-64 bit  
CPUs, if nobody totally disagrees.

Alex

Re: [Qemu-devel] Re: [PATCH 2/3] PPC64: Implement slbmte

[Paul Brook]

> >>  target_ulong helper_load_sr (target_ulong sr_num)
> >>  {
> >> +    if (env->mmu_model & POWERPC_MMU_64)
> >> +        return ppc_load_sr(env, sr_num);
> >>     return env->sr[sr_num];
> >>  }
> >
> > It would be faster to move the MMU model checks to translation time.
>
> The problem is that we don't know which -cpu the user will give us
> when we run qemu-system-ppc64.

You're confusing translation time with compile time.

Paul

[Qemu-devel] Re: [PATCH 2/3] PPC64: Implement slbmte

[Blue Swirl]

On 3/2/09, Alexander Graf <agraf@suse.de> wrote:
>
>  On 02.03.2009, at 19:58, Blue Swirl wrote:
>
>
> > On 3/2/09, Alexander Graf <agraf@suse.de> wrote:
> >
> > > In order to modify SLB entries on recent PPC64 machines, the slbmte
> > > instruction is used.
> > >
> > > This patch implements the slbmte instruction and makes the "bridge"
> > > mode code use the slb set functions, so we can move the SLB into
> > > the CPU struct later.
> > >
> > > This is required for Linux to run on PPC64.
> > >
> > > Signed-off-by: Alexander Graf <alex@csgraf.de>
> > >
> >
> >
> > >  void ppc_store_sr (CPUPPCState *env, int srnum, target_ulong value)
> > >  {
> > >    LOG_MMU("%s: reg=%d " ADDRX " " ADDRX "\n",
> > >                __func__, srnum, value, env->sr[srnum]);
> > > -    if (env->sr[srnum] != value) {
> > > +    if (env->mmu_model & POWERPC_MMU_64) {
> > >
> >
> >
> > >  target_ulong helper_load_sr (target_ulong sr_num)
> > >  {
> > > +    if (env->mmu_model & POWERPC_MMU_64)
> > > +        return ppc_load_sr(env, sr_num);
> > >    return env->sr[sr_num];
> > >  }
> > >
> >
> > It would be faster to move the MMU model checks to translation time.
> >
>
>  The problem is that we don't know which -cpu the user will give us when we
> run qemu-system-ppc64.
>  Of course we could just make qemu-system-ppc64 not take any non-64 bit
> CPUs, if nobody totally disagrees.

env could contain a pointer to (static const) structure describing the
CPU features. This pointer would be copied to corresponding
DisasContext feature pointer, to be used by the translation functions.

See for example target-sparc: translate.c (CHECK_IU_FEATURE macro) and
helper.c for CPU definitions. IIRC MIPS or ARM have even nicer macros.

[Qemu-devel] Re: [PATCH 2/3] PPC64: Implement slbmte

[Alexander Graf]

On 02.03.2009, at 20:28, Blue Swirl wrote:

> On 3/2/09, Alexander Graf <agraf@suse.de> wrote:
>>
>> On 02.03.2009, at 19:58, Blue Swirl wrote:
>>
>>
>>> On 3/2/09, Alexander Graf <agraf@suse.de> wrote:
>>>
>>>> In order to modify SLB entries on recent PPC64 machines, the slbmte
>>>> instruction is used.
>>>>
>>>> This patch implements the slbmte instruction and makes the "bridge"
>>>> mode code use the slb set functions, so we can move the SLB into
>>>> the CPU struct later.
>>>>
>>>> This is required for Linux to run on PPC64.
>>>>
>>>> Signed-off-by: Alexander Graf <alex@csgraf.de>
>>>>
>>>
>>>
>>>> void ppc_store_sr (CPUPPCState *env, int srnum, target_ulong value)
>>>> {
>>>>  LOG_MMU("%s: reg=%d " ADDRX " " ADDRX "\n",
>>>>              __func__, srnum, value, env->sr[srnum]);
>>>> -    if (env->sr[srnum] != value) {
>>>> +    if (env->mmu_model & POWERPC_MMU_64) {
>>>>
>>>
>>>
>>>> target_ulong helper_load_sr (target_ulong sr_num)
>>>> {
>>>> +    if (env->mmu_model & POWERPC_MMU_64)
>>>> +        return ppc_load_sr(env, sr_num);
>>>>  return env->sr[sr_num];
>>>> }
>>>>
>>>
>>> It would be faster to move the MMU model checks to translation time.
>>>
>>
>> The problem is that we don't know which -cpu the user will give us  
>> when we
>> run qemu-system-ppc64.
>> Of course we could just make qemu-system-ppc64 not take any non-64  
>> bit
>> CPUs, if nobody totally disagrees.
>
> env could contain a pointer to (static const) structure describing the
> CPU features. This pointer would be copied to corresponding
> DisasContext feature pointer, to be used by the translation functions.
>
> See for example target-sparc: translate.c (CHECK_IU_FEATURE macro) and
> helper.c for CPU definitions. IIRC MIPS or ARM have even nicer macros.


Right, as soon as I have Linux actually running and spare time to look  
at this, that's a pretty neat optimization :-). Honestly, for now  
speed isn't my main concern.

Alex

[Qemu-devel] Re: [PATCH 1/3] PPC: Circumvent overflow in mtcrf

[Blue Swirl]

On 3/2/09, Alexander Graf <agraf@suse.de> wrote:
> I had a segmentation fault in mtcrf, where ffs() returned 8 and the
>  code then accessed cpu_crf[7 - 8].
>
>  In order to circumvent this, I just put in an & 7 to the ffs result,
>  so we'll never run negative. This is probably not correct, but makes
>  things work for me so far.

ffs does not return bit number (in power of 2 numbering), but one if
the zeroth bit is set, two if the first bit is set etc. So I'd try
instead

crn = ffs(crm) - 1;

Re: [Qemu-devel] [PATCH 1/3] PPC: Circumvent overflow in mtcrf

[Laurent Desnogues]

On Mon, Mar 2, 2009 at 6:07 PM, Alexander Graf <agraf@suse.de> wrote:
> I had a segmentation fault in mtcrf, where ffs() returned 8 and the
> code then accessed cpu_crf[7 - 8].
>
> In order to circumvent this, I just put in an & 7 to the ffs result,
> so we'll never run negative. This is probably not correct, but makes
> things work for me so far.
>
> Signed-off-by: Alexander Graf <alex@csgraf.de>
> ---
>  target-ppc/translate.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 2a06e4c..2e7420f 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -3937,7 +3937,7 @@ GEN_HANDLER(mtcrf, 0x1F, 0x10, 0x04, 0x00000801, PPC_MISC)
>     crm = CRM(ctx->opcode);
>     if (likely((ctx->opcode & 0x00100000) || (crm ^ (crm - 1)) == 0)) {
>         TCGv_i32 temp = tcg_temp_new_i32();
> -        crn = ffs(crm);
> +        crn = ffs(crm) & 7;
>         tcg_gen_trunc_tl_i32(temp, cpu_gpr[rS(ctx->opcode)]);
>         tcg_gen_shri_i32(cpu_crf[7 - crn], temp, crn * 4);
>         tcg_gen_andi_i32(cpu_crf[7 - crn], cpu_crf[7 - crn], 0xf);

I think the problem lies in the if;  could you try this?

    if (likely((ctx->opcode & 0x00100000))
        if (crm ^ (crm - 1)) == 0) {
            TCGv_i32 temp = tcg_temp_new_i32();
            crn = ffs(crm);
            tcg_gen_trunc_tl_i32(temp, cpu_gpr[rS(ctx->opcode)]);
            tcg_gen_shri_i32(cpu_crf[7 - crn], temp, crn * 4);
            tcg_gen_andi_i32(cpu_crf[7 - crn], cpu_crf[7 - crn], 0xf);
            tcg_temp_free_i32(temp);
        }
    } else {

The Power spec says mtocrf is undefined if crm has not
exactly one bit set.  The patch above will just make it a nop
(and will also generate code for mtcrf when it should instead
of wrongly generating buggy code for mtocrf when crm is a
power of 2 no matter what bit 20 [or bit 11 in dumb reversed
bit numbering scheme of Power] is).


Laurent

Re: [Qemu-devel] [PATCH 1/3] PPC: Circumvent overflow in mtcrf

[Alexander Graf]

On 02.03.2009, at 23:45, Laurent Desnogues  
<laurent.desnogues@gmail.com> wrote:

> On Mon, Mar 2, 2009 at 6:07 PM, Alexander Graf <agraf@suse.de> wrote:
>> I had a segmentation fault in mtcrf, where ffs() returned 8 and the
>> code then accessed cpu_crf[7 - 8].
>>
>> In order to circumvent this, I just put in an & 7 to the ffs result,
>> so we'll never run negative. This is probably not correct, but makes
>> things work for me so far.
>>
>> Signed-off-by: Alexander Graf <alex@csgraf.de>
>> ---
>> target-ppc/translate.c |    2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
>> index 2a06e4c..2e7420f 100644
>> --- a/target-ppc/translate.c
>> +++ b/target-ppc/translate.c
>> @@ -3937,7 +3937,7 @@ GEN_HANDLER(mtcrf, 0x1F, 0x10, 0x04,  
>> 0x00000801, PPC_MISC)
>>    crm = CRM(ctx->opcode);
>>    if (likely((ctx->opcode & 0x00100000) || (crm ^ (crm - 1)) ==  
>> 0)) {
>>        TCGv_i32 temp = tcg_temp_new_i32();
>> -        crn = ffs(crm);
>> +        crn = ffs(crm) & 7;
>>        tcg_gen_trunc_tl_i32(temp, cpu_gpr[rS(ctx->opcode)]);
>>        tcg_gen_shri_i32(cpu_crf[7 - crn], temp, crn * 4);
>>        tcg_gen_andi_i32(cpu_crf[7 - crn], cpu_crf[7 - crn], 0xf);
>
> I think the problem lies in the if;  could you try this?
>
>    if (likely((ctx->opcode & 0x00100000))
>        if (crm ^ (crm - 1)) == 0) {
>            TCGv_i32 temp = tcg_temp_new_i32();
>            crn = ffs(crm);
>            tcg_gen_trunc_tl_i32(temp, cpu_gpr[rS(ctx->opcode)]);
>            tcg_gen_shri_i32(cpu_crf[7 - crn], temp, crn * 4);
>            tcg_gen_andi_i32(cpu_crf[7 - crn], cpu_crf[7 - crn], 0xf);
>            tcg_temp_free_i32(temp);
>        }
>    } else {
>
> The Power spec says mtocrf is undefined if crm has not
> exactly one bit set.  The patch above will just make it a nop
> (and will also generate code for mtcrf when it should instead
> of wrongly generating buggy code for mtocrf when crm is a
> power of 2 no matter what bit 20 [or bit 11 in dumb reversed
> bit numbering scheme of Power] is).

I think malc fixed that one just before you hit the send button :-).

I've been running over other incorrectnesses while trying to get Linux  
working (now unpacks the initrd and panics in pmz init). Do you have a  
good link to the current isa so I can check which bits should be what?

Alex

Re: [Qemu-devel] [PATCH 1/3] PPC: Circumvent overflow in mtcrf

[Laurent Desnogues]

On Tue, Mar 3, 2009 at 11:26 AM, Alexander Graf <agraf@suse.de> wrote:
> I think malc fixed that one just before you hit the send button :-).

He did :-)

> I've been running over other incorrectnesses while trying to get Linux
> working (now unpacks the initrd and panics in pmz init). Do you have a good
> link to the current isa so I can check which bits should be what?

Look here:

   http://www.power.org/resources/downloads/

And remember the odd bit numbering scheme...


Laurent