Re: [Qemu-devel] [PATCH 01/27] Clean up PowerPC SLB handling code

Re: [Qemu-devel] [PATCH 01/27] Clean up PowerPC SLB handling code

[Andreas Färber]

QEMU HEAD still uses a 32-bit binary for both 32-bit and 64-bit. That one uses mtsrin so will need the compatibility, it seemed affected, too.

OpenBIOS SVN HEAD (blob) uses slb* as linked to. We're in the preparation of 1.1 and I need to test it before we can update the QEMU binary. ;)

Sorry for top-posting, Android sucks.

Andreas

David Gibson <david@gibson.dropbear.id.au> schrieb:

>On Thu, May 19, 2011 at 08:00:53AM +0200, Andreas Färber wrote:
>> Am 19.05.2011 um 07:39 schrieb David Gibson:
>> 
>> >On Thu, May 19, 2011 at 07:35:30AM +0200, Andreas Färber wrote:
>> >>Am 25.03.2011 um 04:21 schrieb David Gibson:
>> >>
>> >>>Currently the SLB information when emulating a PowerPC 970 is
>> >>>storeed in a structure with the unhelpfully named fields 'tmp'
>> >>>and 'tmp64'.  While the layout in these fields does match the
>> >>>description of the SLB in the architecture document, it is not
>> >>>convenient either for looking up the SLB, or for emulating the
>> >>>slbmte instruction.
>> >>>
>> >>>This patch, therefore, reorganizes the SLB entry structure to be
>> >>>divided in the the "ESID related" and "VSID related" fields as
>> >>>they are divided in instructions accessing the SLB.
>> >>>
>> >>>In addition to making the code smaller and more readable, this will
>> >>>make it easier to implement for the 1TB segments used in more
>> >>>recent PowerPC chips.
>> >>>
>> >>>Signed-off-by: David Gibson <dwg@au1.ibm.com>
>> >>
>> >>According to my bisect, this patch broke ppc64 OpenBIOS.
>> >>
>> >>David, would you please take a look?
>> >
>> >Uh, sure.  can you describe the symptoms of the breakage, and the
>> >exact qemu setup you've observed the problem with?
>> 
>> $ uname -a
>> Darwin PMG5-3.local 9.8.0 Darwin Kernel Version 9.8.0: Wed Jul 15
>> 16:57:01 PDT 2009; root:xnu-1228.15.4~1/RELEASE_PPC Power Macintosh
>> # Mac OS X 10.5.8 on dual-core G5
>> 
>> $ gcc-4.2 --version
>> powerpc-apple-darwin9-gcc-4.2.1 (GCC) 4.2.1 (Apple Inc. build 5577)
>> Copyright (C) 2007 Free Software Foundation, Inc.
>> This is free software; see the source for copying conditions.  There
>> is NO
>> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
>> PURPOSE.
>> 
>> ../qemu/configure --prefix=/Users/andreas/QEMU/latest64 \
>> --target-list=ppc-softmmu,ppc64-softmmu \
>> --extra-cflags="-arch ppc64" --extra-ldflags="-arch ppc64" --cc=gcc-4.2
>> --host-cc=gcc-4.2 \
>> --enable-cocoa --disable-kvm --disable-strip --disable-docs
>> --enable-io-thread $*
>> # I/O thread doesn't matter
>> 
>> /Users/andreas/QEMU/qemu64/ppc64-softmmu/qemu-system-ppc64 -boot d -cdrom
>> /Users/andreas/QEMU/AIX/dvd.1022A4_OBETA_710.iso \
>> -bios /Users/andreas/QEMU/OpenBIOS/openbios/obj-ppc64/openbios-qemu.elf
>> -m 1024 \
>> -M mac99 -prom-env 'auto-boot?=false' -nographic
>> 
>> CD should be irrelevant, -boot c -hda /dev/null should work the same.
>
>Um, yeah, I thought putting an AIX dvd into a Mac was a bit odd.
>
>> Expected: OpenBIOS banner and > prompt
>> Observed: DSI exception on mtmsrd, no serial output
>
>Does this happen only with your openbios image, or does it also happen
>with the one included in qemu?
>
>My working theory is that openbios is using the old 32-64 bridge stuff
>where segment register accesses are emulated to populate certain SLB
>slots.  I have a vague recollection that I saw the code implementing
>that when I was doing the cleanup and convinced myself it was not
>necessary.  More information as I investigate further.
>
>> (The regression on ppc32 AIX observed by Kenneth seems to be a
>> different issue, introduced earlier.)
>
>I don't know what that regression is; I don't read qemu-devel
>exhaustively.  If you want my attention, best to CC me directly.
>
>-- 
>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

Re: [Qemu-devel] [PATCH 01/27] Clean up PowerPC SLB handling code

[David Gibson]

On Thu, May 19, 2011 at 10:25:04AM +0200, Andreas Färber wrote:
> QEMU HEAD still uses a 32-bit binary for both 32-bit and
> 64-bit. That one uses mtsrin so will need the compatibility, it
> seemed affected, too.
> 
> OpenBIOS SVN HEAD (blob) uses slb* as linked to. We're in the
> preparation of 1.1 and I need to test it before we can update the
> QEMU binary. ;)
> 
> Sorry for top-posting, Android sucks.

So, my theory was half right.  It was a problem with 64-bit mtsr
emulation, but it wasn't that I just removed that code with the SLB
cleanup.  The code was still there and *almost* right.  I was off by
one in one shift, causing the storage key bits to end up in the wrong
place in the SLB entry.  I'll send out the patch right after I've sent
this mail.

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

Re: [Qemu-devel] [PATCH 01/27] Clean up PowerPC SLB handling code

[Alexander Graf]

On 20.05.2011, at 05:34, David Gibson wrote:

> On Thu, May 19, 2011 at 10:25:04AM +0200, Andreas Färber wrote:
>> QEMU HEAD still uses a 32-bit binary for both 32-bit and
>> 64-bit. That one uses mtsrin so will need the compatibility, it
>> seemed affected, too.
>> 
>> OpenBIOS SVN HEAD (blob) uses slb* as linked to. We're in the
>> preparation of 1.1 and I need to test it before we can update the
>> QEMU binary. ;)
>> 
>> Sorry for top-posting, Android sucks.
> 
> So, my theory was half right.  It was a problem with 64-bit mtsr
> emulation, but it wasn't that I just removed that code with the SLB
> cleanup.  The code was still there and *almost* right.  I was off by
> one in one shift, causing the storage key bits to end up in the wrong
> place in the SLB entry.  I'll send out the patch right after I've sent
> this mail.

Thanks a lot for tracking it down you two :)

Alex

[Qemu-devel] [0/27] Implement emulation of pSeries logical partitions (v6)

[David Gibson]

This patch series adds a "pseries" machine to qemu, allowing it to
emulate IBM pSeries logical partitions.  More specifically it
implements the interface defined by the "PowerPC Architecture Platform
Requirements" document (PAPR, or sPAPR for short).

Along the way we add a bunch of support for more modern ppc CPUs than
are currently supported.  It also makes some significant cleanups to
the translation code for hash page table based ppc MMUs.

Please apply.

---

Changes since v5 of this series:
 * Include source for SLOF as a git submodule
 * Move our private hcalls (well. only one so far) to a range
   explicitly reserved by PAPR for this purpose.
 * Allocate an ample 1024 irqs in the XICS controller, rather than
   having a somewhat dubious calculation of precisely how many we
   need.

Changes since v4 of this series:
 * Fix build breakages for powerpc targets other than ppc64 full system.
 * Since the pseries platform requires libfdt, only compile it when
   configured with --enable-fdt
 * Give an informative error if invoked with insufficient guest RAM to
   run the partition firmware.  Without this, giving insufficient RAM
   - such as qemu's default 64M - would lead to the firmware failing
   cryptically partway through boot.

Changes since v3 of this series:
 * Many, many checkpatch fixups
 * Integrated feedback from qemu-devel list
 * Added in-partition SLOF firmware

Changes since v2 of this series:
 * Assorted bugfixes and cleanups.

Changes since v1 of this series:
 * numerous coding style fixups
 * incorporated most review comments from initial version
 * moved to a wholly dynamic hypercall registration scheme
 * assorted other cleanups
 * many more patches implementing VIO devices

[Qemu-devel] [PATCH 01/27] Clean up PowerPC SLB handling code

[David Gibson]

Currently the SLB information when emulating a PowerPC 970 is
storeed in a structure with the unhelpfully named fields 'tmp'
and 'tmp64'.  While the layout in these fields does match the
description of the SLB in the architecture document, it is not
convenient either for looking up the SLB, or for emulating the
slbmte instruction.

This patch, therefore, reorganizes the SLB entry structure to be
divided in the the "ESID related" and "VSID related" fields as
they are divided in instructions accessing the SLB.

In addition to making the code smaller and more readable, this will
make it easier to implement for the 1TB segments used in more
recent PowerPC chips.

Signed-off-by: David Gibson <dwg@au1.ibm.com>
---
 target-ppc/cpu.h       |   29 +++++++-
 target-ppc/helper.c    |  178 ++++++++++++++----------------------------------
 target-ppc/helper.h    |    1 -
 target-ppc/op_helper.c |    9 +--
 4 files changed, 80 insertions(+), 137 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index deb8d7c..124bbbf 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -43,6 +43,8 @@
 # define TARGET_VIRT_ADDR_SPACE_BITS 64
 #endif
 
+#define TARGET_PAGE_BITS_16M 24
+
 #else /* defined (TARGET_PPC64) */
 /* PowerPC 32 definitions */
 #define TARGET_LONG_BITS 32
@@ -359,10 +361,31 @@ union ppc_tlb_t {
 
 typedef struct ppc_slb_t ppc_slb_t;
 struct ppc_slb_t {
-    uint64_t tmp64;
-    uint32_t tmp;
+    uint64_t esid;
+    uint64_t vsid;
 };
 
+/* Bits in the SLB ESID word */
+#define SLB_ESID_ESID           0xFFFFFFFFF0000000ULL
+#define SLB_ESID_V              0x0000000008000000ULL /* valid */
+
+/* Bits in the SLB VSID word */
+#define SLB_VSID_SHIFT          12
+#define SLB_VSID_SSIZE_SHIFT    62
+#define SLB_VSID_B              0xc000000000000000ULL
+#define SLB_VSID_B_256M         0x0000000000000000ULL
+#define SLB_VSID_VSID           0x3FFFFFFFFFFFF000ULL
+#define SLB_VSID_KS             0x0000000000000800ULL
+#define SLB_VSID_KP             0x0000000000000400ULL
+#define SLB_VSID_N              0x0000000000000200ULL /* no-execute */
+#define SLB_VSID_L              0x0000000000000100ULL
+#define SLB_VSID_C              0x0000000000000080ULL /* class */
+#define SLB_VSID_LP             0x0000000000000030ULL
+#define SLB_VSID_ATTR           0x0000000000000FFFULL
+
+#define SEGMENT_SHIFT_256M      28
+#define SEGMENT_MASK_256M       (~((1ULL << SEGMENT_SHIFT_256M) - 1))
+
 /*****************************************************************************/
 /* Machine state register bits definition                                    */
 #define MSR_SF   63 /* Sixty-four-bit mode                            hflags */
@@ -755,7 +778,7 @@ void ppc_store_sdr1 (CPUPPCState *env, target_ulong value);
 void ppc_store_asr (CPUPPCState *env, target_ulong value);
 target_ulong ppc_load_slb (CPUPPCState *env, int slb_nr);
 target_ulong ppc_load_sr (CPUPPCState *env, int sr_nr);
-void ppc_store_slb (CPUPPCState *env, target_ulong rb, target_ulong rs);
+int 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 4b49101..2094ca3 100644
--- a/target-ppc/helper.c
+++ b/target-ppc/helper.c
@@ -672,85 +672,36 @@ static inline int find_pte(CPUState *env, mmu_ctx_t *ctx, int h, int rw,
 }
 
 #if defined(TARGET_PPC64)
-static ppc_slb_t *slb_get_entry(CPUPPCState *env, int nr)
-{
-    ppc_slb_t *retval = &env->slb[nr];
-
-#if 0 // XXX implement bridge mode?
-    if (env->spr[SPR_ASR] & 1) {
-        target_phys_addr_t sr_base;
-
-        sr_base = env->spr[SPR_ASR] & 0xfffffffffffff000;
-        sr_base += (12 * nr);
-
-        retval->tmp64 = ldq_phys(sr_base);
-        retval->tmp = ldl_phys(sr_base + 8);
-    }
-#endif
-
-    return retval;
-}
-
-static void slb_set_entry(CPUPPCState *env, int nr, ppc_slb_t *slb)
-{
-    ppc_slb_t *entry = &env->slb[nr];
-
-    if (slb == entry)
-        return;
-
-    entry->tmp64 = slb->tmp64;
-    entry->tmp = slb->tmp;
-}
-
-static inline int slb_is_valid(ppc_slb_t *slb)
-{
-    return (int)(slb->tmp64 & 0x0000000008000000ULL);
-}
-
-static inline void slb_invalidate(ppc_slb_t *slb)
-{
-    slb->tmp64 &= ~0x0000000008000000ULL;
-}
-
 static inline int slb_lookup(CPUPPCState *env, target_ulong eaddr,
                              target_ulong *vsid, target_ulong *page_mask,
                              int *attr, int *target_page_bits)
 {
-    target_ulong mask;
-    int n, ret;
+    uint64_t esid;
+    int n;
 
-    ret = -5;
     LOG_SLB("%s: eaddr " TARGET_FMT_lx "\n", __func__, eaddr);
-    mask = 0x0000000000000000ULL; /* Avoid gcc warning */
+
+    esid = (eaddr & SEGMENT_MASK_256M) | SLB_ESID_V;
+
     for (n = 0; n < env->slb_nr; n++) {
-        ppc_slb_t *slb = slb_get_entry(env, n);
-
-        LOG_SLB("%s: seg %d %016" PRIx64 " %08"
-                    PRIx32 "\n", __func__, n, slb->tmp64, slb->tmp);
-        if (slb_is_valid(slb)) {
-            /* SLB entry is valid */
-            mask = 0xFFFFFFFFF0000000ULL;
-            if (slb->tmp & 0x8) {
-                /* 16 MB PTEs */
-                if (target_page_bits)
-                    *target_page_bits = 24;
-            } else {
-                /* 4 KB PTEs */
-                if (target_page_bits)
-                    *target_page_bits = TARGET_PAGE_BITS;
-            }
-            if ((eaddr & mask) == (slb->tmp64 & mask)) {
-                /* SLB match */
-                *vsid = ((slb->tmp64 << 24) | (slb->tmp >> 8)) & 0x0003FFFFFFFFFFFFULL;
-                *page_mask = ~mask;
-                *attr = slb->tmp & 0xFF;
-                ret = n;
-                break;
+        ppc_slb_t *slb = &env->slb[n];
+
+        LOG_SLB("%s: slot %d %016" PRIx64 " %016"
+                    PRIx64 "\n", __func__, n, slb->esid, slb->vsid);
+        if (slb->esid == esid) {
+            *vsid = (slb->vsid & SLB_VSID_VSID) >> SLB_VSID_SHIFT;
+            *page_mask = ~SEGMENT_MASK_256M;
+            *attr = slb->vsid & SLB_VSID_ATTR;
+            if (target_page_bits) {
+                *target_page_bits = (slb->vsid & SLB_VSID_L)
+                    ? TARGET_PAGE_BITS_16M
+                    : TARGET_PAGE_BITS;
             }
+            return n;
         }
     }
 
-    return ret;
+    return -5;
 }
 
 void ppc_slb_invalidate_all (CPUPPCState *env)
@@ -760,11 +711,10 @@ void ppc_slb_invalidate_all (CPUPPCState *env)
     do_invalidate = 0;
     /* XXX: Warning: slbia never invalidates the first segment */
     for (n = 1; n < env->slb_nr; n++) {
-        ppc_slb_t *slb = slb_get_entry(env, n);
+        ppc_slb_t *slb = &env->slb[n];
 
-        if (slb_is_valid(slb)) {
-            slb_invalidate(slb);
-            slb_set_entry(env, n, slb);
+        if (slb->esid & SLB_ESID_V) {
+            slb->esid &= ~SLB_ESID_V;
             /* XXX: given the fact that segment size is 256 MB or 1TB,
              *      and we still don't have a tlb_flush_mask(env, n, mask)
              *      in Qemu, we just invalidate all TLBs
@@ -781,68 +731,44 @@ void ppc_slb_invalidate_one (CPUPPCState *env, uint64_t T0)
     target_ulong vsid, page_mask;
     int attr;
     int n;
+    ppc_slb_t *slb;
 
     n = slb_lookup(env, T0, &vsid, &page_mask, &attr, NULL);
-    if (n >= 0) {
-        ppc_slb_t *slb = slb_get_entry(env, n);
-
-        if (slb_is_valid(slb)) {
-            slb_invalidate(slb);
-            slb_set_entry(env, n, slb);
-            /* XXX: given the fact that segment size is 256 MB or 1TB,
-             *      and we still don't have a tlb_flush_mask(env, n, mask)
-             *      in Qemu, we just invalidate all TLBs
-             */
-            tlb_flush(env, 1);
-        }
+    if (n < 0) {
+        return;
     }
-}
 
-target_ulong ppc_load_slb (CPUPPCState *env, int slb_nr)
-{
-    target_ulong rt;
-    ppc_slb_t *slb = slb_get_entry(env, slb_nr);
+    slb = &env->slb[n];
 
-    if (slb_is_valid(slb)) {
-        /* SLB entry is valid */
-        /* Copy SLB bits 62:88 to Rt 37:63 (VSID 23:49) */
-        rt = slb->tmp >> 8;             /* 65:88 => 40:63 */
-        rt |= (slb->tmp64 & 0x7) << 24; /* 62:64 => 37:39 */
-        /* Copy SLB bits 89:92 to Rt 33:36 (KsKpNL) */
-        rt |= ((slb->tmp >> 4) & 0xF) << 27;
-    } else {
-        rt = 0;
-    }
-    LOG_SLB("%s: %016" PRIx64 " %08" PRIx32 " => %d "
-            TARGET_FMT_lx "\n", __func__, slb->tmp64, slb->tmp, slb_nr, rt);
+    if (slb->esid & SLB_ESID_V) {
+        slb->esid &= ~SLB_ESID_V;
 
-    return rt;
+        /* XXX: given the fact that segment size is 256 MB or 1TB,
+         *      and we still don't have a tlb_flush_mask(env, n, mask)
+         *      in Qemu, we just invalidate all TLBs
+         */
+        tlb_flush(env, 1);
+    }
 }
 
-void ppc_store_slb (CPUPPCState *env, target_ulong rb, target_ulong rs)
+int ppc_store_slb (CPUPPCState *env, target_ulong rb, target_ulong rs)
 {
-    ppc_slb_t *slb;
-
-    uint64_t vsid;
-    uint64_t esid;
-    int flags, valid, slb_nr;
-
-    vsid = rs >> 12;
-    flags = ((rs >> 8) & 0xf);
+    int slot = rb & 0xfff;
+    uint64_t esid = rb & ~0xfff;
+    ppc_slb_t *slb = &env->slb[slot];
 
-    esid = rb >> 28;
-    valid = (rb & (1 << 27));
-    slb_nr = rb & 0xfff;
+    if (slot >= env->slb_nr) {
+        return -1;
+    }
 
-    slb = slb_get_entry(env, slb_nr);
-    slb->tmp64 = (esid << 28) | valid | (vsid >> 24);
-    slb->tmp = (vsid << 8) | (flags << 3);
+    slb->esid = esid;
+    slb->vsid = rs;
 
     LOG_SLB("%s: %d " TARGET_FMT_lx " - " TARGET_FMT_lx " => %016" PRIx64
-            " %08" PRIx32 "\n", __func__, slb_nr, rb, rs, slb->tmp64,
-            slb->tmp);
+            " %016" PRIx64 "\n", __func__, slot, rb, rs,
+            slb->esid, slb->vsid);
 
-    slb_set_entry(env, slb_nr, slb);
+    return 0;
 }
 #endif /* defined(TARGET_PPC64) */
 
@@ -860,24 +786,22 @@ static inline int get_segment(CPUState *env, mmu_ctx_t *ctx,
 {
     target_phys_addr_t sdr, hash, mask, sdr_mask, htab_mask;
     target_ulong sr, vsid, vsid_mask, pgidx, page_mask;
-#if defined(TARGET_PPC64)
-    int attr;
-#endif
     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) {
+        int attr;
+
         LOG_MMU("Check SLBs\n");
         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;
+        ctx->key = !!(pr ? (attr & SLB_VSID_KP) : (attr & SLB_VSID_KS));
         ds = 0;
-        ctx->nx = attr & 0x10 ? 1 : 0;
+        ctx->nx = !!(attr & SLB_VSID_N);
         ctx->eaddr = eaddr;
         vsid_mask = 0x00003FFFFFFFFF80ULL;
         vsid_sh = 7;
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 2bf9283..d512cb0 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -340,7 +340,6 @@ DEF_HELPER_1(74xx_tlbi, void, tl)
 DEF_HELPER_FLAGS_0(tlbia, TCG_CALL_CONST, void)
 DEF_HELPER_FLAGS_1(tlbie, TCG_CALL_CONST, void, tl)
 #if defined(TARGET_PPC64)
-DEF_HELPER_FLAGS_1(load_slb, TCG_CALL_CONST, tl, tl)
 DEF_HELPER_FLAGS_2(store_slb, TCG_CALL_CONST, void, tl, tl)
 DEF_HELPER_FLAGS_0(slbia, TCG_CALL_CONST, void)
 DEF_HELPER_FLAGS_1(slbie, TCG_CALL_CONST, void, tl)
diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
index 17e070a..bf41627 100644
--- a/target-ppc/op_helper.c
+++ b/target-ppc/op_helper.c
@@ -3746,14 +3746,11 @@ void helper_store_sr (target_ulong sr_num, target_ulong val)
 
 /* SLB management */
 #if defined(TARGET_PPC64)
-target_ulong helper_load_slb (target_ulong slb_nr)
-{
-    return ppc_load_slb(env, slb_nr);
-}
-
 void helper_store_slb (target_ulong rb, target_ulong rs)
 {
-    ppc_store_slb(env, rb, rs);
+    if (ppc_store_slb(env, rb, rs) < 0) {
+        helper_raise_exception_err(POWERPC_EXCP_PROGRAM, POWERPC_EXCP_INVAL);
+    }
 }
 
 void helper_slbia (void)
-- 
1.7.1

[Qemu-devel] [0/27] Implement emulation of pSeries logical partitions (v5)

[David Gibson]

This patch series adds a "pseries" machine to qemu, allowing it to
emulate IBM pSeries logical partitions.  More specifically it
implements the interface defined by the "PowerPC Architecture Platform
Requirements" document (PAPR, or sPAPR for short).

Along the way we add a bunch of support for more modern ppc CPUs than
are currently supported.  It also makes some significant cleanups to
the translation code for hash page table based ppc MMUs.

Please apply.

---

Note that I haven't implemented (yet) a min_ram field in the machine
structure.  There are a number of places where the pseries platform
would benefit from more participation of the machine description in
command line validation.  In want to think a bit more about these
before sending some patches.  For now I've taken the simpler approach
of just adding a meaningful error message to the machine init function
if ram_size is too small.

Changes since v4 of this series:
 * Fix build breakages for powerpc targets other than ppc64 full system.
 * Since the pseries platform requires libfdt, only compile it when
   configured with --enable-fdt
 * Give an informative error if invoked with insufficient guest RAM to
   run the partition firmware.  Without this, giving insufficient RAM
   - such as qemu's default 64M - would lead to the firmware failing
   cryptically partway through boot.

Changes since v3 of this series:
 * Many, many checkpatch fixups
 * Integrated feedback from qemu-devel list
 * Added in-partition SLOF firmware

Changes since v2 of this series:
 * Assorted bugfixes and cleanups.

Changes since v1 of this series:
 * numerous coding style fixups
 * incorporated most review comments from initial version
 * moved to a wholly dynamic hypercall registration scheme
 * assorted other cleanups
 * many more patches implementing VIO devices

[Qemu-devel] [PATCH 01/27] Clean up PowerPC SLB handling code

[David Gibson]

Currently the SLB information when emulating a PowerPC 970 is
storeed in a structure with the unhelpfully named fields 'tmp'
and 'tmp64'.  While the layout in these fields does match the
description of the SLB in the architecture document, it is not
convenient either for looking up the SLB, or for emulating the
slbmte instruction.

This patch, therefore, reorganizes the SLB entry structure to be
divided in the the "ESID related" and "VSID related" fields as
they are divided in instructions accessing the SLB.

In addition to making the code smaller and more readable, this will
make it easier to implement for the 1TB segments used in more
recent PowerPC chips.

Signed-off-by: David Gibson <dwg@au1.ibm.com>
---
 target-ppc/cpu.h       |   29 +++++++-
 target-ppc/helper.c    |  178 ++++++++++++++----------------------------------
 target-ppc/helper.h    |    1 -
 target-ppc/op_helper.c |    9 +--
 4 files changed, 80 insertions(+), 137 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index deb8d7c..124bbbf 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -43,6 +43,8 @@
 # define TARGET_VIRT_ADDR_SPACE_BITS 64
 #endif
 
+#define TARGET_PAGE_BITS_16M 24
+
 #else /* defined (TARGET_PPC64) */
 /* PowerPC 32 definitions */
 #define TARGET_LONG_BITS 32
@@ -359,10 +361,31 @@ union ppc_tlb_t {
 
 typedef struct ppc_slb_t ppc_slb_t;
 struct ppc_slb_t {
-    uint64_t tmp64;
-    uint32_t tmp;
+    uint64_t esid;
+    uint64_t vsid;
 };
 
+/* Bits in the SLB ESID word */
+#define SLB_ESID_ESID           0xFFFFFFFFF0000000ULL
+#define SLB_ESID_V              0x0000000008000000ULL /* valid */
+
+/* Bits in the SLB VSID word */
+#define SLB_VSID_SHIFT          12
+#define SLB_VSID_SSIZE_SHIFT    62
+#define SLB_VSID_B              0xc000000000000000ULL
+#define SLB_VSID_B_256M         0x0000000000000000ULL
+#define SLB_VSID_VSID           0x3FFFFFFFFFFFF000ULL
+#define SLB_VSID_KS             0x0000000000000800ULL
+#define SLB_VSID_KP             0x0000000000000400ULL
+#define SLB_VSID_N              0x0000000000000200ULL /* no-execute */
+#define SLB_VSID_L              0x0000000000000100ULL
+#define SLB_VSID_C              0x0000000000000080ULL /* class */
+#define SLB_VSID_LP             0x0000000000000030ULL
+#define SLB_VSID_ATTR           0x0000000000000FFFULL
+
+#define SEGMENT_SHIFT_256M      28
+#define SEGMENT_MASK_256M       (~((1ULL << SEGMENT_SHIFT_256M) - 1))
+
 /*****************************************************************************/
 /* Machine state register bits definition                                    */
 #define MSR_SF   63 /* Sixty-four-bit mode                            hflags */
@@ -755,7 +778,7 @@ void ppc_store_sdr1 (CPUPPCState *env, target_ulong value);
 void ppc_store_asr (CPUPPCState *env, target_ulong value);
 target_ulong ppc_load_slb (CPUPPCState *env, int slb_nr);
 target_ulong ppc_load_sr (CPUPPCState *env, int sr_nr);
-void ppc_store_slb (CPUPPCState *env, target_ulong rb, target_ulong rs);
+int 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 4b49101..2094ca3 100644
--- a/target-ppc/helper.c
+++ b/target-ppc/helper.c
@@ -672,85 +672,36 @@ static inline int find_pte(CPUState *env, mmu_ctx_t *ctx, int h, int rw,
 }
 
 #if defined(TARGET_PPC64)
-static ppc_slb_t *slb_get_entry(CPUPPCState *env, int nr)
-{
-    ppc_slb_t *retval = &env->slb[nr];
-
-#if 0 // XXX implement bridge mode?
-    if (env->spr[SPR_ASR] & 1) {
-        target_phys_addr_t sr_base;
-
-        sr_base = env->spr[SPR_ASR] & 0xfffffffffffff000;
-        sr_base += (12 * nr);
-
-        retval->tmp64 = ldq_phys(sr_base);
-        retval->tmp = ldl_phys(sr_base + 8);
-    }
-#endif
-
-    return retval;
-}
-
-static void slb_set_entry(CPUPPCState *env, int nr, ppc_slb_t *slb)
-{
-    ppc_slb_t *entry = &env->slb[nr];
-
-    if (slb == entry)
-        return;
-
-    entry->tmp64 = slb->tmp64;
-    entry->tmp = slb->tmp;
-}
-
-static inline int slb_is_valid(ppc_slb_t *slb)
-{
-    return (int)(slb->tmp64 & 0x0000000008000000ULL);
-}
-
-static inline void slb_invalidate(ppc_slb_t *slb)
-{
-    slb->tmp64 &= ~0x0000000008000000ULL;
-}
-
 static inline int slb_lookup(CPUPPCState *env, target_ulong eaddr,
                              target_ulong *vsid, target_ulong *page_mask,
                              int *attr, int *target_page_bits)
 {
-    target_ulong mask;
-    int n, ret;
+    uint64_t esid;
+    int n;
 
-    ret = -5;
     LOG_SLB("%s: eaddr " TARGET_FMT_lx "\n", __func__, eaddr);
-    mask = 0x0000000000000000ULL; /* Avoid gcc warning */
+
+    esid = (eaddr & SEGMENT_MASK_256M) | SLB_ESID_V;
+
     for (n = 0; n < env->slb_nr; n++) {
-        ppc_slb_t *slb = slb_get_entry(env, n);
-
-        LOG_SLB("%s: seg %d %016" PRIx64 " %08"
-                    PRIx32 "\n", __func__, n, slb->tmp64, slb->tmp);
-        if (slb_is_valid(slb)) {
-            /* SLB entry is valid */
-            mask = 0xFFFFFFFFF0000000ULL;
-            if (slb->tmp & 0x8) {
-                /* 16 MB PTEs */
-                if (target_page_bits)
-                    *target_page_bits = 24;
-            } else {
-                /* 4 KB PTEs */
-                if (target_page_bits)
-                    *target_page_bits = TARGET_PAGE_BITS;
-            }
-            if ((eaddr & mask) == (slb->tmp64 & mask)) {
-                /* SLB match */
-                *vsid = ((slb->tmp64 << 24) | (slb->tmp >> 8)) & 0x0003FFFFFFFFFFFFULL;
-                *page_mask = ~mask;
-                *attr = slb->tmp & 0xFF;
-                ret = n;
-                break;
+        ppc_slb_t *slb = &env->slb[n];
+
+        LOG_SLB("%s: slot %d %016" PRIx64 " %016"
+                    PRIx64 "\n", __func__, n, slb->esid, slb->vsid);
+        if (slb->esid == esid) {
+            *vsid = (slb->vsid & SLB_VSID_VSID) >> SLB_VSID_SHIFT;
+            *page_mask = ~SEGMENT_MASK_256M;
+            *attr = slb->vsid & SLB_VSID_ATTR;
+            if (target_page_bits) {
+                *target_page_bits = (slb->vsid & SLB_VSID_L)
+                    ? TARGET_PAGE_BITS_16M
+                    : TARGET_PAGE_BITS;
             }
+            return n;
         }
     }
 
-    return ret;
+    return -5;
 }
 
 void ppc_slb_invalidate_all (CPUPPCState *env)
@@ -760,11 +711,10 @@ void ppc_slb_invalidate_all (CPUPPCState *env)
     do_invalidate = 0;
     /* XXX: Warning: slbia never invalidates the first segment */
     for (n = 1; n < env->slb_nr; n++) {
-        ppc_slb_t *slb = slb_get_entry(env, n);
+        ppc_slb_t *slb = &env->slb[n];
 
-        if (slb_is_valid(slb)) {
-            slb_invalidate(slb);
-            slb_set_entry(env, n, slb);
+        if (slb->esid & SLB_ESID_V) {
+            slb->esid &= ~SLB_ESID_V;
             /* XXX: given the fact that segment size is 256 MB or 1TB,
              *      and we still don't have a tlb_flush_mask(env, n, mask)
              *      in Qemu, we just invalidate all TLBs
@@ -781,68 +731,44 @@ void ppc_slb_invalidate_one (CPUPPCState *env, uint64_t T0)
     target_ulong vsid, page_mask;
     int attr;
     int n;
+    ppc_slb_t *slb;
 
     n = slb_lookup(env, T0, &vsid, &page_mask, &attr, NULL);
-    if (n >= 0) {
-        ppc_slb_t *slb = slb_get_entry(env, n);
-
-        if (slb_is_valid(slb)) {
-            slb_invalidate(slb);
-            slb_set_entry(env, n, slb);
-            /* XXX: given the fact that segment size is 256 MB or 1TB,
-             *      and we still don't have a tlb_flush_mask(env, n, mask)
-             *      in Qemu, we just invalidate all TLBs
-             */
-            tlb_flush(env, 1);
-        }
+    if (n < 0) {
+        return;
     }
-}
 
-target_ulong ppc_load_slb (CPUPPCState *env, int slb_nr)
-{
-    target_ulong rt;
-    ppc_slb_t *slb = slb_get_entry(env, slb_nr);
+    slb = &env->slb[n];
 
-    if (slb_is_valid(slb)) {
-        /* SLB entry is valid */
-        /* Copy SLB bits 62:88 to Rt 37:63 (VSID 23:49) */
-        rt = slb->tmp >> 8;             /* 65:88 => 40:63 */
-        rt |= (slb->tmp64 & 0x7) << 24; /* 62:64 => 37:39 */
-        /* Copy SLB bits 89:92 to Rt 33:36 (KsKpNL) */
-        rt |= ((slb->tmp >> 4) & 0xF) << 27;
-    } else {
-        rt = 0;
-    }
-    LOG_SLB("%s: %016" PRIx64 " %08" PRIx32 " => %d "
-            TARGET_FMT_lx "\n", __func__, slb->tmp64, slb->tmp, slb_nr, rt);
+    if (slb->esid & SLB_ESID_V) {
+        slb->esid &= ~SLB_ESID_V;
 
-    return rt;
+        /* XXX: given the fact that segment size is 256 MB or 1TB,
+         *      and we still don't have a tlb_flush_mask(env, n, mask)
+         *      in Qemu, we just invalidate all TLBs
+         */
+        tlb_flush(env, 1);
+    }
 }
 
-void ppc_store_slb (CPUPPCState *env, target_ulong rb, target_ulong rs)
+int ppc_store_slb (CPUPPCState *env, target_ulong rb, target_ulong rs)
 {
-    ppc_slb_t *slb;
-
-    uint64_t vsid;
-    uint64_t esid;
-    int flags, valid, slb_nr;
-
-    vsid = rs >> 12;
-    flags = ((rs >> 8) & 0xf);
+    int slot = rb & 0xfff;
+    uint64_t esid = rb & ~0xfff;
+    ppc_slb_t *slb = &env->slb[slot];
 
-    esid = rb >> 28;
-    valid = (rb & (1 << 27));
-    slb_nr = rb & 0xfff;
+    if (slot >= env->slb_nr) {
+        return -1;
+    }
 
-    slb = slb_get_entry(env, slb_nr);
-    slb->tmp64 = (esid << 28) | valid | (vsid >> 24);
-    slb->tmp = (vsid << 8) | (flags << 3);
+    slb->esid = esid;
+    slb->vsid = rs;
 
     LOG_SLB("%s: %d " TARGET_FMT_lx " - " TARGET_FMT_lx " => %016" PRIx64
-            " %08" PRIx32 "\n", __func__, slb_nr, rb, rs, slb->tmp64,
-            slb->tmp);
+            " %016" PRIx64 "\n", __func__, slot, rb, rs,
+            slb->esid, slb->vsid);
 
-    slb_set_entry(env, slb_nr, slb);
+    return 0;
 }
 #endif /* defined(TARGET_PPC64) */
 
@@ -860,24 +786,22 @@ static inline int get_segment(CPUState *env, mmu_ctx_t *ctx,
 {
     target_phys_addr_t sdr, hash, mask, sdr_mask, htab_mask;
     target_ulong sr, vsid, vsid_mask, pgidx, page_mask;
-#if defined(TARGET_PPC64)
-    int attr;
-#endif
     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) {
+        int attr;
+
         LOG_MMU("Check SLBs\n");
         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;
+        ctx->key = !!(pr ? (attr & SLB_VSID_KP) : (attr & SLB_VSID_KS));
         ds = 0;
-        ctx->nx = attr & 0x10 ? 1 : 0;
+        ctx->nx = !!(attr & SLB_VSID_N);
         ctx->eaddr = eaddr;
         vsid_mask = 0x00003FFFFFFFFF80ULL;
         vsid_sh = 7;
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 2bf9283..d512cb0 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -340,7 +340,6 @@ DEF_HELPER_1(74xx_tlbi, void, tl)
 DEF_HELPER_FLAGS_0(tlbia, TCG_CALL_CONST, void)
 DEF_HELPER_FLAGS_1(tlbie, TCG_CALL_CONST, void, tl)
 #if defined(TARGET_PPC64)
-DEF_HELPER_FLAGS_1(load_slb, TCG_CALL_CONST, tl, tl)
 DEF_HELPER_FLAGS_2(store_slb, TCG_CALL_CONST, void, tl, tl)
 DEF_HELPER_FLAGS_0(slbia, TCG_CALL_CONST, void)
 DEF_HELPER_FLAGS_1(slbie, TCG_CALL_CONST, void, tl)
diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
index 17e070a..bf41627 100644
--- a/target-ppc/op_helper.c
+++ b/target-ppc/op_helper.c
@@ -3746,14 +3746,11 @@ void helper_store_sr (target_ulong sr_num, target_ulong val)
 
 /* SLB management */
 #if defined(TARGET_PPC64)
-target_ulong helper_load_slb (target_ulong slb_nr)
-{
-    return ppc_load_slb(env, slb_nr);
-}
-
 void helper_store_slb (target_ulong rb, target_ulong rs)
 {
-    ppc_store_slb(env, rb, rs);
+    if (ppc_store_slb(env, rb, rs) < 0) {
+        helper_raise_exception_err(POWERPC_EXCP_PROGRAM, POWERPC_EXCP_INVAL);
+    }
 }
 
 void helper_slbia (void)
-- 
1.7.1

Re: [Qemu-devel] [PATCH 01/27] Clean up PowerPC SLB handling code

[Andreas Färber]

Am 25.03.2011 um 04:21 schrieb David Gibson:

> Currently the SLB information when emulating a PowerPC 970 is
> storeed in a structure with the unhelpfully named fields 'tmp'
> and 'tmp64'.  While the layout in these fields does match the
> description of the SLB in the architecture document, it is not
> convenient either for looking up the SLB, or for emulating the
> slbmte instruction.
>
> This patch, therefore, reorganizes the SLB entry structure to be
> divided in the the "ESID related" and "VSID related" fields as
> they are divided in instructions accessing the SLB.
>
> In addition to making the code smaller and more readable, this will
> make it easier to implement for the 1TB segments used in more
> recent PowerPC chips.
>
> Signed-off-by: David Gibson <dwg@au1.ibm.com>

According to my bisect, this patch broke ppc64 OpenBIOS.

David, would you please take a look?

Thanks,
Andreas


## bad: 3964f535c35c08470ac69bd553282af500bc8bb0 Merge remote-tracking  
branch 'mst/for_anthony' into staging
## bad: 8500e3a91292f253002783da267f3b08aead86c1 Clean up slb_lookup()  
function
# good: [a0843a68c4e79df801342ec37347dfebc591082e] vnc: fix build  
error from VNC_DIRTY_WORDS
git bisect good a0843a68c4e79df801342ec37347dfebc591082e
# good: [5256d8bfad9b0113dc2f9b57706eaad26b008987] pci: use devfn for  
pci_find_device() instead of (slot, fn) pair
git bisect good 5256d8bfad9b0113dc2f9b57706eaad26b008987
# good: [fcda98630b121a63c9de0705df02e59f4dc2fecc] lm32: rename raise  
opcode to scall
git bisect good fcda98630b121a63c9de0705df02e59f4dc2fecc
# bad: [d569956eaff4be808419f1f259a5c388d8789db4] Add a hook to allow  
hypercalls to be emulated on PowerPC
git bisect bad d569956eaff4be808419f1f259a5c388d8789db4
# good: [17d9b3af5b7f93e43d7fbdcb6f14cad54de9f1ae] target-ppc: ext32u  
instead of andi with constant
git bisect good 17d9b3af5b7f93e43d7fbdcb6f14cad54de9f1ae
# bad: [c48974903051ceb7cfbda23c22c159ea4b482d93] Allow  
qemu_devtree_setprop() to take arbitrary values
git bisect bad c48974903051ceb7cfbda23c22c159ea4b482d93
# bad: [81762d6dd0d430d87024f2c83e9c4dcc4329fb7d] Clean up PowerPC SLB  
handling code
git bisect bad 81762d6dd0d430d87024f2c83e9c4dcc4329fb7d


> ---
> target-ppc/cpu.h       |   29 +++++++-
> target-ppc/helper.c    |  178 +++++++++++++ 
> +----------------------------------
> target-ppc/helper.h    |    1 -
> target-ppc/op_helper.c |    9 +--
> 4 files changed, 80 insertions(+), 137 deletions(-)
>
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index deb8d7c..124bbbf 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -43,6 +43,8 @@
> # define TARGET_VIRT_ADDR_SPACE_BITS 64
> #endif
>
> +#define TARGET_PAGE_BITS_16M 24
> +
> #else /* defined (TARGET_PPC64) */
> /* PowerPC 32 definitions */
> #define TARGET_LONG_BITS 32
> @@ -359,10 +361,31 @@ union ppc_tlb_t {
>
> typedef struct ppc_slb_t ppc_slb_t;
> struct ppc_slb_t {
> -    uint64_t tmp64;
> -    uint32_t tmp;
> +    uint64_t esid;
> +    uint64_t vsid;
> };
>
> +/* Bits in the SLB ESID word */
> +#define SLB_ESID_ESID           0xFFFFFFFFF0000000ULL
> +#define SLB_ESID_V              0x0000000008000000ULL /* valid */
> +
> +/* Bits in the SLB VSID word */
> +#define SLB_VSID_SHIFT          12
> +#define SLB_VSID_SSIZE_SHIFT    62
> +#define SLB_VSID_B              0xc000000000000000ULL
> +#define SLB_VSID_B_256M         0x0000000000000000ULL
> +#define SLB_VSID_VSID           0x3FFFFFFFFFFFF000ULL
> +#define SLB_VSID_KS             0x0000000000000800ULL
> +#define SLB_VSID_KP             0x0000000000000400ULL
> +#define SLB_VSID_N              0x0000000000000200ULL /* no-execute  
> */
> +#define SLB_VSID_L              0x0000000000000100ULL
> +#define SLB_VSID_C              0x0000000000000080ULL /* class */
> +#define SLB_VSID_LP             0x0000000000000030ULL
> +#define SLB_VSID_ATTR           0x0000000000000FFFULL
> +
> +#define SEGMENT_SHIFT_256M      28
> +#define SEGMENT_MASK_256M       (~((1ULL << SEGMENT_SHIFT_256M) - 1))
> +
> / 
> *****************************************************************************/
> /* Machine state register bits  
> definition                                    */
> #define MSR_SF   63 /* Sixty-four-bit  
> mode                            hflags */
> @@ -755,7 +778,7 @@ void ppc_store_sdr1 (CPUPPCState *env,  
> target_ulong value);
> void ppc_store_asr (CPUPPCState *env, target_ulong value);
> target_ulong ppc_load_slb (CPUPPCState *env, int slb_nr);
> target_ulong ppc_load_sr (CPUPPCState *env, int sr_nr);
> -void ppc_store_slb (CPUPPCState *env, target_ulong rb, target_ulong  
> rs);
> +int 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 4b49101..2094ca3 100644
> --- a/target-ppc/helper.c
> +++ b/target-ppc/helper.c
> @@ -672,85 +672,36 @@ static inline int find_pte(CPUState *env,  
> mmu_ctx_t *ctx, int h, int rw,
> }
>
> #if defined(TARGET_PPC64)
> -static ppc_slb_t *slb_get_entry(CPUPPCState *env, int nr)
> -{
> -    ppc_slb_t *retval = &env->slb[nr];
> -
> -#if 0 // XXX implement bridge mode?
> -    if (env->spr[SPR_ASR] & 1) {
> -        target_phys_addr_t sr_base;
> -
> -        sr_base = env->spr[SPR_ASR] & 0xfffffffffffff000;
> -        sr_base += (12 * nr);
> -
> -        retval->tmp64 = ldq_phys(sr_base);
> -        retval->tmp = ldl_phys(sr_base + 8);
> -    }
> -#endif
> -
> -    return retval;
> -}
> -
> -static void slb_set_entry(CPUPPCState *env, int nr, ppc_slb_t *slb)
> -{
> -    ppc_slb_t *entry = &env->slb[nr];
> -
> -    if (slb == entry)
> -        return;
> -
> -    entry->tmp64 = slb->tmp64;
> -    entry->tmp = slb->tmp;
> -}
> -
> -static inline int slb_is_valid(ppc_slb_t *slb)
> -{
> -    return (int)(slb->tmp64 & 0x0000000008000000ULL);
> -}
> -
> -static inline void slb_invalidate(ppc_slb_t *slb)
> -{
> -    slb->tmp64 &= ~0x0000000008000000ULL;
> -}
> -
> static inline int slb_lookup(CPUPPCState *env, target_ulong eaddr,
>                              target_ulong *vsid, target_ulong  
> *page_mask,
>                              int *attr, int *target_page_bits)
> {
> -    target_ulong mask;
> -    int n, ret;
> +    uint64_t esid;
> +    int n;
>
> -    ret = -5;
>     LOG_SLB("%s: eaddr " TARGET_FMT_lx "\n", __func__, eaddr);
> -    mask = 0x0000000000000000ULL; /* Avoid gcc warning */
> +
> +    esid = (eaddr & SEGMENT_MASK_256M) | SLB_ESID_V;
> +
>     for (n = 0; n < env->slb_nr; n++) {
> -        ppc_slb_t *slb = slb_get_entry(env, n);
> -
> -        LOG_SLB("%s: seg %d %016" PRIx64 " %08"
> -                    PRIx32 "\n", __func__, n, slb->tmp64, slb->tmp);
> -        if (slb_is_valid(slb)) {
> -            /* SLB entry is valid */
> -            mask = 0xFFFFFFFFF0000000ULL;
> -            if (slb->tmp & 0x8) {
> -                /* 16 MB PTEs */
> -                if (target_page_bits)
> -                    *target_page_bits = 24;
> -            } else {
> -                /* 4 KB PTEs */
> -                if (target_page_bits)
> -                    *target_page_bits = TARGET_PAGE_BITS;
> -            }
> -            if ((eaddr & mask) == (slb->tmp64 & mask)) {
> -                /* SLB match */
> -                *vsid = ((slb->tmp64 << 24) | (slb->tmp >> 8)) &  
> 0x0003FFFFFFFFFFFFULL;
> -                *page_mask = ~mask;
> -                *attr = slb->tmp & 0xFF;
> -                ret = n;
> -                break;
> +        ppc_slb_t *slb = &env->slb[n];
> +
> +        LOG_SLB("%s: slot %d %016" PRIx64 " %016"
> +                    PRIx64 "\n", __func__, n, slb->esid, slb->vsid);
> +        if (slb->esid == esid) {
> +            *vsid = (slb->vsid & SLB_VSID_VSID) >> SLB_VSID_SHIFT;
> +            *page_mask = ~SEGMENT_MASK_256M;
> +            *attr = slb->vsid & SLB_VSID_ATTR;
> +            if (target_page_bits) {
> +                *target_page_bits = (slb->vsid & SLB_VSID_L)
> +                    ? TARGET_PAGE_BITS_16M
> +                    : TARGET_PAGE_BITS;
>             }
> +            return n;
>         }
>     }
>
> -    return ret;
> +    return -5;
> }
>
> void ppc_slb_invalidate_all (CPUPPCState *env)
> @@ -760,11 +711,10 @@ void ppc_slb_invalidate_all (CPUPPCState *env)
>     do_invalidate = 0;
>     /* XXX: Warning: slbia never invalidates the first segment */
>     for (n = 1; n < env->slb_nr; n++) {
> -        ppc_slb_t *slb = slb_get_entry(env, n);
> +        ppc_slb_t *slb = &env->slb[n];
>
> -        if (slb_is_valid(slb)) {
> -            slb_invalidate(slb);
> -            slb_set_entry(env, n, slb);
> +        if (slb->esid & SLB_ESID_V) {
> +            slb->esid &= ~SLB_ESID_V;
>             /* XXX: given the fact that segment size is 256 MB or 1TB,
>              *      and we still don't have a tlb_flush_mask(env, n,  
> mask)
>              *      in Qemu, we just invalidate all TLBs
> @@ -781,68 +731,44 @@ void ppc_slb_invalidate_one (CPUPPCState *env,  
> uint64_t T0)
>     target_ulong vsid, page_mask;
>     int attr;
>     int n;
> +    ppc_slb_t *slb;
>
>     n = slb_lookup(env, T0, &vsid, &page_mask, &attr, NULL);
> -    if (n >= 0) {
> -        ppc_slb_t *slb = slb_get_entry(env, n);
> -
> -        if (slb_is_valid(slb)) {
> -            slb_invalidate(slb);
> -            slb_set_entry(env, n, slb);
> -            /* XXX: given the fact that segment size is 256 MB or  
> 1TB,
> -             *      and we still don't have a tlb_flush_mask(env,  
> n, mask)
> -             *      in Qemu, we just invalidate all TLBs
> -             */
> -            tlb_flush(env, 1);
> -        }
> +    if (n < 0) {
> +        return;
>     }
> -}
>
> -target_ulong ppc_load_slb (CPUPPCState *env, int slb_nr)
> -{
> -    target_ulong rt;
> -    ppc_slb_t *slb = slb_get_entry(env, slb_nr);
> +    slb = &env->slb[n];
>
> -    if (slb_is_valid(slb)) {
> -        /* SLB entry is valid */
> -        /* Copy SLB bits 62:88 to Rt 37:63 (VSID 23:49) */
> -        rt = slb->tmp >> 8;             /* 65:88 => 40:63 */
> -        rt |= (slb->tmp64 & 0x7) << 24; /* 62:64 => 37:39 */
> -        /* Copy SLB bits 89:92 to Rt 33:36 (KsKpNL) */
> -        rt |= ((slb->tmp >> 4) & 0xF) << 27;
> -    } else {
> -        rt = 0;
> -    }
> -    LOG_SLB("%s: %016" PRIx64 " %08" PRIx32 " => %d "
> -            TARGET_FMT_lx "\n", __func__, slb->tmp64, slb->tmp,  
> slb_nr, rt);
> +    if (slb->esid & SLB_ESID_V) {
> +        slb->esid &= ~SLB_ESID_V;
>
> -    return rt;
> +        /* XXX: given the fact that segment size is 256 MB or 1TB,
> +         *      and we still don't have a tlb_flush_mask(env, n,  
> mask)
> +         *      in Qemu, we just invalidate all TLBs
> +         */
> +        tlb_flush(env, 1);
> +    }
> }
>
> -void ppc_store_slb (CPUPPCState *env, target_ulong rb, target_ulong  
> rs)
> +int ppc_store_slb (CPUPPCState *env, target_ulong rb, target_ulong  
> rs)
> {
> -    ppc_slb_t *slb;
> -
> -    uint64_t vsid;
> -    uint64_t esid;
> -    int flags, valid, slb_nr;
> -
> -    vsid = rs >> 12;
> -    flags = ((rs >> 8) & 0xf);
> +    int slot = rb & 0xfff;
> +    uint64_t esid = rb & ~0xfff;
> +    ppc_slb_t *slb = &env->slb[slot];
>
> -    esid = rb >> 28;
> -    valid = (rb & (1 << 27));
> -    slb_nr = rb & 0xfff;
> +    if (slot >= env->slb_nr) {
> +        return -1;
> +    }
>
> -    slb = slb_get_entry(env, slb_nr);
> -    slb->tmp64 = (esid << 28) | valid | (vsid >> 24);
> -    slb->tmp = (vsid << 8) | (flags << 3);
> +    slb->esid = esid;
> +    slb->vsid = rs;
>
>     LOG_SLB("%s: %d " TARGET_FMT_lx " - " TARGET_FMT_lx " => %016"  
> PRIx64
> -            " %08" PRIx32 "\n", __func__, slb_nr, rb, rs, slb->tmp64,
> -            slb->tmp);
> +            " %016" PRIx64 "\n", __func__, slot, rb, rs,
> +            slb->esid, slb->vsid);
>
> -    slb_set_entry(env, slb_nr, slb);
> +    return 0;
> }
> #endif /* defined(TARGET_PPC64) */
>
> @@ -860,24 +786,22 @@ static inline int get_segment(CPUState *env,  
> mmu_ctx_t *ctx,
> {
>     target_phys_addr_t sdr, hash, mask, sdr_mask, htab_mask;
>     target_ulong sr, vsid, vsid_mask, pgidx, page_mask;
> -#if defined(TARGET_PPC64)
> -    int attr;
> -#endif
>     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) {
> +        int attr;
> +
>         LOG_MMU("Check SLBs\n");
>         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;
> +        ctx->key = !!(pr ? (attr & SLB_VSID_KP) : (attr &  
> SLB_VSID_KS));
>         ds = 0;
> -        ctx->nx = attr & 0x10 ? 1 : 0;
> +        ctx->nx = !!(attr & SLB_VSID_N);
>         ctx->eaddr = eaddr;
>         vsid_mask = 0x00003FFFFFFFFF80ULL;
>         vsid_sh = 7;
> diff --git a/target-ppc/helper.h b/target-ppc/helper.h
> index 2bf9283..d512cb0 100644
> --- a/target-ppc/helper.h
> +++ b/target-ppc/helper.h
> @@ -340,7 +340,6 @@ DEF_HELPER_1(74xx_tlbi, void, tl)
> DEF_HELPER_FLAGS_0(tlbia, TCG_CALL_CONST, void)
> DEF_HELPER_FLAGS_1(tlbie, TCG_CALL_CONST, void, tl)
> #if defined(TARGET_PPC64)
> -DEF_HELPER_FLAGS_1(load_slb, TCG_CALL_CONST, tl, tl)
> DEF_HELPER_FLAGS_2(store_slb, TCG_CALL_CONST, void, tl, tl)
> DEF_HELPER_FLAGS_0(slbia, TCG_CALL_CONST, void)
> DEF_HELPER_FLAGS_1(slbie, TCG_CALL_CONST, void, tl)
> diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
> index 17e070a..bf41627 100644
> --- a/target-ppc/op_helper.c
> +++ b/target-ppc/op_helper.c
> @@ -3746,14 +3746,11 @@ void helper_store_sr (target_ulong sr_num,  
> target_ulong val)
>
> /* SLB management */
> #if defined(TARGET_PPC64)
> -target_ulong helper_load_slb (target_ulong slb_nr)
> -{
> -    return ppc_load_slb(env, slb_nr);
> -}
> -
> void helper_store_slb (target_ulong rb, target_ulong rs)
> {
> -    ppc_store_slb(env, rb, rs);
> +    if (ppc_store_slb(env, rb, rs) < 0) {
> +        helper_raise_exception_err(POWERPC_EXCP_PROGRAM,  
> POWERPC_EXCP_INVAL);
> +    }
> }
>
> void helper_slbia (void)
> -- 
> 1.7.1
>
>

Re: [Qemu-devel] [PATCH 01/27] Clean up PowerPC SLB handling code

[David Gibson]

On Thu, May 19, 2011 at 07:35:30AM +0200, Andreas Färber wrote:
> Am 25.03.2011 um 04:21 schrieb David Gibson:
> 
> >Currently the SLB information when emulating a PowerPC 970 is
> >storeed in a structure with the unhelpfully named fields 'tmp'
> >and 'tmp64'.  While the layout in these fields does match the
> >description of the SLB in the architecture document, it is not
> >convenient either for looking up the SLB, or for emulating the
> >slbmte instruction.
> >
> >This patch, therefore, reorganizes the SLB entry structure to be
> >divided in the the "ESID related" and "VSID related" fields as
> >they are divided in instructions accessing the SLB.
> >
> >In addition to making the code smaller and more readable, this will
> >make it easier to implement for the 1TB segments used in more
> >recent PowerPC chips.
> >
> >Signed-off-by: David Gibson <dwg@au1.ibm.com>
> 
> According to my bisect, this patch broke ppc64 OpenBIOS.
> 
> David, would you please take a look?

Uh, sure.  can you describe the symptoms of the breakage, and the
exact qemu setup you've observed the problem with?

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

Re: [Qemu-devel] [PATCH 01/27] Clean up PowerPC SLB handling code

[David Gibson]

On Thu, May 19, 2011 at 08:00:53AM +0200, Andreas Färber wrote:
> Am 19.05.2011 um 07:39 schrieb David Gibson:
> 
> >On Thu, May 19, 2011 at 07:35:30AM +0200, Andreas Färber wrote:
> >>Am 25.03.2011 um 04:21 schrieb David Gibson:
> >>
> >>>Currently the SLB information when emulating a PowerPC 970 is
> >>>storeed in a structure with the unhelpfully named fields 'tmp'
> >>>and 'tmp64'.  While the layout in these fields does match the
> >>>description of the SLB in the architecture document, it is not
> >>>convenient either for looking up the SLB, or for emulating the
> >>>slbmte instruction.
> >>>
> >>>This patch, therefore, reorganizes the SLB entry structure to be
> >>>divided in the the "ESID related" and "VSID related" fields as
> >>>they are divided in instructions accessing the SLB.
> >>>
> >>>In addition to making the code smaller and more readable, this will
> >>>make it easier to implement for the 1TB segments used in more
> >>>recent PowerPC chips.
> >>>
> >>>Signed-off-by: David Gibson <dwg@au1.ibm.com>
> >>
> >>According to my bisect, this patch broke ppc64 OpenBIOS.
> >>
> >>David, would you please take a look?
> >
> >Uh, sure.  can you describe the symptoms of the breakage, and the
> >exact qemu setup you've observed the problem with?
> 
> $ uname -a
> Darwin PMG5-3.local 9.8.0 Darwin Kernel Version 9.8.0: Wed Jul 15
> 16:57:01 PDT 2009; root:xnu-1228.15.4~1/RELEASE_PPC Power Macintosh
> # Mac OS X 10.5.8 on dual-core G5
> 
> $ gcc-4.2 --version
> powerpc-apple-darwin9-gcc-4.2.1 (GCC) 4.2.1 (Apple Inc. build 5577)
> Copyright (C) 2007 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There
> is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
> PURPOSE.
> 
> ../qemu/configure --prefix=/Users/andreas/QEMU/latest64 \
> --target-list=ppc-softmmu,ppc64-softmmu \
> --extra-cflags="-arch ppc64" --extra-ldflags="-arch ppc64" --cc=gcc-4.2
> --host-cc=gcc-4.2 \
> --enable-cocoa --disable-kvm --disable-strip --disable-docs
> --enable-io-thread $*
> # I/O thread doesn't matter
> 
> /Users/andreas/QEMU/qemu64/ppc64-softmmu/qemu-system-ppc64 -boot d -cdrom
> /Users/andreas/QEMU/AIX/dvd.1022A4_OBETA_710.iso \
> -bios /Users/andreas/QEMU/OpenBIOS/openbios/obj-ppc64/openbios-qemu.elf
> -m 1024 \
> -M mac99 -prom-env 'auto-boot?=false' -nographic
> 
> CD should be irrelevant, -boot c -hda /dev/null should work the same.

Um, yeah, I thought putting an AIX dvd into a Mac was a bit odd.

> Expected: OpenBIOS banner and > prompt
> Observed: DSI exception on mtmsrd, no serial output

Does this happen only with your openbios image, or does it also happen
with the one included in qemu?

My working theory is that openbios is using the old 32-64 bridge stuff
where segment register accesses are emulated to populate certain SLB
slots.  I have a vague recollection that I saw the code implementing
that when I was doing the cleanup and convinced myself it was not
necessary.  More information as I investigate further.

> (The regression on ppc32 AIX observed by Kenneth seems to be a
> different issue, introduced earlier.)

I don't know what that regression is; I don't read qemu-devel
exhaustively.  If you want my attention, best to CC me directly.

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

[Qemu-devel] [0/27] Implement emulation of pSeries logical partitions (v4)

[David Gibson]

This patch series adds a "pseries" machine to qemu, allowing it to
emulate IBM pSeries logical partitions.  More specifically it
implements the interface defined by the "PowerPC Architecture Platform
Requirements" document (PAPR, or sPAPR for short).

Along the way we add a bunch of support for more modern ppc CPUs than
are currently supported.  It also makes some significant cleanups to
the translation code for hash page table based ppc MMUs.

Please apply.

Changes since v3 of this series:
 * Many, many checkpatch fixups
 * Integrated feedback from qemu-devel list
 * Added in-partition SLOF firmware

Changes since v2 of this series:
 * Assorted bugfixes and cleanups.

Changes since v1 of this series:
 * numerous coding style fixups
 * incorporated most review comments from initial version
 * moved to a wholly dynamic hypercall registration scheme
 * assorted other cleanups
 * many more patches implementing VIO devices

[Qemu-devel] [PATCH 01/27] Clean up PowerPC SLB handling code

[David Gibson]

Currently the SLB information when emulating a PowerPC 970 is
storeed in a structure with the unhelpfully named fields 'tmp'
and 'tmp64'.  While the layout in these fields does match the
description of the SLB in the architecture document, it is not
convenient either for looking up the SLB, or for emulating the
slbmte instruction.

This patch, therefore, reorganizes the SLB entry structure to be
divided in the the "ESID related" and "VSID related" fields as
they are divided in instructions accessing the SLB.

In addition to making the code smaller and more readable, this will
make it easier to implement for the 1TB segments used in more
recent PowerPC chips.

Signed-off-by: David Gibson <dwg@au1.ibm.com>
---
 target-ppc/cpu.h       |   29 +++++++-
 target-ppc/helper.c    |  178 ++++++++++++++----------------------------------
 target-ppc/helper.h    |    1 -
 target-ppc/op_helper.c |    9 +--
 4 files changed, 80 insertions(+), 137 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index deb8d7c..124bbbf 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -43,6 +43,8 @@
 # define TARGET_VIRT_ADDR_SPACE_BITS 64
 #endif
 
+#define TARGET_PAGE_BITS_16M 24
+
 #else /* defined (TARGET_PPC64) */
 /* PowerPC 32 definitions */
 #define TARGET_LONG_BITS 32
@@ -359,10 +361,31 @@ union ppc_tlb_t {
 
 typedef struct ppc_slb_t ppc_slb_t;
 struct ppc_slb_t {
-    uint64_t tmp64;
-    uint32_t tmp;
+    uint64_t esid;
+    uint64_t vsid;
 };
 
+/* Bits in the SLB ESID word */
+#define SLB_ESID_ESID           0xFFFFFFFFF0000000ULL
+#define SLB_ESID_V              0x0000000008000000ULL /* valid */
+
+/* Bits in the SLB VSID word */
+#define SLB_VSID_SHIFT          12
+#define SLB_VSID_SSIZE_SHIFT    62
+#define SLB_VSID_B              0xc000000000000000ULL
+#define SLB_VSID_B_256M         0x0000000000000000ULL
+#define SLB_VSID_VSID           0x3FFFFFFFFFFFF000ULL
+#define SLB_VSID_KS             0x0000000000000800ULL
+#define SLB_VSID_KP             0x0000000000000400ULL
+#define SLB_VSID_N              0x0000000000000200ULL /* no-execute */
+#define SLB_VSID_L              0x0000000000000100ULL
+#define SLB_VSID_C              0x0000000000000080ULL /* class */
+#define SLB_VSID_LP             0x0000000000000030ULL
+#define SLB_VSID_ATTR           0x0000000000000FFFULL
+
+#define SEGMENT_SHIFT_256M      28
+#define SEGMENT_MASK_256M       (~((1ULL << SEGMENT_SHIFT_256M) - 1))
+
 /*****************************************************************************/
 /* Machine state register bits definition                                    */
 #define MSR_SF   63 /* Sixty-four-bit mode                            hflags */
@@ -755,7 +778,7 @@ void ppc_store_sdr1 (CPUPPCState *env, target_ulong value);
 void ppc_store_asr (CPUPPCState *env, target_ulong value);
 target_ulong ppc_load_slb (CPUPPCState *env, int slb_nr);
 target_ulong ppc_load_sr (CPUPPCState *env, int sr_nr);
-void ppc_store_slb (CPUPPCState *env, target_ulong rb, target_ulong rs);
+int 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 4b49101..2094ca3 100644
--- a/target-ppc/helper.c
+++ b/target-ppc/helper.c
@@ -672,85 +672,36 @@ static inline int find_pte(CPUState *env, mmu_ctx_t *ctx, int h, int rw,
 }
 
 #if defined(TARGET_PPC64)
-static ppc_slb_t *slb_get_entry(CPUPPCState *env, int nr)
-{
-    ppc_slb_t *retval = &env->slb[nr];
-
-#if 0 // XXX implement bridge mode?
-    if (env->spr[SPR_ASR] & 1) {
-        target_phys_addr_t sr_base;
-
-        sr_base = env->spr[SPR_ASR] & 0xfffffffffffff000;
-        sr_base += (12 * nr);
-
-        retval->tmp64 = ldq_phys(sr_base);
-        retval->tmp = ldl_phys(sr_base + 8);
-    }
-#endif
-
-    return retval;
-}
-
-static void slb_set_entry(CPUPPCState *env, int nr, ppc_slb_t *slb)
-{
-    ppc_slb_t *entry = &env->slb[nr];
-
-    if (slb == entry)
-        return;
-
-    entry->tmp64 = slb->tmp64;
-    entry->tmp = slb->tmp;
-}
-
-static inline int slb_is_valid(ppc_slb_t *slb)
-{
-    return (int)(slb->tmp64 & 0x0000000008000000ULL);
-}
-
-static inline void slb_invalidate(ppc_slb_t *slb)
-{
-    slb->tmp64 &= ~0x0000000008000000ULL;
-}
-
 static inline int slb_lookup(CPUPPCState *env, target_ulong eaddr,
                              target_ulong *vsid, target_ulong *page_mask,
                              int *attr, int *target_page_bits)
 {
-    target_ulong mask;
-    int n, ret;
+    uint64_t esid;
+    int n;
 
-    ret = -5;
     LOG_SLB("%s: eaddr " TARGET_FMT_lx "\n", __func__, eaddr);
-    mask = 0x0000000000000000ULL; /* Avoid gcc warning */
+
+    esid = (eaddr & SEGMENT_MASK_256M) | SLB_ESID_V;
+
     for (n = 0; n < env->slb_nr; n++) {
-        ppc_slb_t *slb = slb_get_entry(env, n);
-
-        LOG_SLB("%s: seg %d %016" PRIx64 " %08"
-                    PRIx32 "\n", __func__, n, slb->tmp64, slb->tmp);
-        if (slb_is_valid(slb)) {
-            /* SLB entry is valid */
-            mask = 0xFFFFFFFFF0000000ULL;
-            if (slb->tmp & 0x8) {
-                /* 16 MB PTEs */
-                if (target_page_bits)
-                    *target_page_bits = 24;
-            } else {
-                /* 4 KB PTEs */
-                if (target_page_bits)
-                    *target_page_bits = TARGET_PAGE_BITS;
-            }
-            if ((eaddr & mask) == (slb->tmp64 & mask)) {
-                /* SLB match */
-                *vsid = ((slb->tmp64 << 24) | (slb->tmp >> 8)) & 0x0003FFFFFFFFFFFFULL;
-                *page_mask = ~mask;
-                *attr = slb->tmp & 0xFF;
-                ret = n;
-                break;
+        ppc_slb_t *slb = &env->slb[n];
+
+        LOG_SLB("%s: slot %d %016" PRIx64 " %016"
+                    PRIx64 "\n", __func__, n, slb->esid, slb->vsid);
+        if (slb->esid == esid) {
+            *vsid = (slb->vsid & SLB_VSID_VSID) >> SLB_VSID_SHIFT;
+            *page_mask = ~SEGMENT_MASK_256M;
+            *attr = slb->vsid & SLB_VSID_ATTR;
+            if (target_page_bits) {
+                *target_page_bits = (slb->vsid & SLB_VSID_L)
+                    ? TARGET_PAGE_BITS_16M
+                    : TARGET_PAGE_BITS;
             }
+            return n;
         }
     }
 
-    return ret;
+    return -5;
 }
 
 void ppc_slb_invalidate_all (CPUPPCState *env)
@@ -760,11 +711,10 @@ void ppc_slb_invalidate_all (CPUPPCState *env)
     do_invalidate = 0;
     /* XXX: Warning: slbia never invalidates the first segment */
     for (n = 1; n < env->slb_nr; n++) {
-        ppc_slb_t *slb = slb_get_entry(env, n);
+        ppc_slb_t *slb = &env->slb[n];
 
-        if (slb_is_valid(slb)) {
-            slb_invalidate(slb);
-            slb_set_entry(env, n, slb);
+        if (slb->esid & SLB_ESID_V) {
+            slb->esid &= ~SLB_ESID_V;
             /* XXX: given the fact that segment size is 256 MB or 1TB,
              *      and we still don't have a tlb_flush_mask(env, n, mask)
              *      in Qemu, we just invalidate all TLBs
@@ -781,68 +731,44 @@ void ppc_slb_invalidate_one (CPUPPCState *env, uint64_t T0)
     target_ulong vsid, page_mask;
     int attr;
     int n;
+    ppc_slb_t *slb;
 
     n = slb_lookup(env, T0, &vsid, &page_mask, &attr, NULL);
-    if (n >= 0) {
-        ppc_slb_t *slb = slb_get_entry(env, n);
-
-        if (slb_is_valid(slb)) {
-            slb_invalidate(slb);
-            slb_set_entry(env, n, slb);
-            /* XXX: given the fact that segment size is 256 MB or 1TB,
-             *      and we still don't have a tlb_flush_mask(env, n, mask)
-             *      in Qemu, we just invalidate all TLBs
-             */
-            tlb_flush(env, 1);
-        }
+    if (n < 0) {
+        return;
     }
-}
 
-target_ulong ppc_load_slb (CPUPPCState *env, int slb_nr)
-{
-    target_ulong rt;
-    ppc_slb_t *slb = slb_get_entry(env, slb_nr);
+    slb = &env->slb[n];
 
-    if (slb_is_valid(slb)) {
-        /* SLB entry is valid */
-        /* Copy SLB bits 62:88 to Rt 37:63 (VSID 23:49) */
-        rt = slb->tmp >> 8;             /* 65:88 => 40:63 */
-        rt |= (slb->tmp64 & 0x7) << 24; /* 62:64 => 37:39 */
-        /* Copy SLB bits 89:92 to Rt 33:36 (KsKpNL) */
-        rt |= ((slb->tmp >> 4) & 0xF) << 27;
-    } else {
-        rt = 0;
-    }
-    LOG_SLB("%s: %016" PRIx64 " %08" PRIx32 " => %d "
-            TARGET_FMT_lx "\n", __func__, slb->tmp64, slb->tmp, slb_nr, rt);
+    if (slb->esid & SLB_ESID_V) {
+        slb->esid &= ~SLB_ESID_V;
 
-    return rt;
+        /* XXX: given the fact that segment size is 256 MB or 1TB,
+         *      and we still don't have a tlb_flush_mask(env, n, mask)
+         *      in Qemu, we just invalidate all TLBs
+         */
+        tlb_flush(env, 1);
+    }
 }
 
-void ppc_store_slb (CPUPPCState *env, target_ulong rb, target_ulong rs)
+int ppc_store_slb (CPUPPCState *env, target_ulong rb, target_ulong rs)
 {
-    ppc_slb_t *slb;
-
-    uint64_t vsid;
-    uint64_t esid;
-    int flags, valid, slb_nr;
-
-    vsid = rs >> 12;
-    flags = ((rs >> 8) & 0xf);
+    int slot = rb & 0xfff;
+    uint64_t esid = rb & ~0xfff;
+    ppc_slb_t *slb = &env->slb[slot];
 
-    esid = rb >> 28;
-    valid = (rb & (1 << 27));
-    slb_nr = rb & 0xfff;
+    if (slot >= env->slb_nr) {
+        return -1;
+    }
 
-    slb = slb_get_entry(env, slb_nr);
-    slb->tmp64 = (esid << 28) | valid | (vsid >> 24);
-    slb->tmp = (vsid << 8) | (flags << 3);
+    slb->esid = esid;
+    slb->vsid = rs;
 
     LOG_SLB("%s: %d " TARGET_FMT_lx " - " TARGET_FMT_lx " => %016" PRIx64
-            " %08" PRIx32 "\n", __func__, slb_nr, rb, rs, slb->tmp64,
-            slb->tmp);
+            " %016" PRIx64 "\n", __func__, slot, rb, rs,
+            slb->esid, slb->vsid);
 
-    slb_set_entry(env, slb_nr, slb);
+    return 0;
 }
 #endif /* defined(TARGET_PPC64) */
 
@@ -860,24 +786,22 @@ static inline int get_segment(CPUState *env, mmu_ctx_t *ctx,
 {
     target_phys_addr_t sdr, hash, mask, sdr_mask, htab_mask;
     target_ulong sr, vsid, vsid_mask, pgidx, page_mask;
-#if defined(TARGET_PPC64)
-    int attr;
-#endif
     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) {
+        int attr;
+
         LOG_MMU("Check SLBs\n");
         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;
+        ctx->key = !!(pr ? (attr & SLB_VSID_KP) : (attr & SLB_VSID_KS));
         ds = 0;
-        ctx->nx = attr & 0x10 ? 1 : 0;
+        ctx->nx = !!(attr & SLB_VSID_N);
         ctx->eaddr = eaddr;
         vsid_mask = 0x00003FFFFFFFFF80ULL;
         vsid_sh = 7;
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 2bf9283..d512cb0 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -340,7 +340,6 @@ DEF_HELPER_1(74xx_tlbi, void, tl)
 DEF_HELPER_FLAGS_0(tlbia, TCG_CALL_CONST, void)
 DEF_HELPER_FLAGS_1(tlbie, TCG_CALL_CONST, void, tl)
 #if defined(TARGET_PPC64)
-DEF_HELPER_FLAGS_1(load_slb, TCG_CALL_CONST, tl, tl)
 DEF_HELPER_FLAGS_2(store_slb, TCG_CALL_CONST, void, tl, tl)
 DEF_HELPER_FLAGS_0(slbia, TCG_CALL_CONST, void)
 DEF_HELPER_FLAGS_1(slbie, TCG_CALL_CONST, void, tl)
diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
index 17e070a..bf41627 100644
--- a/target-ppc/op_helper.c
+++ b/target-ppc/op_helper.c
@@ -3746,14 +3746,11 @@ void helper_store_sr (target_ulong sr_num, target_ulong val)
 
 /* SLB management */
 #if defined(TARGET_PPC64)
-target_ulong helper_load_slb (target_ulong slb_nr)
-{
-    return ppc_load_slb(env, slb_nr);
-}
-
 void helper_store_slb (target_ulong rb, target_ulong rs)
 {
-    ppc_store_slb(env, rb, rs);
+    if (ppc_store_slb(env, rb, rs) < 0) {
+        helper_raise_exception_err(POWERPC_EXCP_PROGRAM, POWERPC_EXCP_INVAL);
+    }
 }
 
 void helper_slbia (void)
-- 
1.7.1