On 09/24/2015 11:23 PM, Scott Wood wrote:
> On Thu, 2015-09-24 at 15:57 +0300, Laurentiu Tudor wrote:
>> Book-E MMUv2 present in e6500 cores supports
>> powers of 2K page sizes while older MMUv1 cores
>> support only powers of 4K page sizes, or in other
>> words the LSB of TSIZE on MMUv1 is always 0.
>> Thus, on MMUv2 we must not strip the LSB.
>
> We can get better TLB utilization by not stripping it, but why "must not"
> which makes it sound like a bugfix rather than an optimization?
Not sure i get it. If a guests wants a 2K^^(2n+1) translation
size it will instead get a 2K^^2n size, no? Isn't this an issue?
>> Signed-off-by: Mihai Caraman <mihai.caraman@xxxxxxxxxxxxx>
>> [Laurentiu.Tudor@xxxxxxxxxxxxx: addressed review
>> feedback, split in distinct patch]
>> Signed-off-by: Laurentiu Tudor <Laurentiu.Tudor@xxxxxxxxxxxxx>
>> ---
>> arch/powerpc/kvm/e500_mmu_host.c | 28 +++++++++++++++++++++-------
>> 1 file changed, 21 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/powerpc/kvm/e500_mmu_host.c
>> b/arch/powerpc/kvm/e500_mmu_host.c
>> index 4d33e19..12d5c67 100644
>> --- a/arch/powerpc/kvm/e500_mmu_host.c
>> +++ b/arch/powerpc/kvm/e500_mmu_host.c
>> @@ -371,6 +371,7 @@ static inline int kvmppc_e500_shadow_map(struct
>> kvmppc_vcpu_e500 *vcpu_e500,
>>
>> unsigned long start, end;
>> unsigned long slot_start, slot_end;
>> + int tsize_inc;
>>
>> pfnmap = 1;
>>
>> @@ -392,10 +393,20 @@ static inline int kvmppc_e500_shadow_map(struct
>> kvmppc_vcpu_e500 *vcpu_e500,
>> MAS1_TSIZE_SHIFT;
>>
>> /*
>> - * e500 doesn't implement the lowest tsize bit,
>> - * or 1K pages.
>> + * MMUv1 doesn't implement the lowest tsize bit,
>> + * or translations smaller than 4K.
>> */
>> - tsize = max(BOOK3E_PAGESZ_4K, tsize & ~1);
>> + if (!has_feature(&vcpu_e500->vcpu, VCPU_FTR_MMU_V2))
>> + tsize &= ~1;
>> + tsize = max(BOOK3E_PAGESZ_4K, tsize);
>> +
>> + /*
>> + * Calculate TSIZE increment. MMUv2 supports
>> + * power of 2K translations while MMUv1 is limited
>> + * to power of 4K sizes.
>> + */
>> + tsize_inc = has_feature(&vcpu_e500->vcpu,
>> + VCPU_FTR_MMU_V2) ? 1 : 2;
>
> If you calculate tsize_inc first, then the previous if-statement can become
> "tsize &= ~(tsize_inc - 1);".
Thanks for the tip. Will do this in the next version.
>>
>> /*
>> * Now find the largest tsize (up to what the guest
>> @@ -404,7 +415,8 @@ static inline int kvmppc_e500_shadow_map(struct
>> kvmppc_vcpu_e500 *vcpu_e500,
>> * aligned.
>> */
>>
>> - for (; tsize > BOOK3E_PAGESZ_4K; tsize -= 2) {
>> + for (; tsize > BOOK3E_PAGESZ_4K;
>> + tsize -= tsize_inc) {
>> unsigned long gfn_start, gfn_end;
>> tsize_pages = 1 << (tsize - 2);
>>
>> @@ -437,10 +449,12 @@ static inline int kvmppc_e500_shadow_map(struct
>> kvmppc_vcpu_e500 *vcpu_e500,
>> tsize = min(__ilog2(psize) - 10, tsize);
>>
>> /*
>> - * e500 doesn't implement the lowest tsize bit,
>> - * or 1K pages.
>> + * MMUv1 doesn't implement the lowest tsize bit,
>> + * or translations smaller than 4K.
>> */
>
> This comment makes it sound like MMUv2 might support translations smaller
> than 4K.
Right. I'll rephrase in the next spin.
Thanks for the review!
---
Best Regards, Laurentiu
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
