diff mbox

[V10] target-ppc: Fix htab_mask calculation

Message ID 1392260368-24016-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com
State New
Headers show

Commit Message

Aneesh Kumar K.V Feb. 13, 2014, 2:59 a.m. UTC
From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>

Correctly update the htab_mask using the return value of
KVM_PPC_ALLOCATE_HTAB ioctl. Also we don't update sdr1
on GET_SREGS for HV. We check for external htab and if
found true, we don't need to update sdr1

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
Changes from V9:
* Fix TCG breakage

 hw/ppc/spapr.c           |  8 +++++++-
 hw/ppc/spapr_hcall.c     | 19 +++++++++++++++----
 target-ppc/cpu.h         |  1 +
 target-ppc/kvm.c         |  4 +++-
 target-ppc/machine.c     | 11 +++++++----
 target-ppc/misc_helper.c |  4 +++-
 target-ppc/mmu_helper.c  |  3 ++-
 7 files changed, 38 insertions(+), 12 deletions(-)

Comments

Greg Kurz Feb. 13, 2014, 10:40 a.m. UTC | #1
On Thu, 13 Feb 2014 08:29:28 +0530
"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:

> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
> 
> Correctly update the htab_mask using the return value of
> KVM_PPC_ALLOCATE_HTAB ioctl. Also we don't update sdr1
> on GET_SREGS for HV. We check for external htab and if
> found true, we don't need to update sdr1
> 
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com>

> ---
> Changes from V9:
> * Fix TCG breakage
> 

Alex,

This patch is a complete replacement for commit 1df2d6d900a9 in your
ppc-next branch.

Please apply so that TCG works again ! :)

Thanks.

--
Greg

>  hw/ppc/spapr.c           |  8 +++++++-
>  hw/ppc/spapr_hcall.c     | 19 +++++++++++++++----
>  target-ppc/cpu.h         |  1 +
>  target-ppc/kvm.c         |  4 +++-
>  target-ppc/machine.c     | 11 +++++++----
>  target-ppc/misc_helper.c |  4 +++-
>  target-ppc/mmu_helper.c  |  3 ++-
>  7 files changed, 38 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index ac62c8f9294b..009bb0112cc0 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -748,7 +748,13 @@ static void spapr_cpu_reset(void *opaque)
>          env->external_htab = (void *)1;
>      }
>      env->htab_base = -1;
> -    env->htab_mask = HTAB_SIZE(spapr) - 1;
> +    /*
> +     * htab_mask is the mask used to normalize hash value to PTEG index.
> +     * htab_shift is log2 of hash table size.
> +     * We have 8 hpte per group, and each hpte is 16 bytes.
> +     * ie have 128 bytes per hpte entry.
> +     */
> +    env->htab_mask = (1ULL << ((spapr)->htab_shift - 7)) - 1;
>      env->spr[SPR_SDR1] = (target_ulong)(uintptr_t)spapr->htab |
>          (spapr->htab_shift - 18);
>  }
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index f755a5392317..c6e123b11ab0 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -40,6 +40,17 @@ static target_ulong compute_tlbie_rb(target_ulong v,
> target_ulong r, return rb;
>  }
> 
> +static inline bool valid_pte_index(CPUPPCState *env, target_ulong
> pte_index) +{
> +    /*
> +     * hash value/pteg group index is normalized by htab_mask
> +     */
> +    if (((pte_index & ~7ULL) / HPTES_PER_GROUP) & ~env->htab_mask) {
> +        return false;
> +    }
> +    return true;
> +}
> +
>  static target_ulong h_enter(PowerPCCPU *cpu, sPAPREnvironment *spapr,
>                              target_ulong opcode, target_ulong *args)
>  {
> @@ -91,7 +102,7 @@ static target_ulong h_enter(PowerPCCPU *cpu,
> sPAPREnvironment *spapr,
> 
>      pteh &= ~0x60ULL;
> 
> -    if ((pte_index * HASH_PTE_SIZE_64) & ~env->htab_mask) {
> +    if (!valid_pte_index(env, pte_index)) {
>          return H_PARAMETER;
>      }
>      if (likely((flags & H_EXACT) == 0)) {
> @@ -136,7 +147,7 @@ static RemoveResult remove_hpte(CPUPPCState *env,
> target_ulong ptex, hwaddr hpte;
>      target_ulong v, r, rb;
> 
> -    if ((ptex * HASH_PTE_SIZE_64) & ~env->htab_mask) {
> +    if (!valid_pte_index(env, ptex)) {
>          return REMOVE_PARM;
>      }
> 
> @@ -262,7 +273,7 @@ static target_ulong h_protect(PowerPCCPU *cpu,
> sPAPREnvironment *spapr, hwaddr hpte;
>      target_ulong v, r, rb;
> 
> -    if ((pte_index * HASH_PTE_SIZE_64) & ~env->htab_mask) {
> +    if (!valid_pte_index(env, pte_index)) {
>          return H_PARAMETER;
>      }
> 
> @@ -299,7 +310,7 @@ static target_ulong h_read(PowerPCCPU *cpu,
> sPAPREnvironment *spapr, uint8_t *hpte;
>      int i, ridx, n_entries = 1;
> 
> -    if ((pte_index * HASH_PTE_SIZE_64) & ~env->htab_mask) {
> +    if (!valid_pte_index(env, pte_index)) {
>          return H_PARAMETER;
>      }
> 
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index bb847676a52e..b0f66e5104dd 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -961,6 +961,7 @@ struct CPUPPCState {
>  #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 */
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 781b72f1ea5a..c771ec11ed28 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -1029,7 +1029,9 @@ int kvm_arch_get_registers(CPUState *cs)
>              return ret;
>          }
> 
> -        ppc_store_sdr1(env, sregs.u.s.sdr1);
> +        if (!env->external_htab) {
> +            ppc_store_sdr1(env, sregs.u.s.sdr1);
> +        }
> 
>          /* Sync SLB */
>  #ifdef TARGET_PPC64
> diff --git a/target-ppc/machine.c b/target-ppc/machine.c
> index 12c174f7f3e6..2d46ceccca3a 100644
> --- a/target-ppc/machine.c
> +++ b/target-ppc/machine.c
> @@ -70,7 +70,9 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int
> version_id) qemu_get_betls(f, &env->pb[i]);
>      for (i = 0; i < 1024; i++)
>          qemu_get_betls(f, &env->spr[i]);
> -    ppc_store_sdr1(env, sdr1);
> +    if (!env->external_htab) {
> +        ppc_store_sdr1(env, sdr1);
> +    }
>      qemu_get_be32s(f, &env->vscr);
>      qemu_get_be64s(f, &env->spe_acc);
>      qemu_get_be32s(f, &env->spe_fscr);
> @@ -179,9 +181,10 @@ static int cpu_post_load(void *opaque, int
> version_id) env->IBAT[1][i+4] = env->spr[SPR_IBAT4U + 2*i + 1];
>      }
> 
> -    /* Restore htab_base and htab_mask variables */
> -    ppc_store_sdr1(env, env->spr[SPR_SDR1]);
> -
> +    if (!env->external_htab) {
> +        /* Restore htab_base and htab_mask variables */
> +        ppc_store_sdr1(env, env->spr[SPR_SDR1]);
> +    }
>      hreg_compute_hflags(env);
>      hreg_compute_mem_idx(env);
> 
> diff --git a/target-ppc/misc_helper.c b/target-ppc/misc_helper.c
> index 616aab6fb67d..dc2ebfc4524b 100644
> --- a/target-ppc/misc_helper.c
> +++ b/target-ppc/misc_helper.c
> @@ -38,7 +38,9 @@ void helper_store_dump_spr(CPUPPCState *env, uint32_t
> sprn)
> 
>  void helper_store_sdr1(CPUPPCState *env, target_ulong val)
>  {
> -    ppc_store_sdr1(env, val);
> +    if (!env->external_htab) {
> +        ppc_store_sdr1(env, val);
> +    }
>  }
> 
>  void helper_store_hid0_601(CPUPPCState *env, target_ulong val)
> diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
> index 04a840b01697..8e2f8e736a12 100644
> --- a/target-ppc/mmu_helper.c
> +++ b/target-ppc/mmu_helper.c
> @@ -2014,6 +2014,7 @@ void ppc_tlb_invalidate_one(CPUPPCState *env,
> target_ulong addr) void ppc_store_sdr1(CPUPPCState *env, target_ulong
> value) {
>      LOG_MMU("%s: " TARGET_FMT_lx "\n", __func__, value);
> +    assert(!env->external_htab);
>      if (env->spr[SPR_SDR1] != value) {
>          env->spr[SPR_SDR1] = value;
>  #if defined(TARGET_PPC64)
> @@ -2025,7 +2026,7 @@ void ppc_store_sdr1(CPUPPCState *env, target_ulong
> value) " stored in SDR1\n", htabsize);
>                  htabsize = 28;
>              }
> -            env->htab_mask = (1ULL << (htabsize + 18)) - 1;
> +            env->htab_mask = (1ULL << (htabsize + 18 - 7)) - 1;
>              env->htab_base = value & SDR_64_HTABORG;
>          } else
>  #endif /* defined(TARGET_PPC64) */
Alexander Graf Feb. 13, 2014, 2:51 p.m. UTC | #2
On 13.02.2014, at 11:40, Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:

> On Thu, 13 Feb 2014 08:29:28 +0530
> "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
> 
>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>> 
>> Correctly update the htab_mask using the return value of
>> KVM_PPC_ALLOCATE_HTAB ioctl. Also we don't update sdr1
>> on GET_SREGS for HV. We check for external htab and if
>> found true, we don't need to update sdr1
>> 
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> 
> Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> 
>> ---
>> Changes from V9:
>> * Fix TCG breakage
>> 
> 
> Alex,
> 
> This patch is a complete replacement for commit 1df2d6d900a9 in your
> ppc-next branch.
> 
> Please apply so that TCG works again ! :)

Thanks, applied to ppc-next.


Alex
Alexander Graf Feb. 14, 2014, 1:06 p.m. UTC | #3
On 13.02.2014, at 15:51, Alexander Graf <agraf@suse.de> wrote:

> 
> On 13.02.2014, at 11:40, Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:
> 
>> On Thu, 13 Feb 2014 08:29:28 +0530
>> "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
>> 
>>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>>> 
>>> Correctly update the htab_mask using the return value of
>>> KVM_PPC_ALLOCATE_HTAB ioctl. Also we don't update sdr1
>>> on GET_SREGS for HV. We check for external htab and if
>>> found true, we don't need to update sdr1
>>> 
>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>> 
>> Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
>> 
>>> ---
>>> Changes from V9:
>>> * Fix TCG breakage
>>> 
>> 
>> Alex,
>> 
>> This patch is a complete replacement for commit 1df2d6d900a9 in your
>> ppc-next branch.
>> 
>> Please apply so that TCG works again ! :)
> 
> Thanks, applied to ppc-next.

With this patch applied -M pseries fails to boot a kernel for me.


Alex
Alexander Graf Feb. 14, 2014, 1:54 p.m. UTC | #4
On 14.02.2014, at 14:06, Alexander Graf <agraf@suse.de> wrote:

> 
> On 13.02.2014, at 15:51, Alexander Graf <agraf@suse.de> wrote:
> 
>> 
>> On 13.02.2014, at 11:40, Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:
>> 
>>> On Thu, 13 Feb 2014 08:29:28 +0530
>>> "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
>>> 
>>>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>>>> 
>>>> Correctly update the htab_mask using the return value of
>>>> KVM_PPC_ALLOCATE_HTAB ioctl. Also we don't update sdr1
>>>> on GET_SREGS for HV. We check for external htab and if
>>>> found true, we don't need to update sdr1
>>>> 
>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>> 
>>> Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
>>> 
>>>> ---
>>>> Changes from V9:
>>>> * Fix TCG breakage
>>>> 
>>> 
>>> Alex,
>>> 
>>> This patch is a complete replacement for commit 1df2d6d900a9 in your
>>> ppc-next branch.
>>> 
>>> Please apply so that TCG works again ! :)
>> 
>> Thanks, applied to ppc-next.
> 
> With this patch applied -M pseries fails to boot a kernel for me.

-M mac99 also fails miserably. It almost looks as if the mask cuts off some bits, but please investigate this yourself. I'll remove the patch from the queue meanwhile.


Alex
Alexander Graf Feb. 14, 2014, 2:28 p.m. UTC | #5
On 14.02.2014, at 14:54, Alexander Graf <agraf@suse.de> wrote:

> 
> On 14.02.2014, at 14:06, Alexander Graf <agraf@suse.de> wrote:
> 
>> 
>> On 13.02.2014, at 15:51, Alexander Graf <agraf@suse.de> wrote:
>> 
>>> 
>>> On 13.02.2014, at 11:40, Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:
>>> 
>>>> On Thu, 13 Feb 2014 08:29:28 +0530
>>>> "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
>>>> 
>>>>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>>>>> 
>>>>> Correctly update the htab_mask using the return value of
>>>>> KVM_PPC_ALLOCATE_HTAB ioctl. Also we don't update sdr1
>>>>> on GET_SREGS for HV. We check for external htab and if
>>>>> found true, we don't need to update sdr1
>>>>> 
>>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>> 
>>>> Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
>>>> 
>>>>> ---
>>>>> Changes from V9:
>>>>> * Fix TCG breakage
>>>>> 
>>>> 
>>>> Alex,
>>>> 
>>>> This patch is a complete replacement for commit 1df2d6d900a9 in your
>>>> ppc-next branch.
>>>> 
>>>> Please apply so that TCG works again ! :)
>>> 
>>> Thanks, applied to ppc-next.
>> 
>> With this patch applied -M pseries fails to boot a kernel for me.
> 
> -M mac99 also fails miserably. It almost looks as if the mask cuts off some bits, but please investigate this yourself. I'll remove the patch from the queue meanwhile.

Sigh, this whole series is just way too broken. I'll remove everything as of this patch from my queue. Aneesh, please rework the HTAB load/store patches and make sure that

  - TCG works
  - -M mac99 works
  - 32bit guest works
  - 32bit host works (casts!)

Then we can talk about applying them again.


Alex
Alexander Graf Feb. 14, 2014, 2:42 p.m. UTC | #6
On 14.02.2014, at 15:28, Alexander Graf <agraf@suse.de> wrote:

> 
> On 14.02.2014, at 14:54, Alexander Graf <agraf@suse.de> wrote:
> 
>> 
>> On 14.02.2014, at 14:06, Alexander Graf <agraf@suse.de> wrote:
>> 
>>> 
>>> On 13.02.2014, at 15:51, Alexander Graf <agraf@suse.de> wrote:
>>> 
>>>> 
>>>> On 13.02.2014, at 11:40, Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:
>>>> 
>>>>> On Thu, 13 Feb 2014 08:29:28 +0530
>>>>> "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
>>>>> 
>>>>>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>>>>>> 
>>>>>> Correctly update the htab_mask using the return value of
>>>>>> KVM_PPC_ALLOCATE_HTAB ioctl. Also we don't update sdr1
>>>>>> on GET_SREGS for HV. We check for external htab and if
>>>>>> found true, we don't need to update sdr1
>>>>>> 
>>>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>>> 
>>>>> Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
>>>>> 
>>>>>> ---
>>>>>> Changes from V9:
>>>>>> * Fix TCG breakage
>>>>>> 
>>>>> 
>>>>> Alex,
>>>>> 
>>>>> This patch is a complete replacement for commit 1df2d6d900a9 in your
>>>>> ppc-next branch.
>>>>> 
>>>>> Please apply so that TCG works again ! :)
>>>> 
>>>> Thanks, applied to ppc-next.
>>> 
>>> With this patch applied -M pseries fails to boot a kernel for me.
>> 
>> -M mac99 also fails miserably. It almost looks as if the mask cuts off some bits, but please investigate this yourself. I'll remove the patch from the queue meanwhile.
> 
> Sigh, this whole series is just way too broken. I'll remove everything as of this patch from my queue. Aneesh, please rework the HTAB load/store patches and make sure that
> 
>  - TCG works
>  - -M mac99 works
>  - 32bit guest works
>  - 32bit host works (casts!)

Oh, and please make sure every patch by itself covers the above, so that they're properly bisectable.


Alex
Greg Kurz Feb. 15, 2014, 11:02 a.m. UTC | #7
On Fri, 14 Feb 2014 15:42:49 +0100
Alexander Graf <agraf@suse.de> wrote:
> [...]
> >> -M mac99 also fails miserably. It almost looks as if the mask cuts off
> >> some bits, but please investigate this yourself. I'll remove the patch
> >> from the queue meanwhile.
> > 
> > Sigh, this whole series is just way too broken. I'll remove everything
> > as of this patch from my queue. Aneesh, please rework the HTAB
> > load/store patches and make sure that
> > 
> >  - TCG works
> >  - -M mac99 works
> >  - 32bit guest works
> >  - 32bit host works (casts!)
> 
> Oh, and please make sure every patch by itself covers the above, so that
> they're properly bisectable.
> 
> 

I have a reworked patchset that fixes TCG and the build issues. I'll repost
as soon as I have done all the testing.

> Alex
> 
> 

Cheers.

--
Greg
diff mbox

Patch

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index ac62c8f9294b..009bb0112cc0 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -748,7 +748,13 @@  static void spapr_cpu_reset(void *opaque)
         env->external_htab = (void *)1;
     }
     env->htab_base = -1;
-    env->htab_mask = HTAB_SIZE(spapr) - 1;
+    /*
+     * htab_mask is the mask used to normalize hash value to PTEG index.
+     * htab_shift is log2 of hash table size.
+     * We have 8 hpte per group, and each hpte is 16 bytes.
+     * ie have 128 bytes per hpte entry.
+     */
+    env->htab_mask = (1ULL << ((spapr)->htab_shift - 7)) - 1;
     env->spr[SPR_SDR1] = (target_ulong)(uintptr_t)spapr->htab |
         (spapr->htab_shift - 18);
 }
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index f755a5392317..c6e123b11ab0 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -40,6 +40,17 @@  static target_ulong compute_tlbie_rb(target_ulong v, target_ulong r,
     return rb;
 }
 
+static inline bool valid_pte_index(CPUPPCState *env, target_ulong pte_index)
+{
+    /*
+     * hash value/pteg group index is normalized by htab_mask
+     */
+    if (((pte_index & ~7ULL) / HPTES_PER_GROUP) & ~env->htab_mask) {
+        return false;
+    }
+    return true;
+}
+
 static target_ulong h_enter(PowerPCCPU *cpu, sPAPREnvironment *spapr,
                             target_ulong opcode, target_ulong *args)
 {
@@ -91,7 +102,7 @@  static target_ulong h_enter(PowerPCCPU *cpu, sPAPREnvironment *spapr,
 
     pteh &= ~0x60ULL;
 
-    if ((pte_index * HASH_PTE_SIZE_64) & ~env->htab_mask) {
+    if (!valid_pte_index(env, pte_index)) {
         return H_PARAMETER;
     }
     if (likely((flags & H_EXACT) == 0)) {
@@ -136,7 +147,7 @@  static RemoveResult remove_hpte(CPUPPCState *env, target_ulong ptex,
     hwaddr hpte;
     target_ulong v, r, rb;
 
-    if ((ptex * HASH_PTE_SIZE_64) & ~env->htab_mask) {
+    if (!valid_pte_index(env, ptex)) {
         return REMOVE_PARM;
     }
 
@@ -262,7 +273,7 @@  static target_ulong h_protect(PowerPCCPU *cpu, sPAPREnvironment *spapr,
     hwaddr hpte;
     target_ulong v, r, rb;
 
-    if ((pte_index * HASH_PTE_SIZE_64) & ~env->htab_mask) {
+    if (!valid_pte_index(env, pte_index)) {
         return H_PARAMETER;
     }
 
@@ -299,7 +310,7 @@  static target_ulong h_read(PowerPCCPU *cpu, sPAPREnvironment *spapr,
     uint8_t *hpte;
     int i, ridx, n_entries = 1;
 
-    if ((pte_index * HASH_PTE_SIZE_64) & ~env->htab_mask) {
+    if (!valid_pte_index(env, pte_index)) {
         return H_PARAMETER;
     }
 
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index bb847676a52e..b0f66e5104dd 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -961,6 +961,7 @@  struct CPUPPCState {
 #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 */
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 781b72f1ea5a..c771ec11ed28 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -1029,7 +1029,9 @@  int kvm_arch_get_registers(CPUState *cs)
             return ret;
         }
 
-        ppc_store_sdr1(env, sregs.u.s.sdr1);
+        if (!env->external_htab) {
+            ppc_store_sdr1(env, sregs.u.s.sdr1);
+        }
 
         /* Sync SLB */
 #ifdef TARGET_PPC64
diff --git a/target-ppc/machine.c b/target-ppc/machine.c
index 12c174f7f3e6..2d46ceccca3a 100644
--- a/target-ppc/machine.c
+++ b/target-ppc/machine.c
@@ -70,7 +70,9 @@  static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
         qemu_get_betls(f, &env->pb[i]);
     for (i = 0; i < 1024; i++)
         qemu_get_betls(f, &env->spr[i]);
-    ppc_store_sdr1(env, sdr1);
+    if (!env->external_htab) {
+        ppc_store_sdr1(env, sdr1);
+    }
     qemu_get_be32s(f, &env->vscr);
     qemu_get_be64s(f, &env->spe_acc);
     qemu_get_be32s(f, &env->spe_fscr);
@@ -179,9 +181,10 @@  static int cpu_post_load(void *opaque, int version_id)
         env->IBAT[1][i+4] = env->spr[SPR_IBAT4U + 2*i + 1];
     }
 
-    /* Restore htab_base and htab_mask variables */
-    ppc_store_sdr1(env, env->spr[SPR_SDR1]);
-
+    if (!env->external_htab) {
+        /* Restore htab_base and htab_mask variables */
+        ppc_store_sdr1(env, env->spr[SPR_SDR1]);
+    }
     hreg_compute_hflags(env);
     hreg_compute_mem_idx(env);
 
diff --git a/target-ppc/misc_helper.c b/target-ppc/misc_helper.c
index 616aab6fb67d..dc2ebfc4524b 100644
--- a/target-ppc/misc_helper.c
+++ b/target-ppc/misc_helper.c
@@ -38,7 +38,9 @@  void helper_store_dump_spr(CPUPPCState *env, uint32_t sprn)
 
 void helper_store_sdr1(CPUPPCState *env, target_ulong val)
 {
-    ppc_store_sdr1(env, val);
+    if (!env->external_htab) {
+        ppc_store_sdr1(env, val);
+    }
 }
 
 void helper_store_hid0_601(CPUPPCState *env, target_ulong val)
diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
index 04a840b01697..8e2f8e736a12 100644
--- a/target-ppc/mmu_helper.c
+++ b/target-ppc/mmu_helper.c
@@ -2014,6 +2014,7 @@  void ppc_tlb_invalidate_one(CPUPPCState *env, target_ulong addr)
 void ppc_store_sdr1(CPUPPCState *env, target_ulong value)
 {
     LOG_MMU("%s: " TARGET_FMT_lx "\n", __func__, value);
+    assert(!env->external_htab);
     if (env->spr[SPR_SDR1] != value) {
         env->spr[SPR_SDR1] = value;
 #if defined(TARGET_PPC64)
@@ -2025,7 +2026,7 @@  void ppc_store_sdr1(CPUPPCState *env, target_ulong value)
                         " stored in SDR1\n", htabsize);
                 htabsize = 28;
             }
-            env->htab_mask = (1ULL << (htabsize + 18)) - 1;
+            env->htab_mask = (1ULL << (htabsize + 18 - 7)) - 1;
             env->htab_base = value & SDR_64_HTABORG;
         } else
 #endif /* defined(TARGET_PPC64) */