[PATCH 0/2] KVM: arm64: user_mem_abort() improvements

[PATCH 0/2] KVM: arm64: user_mem_abort() improvements

[Alexandru Elisei]

The first patch is a fix for a bug that I found by code inspection.

The second patch is an enhancement for the way user_mem_abort() handles
hugetlbfs backed VM memory.

Tested on a rockpro64 with 4K pages and hugetlbfs hugepagesz=1G (PUD sized
block mappings).  First test, guest RAM starts at 0x8100 0000
(memslot->base_gfn not aligned to 1GB); second test, guest RAM starts at
0x8000 0000, but is only 512 MB.  In both cases using PUD mappings is not
possible because either the memslot base address is not aligned, or the
mapping would extend beyond the memslot.

Without the changes, user_mem_abort() uses 4K pages to map the guest IPA.
With the patches, user_mem_abort() uses PMD block mappings (2MB) to map the
guest RAM, which means less TLB pressure and fewer stage 2 aborts.

Alexandru Elisei (2):
  KVM: arm64: Update page shift if stage 2 block mapping not supported
  KVM: arm64: Try PMD block mappings if PUD mappings are not supported

 arch/arm64/kvm/mmu.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

-- 
2.28.0

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH 0/2] KVM: arm64: user_mem_abort() improvements

[Alexandru Elisei]

The first patch is a fix for a bug that I found by code inspection.

The second patch is an enhancement for the way user_mem_abort() handles
hugetlbfs backed VM memory.

Tested on a rockpro64 with 4K pages and hugetlbfs hugepagesz=1G (PUD sized
block mappings).  First test, guest RAM starts at 0x8100 0000
(memslot->base_gfn not aligned to 1GB); second test, guest RAM starts at
0x8000 0000, but is only 512 MB.  In both cases using PUD mappings is not
possible because either the memslot base address is not aligned, or the
mapping would extend beyond the memslot.

Without the changes, user_mem_abort() uses 4K pages to map the guest IPA.
With the patches, user_mem_abort() uses PMD block mappings (2MB) to map the
guest RAM, which means less TLB pressure and fewer stage 2 aborts.

Alexandru Elisei (2):
  KVM: arm64: Update page shift if stage 2 block mapping not supported
  KVM: arm64: Try PMD block mappings if PUD mappings are not supported

 arch/arm64/kvm/mmu.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/2] KVM: arm64: Update page shift if stage 2 block mapping not supported

[Alexandru Elisei]

Commit 196f878a7ac2e (" KVM: arm/arm64: Signal SIGBUS when stage2 discovers
hwpoison memory") modifies user_mem_abort() to send a SIGBUS signal when
the fault IPA maps to a hwpoisoned page. Commit 1559b7583ff6 ("KVM:
arm/arm64: Re-check VMA on detecting a poisoned page") changed
kvm_send_hwpoison_signal() to use the page shift instead of the VMA because
at that point the code had already released the mmap lock, which means
userspace could have modified the VMA.

If userspace uses hugetlbfs for the VM memory, user_mem_abort() tries to
map the guest fault IPA using block mappings in stage 2. That is not always
possible, if, for example, userspace uses dirty page logging for the VM.
Update the page shift appropriately in those cases when we downgrade the
stage 2 entry from a block mapping to a page.

Fixes: 1559b7583ff6 ("KVM: arm/arm64: Re-check VMA on detecting a poisoned page")
Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 arch/arm64/kvm/mmu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index ba00bcc0c884..25e7dc52c086 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -1877,6 +1877,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 	    !fault_supports_stage2_huge_mapping(memslot, hva, vma_pagesize)) {
 		force_pte = true;
 		vma_pagesize = PAGE_SIZE;
+		vma_shift = PAGE_SHIFT;
 	}
 
 	/*
-- 
2.28.0

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH 1/2] KVM: arm64: Update page shift if stage 2 block mapping not supported

[Alexandru Elisei]

Commit 196f878a7ac2e (" KVM: arm/arm64: Signal SIGBUS when stage2 discovers
hwpoison memory") modifies user_mem_abort() to send a SIGBUS signal when
the fault IPA maps to a hwpoisoned page. Commit 1559b7583ff6 ("KVM:
arm/arm64: Re-check VMA on detecting a poisoned page") changed
kvm_send_hwpoison_signal() to use the page shift instead of the VMA because
at that point the code had already released the mmap lock, which means
userspace could have modified the VMA.

If userspace uses hugetlbfs for the VM memory, user_mem_abort() tries to
map the guest fault IPA using block mappings in stage 2. That is not always
possible, if, for example, userspace uses dirty page logging for the VM.
Update the page shift appropriately in those cases when we downgrade the
stage 2 entry from a block mapping to a page.

Fixes: 1559b7583ff6 ("KVM: arm/arm64: Re-check VMA on detecting a poisoned page")
Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 arch/arm64/kvm/mmu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index ba00bcc0c884..25e7dc52c086 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -1877,6 +1877,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 	    !fault_supports_stage2_huge_mapping(memslot, hva, vma_pagesize)) {
 		force_pte = true;
 		vma_pagesize = PAGE_SIZE;
+		vma_shift = PAGE_SHIFT;
 	}
 
 	/*
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] KVM: arm64: Update page shift if stage 2 block mapping not supported

[Gavin Shan]

On 9/1/20 11:33 PM, Alexandru Elisei wrote:
> Commit 196f878a7ac2e (" KVM: arm/arm64: Signal SIGBUS when stage2 discovers
> hwpoison memory") modifies user_mem_abort() to send a SIGBUS signal when
> the fault IPA maps to a hwpoisoned page. Commit 1559b7583ff6 ("KVM:
> arm/arm64: Re-check VMA on detecting a poisoned page") changed
> kvm_send_hwpoison_signal() to use the page shift instead of the VMA because
> at that point the code had already released the mmap lock, which means
> userspace could have modified the VMA.
> 
> If userspace uses hugetlbfs for the VM memory, user_mem_abort() tries to
> map the guest fault IPA using block mappings in stage 2. That is not always
> possible, if, for example, userspace uses dirty page logging for the VM.
> Update the page shift appropriately in those cases when we downgrade the
> stage 2 entry from a block mapping to a page.
> 
> Fixes: 1559b7583ff6 ("KVM: arm/arm64: Re-check VMA on detecting a poisoned page")
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---

Reviewed-by: Gavin Shan <gshan@redhat.com>

>   arch/arm64/kvm/mmu.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index ba00bcc0c884..25e7dc52c086 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -1877,6 +1877,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>   	    !fault_supports_stage2_huge_mapping(memslot, hva, vma_pagesize)) {
>   		force_pte = true;
>   		vma_pagesize = PAGE_SIZE;
> +		vma_shift = PAGE_SHIFT;
>   	}
>   
>   	/*
> 

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 1/2] KVM: arm64: Update page shift if stage 2 block mapping not supported

[Gavin Shan]

On 9/1/20 11:33 PM, Alexandru Elisei wrote:
> Commit 196f878a7ac2e (" KVM: arm/arm64: Signal SIGBUS when stage2 discovers
> hwpoison memory") modifies user_mem_abort() to send a SIGBUS signal when
> the fault IPA maps to a hwpoisoned page. Commit 1559b7583ff6 ("KVM:
> arm/arm64: Re-check VMA on detecting a poisoned page") changed
> kvm_send_hwpoison_signal() to use the page shift instead of the VMA because
> at that point the code had already released the mmap lock, which means
> userspace could have modified the VMA.
> 
> If userspace uses hugetlbfs for the VM memory, user_mem_abort() tries to
> map the guest fault IPA using block mappings in stage 2. That is not always
> possible, if, for example, userspace uses dirty page logging for the VM.
> Update the page shift appropriately in those cases when we downgrade the
> stage 2 entry from a block mapping to a page.
> 
> Fixes: 1559b7583ff6 ("KVM: arm/arm64: Re-check VMA on detecting a poisoned page")
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---

Reviewed-by: Gavin Shan <gshan@redhat.com>

>   arch/arm64/kvm/mmu.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index ba00bcc0c884..25e7dc52c086 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -1877,6 +1877,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>   	    !fault_supports_stage2_huge_mapping(memslot, hva, vma_pagesize)) {
>   		force_pte = true;
>   		vma_pagesize = PAGE_SIZE;
> +		vma_shift = PAGE_SHIFT;
>   	}
>   
>   	/*
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/2] KVM: arm64: Try PMD block mappings if PUD mappings are not supported

[Alexandru Elisei]

When userspace uses hugetlbfs for the VM memory, user_mem_abort() tries to
use the same block size to map the faulting IPA in stage 2. If stage 2
cannot use the same size mapping because the block size doesn't fit in the
memslot or the memslot is not properly aligned, user_mem_abort() will fall
back to a page mapping, regardless of the block size. We can do better for
PUD backed hugetlbfs by checking if a PMD block mapping is possible before
deciding to use a page.

vma_pagesize is an unsigned long, use 1UL instead of 1ULL when assigning
its value.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 arch/arm64/kvm/mmu.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 25e7dc52c086..f590f7355cda 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -1871,15 +1871,24 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 	else
 		vma_shift = PAGE_SHIFT;
 
-	vma_pagesize = 1ULL << vma_shift;
 	if (logging_active ||
-	    (vma->vm_flags & VM_PFNMAP) ||
-	    !fault_supports_stage2_huge_mapping(memslot, hva, vma_pagesize)) {
+	    (vma->vm_flags & VM_PFNMAP)) {
 		force_pte = true;
-		vma_pagesize = PAGE_SIZE;
 		vma_shift = PAGE_SHIFT;
 	}
 
+	if (vma_shift == PUD_SHIFT &&
+	    !fault_supports_stage2_huge_mapping(memslot, hva, PUD_SIZE))
+		vma_shift = PMD_SHIFT;
+
+	if (vma_shift == PMD_SHIFT &&
+	    !fault_supports_stage2_huge_mapping(memslot, hva, PMD_SIZE)) {
+		force_pte = true;
+		vma_shift = PAGE_SHIFT;
+	}
+
+	vma_pagesize = 1UL << vma_shift;
+
 	/*
 	 * The stage2 has a minimum of 2 level table (For arm64 see
 	 * kvm_arm_setup_stage2()). Hence, we are guaranteed that we can
@@ -1889,7 +1898,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 	 */
 	if (vma_pagesize == PMD_SIZE ||
 	    (vma_pagesize == PUD_SIZE && kvm_stage2_has_pmd(kvm)))
-		gfn = (fault_ipa & huge_page_mask(hstate_vma(vma))) >> PAGE_SHIFT;
+		gfn = (fault_ipa & ~(vma_pagesize - 1)) >> PAGE_SHIFT;
 	mmap_read_unlock(current->mm);
 
 	/* We need minimum second+third level pages */
-- 
2.28.0

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH 2/2] KVM: arm64: Try PMD block mappings if PUD mappings are not supported

[Alexandru Elisei]

When userspace uses hugetlbfs for the VM memory, user_mem_abort() tries to
use the same block size to map the faulting IPA in stage 2. If stage 2
cannot use the same size mapping because the block size doesn't fit in the
memslot or the memslot is not properly aligned, user_mem_abort() will fall
back to a page mapping, regardless of the block size. We can do better for
PUD backed hugetlbfs by checking if a PMD block mapping is possible before
deciding to use a page.

vma_pagesize is an unsigned long, use 1UL instead of 1ULL when assigning
its value.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 arch/arm64/kvm/mmu.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 25e7dc52c086..f590f7355cda 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -1871,15 +1871,24 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 	else
 		vma_shift = PAGE_SHIFT;
 
-	vma_pagesize = 1ULL << vma_shift;
 	if (logging_active ||
-	    (vma->vm_flags & VM_PFNMAP) ||
-	    !fault_supports_stage2_huge_mapping(memslot, hva, vma_pagesize)) {
+	    (vma->vm_flags & VM_PFNMAP)) {
 		force_pte = true;
-		vma_pagesize = PAGE_SIZE;
 		vma_shift = PAGE_SHIFT;
 	}
 
+	if (vma_shift == PUD_SHIFT &&
+	    !fault_supports_stage2_huge_mapping(memslot, hva, PUD_SIZE))
+		vma_shift = PMD_SHIFT;
+
+	if (vma_shift == PMD_SHIFT &&
+	    !fault_supports_stage2_huge_mapping(memslot, hva, PMD_SIZE)) {
+		force_pte = true;
+		vma_shift = PAGE_SHIFT;
+	}
+
+	vma_pagesize = 1UL << vma_shift;
+
 	/*
 	 * The stage2 has a minimum of 2 level table (For arm64 see
 	 * kvm_arm_setup_stage2()). Hence, we are guaranteed that we can
@@ -1889,7 +1898,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 	 */
 	if (vma_pagesize == PMD_SIZE ||
 	    (vma_pagesize == PUD_SIZE && kvm_stage2_has_pmd(kvm)))
-		gfn = (fault_ipa & huge_page_mask(hstate_vma(vma))) >> PAGE_SHIFT;
+		gfn = (fault_ipa & ~(vma_pagesize - 1)) >> PAGE_SHIFT;
 	mmap_read_unlock(current->mm);
 
 	/* We need minimum second+third level pages */
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] KVM: arm64: Try PMD block mappings if PUD mappings are not supported

[Gavin Shan]

Hi Alexandru,

On 9/1/20 11:33 PM, Alexandru Elisei wrote:
> When userspace uses hugetlbfs for the VM memory, user_mem_abort() tries to
> use the same block size to map the faulting IPA in stage 2. If stage 2
> cannot use the same size mapping because the block size doesn't fit in the
> memslot or the memslot is not properly aligned, user_mem_abort() will fall
> back to a page mapping, regardless of the block size. We can do better for
> PUD backed hugetlbfs by checking if a PMD block mapping is possible before
> deciding to use a page.
> 
> vma_pagesize is an unsigned long, use 1UL instead of 1ULL when assigning
> its value.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>   arch/arm64/kvm/mmu.c | 19 ++++++++++++++-----
>   1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 25e7dc52c086..f590f7355cda 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -1871,15 +1871,24 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>   	else
>   		vma_shift = PAGE_SHIFT;
>   
> -	vma_pagesize = 1ULL << vma_shift;
>   	if (logging_active ||
> -	    (vma->vm_flags & VM_PFNMAP) ||
> -	    !fault_supports_stage2_huge_mapping(memslot, hva, vma_pagesize)) {
> +	    (vma->vm_flags & VM_PFNMAP)) {
>   		force_pte = true;
> -		vma_pagesize = PAGE_SIZE;
>   		vma_shift = PAGE_SHIFT;
>   	}
>  

It looks incorrect because @vma_pagesize wasn't initialized when
it's passed to fault_supports_stage2_huge_mapping() for the checking.
It's assumed you missed the following changes according to the commit
log:

    fault_supports_stage2_huge_mapping(memslot, hva, (1UL << vma_shift))
  
> +	if (vma_shift == PUD_SHIFT &&
> +	    !fault_supports_stage2_huge_mapping(memslot, hva, PUD_SIZE))
> +		vma_shift = PMD_SHIFT;
> +
> +	if (vma_shift == PMD_SHIFT &&
> +	    !fault_supports_stage2_huge_mapping(memslot, hva, PMD_SIZE)) {
> +		force_pte = true;
> +		vma_shift = PAGE_SHIFT;
> +	}
> +
> +	vma_pagesize = 1UL << vma_shift;
> +>   	/*
>   	 * The stage2 has a minimum of 2 level table (For arm64 see
>   	 * kvm_arm_setup_stage2()). Hence, we are guaranteed that we can
> @@ -1889,7 +1898,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>   	 */
>   	if (vma_pagesize == PMD_SIZE ||
>   	    (vma_pagesize == PUD_SIZE && kvm_stage2_has_pmd(kvm)))
> -		gfn = (fault_ipa & huge_page_mask(hstate_vma(vma))) >> PAGE_SHIFT;
> +		gfn = (fault_ipa & ~(vma_pagesize - 1)) >> PAGE_SHIFT;
>   	mmap_read_unlock(current->mm);
>   
>   	/* We need minimum second+third level pages */
> 

Thanks,
Gavin

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 2/2] KVM: arm64: Try PMD block mappings if PUD mappings are not supported

[Gavin Shan]

Hi Alexandru,

On 9/1/20 11:33 PM, Alexandru Elisei wrote:
> When userspace uses hugetlbfs for the VM memory, user_mem_abort() tries to
> use the same block size to map the faulting IPA in stage 2. If stage 2
> cannot use the same size mapping because the block size doesn't fit in the
> memslot or the memslot is not properly aligned, user_mem_abort() will fall
> back to a page mapping, regardless of the block size. We can do better for
> PUD backed hugetlbfs by checking if a PMD block mapping is possible before
> deciding to use a page.
> 
> vma_pagesize is an unsigned long, use 1UL instead of 1ULL when assigning
> its value.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>   arch/arm64/kvm/mmu.c | 19 ++++++++++++++-----
>   1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 25e7dc52c086..f590f7355cda 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -1871,15 +1871,24 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>   	else
>   		vma_shift = PAGE_SHIFT;
>   
> -	vma_pagesize = 1ULL << vma_shift;
>   	if (logging_active ||
> -	    (vma->vm_flags & VM_PFNMAP) ||
> -	    !fault_supports_stage2_huge_mapping(memslot, hva, vma_pagesize)) {
> +	    (vma->vm_flags & VM_PFNMAP)) {
>   		force_pte = true;
> -		vma_pagesize = PAGE_SIZE;
>   		vma_shift = PAGE_SHIFT;
>   	}
>  

It looks incorrect because @vma_pagesize wasn't initialized when
it's passed to fault_supports_stage2_huge_mapping() for the checking.
It's assumed you missed the following changes according to the commit
log:

    fault_supports_stage2_huge_mapping(memslot, hva, (1UL << vma_shift))
  
> +	if (vma_shift == PUD_SHIFT &&
> +	    !fault_supports_stage2_huge_mapping(memslot, hva, PUD_SIZE))
> +		vma_shift = PMD_SHIFT;
> +
> +	if (vma_shift == PMD_SHIFT &&
> +	    !fault_supports_stage2_huge_mapping(memslot, hva, PMD_SIZE)) {
> +		force_pte = true;
> +		vma_shift = PAGE_SHIFT;
> +	}
> +
> +	vma_pagesize = 1UL << vma_shift;
> +>   	/*
>   	 * The stage2 has a minimum of 2 level table (For arm64 see
>   	 * kvm_arm_setup_stage2()). Hence, we are guaranteed that we can
> @@ -1889,7 +1898,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>   	 */
>   	if (vma_pagesize == PMD_SIZE ||
>   	    (vma_pagesize == PUD_SIZE && kvm_stage2_has_pmd(kvm)))
> -		gfn = (fault_ipa & huge_page_mask(hstate_vma(vma))) >> PAGE_SHIFT;
> +		gfn = (fault_ipa & ~(vma_pagesize - 1)) >> PAGE_SHIFT;
>   	mmap_read_unlock(current->mm);
>   
>   	/* We need minimum second+third level pages */
> 

Thanks,
Gavin


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] KVM: arm64: Try PMD block mappings if PUD mappings are not supported

[Alexandru Elisei]

Hi Gavin,

Many thanks for having a look at the patches!

On 9/2/20 2:23 AM, Gavin Shan wrote:
> Hi Alexandru,
>
> On 9/1/20 11:33 PM, Alexandru Elisei wrote:
>> When userspace uses hugetlbfs for the VM memory, user_mem_abort() tries to
>> use the same block size to map the faulting IPA in stage 2. If stage 2
>> cannot use the same size mapping because the block size doesn't fit in the
>> memslot or the memslot is not properly aligned, user_mem_abort() will fall
>> back to a page mapping, regardless of the block size. We can do better for
>> PUD backed hugetlbfs by checking if a PMD block mapping is possible before
>> deciding to use a page.
>>
>> vma_pagesize is an unsigned long, use 1UL instead of 1ULL when assigning
>> its value.
>>
>> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
>> ---
>>   arch/arm64/kvm/mmu.c | 19 ++++++++++++++-----
>>   1 file changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
>> index 25e7dc52c086..f590f7355cda 100644
>> --- a/arch/arm64/kvm/mmu.c
>> +++ b/arch/arm64/kvm/mmu.c
>> @@ -1871,15 +1871,24 @@ static int user_mem_abort(struct kvm_vcpu *vcpu,
>> phys_addr_t fault_ipa,
>>       else
>>           vma_shift = PAGE_SHIFT;
>>   -    vma_pagesize = 1ULL << vma_shift;
>>       if (logging_active ||
>> -        (vma->vm_flags & VM_PFNMAP) ||
>> -        !fault_supports_stage2_huge_mapping(memslot, hva, vma_pagesize)) {
>> +        (vma->vm_flags & VM_PFNMAP)) {
>>           force_pte = true;
>> -        vma_pagesize = PAGE_SIZE;
>>           vma_shift = PAGE_SHIFT;
>>       }
>>  
>
> It looks incorrect because @vma_pagesize wasn't initialized when
> it's passed to fault_supports_stage2_huge_mapping() for the checking.
> It's assumed you missed the following changes according to the commit
> log:
>
>    fault_supports_stage2_huge_mapping(memslot, hva, (1UL << vma_shift))

I'm not sure what you mean. Maybe you've misread the diff? Because the above call
to fault_supports_stage2_huge_mapping() was removed by the patch.

Thanks,

Alex

>  
>> +    if (vma_shift == PUD_SHIFT &&
>> +        !fault_supports_stage2_huge_mapping(memslot, hva, PUD_SIZE))
>> +        vma_shift = PMD_SHIFT;
>> +
>> +    if (vma_shift == PMD_SHIFT &&
>> +        !fault_supports_stage2_huge_mapping(memslot, hva, PMD_SIZE)) {
>> +        force_pte = true;
>> +        vma_shift = PAGE_SHIFT;
>> +    }
>> +
>> +    vma_pagesize = 1UL << vma_shift;
>> +>       /*
>>        * The stage2 has a minimum of 2 level table (For arm64 see
>>        * kvm_arm_setup_stage2()). Hence, we are guaranteed that we can
>> @@ -1889,7 +1898,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu,
>> phys_addr_t fault_ipa,
>>        */
>>       if (vma_pagesize == PMD_SIZE ||
>>           (vma_pagesize == PUD_SIZE && kvm_stage2_has_pmd(kvm)))
>> -        gfn = (fault_ipa & huge_page_mask(hstate_vma(vma))) >> PAGE_SHIFT;
>> +        gfn = (fault_ipa & ~(vma_pagesize - 1)) >> PAGE_SHIFT;
>>       mmap_read_unlock(current->mm);
>>         /* We need minimum second+third level pages */
>>
>
> Thanks,
> Gavin
>
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 2/2] KVM: arm64: Try PMD block mappings if PUD mappings are not supported

[Alexandru Elisei]

Hi Gavin,

Many thanks for having a look at the patches!

On 9/2/20 2:23 AM, Gavin Shan wrote:
> Hi Alexandru,
>
> On 9/1/20 11:33 PM, Alexandru Elisei wrote:
>> When userspace uses hugetlbfs for the VM memory, user_mem_abort() tries to
>> use the same block size to map the faulting IPA in stage 2. If stage 2
>> cannot use the same size mapping because the block size doesn't fit in the
>> memslot or the memslot is not properly aligned, user_mem_abort() will fall
>> back to a page mapping, regardless of the block size. We can do better for
>> PUD backed hugetlbfs by checking if a PMD block mapping is possible before
>> deciding to use a page.
>>
>> vma_pagesize is an unsigned long, use 1UL instead of 1ULL when assigning
>> its value.
>>
>> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
>> ---
>>   arch/arm64/kvm/mmu.c | 19 ++++++++++++++-----
>>   1 file changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
>> index 25e7dc52c086..f590f7355cda 100644
>> --- a/arch/arm64/kvm/mmu.c
>> +++ b/arch/arm64/kvm/mmu.c
>> @@ -1871,15 +1871,24 @@ static int user_mem_abort(struct kvm_vcpu *vcpu,
>> phys_addr_t fault_ipa,
>>       else
>>           vma_shift = PAGE_SHIFT;
>>   -    vma_pagesize = 1ULL << vma_shift;
>>       if (logging_active ||
>> -        (vma->vm_flags & VM_PFNMAP) ||
>> -        !fault_supports_stage2_huge_mapping(memslot, hva, vma_pagesize)) {
>> +        (vma->vm_flags & VM_PFNMAP)) {
>>           force_pte = true;
>> -        vma_pagesize = PAGE_SIZE;
>>           vma_shift = PAGE_SHIFT;
>>       }
>>  
>
> It looks incorrect because @vma_pagesize wasn't initialized when
> it's passed to fault_supports_stage2_huge_mapping() for the checking.
> It's assumed you missed the following changes according to the commit
> log:
>
>    fault_supports_stage2_huge_mapping(memslot, hva, (1UL << vma_shift))

I'm not sure what you mean. Maybe you've misread the diff? Because the above call
to fault_supports_stage2_huge_mapping() was removed by the patch.

Thanks,

Alex

>  
>> +    if (vma_shift == PUD_SHIFT &&
>> +        !fault_supports_stage2_huge_mapping(memslot, hva, PUD_SIZE))
>> +        vma_shift = PMD_SHIFT;
>> +
>> +    if (vma_shift == PMD_SHIFT &&
>> +        !fault_supports_stage2_huge_mapping(memslot, hva, PMD_SIZE)) {
>> +        force_pte = true;
>> +        vma_shift = PAGE_SHIFT;
>> +    }
>> +
>> +    vma_pagesize = 1UL << vma_shift;
>> +>       /*
>>        * The stage2 has a minimum of 2 level table (For arm64 see
>>        * kvm_arm_setup_stage2()). Hence, we are guaranteed that we can
>> @@ -1889,7 +1898,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu,
>> phys_addr_t fault_ipa,
>>        */
>>       if (vma_pagesize == PMD_SIZE ||
>>           (vma_pagesize == PUD_SIZE && kvm_stage2_has_pmd(kvm)))
>> -        gfn = (fault_ipa & huge_page_mask(hstate_vma(vma))) >> PAGE_SHIFT;
>> +        gfn = (fault_ipa & ~(vma_pagesize - 1)) >> PAGE_SHIFT;
>>       mmap_read_unlock(current->mm);
>>         /* We need minimum second+third level pages */
>>
>
> Thanks,
> Gavin
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] KVM: arm64: Try PMD block mappings if PUD mappings are not supported

[Gavin Shan]

Hi Alex,

On 9/2/20 7:01 PM, Alexandru Elisei wrote:
> On 9/2/20 2:23 AM, Gavin Shan wrote:
>> On 9/1/20 11:33 PM, Alexandru Elisei wrote:
>>> When userspace uses hugetlbfs for the VM memory, user_mem_abort() tries to
>>> use the same block size to map the faulting IPA in stage 2. If stage 2
>>> cannot use the same size mapping because the block size doesn't fit in the
>>> memslot or the memslot is not properly aligned, user_mem_abort() will fall
>>> back to a page mapping, regardless of the block size. We can do better for
>>> PUD backed hugetlbfs by checking if a PMD block mapping is possible before
>>> deciding to use a page.
>>>
>>> vma_pagesize is an unsigned long, use 1UL instead of 1ULL when assigning
>>> its value.
>>>
>>> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
>>> ---
>>>    arch/arm64/kvm/mmu.c | 19 ++++++++++++++-----
>>>    1 file changed, 14 insertions(+), 5 deletions(-)
>>>

Reviewed-by: Gavin Shan <gshan@redhat.com>

>>> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
>>> index 25e7dc52c086..f590f7355cda 100644
>>> --- a/arch/arm64/kvm/mmu.c
>>> +++ b/arch/arm64/kvm/mmu.c
>>> @@ -1871,15 +1871,24 @@ static int user_mem_abort(struct kvm_vcpu *vcpu,
>>> phys_addr_t fault_ipa,
>>>        else
>>>            vma_shift = PAGE_SHIFT;
>>>    -    vma_pagesize = 1ULL << vma_shift;
>>>        if (logging_active ||
>>> -        (vma->vm_flags & VM_PFNMAP) ||
>>> -        !fault_supports_stage2_huge_mapping(memslot, hva, vma_pagesize)) {
>>> +        (vma->vm_flags & VM_PFNMAP)) {
>>>            force_pte = true;
>>> -        vma_pagesize = PAGE_SIZE;
>>>            vma_shift = PAGE_SHIFT;
>>>        }
>>>   
>>
>> It looks incorrect because @vma_pagesize wasn't initialized when
>> it's passed to fault_supports_stage2_huge_mapping() for the checking.
>> It's assumed you missed the following changes according to the commit
>> log:
>>
>>     fault_supports_stage2_huge_mapping(memslot, hva, (1UL << vma_shift))
> 
> I'm not sure what you mean. Maybe you've misread the diff? Because the above call
> to fault_supports_stage2_huge_mapping() was removed by the patch.
> 

Yeah, your guess is correct as I looked into the removed code :)

>>   
>>> +    if (vma_shift == PUD_SHIFT &&
>>> +        !fault_supports_stage2_huge_mapping(memslot, hva, PUD_SIZE))
>>> +        vma_shift = PMD_SHIFT;
>>> +
>>> +    if (vma_shift == PMD_SHIFT &&
>>> +        !fault_supports_stage2_huge_mapping(memslot, hva, PMD_SIZE)) {
>>> +        force_pte = true;
>>> +        vma_shift = PAGE_SHIFT;
>>> +    }
>>> +
>>> +    vma_pagesize = 1UL << vma_shift;
>>> +>       /*
>>>         * The stage2 has a minimum of 2 level table (For arm64 see
>>>         * kvm_arm_setup_stage2()). Hence, we are guaranteed that we can
>>> @@ -1889,7 +1898,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu,
>>> phys_addr_t fault_ipa,
>>>         */
>>>        if (vma_pagesize == PMD_SIZE ||
>>>            (vma_pagesize == PUD_SIZE && kvm_stage2_has_pmd(kvm)))
>>> -        gfn = (fault_ipa & huge_page_mask(hstate_vma(vma))) >> PAGE_SHIFT;
>>> +        gfn = (fault_ipa & ~(vma_pagesize - 1)) >> PAGE_SHIFT;
>>>        mmap_read_unlock(current->mm);
>>>          /* We need minimum second+third level pages */
>>>
>>

Thanks,
Gavin

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 2/2] KVM: arm64: Try PMD block mappings if PUD mappings are not supported

[Gavin Shan]

Hi Alex,

On 9/2/20 7:01 PM, Alexandru Elisei wrote:
> On 9/2/20 2:23 AM, Gavin Shan wrote:
>> On 9/1/20 11:33 PM, Alexandru Elisei wrote:
>>> When userspace uses hugetlbfs for the VM memory, user_mem_abort() tries to
>>> use the same block size to map the faulting IPA in stage 2. If stage 2
>>> cannot use the same size mapping because the block size doesn't fit in the
>>> memslot or the memslot is not properly aligned, user_mem_abort() will fall
>>> back to a page mapping, regardless of the block size. We can do better for
>>> PUD backed hugetlbfs by checking if a PMD block mapping is possible before
>>> deciding to use a page.
>>>
>>> vma_pagesize is an unsigned long, use 1UL instead of 1ULL when assigning
>>> its value.
>>>
>>> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
>>> ---
>>>    arch/arm64/kvm/mmu.c | 19 ++++++++++++++-----
>>>    1 file changed, 14 insertions(+), 5 deletions(-)
>>>

Reviewed-by: Gavin Shan <gshan@redhat.com>

>>> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
>>> index 25e7dc52c086..f590f7355cda 100644
>>> --- a/arch/arm64/kvm/mmu.c
>>> +++ b/arch/arm64/kvm/mmu.c
>>> @@ -1871,15 +1871,24 @@ static int user_mem_abort(struct kvm_vcpu *vcpu,
>>> phys_addr_t fault_ipa,
>>>        else
>>>            vma_shift = PAGE_SHIFT;
>>>    -    vma_pagesize = 1ULL << vma_shift;
>>>        if (logging_active ||
>>> -        (vma->vm_flags & VM_PFNMAP) ||
>>> -        !fault_supports_stage2_huge_mapping(memslot, hva, vma_pagesize)) {
>>> +        (vma->vm_flags & VM_PFNMAP)) {
>>>            force_pte = true;
>>> -        vma_pagesize = PAGE_SIZE;
>>>            vma_shift = PAGE_SHIFT;
>>>        }
>>>   
>>
>> It looks incorrect because @vma_pagesize wasn't initialized when
>> it's passed to fault_supports_stage2_huge_mapping() for the checking.
>> It's assumed you missed the following changes according to the commit
>> log:
>>
>>     fault_supports_stage2_huge_mapping(memslot, hva, (1UL << vma_shift))
> 
> I'm not sure what you mean. Maybe you've misread the diff? Because the above call
> to fault_supports_stage2_huge_mapping() was removed by the patch.
> 

Yeah, your guess is correct as I looked into the removed code :)

>>   
>>> +    if (vma_shift == PUD_SHIFT &&
>>> +        !fault_supports_stage2_huge_mapping(memslot, hva, PUD_SIZE))
>>> +        vma_shift = PMD_SHIFT;
>>> +
>>> +    if (vma_shift == PMD_SHIFT &&
>>> +        !fault_supports_stage2_huge_mapping(memslot, hva, PMD_SIZE)) {
>>> +        force_pte = true;
>>> +        vma_shift = PAGE_SHIFT;
>>> +    }
>>> +
>>> +    vma_pagesize = 1UL << vma_shift;
>>> +>       /*
>>>         * The stage2 has a minimum of 2 level table (For arm64 see
>>>         * kvm_arm_setup_stage2()). Hence, we are guaranteed that we can
>>> @@ -1889,7 +1898,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu,
>>> phys_addr_t fault_ipa,
>>>         */
>>>        if (vma_pagesize == PMD_SIZE ||
>>>            (vma_pagesize == PUD_SIZE && kvm_stage2_has_pmd(kvm)))
>>> -        gfn = (fault_ipa & huge_page_mask(hstate_vma(vma))) >> PAGE_SHIFT;
>>> +        gfn = (fault_ipa & ~(vma_pagesize - 1)) >> PAGE_SHIFT;
>>>        mmap_read_unlock(current->mm);
>>>          /* We need minimum second+third level pages */
>>>
>>

Thanks,
Gavin


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] KVM: arm64: Try PMD block mappings if PUD mappings are not supported

[Marc Zyngier]

Hi Alex,

On Tue, 01 Sep 2020 14:33:57 +0100,
Alexandru Elisei <alexandru.elisei@arm.com> wrote:
> 
> When userspace uses hugetlbfs for the VM memory, user_mem_abort() tries to
> use the same block size to map the faulting IPA in stage 2. If stage 2
> cannot use the same size mapping because the block size doesn't fit in the
> memslot or the memslot is not properly aligned, user_mem_abort() will fall
> back to a page mapping, regardless of the block size. We can do better for
> PUD backed hugetlbfs by checking if a PMD block mapping is possible before
> deciding to use a page.
> 
> vma_pagesize is an unsigned long, use 1UL instead of 1ULL when assigning
> its value.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  arch/arm64/kvm/mmu.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 25e7dc52c086..f590f7355cda 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -1871,15 +1871,24 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>  	else
>  		vma_shift = PAGE_SHIFT;
>  
> -	vma_pagesize = 1ULL << vma_shift;
>  	if (logging_active ||
> -	    (vma->vm_flags & VM_PFNMAP) ||
> -	    !fault_supports_stage2_huge_mapping(memslot, hva, vma_pagesize)) {
> +	    (vma->vm_flags & VM_PFNMAP)) {
>  		force_pte = true;
> -		vma_pagesize = PAGE_SIZE;
>  		vma_shift = PAGE_SHIFT;
>  	}
>  
> +	if (vma_shift == PUD_SHIFT &&
> +	    !fault_supports_stage2_huge_mapping(memslot, hva, PUD_SIZE))
> +		vma_shift = PMD_SHIFT;
> +
> +	if (vma_shift == PMD_SHIFT &&
> +	    !fault_supports_stage2_huge_mapping(memslot, hva, PMD_SIZE)) {
> +		force_pte = true;
> +		vma_shift = PAGE_SHIFT;
> +	}
> +
> +	vma_pagesize = 1UL << vma_shift;
> +
>  	/*
>  	 * The stage2 has a minimum of 2 level table (For arm64 see
>  	 * kvm_arm_setup_stage2()). Hence, we are guaranteed that we can
> @@ -1889,7 +1898,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>  	 */
>  	if (vma_pagesize == PMD_SIZE ||
>  	    (vma_pagesize == PUD_SIZE && kvm_stage2_has_pmd(kvm)))
> -		gfn = (fault_ipa & huge_page_mask(hstate_vma(vma))) >> PAGE_SHIFT;
> +		gfn = (fault_ipa & ~(vma_pagesize - 1)) >> PAGE_SHIFT;
>  	mmap_read_unlock(current->mm);
>  
>  	/* We need minimum second+third level pages */

Although this looks like a sensible change, I'm a reluctant to take it
at this stage, given that we already have a bunch of patches from Will
to change the way we deal with PTs.

Could you look into how this could fit into the new code instead?

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 2/2] KVM: arm64: Try PMD block mappings if PUD mappings are not supported

[Marc Zyngier]

Hi Alex,

On Tue, 01 Sep 2020 14:33:57 +0100,
Alexandru Elisei <alexandru.elisei@arm.com> wrote:
> 
> When userspace uses hugetlbfs for the VM memory, user_mem_abort() tries to
> use the same block size to map the faulting IPA in stage 2. If stage 2
> cannot use the same size mapping because the block size doesn't fit in the
> memslot or the memslot is not properly aligned, user_mem_abort() will fall
> back to a page mapping, regardless of the block size. We can do better for
> PUD backed hugetlbfs by checking if a PMD block mapping is possible before
> deciding to use a page.
> 
> vma_pagesize is an unsigned long, use 1UL instead of 1ULL when assigning
> its value.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  arch/arm64/kvm/mmu.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 25e7dc52c086..f590f7355cda 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -1871,15 +1871,24 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>  	else
>  		vma_shift = PAGE_SHIFT;
>  
> -	vma_pagesize = 1ULL << vma_shift;
>  	if (logging_active ||
> -	    (vma->vm_flags & VM_PFNMAP) ||
> -	    !fault_supports_stage2_huge_mapping(memslot, hva, vma_pagesize)) {
> +	    (vma->vm_flags & VM_PFNMAP)) {
>  		force_pte = true;
> -		vma_pagesize = PAGE_SIZE;
>  		vma_shift = PAGE_SHIFT;
>  	}
>  
> +	if (vma_shift == PUD_SHIFT &&
> +	    !fault_supports_stage2_huge_mapping(memslot, hva, PUD_SIZE))
> +		vma_shift = PMD_SHIFT;
> +
> +	if (vma_shift == PMD_SHIFT &&
> +	    !fault_supports_stage2_huge_mapping(memslot, hva, PMD_SIZE)) {
> +		force_pte = true;
> +		vma_shift = PAGE_SHIFT;
> +	}
> +
> +	vma_pagesize = 1UL << vma_shift;
> +
>  	/*
>  	 * The stage2 has a minimum of 2 level table (For arm64 see
>  	 * kvm_arm_setup_stage2()). Hence, we are guaranteed that we can
> @@ -1889,7 +1898,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>  	 */
>  	if (vma_pagesize == PMD_SIZE ||
>  	    (vma_pagesize == PUD_SIZE && kvm_stage2_has_pmd(kvm)))
> -		gfn = (fault_ipa & huge_page_mask(hstate_vma(vma))) >> PAGE_SHIFT;
> +		gfn = (fault_ipa & ~(vma_pagesize - 1)) >> PAGE_SHIFT;
>  	mmap_read_unlock(current->mm);
>  
>  	/* We need minimum second+third level pages */

Although this looks like a sensible change, I'm a reluctant to take it
at this stage, given that we already have a bunch of patches from Will
to change the way we deal with PTs.

Could you look into how this could fit into the new code instead?

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] KVM: arm64: Try PMD block mappings if PUD mappings are not supported

[Alexandru Elisei]

Hi Marc,

On 9/4/20 10:58 AM, Marc Zyngier wrote:
> Hi Alex,
>
> On Tue, 01 Sep 2020 14:33:57 +0100,
> Alexandru Elisei <alexandru.elisei@arm.com> wrote:
>> When userspace uses hugetlbfs for the VM memory, user_mem_abort() tries to
>> use the same block size to map the faulting IPA in stage 2. If stage 2
>> cannot use the same size mapping because the block size doesn't fit in the
>> memslot or the memslot is not properly aligned, user_mem_abort() will fall
>> back to a page mapping, regardless of the block size. We can do better for
>> PUD backed hugetlbfs by checking if a PMD block mapping is possible before
>> deciding to use a page.
>>
>> vma_pagesize is an unsigned long, use 1UL instead of 1ULL when assigning
>> its value.
>>
>> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
>> ---
>>  arch/arm64/kvm/mmu.c | 19 ++++++++++++++-----
>>  1 file changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
>> index 25e7dc52c086..f590f7355cda 100644
>> --- a/arch/arm64/kvm/mmu.c
>> +++ b/arch/arm64/kvm/mmu.c
>> @@ -1871,15 +1871,24 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>>  	else
>>  		vma_shift = PAGE_SHIFT;
>>  
>> -	vma_pagesize = 1ULL << vma_shift;
>>  	if (logging_active ||
>> -	    (vma->vm_flags & VM_PFNMAP) ||
>> -	    !fault_supports_stage2_huge_mapping(memslot, hva, vma_pagesize)) {
>> +	    (vma->vm_flags & VM_PFNMAP)) {
>>  		force_pte = true;
>> -		vma_pagesize = PAGE_SIZE;
>>  		vma_shift = PAGE_SHIFT;
>>  	}
>>  
>> +	if (vma_shift == PUD_SHIFT &&
>> +	    !fault_supports_stage2_huge_mapping(memslot, hva, PUD_SIZE))
>> +		vma_shift = PMD_SHIFT;
>> +
>> +	if (vma_shift == PMD_SHIFT &&
>> +	    !fault_supports_stage2_huge_mapping(memslot, hva, PMD_SIZE)) {
>> +		force_pte = true;
>> +		vma_shift = PAGE_SHIFT;
>> +	}
>> +
>> +	vma_pagesize = 1UL << vma_shift;
>> +
>>  	/*
>>  	 * The stage2 has a minimum of 2 level table (For arm64 see
>>  	 * kvm_arm_setup_stage2()). Hence, we are guaranteed that we can
>> @@ -1889,7 +1898,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>>  	 */
>>  	if (vma_pagesize == PMD_SIZE ||
>>  	    (vma_pagesize == PUD_SIZE && kvm_stage2_has_pmd(kvm)))
>> -		gfn = (fault_ipa & huge_page_mask(hstate_vma(vma))) >> PAGE_SHIFT;
>> +		gfn = (fault_ipa & ~(vma_pagesize - 1)) >> PAGE_SHIFT;
>>  	mmap_read_unlock(current->mm);
>>  
>>  	/* We need minimum second+third level pages */
> Although this looks like a sensible change, I'm a reluctant to take it
> at this stage, given that we already have a bunch of patches from Will
> to change the way we deal with PTs.
>
> Could you look into how this could fit into the new code instead?

Sure, that sounds very sensible. I'm in the process of reviewing Will's series,
and after I'm done I'll rebase this on top of his patches and send it as v2. Does
that sound ok to you? Or do you want me to base this patch on one of your branches?

Thanks,
Alex
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 2/2] KVM: arm64: Try PMD block mappings if PUD mappings are not supported

[Alexandru Elisei]

Hi Marc,

On 9/4/20 10:58 AM, Marc Zyngier wrote:
> Hi Alex,
>
> On Tue, 01 Sep 2020 14:33:57 +0100,
> Alexandru Elisei <alexandru.elisei@arm.com> wrote:
>> When userspace uses hugetlbfs for the VM memory, user_mem_abort() tries to
>> use the same block size to map the faulting IPA in stage 2. If stage 2
>> cannot use the same size mapping because the block size doesn't fit in the
>> memslot or the memslot is not properly aligned, user_mem_abort() will fall
>> back to a page mapping, regardless of the block size. We can do better for
>> PUD backed hugetlbfs by checking if a PMD block mapping is possible before
>> deciding to use a page.
>>
>> vma_pagesize is an unsigned long, use 1UL instead of 1ULL when assigning
>> its value.
>>
>> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
>> ---
>>  arch/arm64/kvm/mmu.c | 19 ++++++++++++++-----
>>  1 file changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
>> index 25e7dc52c086..f590f7355cda 100644
>> --- a/arch/arm64/kvm/mmu.c
>> +++ b/arch/arm64/kvm/mmu.c
>> @@ -1871,15 +1871,24 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>>  	else
>>  		vma_shift = PAGE_SHIFT;
>>  
>> -	vma_pagesize = 1ULL << vma_shift;
>>  	if (logging_active ||
>> -	    (vma->vm_flags & VM_PFNMAP) ||
>> -	    !fault_supports_stage2_huge_mapping(memslot, hva, vma_pagesize)) {
>> +	    (vma->vm_flags & VM_PFNMAP)) {
>>  		force_pte = true;
>> -		vma_pagesize = PAGE_SIZE;
>>  		vma_shift = PAGE_SHIFT;
>>  	}
>>  
>> +	if (vma_shift == PUD_SHIFT &&
>> +	    !fault_supports_stage2_huge_mapping(memslot, hva, PUD_SIZE))
>> +		vma_shift = PMD_SHIFT;
>> +
>> +	if (vma_shift == PMD_SHIFT &&
>> +	    !fault_supports_stage2_huge_mapping(memslot, hva, PMD_SIZE)) {
>> +		force_pte = true;
>> +		vma_shift = PAGE_SHIFT;
>> +	}
>> +
>> +	vma_pagesize = 1UL << vma_shift;
>> +
>>  	/*
>>  	 * The stage2 has a minimum of 2 level table (For arm64 see
>>  	 * kvm_arm_setup_stage2()). Hence, we are guaranteed that we can
>> @@ -1889,7 +1898,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>>  	 */
>>  	if (vma_pagesize == PMD_SIZE ||
>>  	    (vma_pagesize == PUD_SIZE && kvm_stage2_has_pmd(kvm)))
>> -		gfn = (fault_ipa & huge_page_mask(hstate_vma(vma))) >> PAGE_SHIFT;
>> +		gfn = (fault_ipa & ~(vma_pagesize - 1)) >> PAGE_SHIFT;
>>  	mmap_read_unlock(current->mm);
>>  
>>  	/* We need minimum second+third level pages */
> Although this looks like a sensible change, I'm a reluctant to take it
> at this stage, given that we already have a bunch of patches from Will
> to change the way we deal with PTs.
>
> Could you look into how this could fit into the new code instead?

Sure, that sounds very sensible. I'm in the process of reviewing Will's series,
and after I'm done I'll rebase this on top of his patches and send it as v2. Does
that sound ok to you? Or do you want me to base this patch on one of your branches?

Thanks,
Alex

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] KVM: arm64: Try PMD block mappings if PUD mappings are not supported

[Marc Zyngier]

On 2020-09-08 13:23, Alexandru Elisei wrote:
> Hi Marc,
> 
> On 9/4/20 10:58 AM, Marc Zyngier wrote:
>> Hi Alex,
>> 
>> On Tue, 01 Sep 2020 14:33:57 +0100,
>> Alexandru Elisei <alexandru.elisei@arm.com> wrote:
>>> When userspace uses hugetlbfs for the VM memory, user_mem_abort() 
>>> tries to
>>> use the same block size to map the faulting IPA in stage 2. If stage 
>>> 2
>>> cannot use the same size mapping because the block size doesn't fit 
>>> in the
>>> memslot or the memslot is not properly aligned, user_mem_abort() will 
>>> fall
>>> back to a page mapping, regardless of the block size. We can do 
>>> better for
>>> PUD backed hugetlbfs by checking if a PMD block mapping is possible 
>>> before
>>> deciding to use a page.
>>> 
>>> vma_pagesize is an unsigned long, use 1UL instead of 1ULL when 
>>> assigning
>>> its value.
>>> 
>>> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
>>> ---
>>>  arch/arm64/kvm/mmu.c | 19 ++++++++++++++-----
>>>  1 file changed, 14 insertions(+), 5 deletions(-)
>>> 
>>> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
>>> index 25e7dc52c086..f590f7355cda 100644
>>> --- a/arch/arm64/kvm/mmu.c
>>> +++ b/arch/arm64/kvm/mmu.c
>>> @@ -1871,15 +1871,24 @@ static int user_mem_abort(struct kvm_vcpu 
>>> *vcpu, phys_addr_t fault_ipa,
>>>  	else
>>>  		vma_shift = PAGE_SHIFT;
>>> 
>>> -	vma_pagesize = 1ULL << vma_shift;
>>>  	if (logging_active ||
>>> -	    (vma->vm_flags & VM_PFNMAP) ||
>>> -	    !fault_supports_stage2_huge_mapping(memslot, hva, 
>>> vma_pagesize)) {
>>> +	    (vma->vm_flags & VM_PFNMAP)) {
>>>  		force_pte = true;
>>> -		vma_pagesize = PAGE_SIZE;
>>>  		vma_shift = PAGE_SHIFT;
>>>  	}
>>> 
>>> +	if (vma_shift == PUD_SHIFT &&
>>> +	    !fault_supports_stage2_huge_mapping(memslot, hva, PUD_SIZE))
>>> +		vma_shift = PMD_SHIFT;
>>> +
>>> +	if (vma_shift == PMD_SHIFT &&
>>> +	    !fault_supports_stage2_huge_mapping(memslot, hva, PMD_SIZE)) {
>>> +		force_pte = true;
>>> +		vma_shift = PAGE_SHIFT;
>>> +	}
>>> +
>>> +	vma_pagesize = 1UL << vma_shift;
>>> +
>>>  	/*
>>>  	 * The stage2 has a minimum of 2 level table (For arm64 see
>>>  	 * kvm_arm_setup_stage2()). Hence, we are guaranteed that we can
>>> @@ -1889,7 +1898,7 @@ static int user_mem_abort(struct kvm_vcpu 
>>> *vcpu, phys_addr_t fault_ipa,
>>>  	 */
>>>  	if (vma_pagesize == PMD_SIZE ||
>>>  	    (vma_pagesize == PUD_SIZE && kvm_stage2_has_pmd(kvm)))
>>> -		gfn = (fault_ipa & huge_page_mask(hstate_vma(vma))) >> PAGE_SHIFT;
>>> +		gfn = (fault_ipa & ~(vma_pagesize - 1)) >> PAGE_SHIFT;
>>>  	mmap_read_unlock(current->mm);
>>> 
>>>  	/* We need minimum second+third level pages */
>> Although this looks like a sensible change, I'm a reluctant to take it
>> at this stage, given that we already have a bunch of patches from Will
>> to change the way we deal with PTs.
>> 
>> Could you look into how this could fit into the new code instead?
> 
> Sure, that sounds very sensible. I'm in the process of reviewing Will's 
> series,
> and after I'm done I'll rebase this on top of his patches and send it
> as v2. Does
> that sound ok to you? Or do you want me to base this patch on one of
> your branches?

Either way is fine (kvmarm/next has his patches). Just let me know
what this is based on when you  post the patches.

         M.
-- 
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 2/2] KVM: arm64: Try PMD block mappings if PUD mappings are not supported

[Marc Zyngier]

On 2020-09-08 13:23, Alexandru Elisei wrote:
> Hi Marc,
> 
> On 9/4/20 10:58 AM, Marc Zyngier wrote:
>> Hi Alex,
>> 
>> On Tue, 01 Sep 2020 14:33:57 +0100,
>> Alexandru Elisei <alexandru.elisei@arm.com> wrote:
>>> When userspace uses hugetlbfs for the VM memory, user_mem_abort() 
>>> tries to
>>> use the same block size to map the faulting IPA in stage 2. If stage 
>>> 2
>>> cannot use the same size mapping because the block size doesn't fit 
>>> in the
>>> memslot or the memslot is not properly aligned, user_mem_abort() will 
>>> fall
>>> back to a page mapping, regardless of the block size. We can do 
>>> better for
>>> PUD backed hugetlbfs by checking if a PMD block mapping is possible 
>>> before
>>> deciding to use a page.
>>> 
>>> vma_pagesize is an unsigned long, use 1UL instead of 1ULL when 
>>> assigning
>>> its value.
>>> 
>>> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
>>> ---
>>>  arch/arm64/kvm/mmu.c | 19 ++++++++++++++-----
>>>  1 file changed, 14 insertions(+), 5 deletions(-)
>>> 
>>> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
>>> index 25e7dc52c086..f590f7355cda 100644
>>> --- a/arch/arm64/kvm/mmu.c
>>> +++ b/arch/arm64/kvm/mmu.c
>>> @@ -1871,15 +1871,24 @@ static int user_mem_abort(struct kvm_vcpu 
>>> *vcpu, phys_addr_t fault_ipa,
>>>  	else
>>>  		vma_shift = PAGE_SHIFT;
>>> 
>>> -	vma_pagesize = 1ULL << vma_shift;
>>>  	if (logging_active ||
>>> -	    (vma->vm_flags & VM_PFNMAP) ||
>>> -	    !fault_supports_stage2_huge_mapping(memslot, hva, 
>>> vma_pagesize)) {
>>> +	    (vma->vm_flags & VM_PFNMAP)) {
>>>  		force_pte = true;
>>> -		vma_pagesize = PAGE_SIZE;
>>>  		vma_shift = PAGE_SHIFT;
>>>  	}
>>> 
>>> +	if (vma_shift == PUD_SHIFT &&
>>> +	    !fault_supports_stage2_huge_mapping(memslot, hva, PUD_SIZE))
>>> +		vma_shift = PMD_SHIFT;
>>> +
>>> +	if (vma_shift == PMD_SHIFT &&
>>> +	    !fault_supports_stage2_huge_mapping(memslot, hva, PMD_SIZE)) {
>>> +		force_pte = true;
>>> +		vma_shift = PAGE_SHIFT;
>>> +	}
>>> +
>>> +	vma_pagesize = 1UL << vma_shift;
>>> +
>>>  	/*
>>>  	 * The stage2 has a minimum of 2 level table (For arm64 see
>>>  	 * kvm_arm_setup_stage2()). Hence, we are guaranteed that we can
>>> @@ -1889,7 +1898,7 @@ static int user_mem_abort(struct kvm_vcpu 
>>> *vcpu, phys_addr_t fault_ipa,
>>>  	 */
>>>  	if (vma_pagesize == PMD_SIZE ||
>>>  	    (vma_pagesize == PUD_SIZE && kvm_stage2_has_pmd(kvm)))
>>> -		gfn = (fault_ipa & huge_page_mask(hstate_vma(vma))) >> PAGE_SHIFT;
>>> +		gfn = (fault_ipa & ~(vma_pagesize - 1)) >> PAGE_SHIFT;
>>>  	mmap_read_unlock(current->mm);
>>> 
>>>  	/* We need minimum second+third level pages */
>> Although this looks like a sensible change, I'm a reluctant to take it
>> at this stage, given that we already have a bunch of patches from Will
>> to change the way we deal with PTs.
>> 
>> Could you look into how this could fit into the new code instead?
> 
> Sure, that sounds very sensible. I'm in the process of reviewing Will's 
> series,
> and after I'm done I'll rebase this on top of his patches and send it
> as v2. Does
> that sound ok to you? Or do you want me to base this patch on one of
> your branches?

Either way is fine (kvmarm/next has his patches). Just let me know
what this is based on when you  post the patches.

         M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] KVM: arm64: user_mem_abort() improvements

[Marc Zyngier]

On Tue, 1 Sep 2020 14:33:55 +0100, Alexandru Elisei wrote:
> The first patch is a fix for a bug that I found by code inspection.
> 
> The second patch is an enhancement for the way user_mem_abort() handles
> hugetlbfs backed VM memory.
> 
> Tested on a rockpro64 with 4K pages and hugetlbfs hugepagesz=1G (PUD sized
> block mappings).  First test, guest RAM starts at 0x8100 0000
> (memslot->base_gfn not aligned to 1GB); second test, guest RAM starts at
> 0x8000 0000, but is only 512 MB.  In both cases using PUD mappings is not
> possible because either the memslot base address is not aligned, or the
> mapping would extend beyond the memslot.
> 
> [...]

Applied to next, thanks!

[1/2] KVM: arm64: Update page shift if stage 2 block mapping not supported
      commit: 7b75cd5128421c673153efb1236705696a1a9812

Cheers,

	M.
-- 
Without deviation from the norm, progress is not possible.


_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 0/2] KVM: arm64: user_mem_abort() improvements

[Marc Zyngier]

On Tue, 1 Sep 2020 14:33:55 +0100, Alexandru Elisei wrote:
> The first patch is a fix for a bug that I found by code inspection.
> 
> The second patch is an enhancement for the way user_mem_abort() handles
> hugetlbfs backed VM memory.
> 
> Tested on a rockpro64 with 4K pages and hugetlbfs hugepagesz=1G (PUD sized
> block mappings).  First test, guest RAM starts at 0x8100 0000
> (memslot->base_gfn not aligned to 1GB); second test, guest RAM starts at
> 0x8000 0000, but is only 512 MB.  In both cases using PUD mappings is not
> possible because either the memslot base address is not aligned, or the
> mapping would extend beyond the memslot.
> 
> [...]

Applied to next, thanks!

[1/2] KVM: arm64: Update page shift if stage 2 block mapping not supported
      commit: 7b75cd5128421c673153efb1236705696a1a9812

Cheers,

	M.
-- 
Without deviation from the norm, progress is not possible.



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel