[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 5/6] target/ppc: Eliminate htab_base and htab_ma
From: |
David Gibson |
Subject: |
Re: [Qemu-devel] [PATCH 5/6] target/ppc: Eliminate htab_base and htab_mask variables |
Date: |
Fri, 24 Feb 2017 16:34:44 +1100 |
User-agent: |
Mutt/1.7.1 (2016-10-04) |
On Thu, Feb 23, 2017 at 04:52:59PM +1100, Suraj Jitindar Singh wrote:
> On Thu, 2017-02-23 at 13:09 +1100, David Gibson wrote:
> > CPUPPCState includes fields htab_base and htab_mask which store the
> > base
> > address (GPA) and size (as a mask) of the guest's hashed page table
> > (HPT).
> > These are set when the SDR1 register is updated.
> >
> > Keeping these in sync with the SDR1 is actually a little bit fiddly,
> > and
> > probably not useful for performance, since keeping them expands the
> > size of
> > CPUPPCState. It also makes some upcoming changes harder to
> > implement.
> >
> > This patch removes these fields, in favour of calculating them
> > directly
> > from the SDR1 contents when necessary.
> >
> > This does make a change to the behaviour of attempting to write a bad
> > value
> > (invalid HPT size) to the SDR1 with an mtspr
> > instruction. Previously, the
> > bad value would be stored in SDR1 and could be retrieved with a later
> > mfspr, but the HPT size as used by the softmmu would be, clamped to
> > the
> > allowed values. Now, writing a bad value is treated as a no-op. An
> > error
> > message is printed in both new and old versions.
> >
> > I'm not sure which behaviour, if either, matches real hardware. I
> > don't
> > think it matters that much, since it's pretty clear that if an OS
> > writes
> > a bad value to SDR1, it's not going to boot.
> >
> > Signed-off-by: David Gibson <address@hidden>
>
> Comments Below.
>
> > ---
> > hw/ppc/spapr_hcall.c | 4 ++--
> > target/ppc/cpu.h | 8 --------
> > target/ppc/machine.c | 1 -
> > target/ppc/mmu-hash32.c | 14 +++++++-------
> > target/ppc/mmu-hash32.h | 24 ++++++++++++++++++------
> > target/ppc/mmu-hash64.c | 37 ++++++++++++++++---------------------
> > target/ppc/mmu-hash64.h | 13 +++++++++++++
> > target/ppc/mmu_helper.c | 31 ++++++++++++++++---------------
> > 8 files changed, 72 insertions(+), 60 deletions(-)
> >
> > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> > index fd961b5..85d96f6 100644
> > --- a/hw/ppc/spapr_hcall.c
> > +++ b/hw/ppc/spapr_hcall.c
> > @@ -50,9 +50,9 @@ static bool has_spr(PowerPCCPU *cpu, int spr)
> > static inline bool valid_ptex(PowerPCCPU *cpu, target_ulong ptex)
> > {
> > /*
> > - * hash value/pteg group index is normalized by htab_mask
> > + * hash value/pteg group index is normalized by HPT mask
> > */
> > - if (((ptex & ~7ULL) / HPTES_PER_GROUP) & ~cpu->env.htab_mask) {
> > + if (((ptex & ~7ULL) / HPTES_PER_GROUP) &
> > ~ppc_hash64_hpt_mask(cpu)) {
> > return false;
> > }
> > return true;
> > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> > index c89973e..c6cd9ab 100644
> > --- a/target/ppc/cpu.h
> > +++ b/target/ppc/cpu.h
> > @@ -309,11 +309,6 @@ union ppc_tlb_t {
> > #define SDR_32_HTABORG 0xFFFF0000UL
> > #define SDR_32_HTABMASK 0x000001FFUL
> >
> > -#if defined(TARGET_PPC64)
> > -#define SDR_64_HTABORG 0xFFFFFFFFFFFC0000ULL
> > -#define SDR_64_HTABSIZE 0x000000000000001FULL
> > -#endif /* defined(TARGET_PPC64 */
> > -
> > typedef struct ppc_slb_t ppc_slb_t;
> > struct ppc_slb_t {
> > uint64_t esid;
> > @@ -1006,9 +1001,6 @@ struct CPUPPCState {
> > /* tcg TLB needs flush (deferred slb inval instruction
> > typically) */
> > #endif
> > /* segment registers */
> > - hwaddr htab_base;
> > - /* mask used to normalize hash value to PTEG index */
> > - hwaddr htab_mask;
> > target_ulong sr[32];
> > /* externally stored hash table */
> > uint8_t *external_htab;
> > diff --git a/target/ppc/machine.c b/target/ppc/machine.c
> > index df9f7a4..1ccbc8a 100644
> > --- a/target/ppc/machine.c
> > +++ b/target/ppc/machine.c
> > @@ -229,7 +229,6 @@ static int cpu_post_load(void *opaque, int
> > version_id)
> > }
> >
> > if (!env->external_htab) {
> > - /* Restore htab_base and htab_mask variables */
> > ppc_store_sdr1(env, env->spr[SPR_SDR1]);
>
> Do we still need to do this? As far as I can tell it pretty much does:
> env->spr[SPR_SDR1] = env->spr[SPR_SDR1]
> and checks the htab_size, which would have been done when we set SDR1
> in the first place anyway?
>
> > }
> >
> > diff --git a/target/ppc/mmu-hash32.c b/target/ppc/mmu-hash32.c
> > index 29bace6..03ae3c1 100644
> > --- a/target/ppc/mmu-hash32.c
> > +++ b/target/ppc/mmu-hash32.c
> > @@ -304,9 +304,9 @@ static int ppc_hash32_direct_store(PowerPCCPU
> > *cpu, target_ulong sr,
> >
> > hwaddr get_pteg_offset32(PowerPCCPU *cpu, hwaddr hash)
> > {
> > - CPUPPCState *env = &cpu->env;
> > + target_ulong mask = ppc_hash32_hpt_mask(cpu);
> >
> > - return (hash * HASH_PTEG_SIZE_32) & env->htab_mask;
> > + return (hash * HASH_PTEG_SIZE_32) & mask;
> > }
> >
> > static hwaddr ppc_hash32_pteg_search(PowerPCCPU *cpu, hwaddr
> > pteg_off,
> > @@ -339,7 +339,6 @@ static hwaddr ppc_hash32_htab_lookup(PowerPCCPU
> > *cpu,
> > target_ulong sr, target_ulong
> > eaddr,
> > ppc_hash_pte32_t *pte)
> > {
> > - CPUPPCState *env = &cpu->env;
> > hwaddr pteg_off, pte_offset;
> > hwaddr hash;
> > uint32_t vsid, pgidx, ptem;
> > @@ -353,21 +352,22 @@ static hwaddr ppc_hash32_htab_lookup(PowerPCCPU
> > *cpu,
> > qemu_log_mask(CPU_LOG_MMU, "htab_base " TARGET_FMT_plx
> > " htab_mask " TARGET_FMT_plx
> > " hash " TARGET_FMT_plx "\n",
> > - env->htab_base, env->htab_mask, hash);
> > + ppc_hash32_hpt_base(cpu), ppc_hash32_hpt_mask(cpu),
> > hash);
> >
> > /* Primary PTEG lookup */
> > qemu_log_mask(CPU_LOG_MMU, "0 htab=" TARGET_FMT_plx "/"
> > TARGET_FMT_plx
> > " vsid=%" PRIx32 " ptem=%" PRIx32
> > " hash=" TARGET_FMT_plx "\n",
> > - env->htab_base, env->htab_mask, vsid, ptem, hash);
> > + ppc_hash32_hpt_base(cpu), ppc_hash32_hpt_mask(cpu),
> > + vsid, ptem, hash);
> > pteg_off = get_pteg_offset32(cpu, hash);
> > pte_offset = ppc_hash32_pteg_search(cpu, pteg_off, 0, ptem,
> > pte);
> > if (pte_offset == -1) {
> > /* Secondary PTEG lookup */
> > qemu_log_mask(CPU_LOG_MMU, "1 htab=" TARGET_FMT_plx "/"
> > TARGET_FMT_plx
> > " vsid=%" PRIx32 " api=%" PRIx32
> > - " hash=" TARGET_FMT_plx "\n", env->htab_base,
> > - env->htab_mask, vsid, ptem, ~hash);
> > + " hash=" TARGET_FMT_plx "\n",
> > ppc_hash32_hpt_base(cpu),
> > + ppc_hash32_hpt_mask(cpu), vsid, ptem, ~hash);
> > pteg_off = get_pteg_offset32(cpu, ~hash);
> > pte_offset = ppc_hash32_pteg_search(cpu, pteg_off, 1, ptem,
> > pte);
> > }
> > diff --git a/target/ppc/mmu-hash32.h b/target/ppc/mmu-hash32.h
> > index 5b9fb08..054be65 100644
> > --- a/target/ppc/mmu-hash32.h
> > +++ b/target/ppc/mmu-hash32.h
> > @@ -65,42 +65,54 @@ int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu,
> > vaddr address, int rw,
> > #define HPTE32_R_WIMG 0x00000078
> > #define HPTE32_R_PP 0x00000003
> >
> > +static inline hwaddr ppc_hash32_hpt_base(PowerPCCPU *cpu)
> > +{
> > + return cpu->env.spr[SPR_SDR1] & SDR_32_HTABORG;
> > +}
> > +
> > +static inline hwaddr ppc_hash32_hpt_mask(PowerPCCPU *cpu)
> > +{
> > + return ((cpu->env.spr[SPR_SDR1] & SDR_32_HTABMASK) << 16) |
> > 0xFFFF;
> > +}
> > +
> > static inline target_ulong ppc_hash32_load_hpte0(PowerPCCPU *cpu,
> > hwaddr pte_offset)
> > {
> > CPUPPCState *env = &cpu->env;
> > + target_ulong base = ppc_hash32_hpt_base(cpu);
> >
> > assert(!env->external_htab); /* Not supported on 32-bit for now
> > */
> > - return ldl_phys(CPU(cpu)->as, env->htab_base + pte_offset);
> > + return ldl_phys(CPU(cpu)->as, base + pte_offset);
> > }
> >
> > static inline target_ulong ppc_hash32_load_hpte1(PowerPCCPU *cpu,
> > hwaddr pte_offset)
> > {
> > + target_ulong base = ppc_hash32_hpt_base(cpu);
> > CPUPPCState *env = &cpu->env;
> >
> > assert(!env->external_htab); /* Not supported on 32-bit for now
> > */
> > - return ldl_phys(CPU(cpu)->as,
> > - env->htab_base + pte_offset + HASH_PTE_SIZE_32 /
> > 2);
> > + return ldl_phys(CPU(cpu)->as, base + pte_offset +
> > HASH_PTE_SIZE_32 / 2);
> > }
> >
> > static inline void ppc_hash32_store_hpte0(PowerPCCPU *cpu,
> > hwaddr pte_offset,
> > target_ulong pte0)
> > {
> > CPUPPCState *env = &cpu->env;
> > + target_ulong base = ppc_hash32_hpt_base(cpu);
> >
> > assert(!env->external_htab); /* Not supported on 32-bit for now
> > */
> > - stl_phys(CPU(cpu)->as, env->htab_base + pte_offset, pte0);
> > + stl_phys(CPU(cpu)->as, base + pte_offset, pte0);
> > }
> >
> > static inline void ppc_hash32_store_hpte1(PowerPCCPU *cpu,
> > hwaddr pte_offset,
> > target_ulong pte1)
> > {
> > CPUPPCState *env = &cpu->env;
> > + target_ulong base = ppc_hash32_hpt_base(cpu);
> >
> > assert(!env->external_htab); /* Not supported on 32-bit for now
> > */
> > - stl_phys(CPU(cpu)->as,
> > - env->htab_base + pte_offset + HASH_PTE_SIZE_32 / 2,
> > pte1);
> > + stl_phys(CPU(cpu)->as, base + pte_offset + HASH_PTE_SIZE_32 / 2,
> > pte1);
> > }
> >
> > typedef struct {
> > diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> > index c59db47..bb87777 100644
> > --- a/target/ppc/mmu-hash64.c
> > +++ b/target/ppc/mmu-hash64.c
> > @@ -304,15 +304,13 @@ void ppc_hash64_set_sdr1(PowerPCCPU *cpu,
> > target_ulong value,
> > CPUPPCState *env = &cpu->env;
> > target_ulong htabsize = value & SDR_64_HTABSIZE;
> >
> > - env->spr[SPR_SDR1] = value;
> > if (htabsize > 28) {
> > error_setg(errp,
> > "Invalid HTABSIZE 0x" TARGET_FMT_lx" stored in
> > SDR1",
> > htabsize);
> > - htabsize = 28;
> > + return;
> > }
> > - env->htab_mask = (1ULL << (htabsize + 18 - 7)) - 1;
> > - env->htab_base = value & SDR_64_HTABORG;
> > + env->spr[SPR_SDR1] = value;
> > }
> >
> > void ppc_hash64_set_external_hpt(PowerPCCPU *cpu, void *hpt, int
> > shift,
> > @@ -333,10 +331,6 @@ void ppc_hash64_set_external_hpt(PowerPCCPU
> > *cpu, void *hpt, int shift,
> > return;
> > }
> >
> > - /* Not strictly necessary, but makes it clearer that an external
> > - * htab is in use when debugging */
> > - env->htab_base = -1;
> > -
> > if (kvm_enabled()) {
> > if (kvmppc_put_books_sregs(cpu) < 0) {
> > error_setg(errp, "Unable to update SDR1 in KVM");
> > @@ -448,11 +442,12 @@ const ppc_hash_pte64_t
> > *ppc_hash64_map_hptes(PowerPCCPU *cpu,
> > * HTAB is controlled by QEMU. Just point to the internally
> > * accessible PTEG.
> > */
> > - hptes = (ppc_hash_pte64_t *)(cpu->env.external_htab +
> > pte_offset);
> > - } else if (cpu->env.htab_base) {
> > + hptes = (const ppc_hash_pte64_t *)(cpu->env.external_htab +
> > pte_offset);
> ^^^ Does this particular line belong in the previous patch?
Yes, yes it does. Nice catch.
> > + } else if (ppc_hash64_hpt_base(cpu)) {
> > + hwaddr base = ppc_hash64_hpt_base(cpu);
> > hwaddr plen = n * HASH_PTE_SIZE_64;
> > - hptes = address_space_map(CPU(cpu)->as, cpu->env.htab_base +
> > pte_offset,
> > - &plen, false);
> > + hptes = address_space_map(CPU(cpu)->as, base + pte_offset,
> > + &plen, false);
> > if (plen < (n * HASH_PTE_SIZE_64)) {
> > hw_error("%s: Unable to map all requested HPTEs\n",
> > __FUNCTION__);
> > }
> > @@ -513,13 +508,12 @@ static hwaddr ppc_hash64_pteg_search(PowerPCCPU
> > *cpu, hwaddr hash,
> > target_ulong ptem,
> > ppc_hash_pte64_t *pte, unsigned
> > *pshift)
> > {
> > - CPUPPCState *env = &cpu->env;
> > int i;
> > const ppc_hash_pte64_t *pteg;
> > target_ulong pte0, pte1;
> > target_ulong ptex;
> >
> > - ptex = (hash & env->htab_mask) * HPTES_PER_GROUP;
> > + ptex = (hash & ppc_hash64_hpt_mask(cpu)) * HPTES_PER_GROUP;
> > pteg = ppc_hash64_map_hptes(cpu, ptex, HPTES_PER_GROUP);
> > if (!pteg) {
> > return -1;
> > @@ -597,14 +591,15 @@ static hwaddr ppc_hash64_htab_lookup(PowerPCCPU
> > *cpu,
> > qemu_log_mask(CPU_LOG_MMU,
> > "htab_base " TARGET_FMT_plx " htab_mask " TARGET_FMT_plx
> > " hash " TARGET_FMT_plx "\n",
> > - env->htab_base, env->htab_mask, hash);
> > + ppc_hash64_hpt_base(cpu), ppc_hash64_hpt_mask(cpu),
> > hash);
> >
> > /* Primary PTEG lookup */
> > qemu_log_mask(CPU_LOG_MMU,
> > "0 htab=" TARGET_FMT_plx "/" TARGET_FMT_plx
> > " vsid=" TARGET_FMT_lx " ptem=" TARGET_FMT_lx
> > " hash=" TARGET_FMT_plx "\n",
> > - env->htab_base, env->htab_mask, vsid, ptem, hash);
> > + ppc_hash64_hpt_base(cpu), ppc_hash64_hpt_mask(cpu),
> > + vsid, ptem, hash);
> > ptex = ppc_hash64_pteg_search(cpu, hash, sps, ptem, pte,
> > pshift);
> >
> > if (ptex == -1) {
> > @@ -613,8 +608,8 @@ static hwaddr ppc_hash64_htab_lookup(PowerPCCPU
> > *cpu,
> > qemu_log_mask(CPU_LOG_MMU,
> > "1 htab=" TARGET_FMT_plx "/" TARGET_FMT_plx
> > " vsid=" TARGET_FMT_lx " api=" TARGET_FMT_lx
> > - " hash=" TARGET_FMT_plx "\n", env->htab_base,
> > - env->htab_mask, vsid, ptem, ~hash);
> > + " hash=" TARGET_FMT_plx "\n",
> > ppc_hash64_hpt_base(cpu),
> > + ppc_hash64_hpt_mask(cpu), vsid, ptem, ~hash);
> >
> > ptex = ppc_hash64_pteg_search(cpu, ~hash, sps, ptem, pte,
> > pshift);
> > }
> > @@ -932,9 +927,9 @@ void ppc_hash64_store_hpte(PowerPCCPU *cpu,
> > hwaddr ptex,
> > stq_p(env->external_htab + offset, pte0);
> > stq_p(env->external_htab + offset + HASH_PTE_SIZE_64 / 2,
> > pte1);
> > } else {
> > - stq_phys(CPU(cpu)->as, env->htab_base + offset, pte0);
> > - stq_phys(CPU(cpu)->as,
> > - env->htab_base + offset + HASH_PTE_SIZE_64 / 2,
> > pte1);
> > + hwaddr base = ppc_hash64_hpt_base(cpu);
> > + stq_phys(CPU(cpu)->as, base + offset, pte0);
> > + stq_phys(CPU(cpu)->as, base + offset + HASH_PTE_SIZE_64 / 2,
> > pte1);
> > }
> > }
> >
> > diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
> > index 8637fe4..dc0bc99 100644
> > --- a/target/ppc/mmu-hash64.h
> > +++ b/target/ppc/mmu-hash64.h
> > @@ -56,6 +56,9 @@ void ppc_hash64_update_rmls(CPUPPCState *env);
> > * Hash page table definitions
> > */
> >
> > +#define SDR_64_HTABORG 0xFFFFFFFFFFFC0000ULL
>
> I think this was just always wrong, but it should be
> 0x0FFFFFFFFFFC0000ULL
> ^
>
> > +#define SDR_64_HTABSIZE 0x000000000000001FULL
> > +
> > #define HPTES_PER_GROUP 8
> > #define HASH_PTE_SIZE_64 16
> > #define HASH_PTEG_SIZE_64 (HASH_PTE_SIZE_64 * HPTES_PER_GROUP)
> > @@ -91,6 +94,16 @@ void ppc_hash64_update_rmls(CPUPPCState *env);
> > #define HPTE64_V_1TB_SEG 0x4000000000000000ULL
> > #define HPTE64_V_VRMA_MASK 0x4001ffffff000000ULL
> >
> > +static inline hwaddr ppc_hash64_hpt_base(PowerPCCPU *cpu)
> > +{
> > + return cpu->env.spr[SPR_SDR1] & SDR_64_HTABORG;
> > +}
> > +
> > +static inline hwaddr ppc_hash64_hpt_mask(PowerPCCPU *cpu)
> > +{
> > + return (1ULL << ((cpu->env.spr[SPR_SDR1] & SDR_64_HTABSIZE) + 18
> > - 7)) - 1;
> > +}
> > +
> > void ppc_hash64_set_sdr1(PowerPCCPU *cpu, target_ulong value,
> > Error **errp);
> > void ppc_hash64_set_external_hpt(PowerPCCPU *cpu, void *hpt, int
> > shift,
> > diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
> > index eb2d482..1381635 100644
> > --- a/target/ppc/mmu_helper.c
> > +++ b/target/ppc/mmu_helper.c
> > @@ -466,6 +466,7 @@ static int get_bat_6xx_tlb(CPUPPCState *env,
> > mmu_ctx_t *ctx,
> > static inline int get_segment_6xx_tlb(CPUPPCState *env, mmu_ctx_t
> > *ctx,
> > target_ulong eaddr, int rw,
> > int type)
> > {
> > + PowerPCCPU *cpu = ppc_env_get_cpu(env);
> > hwaddr hash;
> > target_ulong vsid;
> > int ds, pr, target_page_bits;
> > @@ -503,7 +504,7 @@ static inline int get_segment_6xx_tlb(CPUPPCState
> > *env, mmu_ctx_t *ctx,
> > qemu_log_mask(CPU_LOG_MMU, "htab_base " TARGET_FMT_plx
> > " htab_mask " TARGET_FMT_plx
> > " hash " TARGET_FMT_plx "\n",
> > - env->htab_base, env->htab_mask, hash);
> > + ppc_hash32_hpt_base(cpu),
> > ppc_hash32_hpt_mask(cpu), hash);
> > ctx->hash[0] = hash;
> > ctx->hash[1] = ~hash;
> >
> > @@ -518,9 +519,11 @@ static inline int
> > get_segment_6xx_tlb(CPUPPCState *env, mmu_ctx_t *ctx,
> > uint32_t a0, a1, a2, a3;
> >
> > qemu_log("Page table: " TARGET_FMT_plx " len "
> > TARGET_FMT_plx
> > - "\n", env->htab_base, env->htab_mask +
> > 0x80);
> > - for (curaddr = env->htab_base;
> > - curaddr < (env->htab_base + env->htab_mask +
> > 0x80);
> > + "\n", ppc_hash32_hpt_base(cpu),
> > + ppc_hash32_hpt_mask(env) + 0x80);
> > + for (curaddr = ppc_hash32_hpt_base(cpu);
> > + curaddr < (ppc_hash32_hpt_base(cpu)
> > + + ppc_hash32_hpt_mask(cpu) + 0x80);
> > curaddr += 16) {
> > a0 = ldl_phys(cs->as, curaddr);
> > a1 = ldl_phys(cs->as, curaddr + 4);
> > @@ -1205,12 +1208,13 @@ static void mmu6xx_dump_BATs(FILE *f,
> > fprintf_function cpu_fprintf,
> > static void mmu6xx_dump_mmu(FILE *f, fprintf_function cpu_fprintf,
> > CPUPPCState *env)
> > {
> > + PowerPCCPU *cpu = ppc_env_get_cpu(env);
> > ppc6xx_tlb_t *tlb;
> > target_ulong sr;
> > int type, way, entry, i;
> >
> > - cpu_fprintf(f, "HTAB base = 0x%"HWADDR_PRIx"\n", env-
> > >htab_base);
> > - cpu_fprintf(f, "HTAB mask = 0x%"HWADDR_PRIx"\n", env-
> > >htab_mask);
> > + cpu_fprintf(f, "HTAB base = 0x%"HWADDR_PRIx"\n",
> > ppc_hash32_hpt_base(cpu));
> > + cpu_fprintf(f, "HTAB mask = 0x%"HWADDR_PRIx"\n",
> > ppc_hash32_hpt_mask(cpu));
> >
> > cpu_fprintf(f, "\nSegment registers:\n");
> > for (i = 0; i < 32; i++) {
> > @@ -1592,9 +1596,9 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState
> > *env, target_ulong address,
> > env->spr[SPR_DCMP] = 0x80000000 | ctx.ptem;
> > tlb_miss:
> > env->error_code |= ctx.key << 19;
> > - env->spr[SPR_HASH1] = env->htab_base +
> > + env->spr[SPR_HASH1] = ppc_hash32_hpt_base(cpu) +
> > get_pteg_offset32(cpu, ctx.hash[0]);
> > - env->spr[SPR_HASH2] = env->htab_base +
> > + env->spr[SPR_HASH2] = ppc_hash32_hpt_base(cpu) +
> > get_pteg_offset32(cpu, ctx.hash[1]);
> > break;
> > case POWERPC_MMU_SOFT_74xx:
> > @@ -1999,7 +2003,6 @@ void ppc_store_sdr1(CPUPPCState *env,
> > target_ulong value)
> > {
> > qemu_log_mask(CPU_LOG_MMU, "%s: " TARGET_FMT_lx "\n", __func__,
> > value);
> > assert(!env->external_htab);
> > - env->spr[SPR_SDR1] = value;
> > #if defined(TARGET_PPC64)
> > if (env->mmu_model & POWERPC_MMU_64) {
> > PowerPCCPU *cpu = ppc_env_get_cpu(env);
> > @@ -2009,14 +2012,12 @@ void ppc_store_sdr1(CPUPPCState *env,
> > target_ulong value)
> > if (local_err) {
> > error_report_err(local_err);
> > error_free(local_err);
> > + return;
> > }
> > - } else
> > -#endif /* defined(TARGET_PPC64) */
> > - {
> > - /* FIXME: Should check for valid HTABMASK values */
> > - env->htab_mask = ((value & SDR_32_HTABMASK) << 16) | 0xFFFF;
> > - env->htab_base = value & SDR_32_HTABORG;
> > }
> > +#endif /* defined(TARGET_PPC64) */
> > + /* FIXME: Should check for valid HTABMASK values in 32-bit case
> > */
> > + env->spr[SPR_SDR1] = value;
> > }
> >
> > /* Segment registers load and store */
>
--
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
signature.asc
Description: PGP signature
- Re: [Qemu-devel] [PATCH 4/6] target/ppc: Cleanup HPTE accessors for 64-bit hash MMU, (continued)