Re: [PATCH] kvm: arm: Enforce PTE mappings at stage2 when needed

2019-03-17 Thread Zenghui Yu

Hi Suzuki,

On 2019/3/12 17:52, Suzuki K Poulose wrote:

commit 6794ad5443a2118 ("KVM: arm/arm64: Fix unintended stage 2 PMD mappings")
made the checks to skip huge mappings, stricter. However it introduced
a bug where we still use huge mappings, ignoring the flag to
use PTE mappings, by not reseting the vma_pagesize to PAGE_SIZE.

Also, the checks do not cover the PUD huge pages, that was
under review during the same period. This patch fixes both
the issues.

Fixes : 6794ad5443a2118 ("KVM: arm/arm64: Fix unintended stage 2 PMD mappings")
Reported-by: Zenghui Yu 
Cc: Zenghui Yu 
Cc: Christoffer Dall 
Cc: Marc Zyngier 
Signed-off-by: Suzuki K Poulose 
---


Thanks for Cc-ing me. It works fine now!


zenghui


  virt/kvm/arm/mmu.c | 43 +--
  1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
index 30251e2..66e0fbb5 100644
--- a/virt/kvm/arm/mmu.c
+++ b/virt/kvm/arm/mmu.c
@@ -1595,8 +1595,9 @@ static void kvm_send_hwpoison_signal(unsigned long 
address,
send_sig_mceerr(BUS_MCEERR_AR, (void __user *)address, lsb, current);
  }
  
-static bool fault_supports_stage2_pmd_mappings(struct kvm_memory_slot *memslot,

-  unsigned long hva)
+static bool fault_supports_stage2_huge_mapping(struct kvm_memory_slot *memslot,
+  unsigned long hva,
+  unsigned long map_size)
  {
gpa_t gpa_start, gpa_end;
hva_t uaddr_start, uaddr_end;
@@ -1612,34 +1613,34 @@ static bool fault_supports_stage2_pmd_mappings(struct 
kvm_memory_slot *memslot,
  
  	/*

 * Pages belonging to memslots that don't have the same alignment
-* within a PMD for userspace and IPA cannot be mapped with stage-2
-* PMD entries, because we'll end up mapping the wrong pages.
+* within a PMD/PUD for userspace and IPA cannot be mapped with stage-2
+* PMD/PUD entries, because we'll end up mapping the wrong pages.
 *
 * Consider a layout like the following:
 *
 *memslot->userspace_addr:
 *+-+++---+
-*|abcde|fgh  Stage-1 PMD|Stage-1 PMD   tv|xyz|
+*|abcde|fgh  Stage-1 block  |Stage-1 block tv|xyz|
 *+-+++---+
 *
 *memslot->base_gfn << PAGE_SIZE:
 *  +---+++-+
-*  |abc|def  Stage-2 PMD|Stage-2 PMD |tvxyz|
+*  |abc|def  Stage-2 block  |Stage-2 block   |tvxyz|
 *  +---+++-+
 *
-* If we create those stage-2 PMDs, we'll end up with this incorrect
+* If we create those stage-2 blocks, we'll end up with this incorrect
 * mapping:
 *   d -> f
 *   e -> g
 *   f -> h
 */
-   if ((gpa_start & ~S2_PMD_MASK) != (uaddr_start & ~S2_PMD_MASK))
+   if ((gpa_start & (map_size - 1)) != (uaddr_start & (map_size - 1)))
return false;
  
  	/*

 * Next, let's make sure we're not trying to map anything not covered
-* by the memslot. This means we have to prohibit PMD size mappings
-* for the beginning and end of a non-PMD aligned and non-PMD sized
+* by the memslot. This means we have to prohibit block size mappings
+* for the beginning and end of a non-block aligned and non-block sized
 * memory slot (illustrated by the head and tail parts of the
 * userspace view above containing pages 'abcde' and 'xyz',
 * respectively).
@@ -1648,8 +1649,8 @@ static bool fault_supports_stage2_pmd_mappings(struct 
kvm_memory_slot *memslot,
 * userspace_addr or the base_gfn, as both are equally aligned (per
 * the check above) and equally sized.
 */
-   return (hva & S2_PMD_MASK) >= uaddr_start &&
-  (hva & S2_PMD_MASK) + S2_PMD_SIZE <= uaddr_end;
+   return (hva & ~(map_size - 1)) >= uaddr_start &&
+  (hva & ~(map_size - 1)) + map_size <= uaddr_end;
  }
  
  static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,

@@ -1678,12 +1679,6 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, 
phys_addr_t fault_ipa,
return -EFAULT;
}
  
-	if (!fault_supports_stage2_pmd_mappings(memslot, hva))

-   force_pte = true;
-
-   if (logging_active)
-   force_pte = true;
-
/* Let's check if we will get back a huge page backed by hugetlbfs */
down_read(>mm->mmap_sem);
vma = find_vma_intersection(current->mm, hva, hva + 1);
@@ -1694,6 +1689,12 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, 
phys_addr_t fault_ipa,
}
  
  	vma_pagesize = vma_kernel_pagesize(vma);

+   if 

[PATCH] kvm: arm: Enforce PTE mappings at stage2 when needed

2019-03-12 Thread Suzuki K Poulose
commit 6794ad5443a2118 ("KVM: arm/arm64: Fix unintended stage 2 PMD mappings")
made the checks to skip huge mappings, stricter. However it introduced
a bug where we still use huge mappings, ignoring the flag to
use PTE mappings, by not reseting the vma_pagesize to PAGE_SIZE.

Also, the checks do not cover the PUD huge pages, that was
under review during the same period. This patch fixes both
the issues.

Fixes : 6794ad5443a2118 ("KVM: arm/arm64: Fix unintended stage 2 PMD mappings")
Reported-by: Zenghui Yu 
Cc: Zenghui Yu 
Cc: Christoffer Dall 
Cc: Marc Zyngier 
Signed-off-by: Suzuki K Poulose 
---
 virt/kvm/arm/mmu.c | 43 +--
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
index 30251e2..66e0fbb5 100644
--- a/virt/kvm/arm/mmu.c
+++ b/virt/kvm/arm/mmu.c
@@ -1595,8 +1595,9 @@ static void kvm_send_hwpoison_signal(unsigned long 
address,
send_sig_mceerr(BUS_MCEERR_AR, (void __user *)address, lsb, current);
 }
 
-static bool fault_supports_stage2_pmd_mappings(struct kvm_memory_slot *memslot,
-  unsigned long hva)
+static bool fault_supports_stage2_huge_mapping(struct kvm_memory_slot *memslot,
+  unsigned long hva,
+  unsigned long map_size)
 {
gpa_t gpa_start, gpa_end;
hva_t uaddr_start, uaddr_end;
@@ -1612,34 +1613,34 @@ static bool fault_supports_stage2_pmd_mappings(struct 
kvm_memory_slot *memslot,
 
/*
 * Pages belonging to memslots that don't have the same alignment
-* within a PMD for userspace and IPA cannot be mapped with stage-2
-* PMD entries, because we'll end up mapping the wrong pages.
+* within a PMD/PUD for userspace and IPA cannot be mapped with stage-2
+* PMD/PUD entries, because we'll end up mapping the wrong pages.
 *
 * Consider a layout like the following:
 *
 *memslot->userspace_addr:
 *+-+++---+
-*|abcde|fgh  Stage-1 PMD|Stage-1 PMD   tv|xyz|
+*|abcde|fgh  Stage-1 block  |Stage-1 block tv|xyz|
 *+-+++---+
 *
 *memslot->base_gfn << PAGE_SIZE:
 *  +---+++-+
-*  |abc|def  Stage-2 PMD|Stage-2 PMD |tvxyz|
+*  |abc|def  Stage-2 block  |Stage-2 block   |tvxyz|
 *  +---+++-+
 *
-* If we create those stage-2 PMDs, we'll end up with this incorrect
+* If we create those stage-2 blocks, we'll end up with this incorrect
 * mapping:
 *   d -> f
 *   e -> g
 *   f -> h
 */
-   if ((gpa_start & ~S2_PMD_MASK) != (uaddr_start & ~S2_PMD_MASK))
+   if ((gpa_start & (map_size - 1)) != (uaddr_start & (map_size - 1)))
return false;
 
/*
 * Next, let's make sure we're not trying to map anything not covered
-* by the memslot. This means we have to prohibit PMD size mappings
-* for the beginning and end of a non-PMD aligned and non-PMD sized
+* by the memslot. This means we have to prohibit block size mappings
+* for the beginning and end of a non-block aligned and non-block sized
 * memory slot (illustrated by the head and tail parts of the
 * userspace view above containing pages 'abcde' and 'xyz',
 * respectively).
@@ -1648,8 +1649,8 @@ static bool fault_supports_stage2_pmd_mappings(struct 
kvm_memory_slot *memslot,
 * userspace_addr or the base_gfn, as both are equally aligned (per
 * the check above) and equally sized.
 */
-   return (hva & S2_PMD_MASK) >= uaddr_start &&
-  (hva & S2_PMD_MASK) + S2_PMD_SIZE <= uaddr_end;
+   return (hva & ~(map_size - 1)) >= uaddr_start &&
+  (hva & ~(map_size - 1)) + map_size <= uaddr_end;
 }
 
 static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
@@ -1678,12 +1679,6 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, 
phys_addr_t fault_ipa,
return -EFAULT;
}
 
-   if (!fault_supports_stage2_pmd_mappings(memslot, hva))
-   force_pte = true;
-
-   if (logging_active)
-   force_pte = true;
-
/* Let's check if we will get back a huge page backed by hugetlbfs */
down_read(>mm->mmap_sem);
vma = find_vma_intersection(current->mm, hva, hva + 1);
@@ -1694,6 +1689,12 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, 
phys_addr_t fault_ipa,
}
 
vma_pagesize = vma_kernel_pagesize(vma);
+   if (logging_active ||
+   !fault_supports_stage2_huge_mapping(memslot, hva, vma_pagesize)) {
+