[PATCH] KVM: PPC: E500: Support hugetlbfs

[PATCH] KVM: PPC: E500: Support hugetlbfs

[Alexander Graf]

With hugetlbfs support emerging on e500, we should also support KVM
backing its guest memory by it.

This patch adds support for hugetlbfs into the e500 shadow mmu code.

Signed-off-by: Alexander Graf <agraf@suse.de>

---

v1 -> v2:

  - address scott's comments
---
 arch/powerpc/kvm/e500_tlb.c |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/kvm/e500_tlb.c b/arch/powerpc/kvm/e500_tlb.c
index ec17148..1dd96a9 100644
--- a/arch/powerpc/kvm/e500_tlb.c
+++ b/arch/powerpc/kvm/e500_tlb.c
@@ -24,6 +24,7 @@
 #include <linux/sched.h>
 #include <linux/rwsem.h>
 #include <linux/vmalloc.h>
+#include <linux/hugetlb.h>
 #include <asm/kvm_ppc.h>
 #include <asm/kvm_e500.h>
 
@@ -673,12 +674,31 @@ static inline void kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
 				pfn &= ~(tsize_pages - 1);
 				break;
 			}
+		} else if (vma && hva >= vma->vm_start &&
+                           (vma->vm_flags & VM_HUGETLB)) {
+			unsigned long psize = vma_kernel_pagesize(vma);
+
+			tsize = (gtlbe->mas1 & MAS1_TSIZE_MASK) >>
+				MAS1_TSIZE_SHIFT;
+
+			/*
+			 * Take the largest page size that satisfies both host
+			 * and guest mapping
+			 */
+			tsize = min(__ilog2(psize) - 10, tsize);
+
+			/*
+			 * e500 doesn't implement the lowest tsize bit,
+			 * or 1K pages.
+			 */
+			tsize = max(BOOK3E_PAGESZ_4K, tsize & ~1);
 		}
 
 		up_read(&current->mm->mmap_sem);
 	}
 
 	if (likely(!pfnmap)) {
+		unsigned long tsize_pages = 1 << (tsize + 10 - PAGE_SHIFT);
 		pfn = gfn_to_pfn_memslot(vcpu_e500->vcpu.kvm, slot, gfn);
 		if (is_error_pfn(pfn)) {
 			printk(KERN_ERR "Couldn't get real page for gfn %lx!\n",
@@ -686,6 +706,10 @@ static inline void kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
 			kvm_release_pfn_clean(pfn);
 			return;
 		}
+
+		/* Align guest and physical address to page map boundaries */
+		pfn &= ~(tsize_pages - 1);
+		gvaddr &= ~((tsize_pages << PAGE_SHIFT) - 1);
 	}
 
 	/* Drop old ref and setup new one. */
-- 
1.6.0.2

Re: [PATCH] KVM: PPC: E500: Support hugetlbfs

[Scott Wood]

On 10/05/2011 09:37 AM, Alexander Graf wrote:
> diff --git a/arch/powerpc/kvm/e500_tlb.c b/arch/powerpc/kvm/e500_tlb.c
> index ec17148..1dd96a9 100644
> --- a/arch/powerpc/kvm/e500_tlb.c
> +++ b/arch/powerpc/kvm/e500_tlb.c
> @@ -24,6 +24,7 @@
>  #include <linux/sched.h>
>  #include <linux/rwsem.h>
>  #include <linux/vmalloc.h>
> +#include <linux/hugetlb.h>
>  #include <asm/kvm_ppc.h>
>  #include <asm/kvm_e500.h>
>  
> @@ -673,12 +674,31 @@ static inline void kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
>  				pfn &= ~(tsize_pages - 1);
>  				break;
>  			}
> +		} else if (vma && hva >= vma->vm_start &&
> +                           (vma->vm_flags & VM_HUGETLB)) {
> +			unsigned long psize = vma_kernel_pagesize(vma);
> +
> +			tsize = (gtlbe->mas1 & MAS1_TSIZE_MASK) >>
> +				MAS1_TSIZE_SHIFT;
> +
> +			/*
> +			 * Take the largest page size that satisfies both host
> +			 * and guest mapping
> +			 */
> +			tsize = min(__ilog2(psize) - 10, tsize);

Any reason for __ilog2() rather than ilog2()?  Shouldn't make a
difference, just curious about avoiding the public interface.

Either way,
Acked-by: Scott Wood <scottwood@freescale.com>

-Scott

Re: [PATCH] KVM: PPC: E500: Support hugetlbfs

[Alexander Graf]

On 05.10.2011, at 18:06, Scott Wood wrote:

> On 10/05/2011 09:37 AM, Alexander Graf wrote:
>> diff --git a/arch/powerpc/kvm/e500_tlb.c b/arch/powerpc/kvm/e500_tlb.c
>> index ec17148..1dd96a9 100644
>> --- a/arch/powerpc/kvm/e500_tlb.c
>> +++ b/arch/powerpc/kvm/e500_tlb.c
>> @@ -24,6 +24,7 @@
>> #include <linux/sched.h>
>> #include <linux/rwsem.h>
>> #include <linux/vmalloc.h>
>> +#include <linux/hugetlb.h>
>> #include <asm/kvm_ppc.h>
>> #include <asm/kvm_e500.h>
>> 
>> @@ -673,12 +674,31 @@ static inline void kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
>> 				pfn &= ~(tsize_pages - 1);
>> 				break;
>> 			}
>> +		} else if (vma && hva >= vma->vm_start &&
>> +                           (vma->vm_flags & VM_HUGETLB)) {
>> +			unsigned long psize = vma_kernel_pagesize(vma);
>> +
>> +			tsize = (gtlbe->mas1 & MAS1_TSIZE_MASK) >>
>> +				MAS1_TSIZE_SHIFT;
>> +
>> +			/*
>> +			 * Take the largest page size that satisfies both host
>> +			 * and guest mapping
>> +			 */
>> +			tsize = min(__ilog2(psize) - 10, tsize);
> 
> Any reason for __ilog2() rather than ilog2()?  Shouldn't make a
> difference, just curious about avoiding the public interface.

I grep'ed through the kernel tree and only found __ilog2 defined as well as mostly users for __ilog2, so I figured there's got to be a reason ;)


Alex

Re: [PATCH] KVM: PPC: E500: Support hugetlbfs

[Scott Wood]

On 10/05/2011 01:55 PM, Alexander Graf wrote:
> 
> On 05.10.2011, at 18:06, Scott Wood wrote:
> 
>> Any reason for __ilog2() rather than ilog2()?  Shouldn't make a
>> difference, just curious about avoiding the public interface.
> 
> I grep'ed through the kernel tree and only found __ilog2 defined as well as mostly users for __ilog2, so I figured there's got to be a reason ;)

ilog2() is defined in include/linux/ilog2.h.  It produces constant
output if the input is constant, and appears to be the "front door" to
__ilog2_u32/__ilog2_u64.  Plain __ilog2 is older and powerpc-specific,
which is probably why there are more users of that in arch/powerpc.

-Scott

[PATCH] KVM: PPC: E500: Support hugetlbfs

[Alexander Graf]

With hugetlbfs support emerging on e500, we should also support KVM
backing its guest memory by it.

This patch adds support for hugetlbfs into the e500 shadow mmu code.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/powerpc/kvm/e500_tlb.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/kvm/e500_tlb.c b/arch/powerpc/kvm/e500_tlb.c
index ec17148..64f75eb 100644
--- a/arch/powerpc/kvm/e500_tlb.c
+++ b/arch/powerpc/kvm/e500_tlb.c
@@ -24,6 +24,7 @@
 #include <linux/sched.h>
 #include <linux/rwsem.h>
 #include <linux/vmalloc.h>
+#include <linux/hugetlb.h>
 #include <asm/kvm_ppc.h>
 #include <asm/kvm_e500.h>
 
@@ -673,13 +674,34 @@ static inline void kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
 				pfn &= ~(tsize_pages - 1);
 				break;
 			}
+		} else if (vma && hva >= vma->vm_start &&
+                           (vma->vm_flags & VM_HUGETLB)) {
+			unsigned long psize = vma_kernel_pagesize(vma);
+			int lz;
+
+			tsize = (gtlbe->mas1 & MAS1_TSIZE_MASK) >>
+				MAS1_TSIZE_SHIFT;
+
+			/*
+			 * e500 doesn't implement the lowest tsize bit,
+			 * or 1K pages.
+			 */
+			tsize = max(BOOK3E_PAGESZ_4K, tsize & ~1);
+
+			/* take the smallest page size that satisfies host and
+			   guest mapping */
+			asm (PPC_CNTLZL "%0,%1" : "=r" (lz) : "r" (psize));
+			tsize = min(21 - lz, tsize);
 		}
 
 		up_read(&current->mm->mmap_sem);
 	}
 
 	if (likely(!pfnmap)) {
+		unsigned long tsize_pages = 1 << (tsize - 2);
 		pfn = gfn_to_pfn_memslot(vcpu_e500->vcpu.kvm, slot, gfn);
+		pfn &= ~(tsize_pages - 1);
+		gvaddr &= ~((tsize_pages << PAGE_SHIFT) - 1);
 		if (is_error_pfn(pfn)) {
 			printk(KERN_ERR "Couldn't get real page for gfn %lx!\n",
 					(long)gfn);
-- 
1.6.0.2

Re: [PATCH] KVM: PPC: E500: Support hugetlbfs

[Scott Wood]

On 09/19/2011 06:35 PM, Alexander Graf wrote:
> +			/*
> +			 * e500 doesn't implement the lowest tsize bit,
> +			 * or 1K pages.
> +			 */
> +			tsize = max(BOOK3E_PAGESZ_4K, tsize & ~1);
> +
> +			/* take the smallest page size that satisfies host and
> +			   guest mapping */

kernel comment style...

I don't personally care much, but this leaves a checkpatch complaint for
the next person to modify the comment.  Such as to make it say "take the
largest page size that...". :-)

> +			asm (PPC_CNTLZL "%0,%1" : "=r" (lz) : "r" (psize));
> +			tsize = min(21 - lz, tsize);

No need to open-code the cntlz and subtract-from-21:

	tsize = min(ilog2(psize) - 10, tsize);

	/*
	 * e500 doesn't implement the lowest tsize bit,
	 * or 1K pages.
	 */
	tsize = max(BOOK3E_PAGESZ_4K, tsize & ~1);

There's still an open-coded subtraction of 10, but that relates more
straightforwardly to the definition of tsize (and could be factored out
into a size-to-tsize function).

>  		}
>  
>  		up_read(&current->mm->mmap_sem);
>  	}
>  
>  	if (likely(!pfnmap)) {
> +		unsigned long tsize_pages = 1 << (tsize - 2);

1 << (tsize + 10 - PAGE_SHIFT);

>  		pfn = gfn_to_pfn_memslot(vcpu_e500->vcpu.kvm, slot, gfn);
> +		pfn &= ~(tsize_pages - 1);
> +		gvaddr &= ~((tsize_pages << PAGE_SHIFT) - 1);
>  		if (is_error_pfn(pfn)) {

Won't the masking of pfn affect is_error_pfn()?

-Scott

Re: [PATCH] KVM: PPC: E500: Support hugetlbfs

[Alexander Graf]

On 20.09.2011, at 19:54, Scott Wood wrote:

> On 09/19/2011 06:35 PM, Alexander Graf wrote:
>> +			/*
>> +			 * e500 doesn't implement the lowest tsize bit,
>> +			 * or 1K pages.
>> +			 */
>> +			tsize = max(BOOK3E_PAGESZ_4K, tsize & ~1);
>> +
>> +			/* take the smallest page size that satisfies host and
>> +			   guest mapping */
> 
> kernel comment style...
> 
> I don't personally care much, but this leaves a checkpatch complaint for
> the next person to modify the comment.  Such as to make it say "take the
> largest page size that...". :-)

Ahem :). Changed.

> 
>> +			asm (PPC_CNTLZL "%0,%1" : "=r" (lz) : "r" (psize));
>> +			tsize = min(21 - lz, tsize);
> 
> No need to open-code the cntlz and subtract-from-21:
> 
> 	tsize = min(ilog2(psize) - 10, tsize);
> 
> 	/*
> 	 * e500 doesn't implement the lowest tsize bit,
> 	 * or 1K pages.
> 	 */
> 	tsize = max(BOOK3E_PAGESZ_4K, tsize & ~1);
> 
> There's still an open-coded subtraction of 10, but that relates more
> straightforwardly to the definition of tsize (and could be factored out
> into a size-to-tsize function).

Yeah, no need to micro-optimized those few bits. The reason I used the asm statement was that I copied the hugetlbfs code which does it that way :).

> 
>> 		}
>> 
>> 		up_read(&current->mm->mmap_sem);
>> 	}
>> 
>> 	if (likely(!pfnmap)) {
>> +		unsigned long tsize_pages = 1 << (tsize - 2);
> 
> 1 << (tsize + 10 - PAGE_SHIFT);

Are we getting variable page sizes anytime soon? Will change it nevertheless, just curious :).

> 
>> 		pfn = gfn_to_pfn_memslot(vcpu_e500->vcpu.kvm, slot, gfn);
>> +		pfn &= ~(tsize_pages - 1);
>> +		gvaddr &= ~((tsize_pages << PAGE_SHIFT) - 1);
>> 		if (is_error_pfn(pfn)) {
> 
> Won't the masking of pfn affect is_error_pfn()?

Yes, it would. Good catch!


Alex

Re: [PATCH] KVM: PPC: E500: Support hugetlbfs

[Scott Wood]

On 09/24/2011 02:44 AM, Alexander Graf wrote:
> On 20.09.2011, at 19:54, Scott Wood wrote:
>> On 09/19/2011 06:35 PM, Alexander Graf wrote:
>>> +			asm (PPC_CNTLZL "%0,%1" : "=r" (lz) : "r" (psize));
>>> +			tsize = min(21 - lz, tsize);
>>
>> No need to open-code the cntlz and subtract-from-21:
>>
>> 	tsize = min(ilog2(psize) - 10, tsize);
>>
>> 	/*
>> 	 * e500 doesn't implement the lowest tsize bit,
>> 	 * or 1K pages.
>> 	 */
>> 	tsize = max(BOOK3E_PAGESZ_4K, tsize & ~1);
>>
>> There's still an open-coded subtraction of 10, but that relates more
>> straightforwardly to the definition of tsize (and could be factored out
>> into a size-to-tsize function).
> 
> Yeah, no need to micro-optimized those few bits. The reason I used the asm statement was that I copied the hugetlbfs code which does it that way :).

The "21 - lz" thing is broken on 64-bit, FWIW.  It works by chance in
the hugetlbfs code (and in settlbcam) because it results in tsize being
too low by 32, which only affects bits that subsequently get masked off.

>>> 		}
>>>
>>> 		up_read(&current->mm->mmap_sem);
>>> 	}
>>>
>>> 	if (likely(!pfnmap)) {
>>> +		unsigned long tsize_pages = 1 << (tsize - 2);
>>
>> 1 << (tsize + 10 - PAGE_SHIFT);
> 
> Are we getting variable page sizes anytime soon? Will change it nevertheless, just curious :).

Nothing imminent on our chips AFAIK, but there's some 64K page support
in Linux for IBM's book3e chips, and it's not nice to hardcode
(especially in a hidden way) regardless.

-Scott

RE: [PATCH] KVM: PPC: E500: Support hugetlbfs

[Liu Yu-B13201]

 

> -----Original Message-----
> From: kvm-ppc-owner@vger.kernel.org 
> [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of Alexander Graf
> Sent: Tuesday, September 20, 2011 7:36 AM
> To: kvm-ppc@vger.kernel.org
> Cc: kvm@vger.kernel.org
> Subject: [PATCH] KVM: PPC: E500: Support hugetlbfs
> 
> With hugetlbfs support emerging on e500, we should also support KVM
> backing its guest memory by it.
> 
> This patch adds support for hugetlbfs into the e500 shadow mmu code.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  arch/powerpc/kvm/e500_tlb.c |   22 ++++++++++++++++++++++
>  1 files changed, 22 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/e500_tlb.c b/arch/powerpc/kvm/e500_tlb.c
> index ec17148..64f75eb 100644
> --- a/arch/powerpc/kvm/e500_tlb.c
> +++ b/arch/powerpc/kvm/e500_tlb.c
> @@ -24,6 +24,7 @@
>  #include <linux/sched.h>
>  #include <linux/rwsem.h>
>  #include <linux/vmalloc.h>
> +#include <linux/hugetlb.h>
>  #include <asm/kvm_ppc.h>
>  #include <asm/kvm_e500.h>
>  
> @@ -673,13 +674,34 @@ static inline void 
> kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
>  				pfn &= ~(tsize_pages - 1);
>  				break;
>  			}
> +		} else if (vma && hva >= vma->vm_start &&
> +                           (vma->vm_flags & VM_HUGETLB)) {

Why check (vma && hva >= vma->vm_start) twice?

Thanks,
Yu

Re: [PATCH] KVM: PPC: E500: Support hugetlbfs

[Alexander Graf]

On 22.09.2011, at 08:50, Liu Yu-B13201 wrote:

> 
> 
>> -----Original Message-----
>> From: kvm-ppc-owner@vger.kernel.org 
>> [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of Alexander Graf
>> Sent: Tuesday, September 20, 2011 7:36 AM
>> To: kvm-ppc@vger.kernel.org
>> Cc: kvm@vger.kernel.org
>> Subject: [PATCH] KVM: PPC: E500: Support hugetlbfs
>> 
>> With hugetlbfs support emerging on e500, we should also support KVM
>> backing its guest memory by it.
>> 
>> This patch adds support for hugetlbfs into the e500 shadow mmu code.
>> 
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>> arch/powerpc/kvm/e500_tlb.c |   22 ++++++++++++++++++++++
>> 1 files changed, 22 insertions(+), 0 deletions(-)
>> 
>> diff --git a/arch/powerpc/kvm/e500_tlb.c b/arch/powerpc/kvm/e500_tlb.c
>> index ec17148..64f75eb 100644
>> --- a/arch/powerpc/kvm/e500_tlb.c
>> +++ b/arch/powerpc/kvm/e500_tlb.c
>> @@ -24,6 +24,7 @@
>> #include <linux/sched.h>
>> #include <linux/rwsem.h>
>> #include <linux/vmalloc.h>
>> +#include <linux/hugetlb.h>
>> #include <asm/kvm_ppc.h>
>> #include <asm/kvm_e500.h>
>> 
>> @@ -673,13 +674,34 @@ static inline void 
>> kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
>> 				pfn &= ~(tsize_pages - 1);
>> 				break;
>> 			}
>> +		} else if (vma && hva >= vma->vm_start &&
>> +                           (vma->vm_flags & VM_HUGETLB)) {
> 
> Why check (vma && hva >= vma->vm_start) twice?

What would you do? :)

In fact, I only copied the vm_start condition from the pfn code. Scott, why do we have to check this in the first place? We're calling find_vma. Can that return a vma that does not cover the hva we're passing in?


Alex

Re: [PATCH] KVM: PPC: E500: Support hugetlbfs

[Scott Wood]

On 09/24/2011 02:47 AM, Alexander Graf wrote:
> 
> On 22.09.2011, at 08:50, Liu Yu-B13201 wrote:
> 
>>
>>
>>> -----Original Message-----
>>> From: kvm-ppc-owner@vger.kernel.org 
>>> [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of Alexander Graf
>>> Sent: Tuesday, September 20, 2011 7:36 AM
>>> To: kvm-ppc@vger.kernel.org
>>> Cc: kvm@vger.kernel.org
>>> Subject: [PATCH] KVM: PPC: E500: Support hugetlbfs
>>>
>>> With hugetlbfs support emerging on e500, we should also support KVM
>>> backing its guest memory by it.
>>>
>>> This patch adds support for hugetlbfs into the e500 shadow mmu code.
>>>
>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>> ---
>>> arch/powerpc/kvm/e500_tlb.c |   22 ++++++++++++++++++++++
>>> 1 files changed, 22 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/powerpc/kvm/e500_tlb.c b/arch/powerpc/kvm/e500_tlb.c
>>> index ec17148..64f75eb 100644
>>> --- a/arch/powerpc/kvm/e500_tlb.c
>>> +++ b/arch/powerpc/kvm/e500_tlb.c
>>> @@ -24,6 +24,7 @@
>>> #include <linux/sched.h>
>>> #include <linux/rwsem.h>
>>> #include <linux/vmalloc.h>
>>> +#include <linux/hugetlb.h>
>>> #include <asm/kvm_ppc.h>
>>> #include <asm/kvm_e500.h>
>>>
>>> @@ -673,13 +674,34 @@ static inline void 
>>> kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
>>> 				pfn &= ~(tsize_pages - 1);
>>> 				break;
>>> 			}
>>> +		} else if (vma && hva >= vma->vm_start &&
>>> +                           (vma->vm_flags & VM_HUGETLB)) {
>>
>> Why check (vma && hva >= vma->vm_start) twice?
> 
> What would you do? :)

I think he's just complaining about doing the check twice, in which case
the answer could be "it avoids extra indentation and the compiler should
be able to factor out the common subexpression".

> In fact, I only copied the vm_start condition from the pfn code.
> Scott, why do we have to check this in the first place? We're calling
> find_vma. Can that return a vma that does not cover the hva we're
> passing in?

Yes, it can.  From find_vma():

/* Look up the first VMA which satisfies  addr < vm_end,  NULL if none. */

You'll find similar checks in a lot of other places where find_vma() is
used.

-Scott