Patchwork [01/27] Clean up PowerPC SLB handling code

login
register
mail settings
Submitter David Gibson
Date March 25, 2011, 3:21 a.m.
Message ID <1301023292-24977-2-git-send-email-david@gibson.dropbear.id.au>
Download mbox | patch
Permalink /patch/88302/
State New
Headers show

Comments

David Gibson - March 25, 2011, 3:21 a.m.
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(-)
Andreas Färber - May 19, 2011, 5:35 a.m.
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
>
>
David Gibson - May 19, 2011, 5:39 a.m.
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 - May 19, 2011, 6:45 a.m.
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.

Patch

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)