Re: [PATCH 2/2] KVM: PPC: Remove page table walk helpers

2015-03-29 Thread Benjamin Herrenschmidt
On Mon, 2015-03-30 at 10:39 +0530, Aneesh Kumar K.V wrote:
 This patch remove helpers which we had used only once in the code.
 Limiting page table walk variants help in ensuring that we won't
 end up with code walking page table with wrong assumptions.
 
 Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com

Alex, it would be preferable to have this (and the previous one) in the
powerpc tree due to dependencies with further fixes to our page table
walking vs. THP, so if you're happy, just give us an ack.

Cheers,
Ben.

 ---
  arch/powerpc/include/asm/pgtable.h  | 21 -
  arch/powerpc/kvm/book3s_hv_rm_mmu.c | 62 
 -
  arch/powerpc/kvm/e500_mmu_host.c|  2 +-
  3 files changed, 28 insertions(+), 57 deletions(-)
 
 diff --git a/arch/powerpc/include/asm/pgtable.h 
 b/arch/powerpc/include/asm/pgtable.h
 index 9835ac4173b7..92fe01c355a9 100644
 --- a/arch/powerpc/include/asm/pgtable.h
 +++ b/arch/powerpc/include/asm/pgtable.h
 @@ -249,27 +249,6 @@ extern int gup_hugepte(pte_t *ptep, unsigned long sz, 
 unsigned long addr,
  #endif
  pte_t *find_linux_pte_or_hugepte(pgd_t *pgdir, unsigned long ea,
unsigned *shift);
 -
 -static inline pte_t *lookup_linux_ptep(pgd_t *pgdir, unsigned long hva,
 -  unsigned long *pte_sizep)
 -{
 - pte_t *ptep;
 - unsigned long ps = *pte_sizep;
 - unsigned int shift;
 -
 - ptep = find_linux_pte_or_hugepte(pgdir, hva, shift);
 - if (!ptep)
 - return NULL;
 - if (shift)
 - *pte_sizep = 1ul  shift;
 - else
 - *pte_sizep = PAGE_SIZE;
 -
 - if (ps  *pte_sizep)
 - return NULL;
 -
 - return ptep;
 -}
  #endif /* __ASSEMBLY__ */
  
  #endif /* __KERNEL__ */
 diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c 
 b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
 index 625407e4d3b0..73e083cb9f7e 100644
 --- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c
 +++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
 @@ -131,25 +131,6 @@ static void remove_revmap_chain(struct kvm *kvm, long 
 pte_index,
   unlock_rmap(rmap);
  }
  
 -static pte_t lookup_linux_pte_and_update(pgd_t *pgdir, unsigned long hva,
 -   int writing, unsigned long *pte_sizep)
 -{
 - pte_t *ptep;
 - unsigned long ps = *pte_sizep;
 - unsigned int hugepage_shift;
 -
 - ptep = find_linux_pte_or_hugepte(pgdir, hva, hugepage_shift);
 - if (!ptep)
 - return __pte(0);
 - if (hugepage_shift)
 - *pte_sizep = 1ul  hugepage_shift;
 - else
 - *pte_sizep = PAGE_SIZE;
 - if (ps  *pte_sizep)
 - return __pte(0);
 - return kvmppc_read_update_linux_pte(ptep, writing, hugepage_shift);
 -}
 -
  static inline void unlock_hpte(__be64 *hpte, unsigned long hpte_v)
  {
   asm volatile(PPC_RELEASE_BARRIER  : : : memory);
 @@ -166,10 +147,10 @@ long kvmppc_do_h_enter(struct kvm *kvm, unsigned long 
 flags,
   struct revmap_entry *rev;
   unsigned long g_ptel;
   struct kvm_memory_slot *memslot;
 - unsigned long pte_size;
 + unsigned hpage_shift;
   unsigned long is_io;
   unsigned long *rmap;
 - pte_t pte;
 + pte_t *ptep;
   unsigned int writing;
   unsigned long mmu_seq;
   unsigned long rcbits;
 @@ -208,22 +189,33 @@ long kvmppc_do_h_enter(struct kvm *kvm, unsigned long 
 flags,
  
   /* Translate to host virtual address */
   hva = __gfn_to_hva_memslot(memslot, gfn);
 + ptep = find_linux_pte_or_hugepte(pgdir, hva, hpage_shift);
 + if (ptep) {
 + pte_t pte;
 + unsigned int host_pte_size;
  
 - /* Look up the Linux PTE for the backing page */
 - pte_size = psize;
 - pte = lookup_linux_pte_and_update(pgdir, hva, writing, pte_size);
 - if (pte_present(pte)  !pte_protnone(pte)) {
 - if (writing  !pte_write(pte))
 - /* make the actual HPTE be read-only */
 - ptel = hpte_make_readonly(ptel);
 - is_io = hpte_cache_bits(pte_val(pte));
 - pa = pte_pfn(pte)  PAGE_SHIFT;
 - pa |= hva  (pte_size - 1);
 - pa |= gpa  ~PAGE_MASK;
 - }
 + if (hpage_shift)
 + host_pte_size = 1ul  hpage_shift;
 + else
 + host_pte_size = PAGE_SIZE;
 + /*
 +  * We should always find the guest page size
 +  * to = host page size, if host is using hugepage
 +  */
 + if (host_pte_size  psize)
 + return H_PARAMETER;
  
 - if (pte_size  psize)
 - return H_PARAMETER;
 + pte = kvmppc_read_update_linux_pte(ptep, writing, hpage_shift);
 + if (pte_present(pte)  !pte_protnone(pte)) {
 + if (writing  !pte_write(pte))
 + /* make the actual HPTE be read-only */
 +  

Re: [PATCH 2/2] KVM: PPC: Remove page table walk helpers

2015-03-29 Thread Benjamin Herrenschmidt
On Mon, 2015-03-30 at 10:39 +0530, Aneesh Kumar K.V wrote:
 This patch remove helpers which we had used only once in the code.
 Limiting page table walk variants help in ensuring that we won't
 end up with code walking page table with wrong assumptions.
 
 Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com

Alex, it would be preferable to have this (and the previous one) in the
powerpc tree due to dependencies with further fixes to our page table
walking vs. THP, so if you're happy, just give us an ack.

Cheers,
Ben.

 ---
  arch/powerpc/include/asm/pgtable.h  | 21 -
  arch/powerpc/kvm/book3s_hv_rm_mmu.c | 62 
 -
  arch/powerpc/kvm/e500_mmu_host.c|  2 +-
  3 files changed, 28 insertions(+), 57 deletions(-)
 
 diff --git a/arch/powerpc/include/asm/pgtable.h 
 b/arch/powerpc/include/asm/pgtable.h
 index 9835ac4173b7..92fe01c355a9 100644
 --- a/arch/powerpc/include/asm/pgtable.h
 +++ b/arch/powerpc/include/asm/pgtable.h
 @@ -249,27 +249,6 @@ extern int gup_hugepte(pte_t *ptep, unsigned long sz, 
 unsigned long addr,
  #endif
  pte_t *find_linux_pte_or_hugepte(pgd_t *pgdir, unsigned long ea,
unsigned *shift);
 -
 -static inline pte_t *lookup_linux_ptep(pgd_t *pgdir, unsigned long hva,
 -  unsigned long *pte_sizep)
 -{
 - pte_t *ptep;
 - unsigned long ps = *pte_sizep;
 - unsigned int shift;
 -
 - ptep = find_linux_pte_or_hugepte(pgdir, hva, shift);
 - if (!ptep)
 - return NULL;
 - if (shift)
 - *pte_sizep = 1ul  shift;
 - else
 - *pte_sizep = PAGE_SIZE;
 -
 - if (ps  *pte_sizep)
 - return NULL;
 -
 - return ptep;
 -}
  #endif /* __ASSEMBLY__ */
  
  #endif /* __KERNEL__ */
 diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c 
 b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
 index 625407e4d3b0..73e083cb9f7e 100644
 --- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c
 +++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
 @@ -131,25 +131,6 @@ static void remove_revmap_chain(struct kvm *kvm, long 
 pte_index,
   unlock_rmap(rmap);
  }
  
 -static pte_t lookup_linux_pte_and_update(pgd_t *pgdir, unsigned long hva,
 -   int writing, unsigned long *pte_sizep)
 -{
 - pte_t *ptep;
 - unsigned long ps = *pte_sizep;
 - unsigned int hugepage_shift;
 -
 - ptep = find_linux_pte_or_hugepte(pgdir, hva, hugepage_shift);
 - if (!ptep)
 - return __pte(0);
 - if (hugepage_shift)
 - *pte_sizep = 1ul  hugepage_shift;
 - else
 - *pte_sizep = PAGE_SIZE;
 - if (ps  *pte_sizep)
 - return __pte(0);
 - return kvmppc_read_update_linux_pte(ptep, writing, hugepage_shift);
 -}
 -
  static inline void unlock_hpte(__be64 *hpte, unsigned long hpte_v)
  {
   asm volatile(PPC_RELEASE_BARRIER  : : : memory);
 @@ -166,10 +147,10 @@ long kvmppc_do_h_enter(struct kvm *kvm, unsigned long 
 flags,
   struct revmap_entry *rev;
   unsigned long g_ptel;
   struct kvm_memory_slot *memslot;
 - unsigned long pte_size;
 + unsigned hpage_shift;
   unsigned long is_io;
   unsigned long *rmap;
 - pte_t pte;
 + pte_t *ptep;
   unsigned int writing;
   unsigned long mmu_seq;
   unsigned long rcbits;
 @@ -208,22 +189,33 @@ long kvmppc_do_h_enter(struct kvm *kvm, unsigned long 
 flags,
  
   /* Translate to host virtual address */
   hva = __gfn_to_hva_memslot(memslot, gfn);
 + ptep = find_linux_pte_or_hugepte(pgdir, hva, hpage_shift);
 + if (ptep) {
 + pte_t pte;
 + unsigned int host_pte_size;
  
 - /* Look up the Linux PTE for the backing page */
 - pte_size = psize;
 - pte = lookup_linux_pte_and_update(pgdir, hva, writing, pte_size);
 - if (pte_present(pte)  !pte_protnone(pte)) {
 - if (writing  !pte_write(pte))
 - /* make the actual HPTE be read-only */
 - ptel = hpte_make_readonly(ptel);
 - is_io = hpte_cache_bits(pte_val(pte));
 - pa = pte_pfn(pte)  PAGE_SHIFT;
 - pa |= hva  (pte_size - 1);
 - pa |= gpa  ~PAGE_MASK;
 - }
 + if (hpage_shift)
 + host_pte_size = 1ul  hpage_shift;
 + else
 + host_pte_size = PAGE_SIZE;
 + /*
 +  * We should always find the guest page size
 +  * to = host page size, if host is using hugepage
 +  */
 + if (host_pte_size  psize)
 + return H_PARAMETER;
  
 - if (pte_size  psize)
 - return H_PARAMETER;
 + pte = kvmppc_read_update_linux_pte(ptep, writing, hpage_shift);
 + if (pte_present(pte)  !pte_protnone(pte)) {
 + if (writing  !pte_write(pte))
 + /* make the actual HPTE be read-only */
 +  

[PATCH 2/2] KVM: PPC: Remove page table walk helpers

2015-03-29 Thread Aneesh Kumar K.V
This patch remove helpers which we had used only once in the code.
Limiting page table walk variants help in ensuring that we won't
end up with code walking page table with wrong assumptions.

Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
---
 arch/powerpc/include/asm/pgtable.h  | 21 -
 arch/powerpc/kvm/book3s_hv_rm_mmu.c | 62 -
 arch/powerpc/kvm/e500_mmu_host.c|  2 +-
 3 files changed, 28 insertions(+), 57 deletions(-)

diff --git a/arch/powerpc/include/asm/pgtable.h 
b/arch/powerpc/include/asm/pgtable.h
index 9835ac4173b7..92fe01c355a9 100644
--- a/arch/powerpc/include/asm/pgtable.h
+++ b/arch/powerpc/include/asm/pgtable.h
@@ -249,27 +249,6 @@ extern int gup_hugepte(pte_t *ptep, unsigned long sz, 
unsigned long addr,
 #endif
 pte_t *find_linux_pte_or_hugepte(pgd_t *pgdir, unsigned long ea,
 unsigned *shift);
-
-static inline pte_t *lookup_linux_ptep(pgd_t *pgdir, unsigned long hva,
-unsigned long *pte_sizep)
-{
-   pte_t *ptep;
-   unsigned long ps = *pte_sizep;
-   unsigned int shift;
-
-   ptep = find_linux_pte_or_hugepte(pgdir, hva, shift);
-   if (!ptep)
-   return NULL;
-   if (shift)
-   *pte_sizep = 1ul  shift;
-   else
-   *pte_sizep = PAGE_SIZE;
-
-   if (ps  *pte_sizep)
-   return NULL;
-
-   return ptep;
-}
 #endif /* __ASSEMBLY__ */
 
 #endif /* __KERNEL__ */
diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c 
b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
index 625407e4d3b0..73e083cb9f7e 100644
--- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c
+++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
@@ -131,25 +131,6 @@ static void remove_revmap_chain(struct kvm *kvm, long 
pte_index,
unlock_rmap(rmap);
 }
 
-static pte_t lookup_linux_pte_and_update(pgd_t *pgdir, unsigned long hva,
- int writing, unsigned long *pte_sizep)
-{
-   pte_t *ptep;
-   unsigned long ps = *pte_sizep;
-   unsigned int hugepage_shift;
-
-   ptep = find_linux_pte_or_hugepte(pgdir, hva, hugepage_shift);
-   if (!ptep)
-   return __pte(0);
-   if (hugepage_shift)
-   *pte_sizep = 1ul  hugepage_shift;
-   else
-   *pte_sizep = PAGE_SIZE;
-   if (ps  *pte_sizep)
-   return __pte(0);
-   return kvmppc_read_update_linux_pte(ptep, writing, hugepage_shift);
-}
-
 static inline void unlock_hpte(__be64 *hpte, unsigned long hpte_v)
 {
asm volatile(PPC_RELEASE_BARRIER  : : : memory);
@@ -166,10 +147,10 @@ long kvmppc_do_h_enter(struct kvm *kvm, unsigned long 
flags,
struct revmap_entry *rev;
unsigned long g_ptel;
struct kvm_memory_slot *memslot;
-   unsigned long pte_size;
+   unsigned hpage_shift;
unsigned long is_io;
unsigned long *rmap;
-   pte_t pte;
+   pte_t *ptep;
unsigned int writing;
unsigned long mmu_seq;
unsigned long rcbits;
@@ -208,22 +189,33 @@ long kvmppc_do_h_enter(struct kvm *kvm, unsigned long 
flags,
 
/* Translate to host virtual address */
hva = __gfn_to_hva_memslot(memslot, gfn);
+   ptep = find_linux_pte_or_hugepte(pgdir, hva, hpage_shift);
+   if (ptep) {
+   pte_t pte;
+   unsigned int host_pte_size;
 
-   /* Look up the Linux PTE for the backing page */
-   pte_size = psize;
-   pte = lookup_linux_pte_and_update(pgdir, hva, writing, pte_size);
-   if (pte_present(pte)  !pte_protnone(pte)) {
-   if (writing  !pte_write(pte))
-   /* make the actual HPTE be read-only */
-   ptel = hpte_make_readonly(ptel);
-   is_io = hpte_cache_bits(pte_val(pte));
-   pa = pte_pfn(pte)  PAGE_SHIFT;
-   pa |= hva  (pte_size - 1);
-   pa |= gpa  ~PAGE_MASK;
-   }
+   if (hpage_shift)
+   host_pte_size = 1ul  hpage_shift;
+   else
+   host_pte_size = PAGE_SIZE;
+   /*
+* We should always find the guest page size
+* to = host page size, if host is using hugepage
+*/
+   if (host_pte_size  psize)
+   return H_PARAMETER;
 
-   if (pte_size  psize)
-   return H_PARAMETER;
+   pte = kvmppc_read_update_linux_pte(ptep, writing, hpage_shift);
+   if (pte_present(pte)  !pte_protnone(pte)) {
+   if (writing  !pte_write(pte))
+   /* make the actual HPTE be read-only */
+   ptel = hpte_make_readonly(ptel);
+   is_io = hpte_cache_bits(pte_val(pte));
+   pa = pte_pfn(pte)  PAGE_SHIFT;
+   pa |= hva  (host_pte_size - 1);
+   pa |= gpa  

[PATCH 2/2] KVM: PPC: Remove page table walk helpers

2015-03-29 Thread Aneesh Kumar K.V
This patch remove helpers which we had used only once in the code.
Limiting page table walk variants help in ensuring that we won't
end up with code walking page table with wrong assumptions.

Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
---
 arch/powerpc/include/asm/pgtable.h  | 21 -
 arch/powerpc/kvm/book3s_hv_rm_mmu.c | 62 -
 arch/powerpc/kvm/e500_mmu_host.c|  2 +-
 3 files changed, 28 insertions(+), 57 deletions(-)

diff --git a/arch/powerpc/include/asm/pgtable.h 
b/arch/powerpc/include/asm/pgtable.h
index 9835ac4173b7..92fe01c355a9 100644
--- a/arch/powerpc/include/asm/pgtable.h
+++ b/arch/powerpc/include/asm/pgtable.h
@@ -249,27 +249,6 @@ extern int gup_hugepte(pte_t *ptep, unsigned long sz, 
unsigned long addr,
 #endif
 pte_t *find_linux_pte_or_hugepte(pgd_t *pgdir, unsigned long ea,
 unsigned *shift);
-
-static inline pte_t *lookup_linux_ptep(pgd_t *pgdir, unsigned long hva,
-unsigned long *pte_sizep)
-{
-   pte_t *ptep;
-   unsigned long ps = *pte_sizep;
-   unsigned int shift;
-
-   ptep = find_linux_pte_or_hugepte(pgdir, hva, shift);
-   if (!ptep)
-   return NULL;
-   if (shift)
-   *pte_sizep = 1ul  shift;
-   else
-   *pte_sizep = PAGE_SIZE;
-
-   if (ps  *pte_sizep)
-   return NULL;
-
-   return ptep;
-}
 #endif /* __ASSEMBLY__ */
 
 #endif /* __KERNEL__ */
diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c 
b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
index 625407e4d3b0..73e083cb9f7e 100644
--- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c
+++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
@@ -131,25 +131,6 @@ static void remove_revmap_chain(struct kvm *kvm, long 
pte_index,
unlock_rmap(rmap);
 }
 
-static pte_t lookup_linux_pte_and_update(pgd_t *pgdir, unsigned long hva,
- int writing, unsigned long *pte_sizep)
-{
-   pte_t *ptep;
-   unsigned long ps = *pte_sizep;
-   unsigned int hugepage_shift;
-
-   ptep = find_linux_pte_or_hugepte(pgdir, hva, hugepage_shift);
-   if (!ptep)
-   return __pte(0);
-   if (hugepage_shift)
-   *pte_sizep = 1ul  hugepage_shift;
-   else
-   *pte_sizep = PAGE_SIZE;
-   if (ps  *pte_sizep)
-   return __pte(0);
-   return kvmppc_read_update_linux_pte(ptep, writing, hugepage_shift);
-}
-
 static inline void unlock_hpte(__be64 *hpte, unsigned long hpte_v)
 {
asm volatile(PPC_RELEASE_BARRIER  : : : memory);
@@ -166,10 +147,10 @@ long kvmppc_do_h_enter(struct kvm *kvm, unsigned long 
flags,
struct revmap_entry *rev;
unsigned long g_ptel;
struct kvm_memory_slot *memslot;
-   unsigned long pte_size;
+   unsigned hpage_shift;
unsigned long is_io;
unsigned long *rmap;
-   pte_t pte;
+   pte_t *ptep;
unsigned int writing;
unsigned long mmu_seq;
unsigned long rcbits;
@@ -208,22 +189,33 @@ long kvmppc_do_h_enter(struct kvm *kvm, unsigned long 
flags,
 
/* Translate to host virtual address */
hva = __gfn_to_hva_memslot(memslot, gfn);
+   ptep = find_linux_pte_or_hugepte(pgdir, hva, hpage_shift);
+   if (ptep) {
+   pte_t pte;
+   unsigned int host_pte_size;
 
-   /* Look up the Linux PTE for the backing page */
-   pte_size = psize;
-   pte = lookup_linux_pte_and_update(pgdir, hva, writing, pte_size);
-   if (pte_present(pte)  !pte_protnone(pte)) {
-   if (writing  !pte_write(pte))
-   /* make the actual HPTE be read-only */
-   ptel = hpte_make_readonly(ptel);
-   is_io = hpte_cache_bits(pte_val(pte));
-   pa = pte_pfn(pte)  PAGE_SHIFT;
-   pa |= hva  (pte_size - 1);
-   pa |= gpa  ~PAGE_MASK;
-   }
+   if (hpage_shift)
+   host_pte_size = 1ul  hpage_shift;
+   else
+   host_pte_size = PAGE_SIZE;
+   /*
+* We should always find the guest page size
+* to = host page size, if host is using hugepage
+*/
+   if (host_pte_size  psize)
+   return H_PARAMETER;
 
-   if (pte_size  psize)
-   return H_PARAMETER;
+   pte = kvmppc_read_update_linux_pte(ptep, writing, hpage_shift);
+   if (pte_present(pte)  !pte_protnone(pte)) {
+   if (writing  !pte_write(pte))
+   /* make the actual HPTE be read-only */
+   ptel = hpte_make_readonly(ptel);
+   is_io = hpte_cache_bits(pte_val(pte));
+   pa = pte_pfn(pte)  PAGE_SHIFT;
+   pa |= hva  (host_pte_size - 1);
+   pa |= gpa