diff mbox

[-V8,1/3] target-ppc: Fix htab_mask calculation

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

Commit Message

Aneesh Kumar K.V Dec. 23, 2013, 5:22 p.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. So don't update htab_mask if sdr1
is found to be zero. Fix the pte index calculation to be
same as that found in the kernel

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 hw/ppc/spapr.c          | 8 +++++++-
 target-ppc/cpu.h        | 1 +
 target-ppc/mmu_helper.c | 4 +++-
 3 files changed, 11 insertions(+), 2 deletions(-)

Comments

Alexander Graf Jan. 9, 2014, 12:21 p.m. UTC | #1
On 23.12.2013, at 18:22, 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. So don't update htab_mask if sdr1
> is found to be zero. Fix the pte index calculation to be
> same as that found in the kernel
> 
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
> hw/ppc/spapr.c          | 8 +++++++-
> target-ppc/cpu.h        | 1 +
> target-ppc/mmu_helper.c | 4 +++-
> 3 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 267a47d6cc4d..e99a66170661 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -731,7 +731,13 @@ static void spapr_cpu_reset(void *opaque)
> 
>     env->external_htab = (uint8_t *)spapr->htab;
>     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/target-ppc/cpu.h b/target-ppc/cpu.h
> index 26acdba847fa..82db9a442be5 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -955,6 +955,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/mmu_helper.c b/target-ppc/mmu_helper.c
> index 04a840b01697..c39cb7b89ee0 100644
> --- a/target-ppc/mmu_helper.c
> +++ b/target-ppc/mmu_helper.c
> @@ -2025,7 +2025,9 @@ void ppc_store_sdr1(CPUPPCState *env, target_ulong value)
>                         " stored in SDR1\n", htabsize);
>                 htabsize = 28;
>             }
> -            env->htab_mask = (1ULL << (htabsize + 18)) - 1;
> +            if (htabsize) {

Better check for external_htab in the caller and never store SDR1 if we use an external htab. If you like, add an assert(!env->external_htab) at the beginning of ppc_store_sdr1() to make sure we really never get here.


Alex
Aneesh Kumar K.V Jan. 9, 2014, 3:44 p.m. UTC | #2
Alexander Graf <agraf@suse.de> writes:

> On 23.12.2013, at 18:22, 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. So don't update htab_mask if sdr1
>> is found to be zero. Fix the pte index calculation to be
>> same as that found in the kernel
>> 
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>> ---
>> hw/ppc/spapr.c          | 8 +++++++-
>> target-ppc/cpu.h        | 1 +
>> target-ppc/mmu_helper.c | 4 +++-
>> 3 files changed, 11 insertions(+), 2 deletions(-)
>> 
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 267a47d6cc4d..e99a66170661 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -731,7 +731,13 @@ static void spapr_cpu_reset(void *opaque)
>> 
>>     env->external_htab = (uint8_t *)spapr->htab;
>>     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/target-ppc/cpu.h b/target-ppc/cpu.h
>> index 26acdba847fa..82db9a442be5 100644
>> --- a/target-ppc/cpu.h
>> +++ b/target-ppc/cpu.h
>> @@ -955,6 +955,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/mmu_helper.c b/target-ppc/mmu_helper.c
>> index 04a840b01697..c39cb7b89ee0 100644
>> --- a/target-ppc/mmu_helper.c
>> +++ b/target-ppc/mmu_helper.c
>> @@ -2025,7 +2025,9 @@ void ppc_store_sdr1(CPUPPCState *env, target_ulong value)
>>                         " stored in SDR1\n", htabsize);
>>                 htabsize = 28;
>>             }
>> -            env->htab_mask = (1ULL << (htabsize + 18)) - 1;
>> +            if (htabsize) {
>
> Better check for external_htab in the caller and never store SDR1 if
> we use an external htab. If you like, add an
> assert(!env->external_htab) at the beginning of ppc_store_sdr1() to
> make sure we really never get here.


will do

-aneesh
diff mbox

Patch

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 267a47d6cc4d..e99a66170661 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -731,7 +731,13 @@  static void spapr_cpu_reset(void *opaque)
 
     env->external_htab = (uint8_t *)spapr->htab;
     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/target-ppc/cpu.h b/target-ppc/cpu.h
index 26acdba847fa..82db9a442be5 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -955,6 +955,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/mmu_helper.c b/target-ppc/mmu_helper.c
index 04a840b01697..c39cb7b89ee0 100644
--- a/target-ppc/mmu_helper.c
+++ b/target-ppc/mmu_helper.c
@@ -2025,7 +2025,9 @@  void ppc_store_sdr1(CPUPPCState *env, target_ulong value)
                         " stored in SDR1\n", htabsize);
                 htabsize = 28;
             }
-            env->htab_mask = (1ULL << (htabsize + 18)) - 1;
+            if (htabsize) {
+                env->htab_mask = (1ULL << (htabsize + 18 - 7)) - 1;
+            }
             env->htab_base = value & SDR_64_HTABORG;
         } else
 #endif /* defined(TARGET_PPC64) */