[PATCH 0/6 v2] kvm: powerpc: use cache attributes from linux pte

From: Bharat Bhushan <redacted>

First patch is a typo fix where book3e define _PAGE_LENDIAN while it should be
defined as _PAGE_ENDIAN. This seems to show that this is never exercised :-)

Second and third patch is to allow guest controlling "G"-Guarded and
"E"-Endiany TLB attributes respectively.

Rest of patches is about setting caching attributes (TLB.WIMGE) using
corresponding Linux pte.

v1->v2
 - Earlier caching attributes (WIMGE) were set based of page is RAM or not
   But now we get these attributes from corresponding Linux PTE.

Bharat Bhushan (6):
  powerpc: book3e: _PAGE_LENDIAN must be _PAGE_ENDIAN
  kvm: powerpc: allow guest control "E" attribute in mas2
  kvm: powerpc: allow guest control "G" attribute in mas2
  powerpc: move linux pte/hugepte search to more generic file
  kvm: powerpc: booke: Add linux pte lookup like booke3s
  kvm: powerpc: use caching attributes as per linux pte

 arch/powerpc/include/asm/kvm_booke.h     |   73 ++++++++++++++++++++++++++++++
 arch/powerpc/include/asm/kvm_host.h      |    2 +-
 arch/powerpc/include/asm/pgtable-ppc64.h |   36 ---------------
 arch/powerpc/include/asm/pgtable.h       |   37 +++++++++++++++
 arch/powerpc/include/asm/pte-book3e.h    |    2 +-
 arch/powerpc/kvm/booke.c                 |    2 +-
 arch/powerpc/kvm/e500.h                  |   10 +++--
 arch/powerpc/kvm/e500_mmu_host.c         |   31 +++++++-----
 8 files changed, 137 insertions(+), 56 deletions(-)

For booke3e _PAGE_ENDIAN is not defined. Infact what is defined
is "_PAGE_LENDIAN" which is wrong and should be _PAGE_ENDIAN.
There are no compilation errors as
arch/powerpc/include/asm/pte-common.h defines _PAGE_ENDIAN to 0
as it is not defined anywhere.

Signed-off-by: Bharat Bhushan <redacted>
---
v1->v2
 - no change

 arch/powerpc/include/asm/pte-book3e.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/include/asm/pte-book3e.h b/arch/powerpc/include/asm/pte-book3e.h
index 0156702..576ad88 100644
--- a/arch/powerpc/include/asm/pte-book3e.h
+++ b/arch/powerpc/include/asm/pte-book3e.h

@@ -40,7 +40,7 @@
 #define _PAGE_U1	0x010000
 #define _PAGE_U0	0x020000
 #define _PAGE_ACCESSED	0x040000
-#define _PAGE_LENDIAN	0x080000
+#define _PAGE_ENDIAN	0x080000
 #define _PAGE_GUARDED	0x100000
 #define _PAGE_COHERENT	0x200000 /* M: enforce memory coherence */
 #define _PAGE_NO_CACHE	0x400000 /* I: cache inhibit */

-- 
1.7.0.4

"E" bit in MAS2 bit indicates whether the page is accessed
in Little-Endian or Big-Endian byte order.
There is no reason to stop guest setting  "E", so allow him."

Signed-off-by: Bharat Bhushan <redacted>
---
v1->v2
 - no change

 arch/powerpc/kvm/e500.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/kvm/e500.h b/arch/powerpc/kvm/e500.h
index c2e5e98..277cb18 100644
--- a/arch/powerpc/kvm/e500.h
+++ b/arch/powerpc/kvm/e500.h

@@ -117,7 +117,7 @@ static inline struct kvmppc_vcpu_e500 *to_e500(struct kvm_vcpu *vcpu)
 #define E500_TLB_USER_PERM_MASK (MAS3_UX|MAS3_UR|MAS3_UW)
 #define E500_TLB_SUPER_PERM_MASK (MAS3_SX|MAS3_SR|MAS3_SW)
 #define MAS2_ATTRIB_MASK \
-	  (MAS2_X0 | MAS2_X1)
+	  (MAS2_X0 | MAS2_X1 | MAS2_E)
 #define MAS3_ATTRIB_MASK \
 	  (MAS3_U0 | MAS3_U1 | MAS3_U2 | MAS3_U3 \
 	   | E500_TLB_USER_PERM_MASK | E500_TLB_SUPER_PERM_MASK)

-- 
1.7.0.4

"G" bit in MAS2 indicates whether the page is Guarded.
There is no reason to stop guest setting  "E", so allow him.

Signed-off-by: Bharat Bhushan <redacted>
---
v1->v2
 - no change

 arch/powerpc/kvm/e500.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/kvm/e500.h b/arch/powerpc/kvm/e500.h
index 277cb18..4fd9650 100644
--- a/arch/powerpc/kvm/e500.h
+++ b/arch/powerpc/kvm/e500.h

@@ -117,7 +117,7 @@ static inline struct kvmppc_vcpu_e500 *to_e500(struct kvm_vcpu *vcpu)
 #define E500_TLB_USER_PERM_MASK (MAS3_UX|MAS3_UR|MAS3_UW)
 #define E500_TLB_SUPER_PERM_MASK (MAS3_SX|MAS3_SR|MAS3_SW)
 #define MAS2_ATTRIB_MASK \
-	  (MAS2_X0 | MAS2_X1 | MAS2_E)
+	  (MAS2_X0 | MAS2_X1 | MAS2_E | MAS2_G)
 #define MAS3_ATTRIB_MASK \
 	  (MAS3_U0 | MAS3_U1 | MAS3_U2 | MAS3_U3 \
 	   | E500_TLB_USER_PERM_MASK | E500_TLB_SUPER_PERM_MASK)

-- 
1.7.0.4

Linux pte search functions find_linux_pte_or_hugepte() and
find_linux_pte() have nothing specific to 64bit anymore.
So they are move from pgtable-ppc64.h to asm/pgtable.h

Signed-off-by: Bharat Bhushan <redacted>
---
v1->v2
 - This is a new change in this version

 arch/powerpc/include/asm/pgtable-ppc64.h |   36 -----------------------------
 arch/powerpc/include/asm/pgtable.h       |   37 ++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 36 deletions(-)

diff --git a/arch/powerpc/include/asm/pgtable-ppc64.h b/arch/powerpc/include/asm/pgtable-ppc64.h
index e3d55f6..d257d98 100644
--- a/arch/powerpc/include/asm/pgtable-ppc64.h
+++ b/arch/powerpc/include/asm/pgtable-ppc64.h

@@ -340,42 +340,6 @@ static inline void __ptep_set_access_flags(pte_t *ptep, pte_t entry)
 void pgtable_cache_add(unsigned shift, void (*ctor)(void *));
 void pgtable_cache_init(void);
 
-/*
- * find_linux_pte returns the address of a linux pte for a given
- * effective address and directory.  If not found, it returns zero.
- */
-static inline pte_t *find_linux_pte(pgd_t *pgdir, unsigned long ea)
-{
-	pgd_t *pg;
-	pud_t *pu;
-	pmd_t *pm;
-	pte_t *pt = NULL;
-
-	pg = pgdir + pgd_index(ea);
-	if (!pgd_none(*pg)) {
-		pu = pud_offset(pg, ea);
-		if (!pud_none(*pu)) {
-			pm = pmd_offset(pu, ea);
-			if (pmd_present(*pm))
-				pt = pte_offset_kernel(pm, ea);
-		}
-	}
-	return pt;
-}
-
-#ifdef CONFIG_HUGETLB_PAGE
-pte_t *find_linux_pte_or_hugepte(pgd_t *pgdir, unsigned long ea,
-				 unsigned *shift);
-#else
-static inline pte_t *find_linux_pte_or_hugepte(pgd_t *pgdir, unsigned long ea,
-					       unsigned *shift)
-{
-	if (shift)
-		*shift = 0;
-	return find_linux_pte(pgdir, ea);
-}
-#endif /* !CONFIG_HUGETLB_PAGE */
-
 #endif /* __ASSEMBLY__ */
 
 #endif /* _ASM_POWERPC_PGTABLE_PPC64_H_ */

diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h
index b6293d2..690c8c2 100644
--- a/arch/powerpc/include/asm/pgtable.h
+++ b/arch/powerpc/include/asm/pgtable.h

@@ -217,6 +217,43 @@ extern int gup_hugepd(hugepd_t *hugepd, unsigned pdshift, unsigned long addr,
 
 extern int gup_hugepte(pte_t *ptep, unsigned long sz, unsigned long addr,
 		       unsigned long end, int write, struct page **pages, int *nr);
+
+/*
+ * find_linux_pte returns the address of a linux pte for a given
+ * effective address and directory.  If not found, it returns zero.
+ */
+static inline pte_t *find_linux_pte(pgd_t *pgdir, unsigned long ea)
+{
+	pgd_t *pg;
+	pud_t *pu;
+	pmd_t *pm;
+	pte_t *pt = NULL;
+
+	pg = pgdir + pgd_index(ea);
+	if (!pgd_none(*pg)) {
+		pu = pud_offset(pg, ea);
+		if (!pud_none(*pu)) {
+			pm = pmd_offset(pu, ea);
+			if (pmd_present(*pm))
+				pt = pte_offset_kernel(pm, ea);
+		}
+	}
+	return pt;
+}
+
+#ifdef CONFIG_HUGETLB_PAGE
+pte_t *find_linux_pte_or_hugepte(pgd_t *pgdir, unsigned long ea,
+				 unsigned *shift);
+#else
+static inline pte_t *find_linux_pte_or_hugepte(pgd_t *pgdir, unsigned long ea,
+					       unsigned *shift)
+{
+	if (shift)
+		*shift = 0;
+	return find_linux_pte(pgdir, ea);
+}
+#endif /* !CONFIG_HUGETLB_PAGE */
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* __KERNEL__ */

-- 
1.7.0.4

KVM need to lookup linux pte for getting TLB attributes (WIMGE).
This is similar to how book3s does.
This will be used in follow-up patches.

Signed-off-by: Bharat Bhushan <redacted>
---
v1->v2
 - This is a new change in this version

 arch/powerpc/include/asm/kvm_booke.h |   73 ++++++++++++++++++++++++++++++++++
 1 files changed, 73 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm_booke.h b/arch/powerpc/include/asm/kvm_booke.h
index d3c1eb3..903624d 100644
--- a/arch/powerpc/include/asm/kvm_booke.h
+++ b/arch/powerpc/include/asm/kvm_booke.h

@@ -102,4 +102,77 @@ static inline ulong kvmppc_get_msr(struct kvm_vcpu *vcpu)
 {
 	return vcpu->arch.shared->msr;
 }
+
+/*
+ * Lock and read a linux PTE.  If it's present and writable, atomically
+ * set dirty and referenced bits and return the PTE, otherwise return 0.
+ */
+static inline pte_t kvmppc_read_update_linux_pte(pte_t *p, int writing)
+{
+	pte_t pte;
+
+#ifdef PTE_ATOMIC_UPDATES
+	pte_t tmp;
+        /* wait until _PAGE_BUSY is clear then set it atomically */
+#ifdef CONFIG_PPC64
+	__asm__ __volatile__ (
+		"1:	ldarx	%0,0,%3\n"
+		"	andi.	%1,%0,%4\n"
+		"	bne-	1b\n"
+		"	ori	%1,%0,%4\n"
+		"	stdcx.	%1,0,%3\n"
+		"	bne-	1b"
+		: "=&r" (pte), "=&r" (tmp), "=m" (*p)
+		: "r" (p), "i" (_PAGE_BUSY)
+		: "cc");
+#else
+        __asm__ __volatile__ (
+                "1:     lwarx   %0,0,%3\n"
+                "       andi.   %1,%0,%4\n"
+                "       bne-    1b\n"
+                "       ori     %1,%0,%4\n"
+                "       stwcx.  %1,0,%3\n"
+                "       bne-    1b"
+                : "=&r" (pte), "=&r" (tmp), "=m" (*p)
+                : "r" (p), "i" (_PAGE_BUSY)
+                : "cc");
+#endif
+#else
+	pte = pte_val(*p);
+#endif
+
+	if (pte_present(pte)) {
+		pte = pte_mkyoung(pte);
+		if (writing && pte_write(pte))
+			pte = pte_mkdirty(pte);
+	}
+
+	*p = pte;	/* clears _PAGE_BUSY */
+
+	return pte;
+}
+
+static inline pte_t lookup_linux_pte(pgd_t *pgdir, unsigned long hva,
+			      int writing, 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 __pte(0);
+	if (shift)
+		*pte_sizep = 1ul << shift;
+	else
+		*pte_sizep = PAGE_SIZE;
+
+	if (ps > *pte_sizep)
+		return __pte(0);
+	if (!pte_present(*ptep))
+		return __pte(0);
+
+	return kvmppc_read_update_linux_pte(ptep, writing);
+}
+
 #endif /* __ASM_KVM_BOOKE_H__ */

-- 
1.7.0.4

KVM uses same WIM tlb attributes as the corresponding qemu pte.
For this we now search the linux pte for the requested page and
get these cache caching/coherency attributes from pte.

Signed-off-by: Bharat Bhushan <redacted>
---
v1->v2
 - Use Linux pte for wimge rather than RAM/no-RAM mechanism

 arch/powerpc/include/asm/kvm_host.h |    2 +-
 arch/powerpc/kvm/booke.c            |    2 +-
 arch/powerpc/kvm/e500.h             |    8 +++++---
 arch/powerpc/kvm/e500_mmu_host.c    |   31 ++++++++++++++++++-------------
 4 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index 3328353..583d405 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h

@@ -535,6 +535,7 @@ struct kvm_vcpu_arch {
 #endif
 	gpa_t paddr_accessed;
 	gva_t vaddr_accessed;
+	pgd_t *pgdir;
 
 	u8 io_gpr; /* GPR used as IO source/target */
 	u8 mmio_is_bigendian;

@@ -592,7 +593,6 @@ struct kvm_vcpu_arch {
 	struct list_head run_list;
 	struct task_struct *run_task;
 	struct kvm_run *kvm_run;
-	pgd_t *pgdir;
 
 	spinlock_t vpa_update_lock;
 	struct kvmppc_vpa vpa;

diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 17722d8..ebcccc2 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c

@@ -697,7 +697,7 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu)
 #endif
 
 	kvmppc_fix_ee_before_entry();
-
+	vcpu->arch.pgdir = current->mm->pgd;
 	ret = __kvmppc_vcpu_run(kvm_run, vcpu);
 
 	/* No need for kvm_guest_exit. It's done in handle_exit.

diff --git a/arch/powerpc/kvm/e500.h b/arch/powerpc/kvm/e500.h
index 4fd9650..fc4b2f6 100644
--- a/arch/powerpc/kvm/e500.h
+++ b/arch/powerpc/kvm/e500.h

@@ -31,11 +31,13 @@ enum vcpu_ftr {
 #define E500_TLB_NUM   2
 
 /* entry is mapped somewhere in host TLB */
-#define E500_TLB_VALID		(1 << 0)
+#define E500_TLB_VALID		(1 << 31)
 /* TLB1 entry is mapped by host TLB1, tracked by bitmaps */
-#define E500_TLB_BITMAP		(1 << 1)
+#define E500_TLB_BITMAP		(1 << 30)
 /* TLB1 entry is mapped by host TLB0 */
-#define E500_TLB_TLB0		(1 << 2)
+#define E500_TLB_TLB0		(1 << 29)
+/* Lower 5 bits have WIMGE value */
+#define E500_TLB_WIMGE_MASK	(0x1f)
 
 struct tlbe_ref {
 	pfn_t pfn;		/* valid only for TLB0, except briefly */

diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..9b10b0b 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c

@@ -64,15 +64,6 @@ static inline u32 e500_shadow_mas3_attrib(u32 mas3, int usermode)
 	return mas3;
 }
 
-static inline u32 e500_shadow_mas2_attrib(u32 mas2, int usermode)
-{
-#ifdef CONFIG_SMP
-	return (mas2 & MAS2_ATTRIB_MASK) | MAS2_M;
-#else
-	return mas2 & MAS2_ATTRIB_MASK;
-#endif
-}
-
 /*
  * writing shadow tlb entry to host TLB
  */

@@ -248,10 +239,12 @@ static inline int tlbe_is_writable(struct kvm_book3e_206_tlb_entry *tlbe)
 
 static inline void kvmppc_e500_ref_setup(struct tlbe_ref *ref,
 					 struct kvm_book3e_206_tlb_entry *gtlbe,
-					 pfn_t pfn)
+					 pfn_t pfn, int wimg)
 {
 	ref->pfn = pfn;
 	ref->flags |= E500_TLB_VALID;
+	/* Use guest supplied MAS2_G and MAS2_E */
+	ref->flags |= (gtlbe->mas2 & MAS2_ATTRIB_MASK) | wimg;
 
 	if (tlbe_is_writable(gtlbe))
 		kvm_set_pfn_dirty(pfn);

@@ -312,8 +305,7 @@ static void kvmppc_e500_setup_stlbe(
 
 	/* Force IPROT=0 for all guest mappings. */
 	stlbe->mas1 = MAS1_TSIZE(tsize) | get_tlb_sts(gtlbe) | MAS1_VALID;
-	stlbe->mas2 = (gvaddr & MAS2_EPN) |
-		      e500_shadow_mas2_attrib(gtlbe->mas2, pr);
+	stlbe->mas2 = (gvaddr & MAS2_EPN) | (ref->flags & E500_TLB_WIMGE_MASK);
 	stlbe->mas7_3 = ((u64)pfn << PAGE_SHIFT) |
 			e500_shadow_mas3_attrib(gtlbe->mas7_3, pr);
 

@@ -332,6 +324,8 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
 	unsigned long hva;
 	int pfnmap = 0;
 	int tsize = BOOK3E_PAGESZ_4K;
+	pte_t pte;
+	int wimg = 0;
 
 	/*
 	 * Translate guest physical to true physical, acquiring

@@ -437,6 +431,8 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
 
 	if (likely(!pfnmap)) {
 		unsigned long tsize_pages = 1 << (tsize + 10 - PAGE_SHIFT);
+		pgd_t *pgdir;
+
 		pfn = gfn_to_pfn_memslot(slot, gfn);
 		if (is_error_noslot_pfn(pfn)) {
 			printk(KERN_ERR "Couldn't get real page for gfn %lx!\n",

@@ -447,9 +443,18 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
 		/* Align guest and physical address to page map boundaries */
 		pfn &= ~(tsize_pages - 1);
 		gvaddr &= ~((tsize_pages << PAGE_SHIFT) - 1);
+		pgdir = vcpu_e500->vcpu.arch.pgdir;
+		pte = lookup_linux_pte(pgdir, hva, 1, &tsize_pages);
+		if (pte_present(pte)) {
+			wimg = (pte >> PTE_WIMGE_SHIFT) & MAS2_WIMGE_MASK;
+		} else {
+			printk(KERN_ERR "pte not present: gfn %lx, pfn %lx\n",
+					(long)gfn, pfn);
+			return -EINVAL;
+		}
 	}
 
-	kvmppc_e500_ref_setup(ref, gtlbe, pfn);
+	kvmppc_e500_ref_setup(ref, gtlbe, pfn, wimg);
 
 	kvmppc_e500_setup_stlbe(&vcpu_e500->vcpu, gtlbe, tsize,
 				ref, gvaddr, stlbe);

-- 
1.7.0.4

On 08/01/2013 07:12 PM, Bharat Bhushan wrote:

99 more quoted lines

This should be
		wimg = (pte_val(pte) >> PTE_WIMGE_SHIFT) & MAS2_WIMGE_MASK;

Tiejun

On 08/01/2013 07:12 PM, Bharat Bhushan wrote:

32 more quoted lines

Looks this function is as same as book3s, so why not improve that as common :)

Tiejun

On 08/01/2013 07:12 PM, Bharat Bhushan wrote:

"G" bit in MAS2 indicates whether the page is Guarded.
There is no reason to stop guest setting  "E", so allow him.

Could we merge patch 2 and 3 into only one.

Tiejun

On Thu, 2013-08-01 at 16:42 +0530, Bharat Bhushan wrote:

_PAGE_BUSY is 0 on book3e.

What about 64-bit PTEs on 32-bit kernels?

In any case, this code does not belong in KVM.  It should be in the main
PPC mm code, even if KVM is the only user.

-Scott

On Thu, Aug 01, 2013 at 04:42:38PM +0530, Bharat Bhushan wrote:

kvmppc_fix_ee_before_entry() is supposed to be the last thing that
happens before __kvmppc_vcpu_run().

How does wimg get set in the pfnmap case?

Could you explain why we need to set dirty/referenced on the PTE, when we
didn't need to do that before?  All we're getting from the PTE is wimg. 
We have MMU notifiers to take care of the page being unmapped, and we've
already marked the page itself as dirty if the TLB entry is writeable.

-Scott

On Fri, 2013-08-02 at 17:58 -0500, Scott Wood wrote:

What about 64-bit PTEs on 32-bit kernels?

In any case, this code does not belong in KVM.  It should be in the
main
PPC mm code, even if KVM is the only user.

Also don't we do similar things in BookS KVM ? At the very least that
sutff should become common. And yes, I agree, it should probably also
move to pgtable*

Cheers,
Ben.

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogQmVuamFtaW4gSGVycmVu
c2NobWlkdCBbbWFpbHRvOmJlbmhAa2VybmVsLmNyYXNoaW5nLm9yZ10NCj4gU2VudDogU2F0dXJk
YXksIEF1Z3VzdCAwMywgMjAxMyA0OjQ3IEFNDQo+IFRvOiBXb29kIFNjb3R0LUIwNzQyMQ0KPiBD
YzogQmh1c2hhbiBCaGFyYXQtUjY1Nzc3OyBhZ3JhZkBzdXNlLmRlOyBrdm0tcHBjQHZnZXIua2Vy
bmVsLm9yZzsNCj4ga3ZtQHZnZXIua2VybmVsLm9yZzsgbGludXhwcGMtZGV2QGxpc3RzLm96bGFi
cy5vcmc7IEJodXNoYW4gQmhhcmF0LVI2NTc3Nw0KPiBTdWJqZWN0OiBSZTogW1BBVENIIDUvNiB2
Ml0ga3ZtOiBwb3dlcnBjOiBib29rZTogQWRkIGxpbnV4IHB0ZSBsb29rdXAgbGlrZQ0KPiBib29r
ZTNzDQo+IA0KPiBPbiBGcmksIDIwMTMtMDgtMDIgYXQgMTc6NTggLTA1MDAsIFNjb3R0IFdvb2Qg
d3JvdGU6DQo+ID4NCj4gPiBXaGF0IGFib3V0IDY0LWJpdCBQVEVzIG9uIDMyLWJpdCBrZXJuZWxz
Pw0KPiA+DQo+ID4gSW4gYW55IGNhc2UsIHRoaXMgY29kZSBkb2VzIG5vdCBiZWxvbmcgaW4gS1ZN
LiAgSXQgc2hvdWxkIGJlIGluIHRoZQ0KPiA+IG1haW4gUFBDIG1tIGNvZGUsIGV2ZW4gaWYgS1ZN
IGlzIHRoZSBvbmx5IHVzZXIuDQo+IA0KPiBBbHNvIGRvbid0IHdlIGRvIHNpbWlsYXIgdGhpbmdz
IGluIEJvb2tTIEtWTSA/IEF0IHRoZSB2ZXJ5IGxlYXN0IHRoYXQgc3V0ZmYNCj4gc2hvdWxkIGJl
Y29tZSBjb21tb24uIEFuZCB5ZXMsIEkgYWdyZWUsIGl0IHNob3VsZCBwcm9iYWJseSBhbHNvIG1v
dmUgdG8gcGd0YWJsZSoNCg0KT25lIG9mIHRoZSBwcm9ibGVtIEkgc2F3IHdhcyB0aGF0IGlmIEkg
cHV0IHRoaXMgY29kZSBpbiBhc20vcGd0YWJsZS0zMi5oIGFuZCBhc20vcGd0YWJsZS02NC5oIHRo
ZW4gcHRlX3BlcnNlbnQoKSBhbmQgb3RoZXIgZnJpZW5kIGZ1bmN0aW9uIChvbiB3aGljaCB0aGlz
IGNvZGUgZGVwZW5kcykgYXJlIGRlZmluZWQgaW4gcGd0YWJsZS5oLiBBbmQgcGd0YWJsZS5oIGlu
Y2x1ZGVzIGFzbS9wZ3RhYmxlLTMyLmggYW5kIGFzbS9wZ3RhYmxlLTY0LmggYmVmb3JlIGl0IGRl
ZmluZXMgcHRlX3ByZXNlbnQoKSBhbmQgZnJpZW5kcyBmdW5jdGlvbnMuDQoNCk9rIEkgbW92ZSB3
b3ZlIHRoaXMgaW4gYXNtL3BndGFibGUqLmgsIGluaXRpYWxseSBJIGZvdWdodCB3aXRoIG15c2Vs
ZiB0byB0YWtlIHRoaXMgY29kZSBpbiBwZ3RhYmxlKiBidXQgZmluYWxseSBlbmQgdXAgZG9pbmcg
aGVyZSAoZ290IGJpYXNlZCBieSBib29rM3MgOikpLg0KDQpUaGFua3MNCi1CaGFyYXQNCg0KPiAN
Cj4gQ2hlZXJzLA0KPiBCZW4uDQo+IA0KPiANCg0K

-----Original Message-----
From: Wood Scott-B07421
Sent: Saturday, August 03, 2013 5:05 AM
To: Bhushan Bharat-R65777
Cc: benh@kernel.crashing.org; <redacted>; <redacted>;
kvm@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; Bhushan Bharat-R65777
Subject: Re: [PATCH 6/6 v2] kvm: powerpc: use caching attributes as per l=

inux

pte
=20
On Thu, Aug 01, 2013 at 04:42:38PM +0530, Bharat Bhushan wrote:

quoted

diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index

17722d8..ebcccc2 100644

--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c

@@ -697,7 +697,7 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run,

struct kvm_vcpu *vcpu)  #endif

 	kvmppc_fix_ee_before_entry();
-
+	vcpu->arch.pgdir =3D current->mm->pgd;
 	ret =3D __kvmppc_vcpu_run(kvm_run, vcpu);

=20
kvmppc_fix_ee_before_entry() is supposed to be the last thing that happen=

s

before __kvmppc_vcpu_run().
=20

quoted

@@ -332,6 +324,8 @@ static inline int kvmppc_e500_shadow_map(struct

kvmppc_vcpu_e500 *vcpu_e500,

quoted

 	unsigned long hva;
 	int pfnmap =3D 0;
 	int tsize =3D BOOK3E_PAGESZ_4K;
+	pte_t pte;
+	int wimg =3D 0;

 	/*
 	 * Translate guest physical to true physical, acquiring @@ -437,6
+431,8 @@ static inline int kvmppc_e500_shadow_map(struct
kvmppc_vcpu_e500 *vcpu_e500,

 	if (likely(!pfnmap)) {
 		unsigned long tsize_pages =3D 1 << (tsize + 10 - PAGE_SHIFT);
+		pgd_t *pgdir;
+
 		pfn =3D gfn_to_pfn_memslot(slot, gfn);
 		if (is_error_noslot_pfn(pfn)) {
 			printk(KERN_ERR "Couldn't get real page for gfn %lx!\n", @@

-447,9

quoted

+443,18 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_=

e500

*vcpu_e500,

quoted

 		/* Align guest and physical address to page map boundaries */
 		pfn &=3D ~(tsize_pages - 1);
 		gvaddr &=3D ~((tsize_pages << PAGE_SHIFT) - 1);
+		pgdir =3D vcpu_e500->vcpu.arch.pgdir;
+		pte =3D lookup_linux_pte(pgdir, hva, 1, &tsize_pages);
+		if (pte_present(pte)) {
+			wimg =3D (pte >> PTE_WIMGE_SHIFT) & MAS2_WIMGE_MASK;
+		} else {
+			printk(KERN_ERR "pte not present: gfn %lx, pfn %lx\n",
+					(long)gfn, pfn);
+			return -EINVAL;
+		}
 	}

=20
How does wimg get set in the pfnmap case?

Pfnmap is not kernel managed pages, right? So should we set I+G there ?

=20
Could you explain why we need to set dirty/referenced on the PTE, when we=

 didn't

need to do that before? All we're getting from the PTE is wimg.
We have MMU notifiers to take care of the page being unmapped, and we've =

already

marked the page itself as dirty if the TLB entry is writeable.

I pulled this code from book3s.

Ben, can you describe why we need this on book3s ?

Thanks
-Bharat

=20
-Scott

On Sat, 2013-08-03 at 02:58 +0000, Bhushan Bharat-R65777 wrote:

One of the problem I saw was that if I put this code in
asm/pgtable-32.h and asm/pgtable-64.h then pte_persent() and other
friend function (on which this code depends) are defined in pgtable.h.
And pgtable.h includes asm/pgtable-32.h and asm/pgtable-64.h before it
defines pte_present() and friends functions.

Ok I move wove this in asm/pgtable*.h, initially I fought with myself
to take this code in pgtable* but finally end up doing here (got
biased by book3s :)).

Is there a reason why these routines can not be completely generic
in pgtable.h ?

Ben.

On Sat, 2013-08-03 at 03:11 +0000, Bhushan Bharat-R65777 wrote:

quoted

Could you explain why we need to set dirty/referenced on the PTE, when we didn't
need to do that before? All we're getting from the PTE is wimg.
We have MMU notifiers to take care of the page being unmapped, and we've already
marked the page itself as dirty if the TLB entry is writeable.

I pulled this code from book3s.

Ben, can you describe why we need this on book3s ?

If you let the guest write to the page you must set the dirty bit on the PTE
(or the struct page, at least one of them), similar with accessed on any access.

If you don't, the VM might swap the page out without writing it back to disk
for example, assuming it contains no modified data.

Cheers,
Ben.

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogQmVuamFtaW4gSGVycmVu
c2NobWlkdCBbbWFpbHRvOmJlbmhAa2VybmVsLmNyYXNoaW5nLm9yZ10NCj4gU2VudDogU2F0dXJk
YXksIEF1Z3VzdCAwMywgMjAxMyA5OjU0IEFNDQo+IFRvOiBCaHVzaGFuIEJoYXJhdC1SNjU3NzcN
Cj4gQ2M6IFdvb2QgU2NvdHQtQjA3NDIxOyBhZ3JhZkBzdXNlLmRlOyBrdm0tcHBjQHZnZXIua2Vy
bmVsLm9yZzsNCj4ga3ZtQHZnZXIua2VybmVsLm9yZzsgbGludXhwcGMtZGV2QGxpc3RzLm96bGFi
cy5vcmcNCj4gU3ViamVjdDogUmU6IFtQQVRDSCA1LzYgdjJdIGt2bTogcG93ZXJwYzogYm9va2U6
IEFkZCBsaW51eCBwdGUgbG9va3VwIGxpa2UNCj4gYm9va2Uzcw0KPiANCj4gT24gU2F0LCAyMDEz
LTA4LTAzIGF0IDAyOjU4ICswMDAwLCBCaHVzaGFuIEJoYXJhdC1SNjU3Nzcgd3JvdGU6DQo+ID4g
T25lIG9mIHRoZSBwcm9ibGVtIEkgc2F3IHdhcyB0aGF0IGlmIEkgcHV0IHRoaXMgY29kZSBpbg0K
PiA+IGFzbS9wZ3RhYmxlLTMyLmggYW5kIGFzbS9wZ3RhYmxlLTY0LmggdGhlbiBwdGVfcGVyc2Vu
dCgpIGFuZCBvdGhlcg0KPiA+IGZyaWVuZCBmdW5jdGlvbiAob24gd2hpY2ggdGhpcyBjb2RlIGRl
cGVuZHMpIGFyZSBkZWZpbmVkIGluIHBndGFibGUuaC4NCj4gPiBBbmQgcGd0YWJsZS5oIGluY2x1
ZGVzIGFzbS9wZ3RhYmxlLTMyLmggYW5kIGFzbS9wZ3RhYmxlLTY0LmggYmVmb3JlIGl0DQo+ID4g
ZGVmaW5lcyBwdGVfcHJlc2VudCgpIGFuZCBmcmllbmRzIGZ1bmN0aW9ucy4NCj4gPg0KPiA+IE9r
IEkgbW92ZSB3b3ZlIHRoaXMgaW4gYXNtL3BndGFibGUqLmgsIGluaXRpYWxseSBJIGZvdWdodCB3
aXRoIG15c2VsZg0KPiA+IHRvIHRha2UgdGhpcyBjb2RlIGluIHBndGFibGUqIGJ1dCBmaW5hbGx5
IGVuZCB1cCBkb2luZyBoZXJlIChnb3QNCj4gPiBiaWFzZWQgYnkgYm9vazNzIDopKS4NCj4gDQo+
IElzIHRoZXJlIGEgcmVhc29uIHdoeSB0aGVzZSByb3V0aW5lcyBjYW4gbm90IGJlIGNvbXBsZXRl
bHkgZ2VuZXJpYyBpbiBwZ3RhYmxlLmgNCj4gPw0KDQpIb3cgYWJvdXQgdGhlIGdlbmVyaWMgZnVu
Y3Rpb246DQoNCmRpZmYgLS1naXQgYS9hcmNoL3Bvd2VycGMvaW5jbHVkZS9hc20vcGd0YWJsZS1w
cGM2NC5oIGIvYXJjaC9wb3dlcnBjL2luY2x1ZGUvYXNtL3BndGFibGUtcHBjNjQuaA0KaW5kZXgg
ZDI1N2Q5OC4uMjFkYWYyOCAxMDA2NDQNCi0tLSBhL2FyY2gvcG93ZXJwYy9pbmNsdWRlL2FzbS9w
Z3RhYmxlLXBwYzY0LmgNCisrKyBiL2FyY2gvcG93ZXJwYy9pbmNsdWRlL2FzbS9wZ3RhYmxlLXBw
YzY0LmgNCkBAIC0yMjEsNiArMjIxLDI3IEBAIHN0YXRpYyBpbmxpbmUgdW5zaWduZWQgbG9uZyBw
dGVfdXBkYXRlKHN0cnVjdCBtbV9zdHJ1Y3QgKm1tLA0KICAgICAgICByZXR1cm4gb2xkOw0KIH0N
Cg0KK3N0YXRpYyBpbmxpbmUgdW5zaWduZWQgbG9uZyBwdGVfcmVhZChwdGVfdCAqcCkNCit7DQor
I2lmZGVmIFBURV9BVE9NSUNfVVBEQVRFUw0KKyAgICAgICBwdGVfdCBwdGU7DQorICAgICAgIHB0
ZV90IHRtcDsNCisgICAgICAgX19hc21fXyBfX3ZvbGF0aWxlX18gKA0KKyAgICAgICAiMTogICAg
IGxkYXJ4ICAgJTAsMCwlM1xuIg0KKyAgICAgICAiICAgICAgIGFuZGkuICAgJTEsJTAsJTRcbiIN
CisgICAgICAgIiAgICAgICBibmUtICAgIDFiXG4iDQorICAgICAgICIgICAgICAgb3JpICAgICAl
MSwlMCwlNFxuIg0KKyAgICAgICAiICAgICAgIHN0ZGN4LiAgJTEsMCwlM1xuIg0KKyAgICAgICAi
ICAgICAgIGJuZS0gICAgMWIiDQorICAgICAgIDogIj0mciIgKHB0ZSksICI9JnIiICh0bXApLCAi
PW0iICgqcCkNCisgICAgICAgOiAiciIgKHApLCAiaSIgKF9QQUdFX0JVU1kpDQorICAgICAgIDog
ImNjIik7DQorDQorICAgICAgIHJldHVybiBwdGU7DQorI2Vsc2UgIA0KKyAgICAgICByZXR1cm4g
cHRlX3ZhbCgqcCk7DQorI2VuZGlmDQorI2VuZGlmDQorfQ0KIHN0YXRpYyBpbmxpbmUgaW50IF9f
cHRlcF90ZXN0X2FuZF9jbGVhcl95b3VuZyhzdHJ1Y3QgbW1fc3RydWN0ICptbSwNCiAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICB1bnNpZ25lZCBsb25nIGFkZHIs
IHB0ZV90ICpwdGVwKQ0KIHsNCmRpZmYgLS1naXQgYS9hcmNoL3Bvd2VycGMvaW5jbHVkZS9hc20v
cGd0YWJsZS5oIGIvYXJjaC9wb3dlcnBjL2luY2x1ZGUvYXNtL3BndGFibGUuaA0KaW5kZXggNjkw
YzhjMi4uZGFkNzEyYyAxMDA2NDQNCi0tLSBhL2FyY2gvcG93ZXJwYy9pbmNsdWRlL2FzbS9wZ3Rh

24 more lines

On Sat, 2013-08-03 at 14:25 +1000, Benjamin Herrenschmidt wrote:

On Sat, 2013-08-03 at 03:11 +0000, Bhushan Bharat-R65777 wrote:

quoted

quoted

Could you explain why we need to set dirty/referenced on the PTE, when we didn't
need to do that before? All we're getting from the PTE is wimg.
We have MMU notifiers to take care of the page being unmapped, and we've already
marked the page itself as dirty if the TLB entry is writeable.

I pulled this code from book3s.

Ben, can you describe why we need this on book3s ?

If you let the guest write to the page you must set the dirty bit on the PTE
(or the struct page, at least one of them), similar with accessed on any access.

If you don't, the VM might swap the page out without writing it back to disk
for example, assuming it contains no modified data.

We've already marked the page itself as dirty using kvm_set_pfn_dirty(),
and if the VM swaps it out we'll get an MMU notifier callback.  If we
marked the PTE dirty/accessed instead, is there any guarantee it will
stay marked dirty/accessed until the next MMU notifier?

-Scott

On Fri, 2013-08-02 at 22:11 -0500, Bhushan Bharat-R65777 wrote:

quoted

How does wimg get set in the pfnmap case?

Pfnmap is not kernel managed pages, right? So should we set I+G there ?

It could depend on ppc_md.phys_mem_access_prot().  Can't you pull it
from the PTE regardless of pfnmap?

-Scott

On Mon, 2013-08-05 at 09:27 -0500, Bhushan Bharat-R65777 wrote:

18 more quoted lines

Please leave a blank line between functions.

The name implies that it just reads the PTE.  Setting accessed/dirty
shouldn't be an undocumented side-effect.  Why can't the caller do that
(or a different function that the caller calls afterward if desired)?  

Though even then you have the undocumented side effect of locking the
PTE on certain targets.

What about 32-bit platforms that need atomic PTEs?

-Scott

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogV29vZCBTY290dC1CMDc0
MjENCj4gU2VudDogVHVlc2RheSwgQXVndXN0IDA2LCAyMDEzIDEyOjQ5IEFNDQo+IFRvOiBCaHVz
aGFuIEJoYXJhdC1SNjU3NzcNCj4gQ2M6IEJlbmphbWluIEhlcnJlbnNjaG1pZHQ7IFdvb2QgU2Nv
dHQtQjA3NDIxOyBhZ3JhZkBzdXNlLmRlOyBrdm0tDQo+IHBwY0B2Z2VyLmtlcm5lbC5vcmc7IGt2
bUB2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4cHBjLWRldkBsaXN0cy5vemxhYnMub3JnDQo+IFN1Ympl
Y3Q6IFJlOiBbUEFUQ0ggNS82IHYyXSBrdm06IHBvd2VycGM6IGJvb2tlOiBBZGQgbGludXggcHRl
IGxvb2t1cCBsaWtlDQo+IGJvb2tlM3MNCj4gDQo+IE9uIE1vbiwgMjAxMy0wOC0wNSBhdCAwOToy
NyAtMDUwMCwgQmh1c2hhbiBCaGFyYXQtUjY1Nzc3IHdyb3RlOg0KPiA+DQo+ID4gPiAtLS0tLU9y
aWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiA+ID4gRnJvbTogQmVuamFtaW4gSGVycmVuc2NobWlkdCBb
bWFpbHRvOmJlbmhAa2VybmVsLmNyYXNoaW5nLm9yZ10NCj4gPiA+IFNlbnQ6IFNhdHVyZGF5LCBB
dWd1c3QgMDMsIDIwMTMgOTo1NCBBTQ0KPiA+ID4gVG86IEJodXNoYW4gQmhhcmF0LVI2NTc3Nw0K
PiA+ID4gQ2M6IFdvb2QgU2NvdHQtQjA3NDIxOyBhZ3JhZkBzdXNlLmRlOyBrdm0tcHBjQHZnZXIu
a2VybmVsLm9yZzsNCj4gPiA+IGt2bUB2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4cHBjLWRldkBsaXN0
cy5vemxhYnMub3JnDQo+ID4gPiBTdWJqZWN0OiBSZTogW1BBVENIIDUvNiB2Ml0ga3ZtOiBwb3dl
cnBjOiBib29rZTogQWRkIGxpbnV4IHB0ZQ0KPiA+ID4gbG9va3VwIGxpa2UgYm9va2Uzcw0KPiA+
ID4NCj4gPiA+IE9uIFNhdCwgMjAxMy0wOC0wMyBhdCAwMjo1OCArMDAwMCwgQmh1c2hhbiBCaGFy
YXQtUjY1Nzc3IHdyb3RlOg0KPiA+ID4gPiBPbmUgb2YgdGhlIHByb2JsZW0gSSBzYXcgd2FzIHRo
YXQgaWYgSSBwdXQgdGhpcyBjb2RlIGluDQo+ID4gPiA+IGFzbS9wZ3RhYmxlLTMyLmggYW5kIGFz
bS9wZ3RhYmxlLTY0LmggdGhlbiBwdGVfcGVyc2VudCgpIGFuZCBvdGhlcg0KPiA+ID4gPiBmcmll
bmQgZnVuY3Rpb24gKG9uIHdoaWNoIHRoaXMgY29kZSBkZXBlbmRzKSBhcmUgZGVmaW5lZCBpbiBw
Z3RhYmxlLmguDQo+ID4gPiA+IEFuZCBwZ3RhYmxlLmggaW5jbHVkZXMgYXNtL3BndGFibGUtMzIu
aCBhbmQgYXNtL3BndGFibGUtNjQuaA0KPiA+ID4gPiBiZWZvcmUgaXQgZGVmaW5lcyBwdGVfcHJl
c2VudCgpIGFuZCBmcmllbmRzIGZ1bmN0aW9ucy4NCj4gPiA+ID4NCj4gPiA+ID4gT2sgSSBtb3Zl
IHdvdmUgdGhpcyBpbiBhc20vcGd0YWJsZSouaCwgaW5pdGlhbGx5IEkgZm91Z2h0IHdpdGgNCj4g
PiA+ID4gbXlzZWxmIHRvIHRha2UgdGhpcyBjb2RlIGluIHBndGFibGUqIGJ1dCBmaW5hbGx5IGVu
ZCB1cCBkb2luZyBoZXJlDQo+ID4gPiA+IChnb3QgYmlhc2VkIGJ5IGJvb2szcyA6KSkuDQo+ID4g
Pg0KPiA+ID4gSXMgdGhlcmUgYSByZWFzb24gd2h5IHRoZXNlIHJvdXRpbmVzIGNhbiBub3QgYmUg
Y29tcGxldGVseSBnZW5lcmljDQo+ID4gPiBpbiBwZ3RhYmxlLmggPw0KPiA+DQo+ID4gSG93IGFi
b3V0IHRoZSBnZW5lcmljIGZ1bmN0aW9uOg0KPiA+DQo+ID4gZGlmZiAtLWdpdCBhL2FyY2gvcG93
ZXJwYy9pbmNsdWRlL2FzbS9wZ3RhYmxlLXBwYzY0LmgNCj4gPiBiL2FyY2gvcG93ZXJwYy9pbmNs
dWRlL2FzbS9wZ3RhYmxlLXBwYzY0LmgNCj4gPiBpbmRleCBkMjU3ZDk4Li4yMWRhZjI4IDEwMDY0
NA0KPiA+IC0tLSBhL2FyY2gvcG93ZXJwYy9pbmNsdWRlL2FzbS9wZ3RhYmxlLXBwYzY0LmgNCj4g
PiArKysgYi9hcmNoL3Bvd2VycGMvaW5jbHVkZS9hc20vcGd0YWJsZS1wcGM2NC5oDQo+ID4gQEAg
LTIyMSw2ICsyMjEsMjcgQEAgc3RhdGljIGlubGluZSB1bnNpZ25lZCBsb25nIHB0ZV91cGRhdGUo
c3RydWN0IG1tX3N0cnVjdA0KPiAqbW0sDQo+ID4gICAgICAgICByZXR1cm4gb2xkOw0KPiA+ICB9
DQo+ID4NCj4gPiArc3RhdGljIGlubGluZSB1bnNpZ25lZCBsb25nIHB0ZV9yZWFkKHB0ZV90ICpw
KSB7ICNpZmRlZg0KPiA+ICtQVEVfQVRPTUlDX1VQREFURVMNCj4gPiArICAgICAgIHB0ZV90IHB0
ZTsNCj4gPiArICAgICAgIHB0ZV90IHRtcDsNCj4gPiArICAgICAgIF9fYXNtX18gX192b2xhdGls
ZV9fICgNCj4gPiArICAgICAgICIxOiAgICAgbGRhcnggICAlMCwwLCUzXG4iDQo+ID4gKyAgICAg
ICAiICAgICAgIGFuZGkuICAgJTEsJTAsJTRcbiINCj4gPiArICAgICAgICIgICAgICAgYm5lLSAg

54 more lines

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogQmh1c2hhbiBCaGFyYXQt
UjY1Nzc3DQo+IFNlbnQ6IFR1ZXNkYXksIEF1Z3VzdCAwNiwgMjAxMyA2OjQyIEFNDQo+IFRvOiBX
b29kIFNjb3R0LUIwNzQyMQ0KPiBDYzogQmVuamFtaW4gSGVycmVuc2NobWlkdDsgYWdyYWZAc3Vz
ZS5kZTsga3ZtLXBwY0B2Z2VyLmtlcm5lbC5vcmc7DQo+IGt2bUB2Z2VyLmtlcm5lbC5vcmc7IGxp
bnV4cHBjLWRldkBsaXN0cy5vemxhYnMub3JnDQo+IFN1YmplY3Q6IFJFOiBbUEFUQ0ggNS82IHYy
XSBrdm06IHBvd2VycGM6IGJvb2tlOiBBZGQgbGludXggcHRlIGxvb2t1cCBsaWtlDQo+IGJvb2tl
M3MNCj4gDQo+IA0KPiANCj4gPiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiA+IEZyb206
IFdvb2QgU2NvdHQtQjA3NDIxDQo+ID4gU2VudDogVHVlc2RheSwgQXVndXN0IDA2LCAyMDEzIDEy
OjQ5IEFNDQo+ID4gVG86IEJodXNoYW4gQmhhcmF0LVI2NTc3Nw0KPiA+IENjOiBCZW5qYW1pbiBI
ZXJyZW5zY2htaWR0OyBXb29kIFNjb3R0LUIwNzQyMTsgYWdyYWZAc3VzZS5kZTsga3ZtLQ0KPiA+
IHBwY0B2Z2VyLmtlcm5lbC5vcmc7IGt2bUB2Z2VyLmtlcm5lbC5vcmc7DQo+ID4gbGludXhwcGMt
ZGV2QGxpc3RzLm96bGFicy5vcmcNCj4gPiBTdWJqZWN0OiBSZTogW1BBVENIIDUvNiB2Ml0ga3Zt
OiBwb3dlcnBjOiBib29rZTogQWRkIGxpbnV4IHB0ZSBsb29rdXANCj4gPiBsaWtlIGJvb2tlM3MN
Cj4gPg0KPiA+IE9uIE1vbiwgMjAxMy0wOC0wNSBhdCAwOToyNyAtMDUwMCwgQmh1c2hhbiBCaGFy
YXQtUjY1Nzc3IHdyb3RlOg0KPiA+ID4NCj4gPiA+ID4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0t
LS0NCj4gPiA+ID4gRnJvbTogQmVuamFtaW4gSGVycmVuc2NobWlkdCBbbWFpbHRvOmJlbmhAa2Vy
bmVsLmNyYXNoaW5nLm9yZ10NCj4gPiA+ID4gU2VudDogU2F0dXJkYXksIEF1Z3VzdCAwMywgMjAx
MyA5OjU0IEFNDQo+ID4gPiA+IFRvOiBCaHVzaGFuIEJoYXJhdC1SNjU3NzcNCj4gPiA+ID4gQ2M6
IFdvb2QgU2NvdHQtQjA3NDIxOyBhZ3JhZkBzdXNlLmRlOyBrdm0tcHBjQHZnZXIua2VybmVsLm9y
ZzsNCj4gPiA+ID4ga3ZtQHZnZXIua2VybmVsLm9yZzsgbGludXhwcGMtZGV2QGxpc3RzLm96bGFi
cy5vcmcNCj4gPiA+ID4gU3ViamVjdDogUmU6IFtQQVRDSCA1LzYgdjJdIGt2bTogcG93ZXJwYzog
Ym9va2U6IEFkZCBsaW51eCBwdGUNCj4gPiA+ID4gbG9va3VwIGxpa2UgYm9va2Uzcw0KPiA+ID4g
Pg0KPiA+ID4gPiBPbiBTYXQsIDIwMTMtMDgtMDMgYXQgMDI6NTggKzAwMDAsIEJodXNoYW4gQmhh
cmF0LVI2NTc3NyB3cm90ZToNCj4gPiA+ID4gPiBPbmUgb2YgdGhlIHByb2JsZW0gSSBzYXcgd2Fz
IHRoYXQgaWYgSSBwdXQgdGhpcyBjb2RlIGluDQo+ID4gPiA+ID4gYXNtL3BndGFibGUtMzIuaCBh
bmQgYXNtL3BndGFibGUtNjQuaCB0aGVuIHB0ZV9wZXJzZW50KCkgYW5kDQo+ID4gPiA+ID4gb3Ro
ZXIgZnJpZW5kIGZ1bmN0aW9uIChvbiB3aGljaCB0aGlzIGNvZGUgZGVwZW5kcykgYXJlIGRlZmlu
ZWQgaW4NCj4gcGd0YWJsZS5oLg0KPiA+ID4gPiA+IEFuZCBwZ3RhYmxlLmggaW5jbHVkZXMgYXNt
L3BndGFibGUtMzIuaCBhbmQgYXNtL3BndGFibGUtNjQuaA0KPiA+ID4gPiA+IGJlZm9yZSBpdCBk
ZWZpbmVzIHB0ZV9wcmVzZW50KCkgYW5kIGZyaWVuZHMgZnVuY3Rpb25zLg0KPiA+ID4gPiA+DQo+
ID4gPiA+ID4gT2sgSSBtb3ZlIHdvdmUgdGhpcyBpbiBhc20vcGd0YWJsZSouaCwgaW5pdGlhbGx5
IEkgZm91Z2h0IHdpdGgNCj4gPiA+ID4gPiBteXNlbGYgdG8gdGFrZSB0aGlzIGNvZGUgaW4gcGd0
YWJsZSogYnV0IGZpbmFsbHkgZW5kIHVwIGRvaW5nDQo+ID4gPiA+ID4gaGVyZSAoZ290IGJpYXNl
ZCBieSBib29rM3MgOikpLg0KPiA+ID4gPg0KPiA+ID4gPiBJcyB0aGVyZSBhIHJlYXNvbiB3aHkg
dGhlc2Ugcm91dGluZXMgY2FuIG5vdCBiZSBjb21wbGV0ZWx5IGdlbmVyaWMNCj4gPiA+ID4gaW4g
cGd0YWJsZS5oID8NCj4gPiA+DQo+ID4gPiBIb3cgYWJvdXQgdGhlIGdlbmVyaWMgZnVuY3Rpb246
DQo+ID4gPg0KPiA+ID4gZGlmZiAtLWdpdCBhL2FyY2gvcG93ZXJwYy9pbmNsdWRlL2FzbS9wZ3Rh
YmxlLXBwYzY0LmgNCj4gPiA+IGIvYXJjaC9wb3dlcnBjL2luY2x1ZGUvYXNtL3BndGFibGUtcHBj
NjQuaA0KPiA+ID4gaW5kZXggZDI1N2Q5OC4uMjFkYWYyOCAxMDA2NDQNCj4gPiA+IC0tLSBhL2Fy
Y2gvcG93ZXJwYy9pbmNsdWRlL2FzbS9wZ3RhYmxlLXBwYzY0LmgNCj4gPiA+ICsrKyBiL2FyY2gv

98 more lines

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogV29vZCBTY290dC1CMDc0
MjENCj4gU2VudDogVHVlc2RheSwgQXVndXN0IDA2LCAyMDEzIDEyOjQ5IEFNDQo+IFRvOiBCaHVz
aGFuIEJoYXJhdC1SNjU3NzcNCj4gQ2M6IEJlbmphbWluIEhlcnJlbnNjaG1pZHQ7IFdvb2QgU2Nv
dHQtQjA3NDIxOyBhZ3JhZkBzdXNlLmRlOyBrdm0tDQo+IHBwY0B2Z2VyLmtlcm5lbC5vcmc7IGt2
bUB2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4cHBjLWRldkBsaXN0cy5vemxhYnMub3JnDQo+IFN1Ympl
Y3Q6IFJlOiBbUEFUQ0ggNS82IHYyXSBrdm06IHBvd2VycGM6IGJvb2tlOiBBZGQgbGludXggcHRl
IGxvb2t1cCBsaWtlDQo+IGJvb2tlM3MNCj4gDQo+IE9uIE1vbiwgMjAxMy0wOC0wNSBhdCAwOToy
NyAtMDUwMCwgQmh1c2hhbiBCaGFyYXQtUjY1Nzc3IHdyb3RlOg0KPiA+DQo+ID4gPiAtLS0tLU9y
aWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiA+ID4gRnJvbTogQmVuamFtaW4gSGVycmVuc2NobWlkdCBb
bWFpbHRvOmJlbmhAa2VybmVsLmNyYXNoaW5nLm9yZ10NCj4gPiA+IFNlbnQ6IFNhdHVyZGF5LCBB
dWd1c3QgMDMsIDIwMTMgOTo1NCBBTQ0KPiA+ID4gVG86IEJodXNoYW4gQmhhcmF0LVI2NTc3Nw0K
PiA+ID4gQ2M6IFdvb2QgU2NvdHQtQjA3NDIxOyBhZ3JhZkBzdXNlLmRlOyBrdm0tcHBjQHZnZXIu
a2VybmVsLm9yZzsNCj4gPiA+IGt2bUB2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4cHBjLWRldkBsaXN0
cy5vemxhYnMub3JnDQo+ID4gPiBTdWJqZWN0OiBSZTogW1BBVENIIDUvNiB2Ml0ga3ZtOiBwb3dl
cnBjOiBib29rZTogQWRkIGxpbnV4IHB0ZQ0KPiA+ID4gbG9va3VwIGxpa2UgYm9va2Uzcw0KPiA+
ID4NCj4gPiA+IE9uIFNhdCwgMjAxMy0wOC0wMyBhdCAwMjo1OCArMDAwMCwgQmh1c2hhbiBCaGFy
YXQtUjY1Nzc3IHdyb3RlOg0KPiA+ID4gPiBPbmUgb2YgdGhlIHByb2JsZW0gSSBzYXcgd2FzIHRo
YXQgaWYgSSBwdXQgdGhpcyBjb2RlIGluDQo+ID4gPiA+IGFzbS9wZ3RhYmxlLTMyLmggYW5kIGFz
bS9wZ3RhYmxlLTY0LmggdGhlbiBwdGVfcGVyc2VudCgpIGFuZCBvdGhlcg0KPiA+ID4gPiBmcmll
bmQgZnVuY3Rpb24gKG9uIHdoaWNoIHRoaXMgY29kZSBkZXBlbmRzKSBhcmUgZGVmaW5lZCBpbiBw
Z3RhYmxlLmguDQo+ID4gPiA+IEFuZCBwZ3RhYmxlLmggaW5jbHVkZXMgYXNtL3BndGFibGUtMzIu
aCBhbmQgYXNtL3BndGFibGUtNjQuaA0KPiA+ID4gPiBiZWZvcmUgaXQgZGVmaW5lcyBwdGVfcHJl
c2VudCgpIGFuZCBmcmllbmRzIGZ1bmN0aW9ucy4NCj4gPiA+ID4NCj4gPiA+ID4gT2sgSSBtb3Zl
IHdvdmUgdGhpcyBpbiBhc20vcGd0YWJsZSouaCwgaW5pdGlhbGx5IEkgZm91Z2h0IHdpdGgNCj4g
PiA+ID4gbXlzZWxmIHRvIHRha2UgdGhpcyBjb2RlIGluIHBndGFibGUqIGJ1dCBmaW5hbGx5IGVu
ZCB1cCBkb2luZyBoZXJlDQo+ID4gPiA+IChnb3QgYmlhc2VkIGJ5IGJvb2szcyA6KSkuDQo+ID4g
Pg0KPiA+ID4gSXMgdGhlcmUgYSByZWFzb24gd2h5IHRoZXNlIHJvdXRpbmVzIGNhbiBub3QgYmUg
Y29tcGxldGVseSBnZW5lcmljDQo+ID4gPiBpbiBwZ3RhYmxlLmggPw0KPiA+DQo+ID4gSG93IGFi
b3V0IHRoZSBnZW5lcmljIGZ1bmN0aW9uOg0KPiA+DQo+ID4gZGlmZiAtLWdpdCBhL2FyY2gvcG93
ZXJwYy9pbmNsdWRlL2FzbS9wZ3RhYmxlLXBwYzY0LmgNCj4gPiBiL2FyY2gvcG93ZXJwYy9pbmNs
dWRlL2FzbS9wZ3RhYmxlLXBwYzY0LmgNCj4gPiBpbmRleCBkMjU3ZDk4Li4yMWRhZjI4IDEwMDY0
NA0KPiA+IC0tLSBhL2FyY2gvcG93ZXJwYy9pbmNsdWRlL2FzbS9wZ3RhYmxlLXBwYzY0LmgNCj4g
PiArKysgYi9hcmNoL3Bvd2VycGMvaW5jbHVkZS9hc20vcGd0YWJsZS1wcGM2NC5oDQo+ID4gQEAg
LTIyMSw2ICsyMjEsMjcgQEAgc3RhdGljIGlubGluZSB1bnNpZ25lZCBsb25nIHB0ZV91cGRhdGUo
c3RydWN0IG1tX3N0cnVjdA0KPiAqbW0sDQo+ID4gICAgICAgICByZXR1cm4gb2xkOw0KPiA+ICB9
DQo+ID4NCj4gPiArc3RhdGljIGlubGluZSB1bnNpZ25lZCBsb25nIHB0ZV9yZWFkKHB0ZV90ICpw
KSB7ICNpZmRlZg0KPiA+ICtQVEVfQVRPTUlDX1VQREFURVMNCj4gPiArICAgICAgIHB0ZV90IHB0
ZTsNCj4gPiArICAgICAgIHB0ZV90IHRtcDsNCj4gPiArICAgICAgIF9fYXNtX18gX192b2xhdGls
ZV9fICgNCj4gPiArICAgICAgICIxOiAgICAgbGRhcnggICAlMCwwLCUzXG4iDQo+ID4gKyAgICAg
ICAiICAgICAgIGFuZGkuICAgJTEsJTAsJTRcbiINCj4gPiArICAgICAgICIgICAgICAgYm5lLSAg

44 more lines

On Tue, Aug 06, 2013 at 07:02:48AM +0000, Bhushan Bharat-R65777 wrote:

I am trying to me the Linux pte search and update generic so that this can be used for powerpc as well.

I am not sure which of the below two should be ok, please help

Given that the BookE code uses gfn_to_pfn_memslot() to get the host
pfn, and then kvm_set_pfn_dirty(pfn) on pages that you're going to let
the guest write to, I don't think you need to set the dirty and/or
accessed bits in the Linux PTE explicitly.  If you care about the
WIMGE bits you can do find_linux_pte_or_hugepte() and just look at the
PTE, but you really should be using mmu_notifier_retry() to guard
against concurrent changes to the Linux PTE.  See the HV KVM code or
patch 21 of my recent series to see how it's used.  You probably
should be calling kvm_set_pfn_accessed() as well.

Paul.

On Wed, 2013-08-07 at 10:24 +1000, Paul Mackerras wrote:

On Tue, Aug 06, 2013 at 07:02:48AM +0000, Bhushan Bharat-R65777 wrote:

quoted

I am trying to me the Linux pte search and update generic so that this can be used for powerpc as well.

I am not sure which of the below two should be ok, please help

Given that the BookE code uses gfn_to_pfn_memslot() to get the host
pfn, and then kvm_set_pfn_dirty(pfn) on pages that you're going to let
the guest write to, I don't think you need to set the dirty and/or
accessed bits in the Linux PTE explicitly.  If you care about the
WIMGE bits you can do find_linux_pte_or_hugepte() and just look at the
PTE, but you really should be using mmu_notifier_retry() to guard
against concurrent changes to the Linux PTE.  See the HV KVM code or
patch 21 of my recent series to see how it's used. 

Hmm... we only get a callback on invalidate_range_start(), not
invalidate_range_end() (and even if we did get a callback for the
latter, it'd probably be racy).  So we may have a problem here
regardless of getting WIMG from the PTE, unless it's guaranteed that
hva_to_pfn() will fail after invalidate_range_start().

 You probably should be calling kvm_set_pfn_accessed() as well.

Yeah...  I think it'll only affect the quality of page-out decisions (as
opposed to corruption and such), but still it should be fixed.

-Scott

On Tue, Aug 06, 2013 at 08:11:34PM -0500, Scott Wood wrote:

On Wed, 2013-08-07 at 10:24 +1000, Paul Mackerras wrote:

quoted

On Tue, Aug 06, 2013 at 07:02:48AM +0000, Bhushan Bharat-R65777 wrote:

quoted

I am trying to me the Linux pte search and update generic so that this can be used for powerpc as well.

I am not sure which of the below two should be ok, please help

Given that the BookE code uses gfn_to_pfn_memslot() to get the host
pfn, and then kvm_set_pfn_dirty(pfn) on pages that you're going to let
the guest write to, I don't think you need to set the dirty and/or
accessed bits in the Linux PTE explicitly.  If you care about the
WIMGE bits you can do find_linux_pte_or_hugepte() and just look at the
PTE, but you really should be using mmu_notifier_retry() to guard
against concurrent changes to the Linux PTE.  See the HV KVM code or
patch 21 of my recent series to see how it's used. 

Hmm... we only get a callback on invalidate_range_start(), not
invalidate_range_end() (and even if we did get a callback for the
latter, it'd probably be racy).  So we may have a problem here
regardless of getting WIMG from the PTE, unless it's guaranteed that
hva_to_pfn() will fail after invalidate_range_start().

No, it's not guaranteed.  You have to use mmu_notifier_retry().  It
tells you if either (a) some sort of invalidation has happened since
you snapshotted kvm->mmu_notifier_seq, or (b) an
invalidate_range_start...end sequence is currently in progress.  In
either case you should discard any PTE or pfn information you
collected and retry.

quoted

 You probably should be calling kvm_set_pfn_accessed() as well.

Yeah...  I think it'll only affect the quality of page-out decisions (as
opposed to corruption and such), but still it should be fixed.

Right.

Paul.