Alexander Graf <ag...@suse.de> writes:
> On 15.10.2013, at 01:58, Aneesh Kumar K.V <aneesh.ku...@linux.vnet.ibm.com>
> wrote:
>
>> From: "Aneesh Kumar K.V" <aneesh.ku...@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.ku...@linux.vnet.ibm.com>
>> ---
>> hw/ppc/spapr.c | 3 ++-
>> target-ppc/mmu-hash64.c | 2 +-
>> target-ppc/mmu_helper.c | 4 +++-
>> 3 files changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 22f2a8a..d4f3502 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -724,7 +724,8 @@ static void spapr_cpu_reset(void *opaque)
>> env->external_htab = (void *)1;
>> }
>> env->htab_base = -1;
>> - env->htab_mask = HTAB_SIZE(spapr) - 1;
>> + /* 128 (2**7) bytes in each HPTEG */
>> + env->htab_mask = (1ULL << ((spapr)->htab_shift - 7)) - 1;
>
> HTAB_SIZE(spapr) / 128? The compiler should be smart enough to produce
> the same code out of that.
(HTAB_SIZE(spapr) - 1) / 128. I am not sure that is any better ?
>
> However, could you please explain why it's better to have the mask be
> on the PTEG rather than the offset? Is this something you missed in
> the previous patch? If so, please change the semantics on what
> htab_mask means before you break the code as that makes bisecting
> hard.
That is how kernel does the masking. In kvmppc_alloc_hpt() we have
/* 128 (2**7) bytes in each HPTEG */
kvm->arch.hpt_mask = (1ul << (order - 7)) - 1;
If we don't have it same, we would end up with wrong hpte index
>
> Furthermore, since you are changing the semantics of htab_mask, have
> you checked all other users of it? Most notably the hash32 code.
I can verify the code, but i don't have setup
-aneesh
