RE: [PATCH 5/6 v2] kvm: powerpc: booke: Add linux pte lookup like booke3s
-Original Message-
From: Bhushan Bharat-R65777
Sent: Tuesday, August 06, 2013 6:42 AM
To: Wood Scott-B07421
Cc: Benjamin Herrenschmidt; ag...@suse.de; kvm-ppc@vger.kernel.org;
k...@vger.kernel.org; linuxppc-...@lists.ozlabs.org
Subject: RE: [PATCH 5/6 v2] kvm: powerpc: booke: Add linux pte lookup like
booke3s
-Original Message-
From: Wood Scott-B07421
Sent: Tuesday, August 06, 2013 12:49 AM
To: Bhushan Bharat-R65777
Cc: Benjamin Herrenschmidt; Wood Scott-B07421; ag...@suse.de; kvm-
p...@vger.kernel.org; k...@vger.kernel.org;
linuxppc-...@lists.ozlabs.org
Subject: Re: [PATCH 5/6 v2] kvm: powerpc: booke: Add linux pte lookup
like booke3s
On Mon, 2013-08-05 at 09:27 -0500, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Benjamin Herrenschmidt [mailto:b...@kernel.crashing.org]
Sent: Saturday, August 03, 2013 9:54 AM
To: Bhushan Bharat-R65777
Cc: Wood Scott-B07421; ag...@suse.de; kvm-ppc@vger.kernel.org;
k...@vger.kernel.org; linuxppc-...@lists.ozlabs.org
Subject: Re: [PATCH 5/6 v2] kvm: powerpc: booke: Add linux pte
lookup like booke3s
On Sat, 2013-08-03 at 02:58 +, 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 ?
How about the generic function:
diff --git a/arch/powerpc/include/asm/pgtable-ppc64.h
b/arch/powerpc/include/asm/pgtable-ppc64.h
index d257d98..21daf28 100644
--- a/arch/powerpc/include/asm/pgtable-ppc64.h
+++ b/arch/powerpc/include/asm/pgtable-ppc64.h
@@ -221,6 +221,27 @@ static inline unsigned long pte_update(struct
mm_struct
*mm,
return old;
}
+static inline unsigned long pte_read(pte_t *p) { #ifdef
+PTE_ATOMIC_UPDATES
+ pte_t pte;
+ pte_t tmp;
+ __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);
+
+ return pte;
+#else
+ return pte_val(*p);
+#endif
+#endif
+}
static inline int __ptep_test_and_clear_young(struct mm_struct *mm,
unsigned long addr,
pte_t *ptep)
Please leave a blank line between functions.
{
diff --git a/arch/powerpc/include/asm/pgtable.h
b/arch/powerpc/include/asm/pgtable.h
index 690c8c2..dad712c 100644
--- a/arch/powerpc/include/asm/pgtable.h
+++ b/arch/powerpc/include/asm/pgtable.h
@@ -254,6 +254,45 @@ static inline pte_t
*find_linux_pte_or_hugepte(pgd_t *pgdir, unsigned long ea, }
#endif
/* !CONFIG_HUGETLB_PAGE */
+static inline pte_t lookup_linux_pte(pgd_t *pgdir, unsigned long hva,
+int writing, unsigned long
+*pte_sizep)
The name implies that it just reads the PTE. Setting accessed/dirty
shouldn't be an undocumented side-effect.
Ok, will rename and document.
Why can't the caller do that (or a different function that the caller
calls afterward if desired)?
The current implementation in book3s is;
1) find a pte/hugepte
2) return null if pte not present
3) take _PAGE_BUSY lock
4) set accessed/dirty
5) clear _PAGE_BUSY.
What I tried was
1) find a pte/hugepte
2) return null if pte not present
3) return pte (not take lock by not setting _PAGE_BUSY)
4) then user calls __ptep_set_access_flags() to atomic update the
dirty/accessed flags in pte.
- but the benchmark results were not good
- Also can there be race as we do not take lock in step 3 and update in step
4 ?
Though even then you have the undocumented side effect of locking the
PTE on certain targets.
+{
+ pte_t *ptep;
+ pte_t pte;
+ 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);
+
+#ifdef CONFIG_PPC64
+ /* Lock
[PATCH 2/2] KVM: PPC: Book3E HV: call SOFT_DISABLE_INTS to sync the software state
We enter with interrupts disabled in hardware, but we need to call SOFT_DISABLE_INTS anyway to ensure that the software state is kept in sync. Signed-off-by: Tiejun Chen tiejun.c...@windriver.com --- arch/powerpc/kvm/bookehv_interrupts.S | 10 ++ 1 file changed, 10 insertions(+) diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S index e8ed7d6..4deaf2e 100644 --- a/arch/powerpc/kvm/bookehv_interrupts.S +++ b/arch/powerpc/kvm/bookehv_interrupts.S @@ -33,6 +33,8 @@ #ifdef CONFIG_64BIT #include asm/exception-64e.h +#include asm/hw_irq.h +#include asm/irqflags.h #else #include ../kernel/head_booke.h /* for THREAD_NORMSAVE() */ #endif @@ -465,6 +467,14 @@ _GLOBAL(kvmppc_resume_host) mtspr SPRN_EPCR, r3 isync +#ifdef CONFIG_64BIT + /* +* We enter with interrupts disabled in hardware, but +* we need to call SOFT_DISABLE_INTS anyway to ensure +* that the software state is kept in sync. +*/ + SOFT_DISABLE_INTS(r3,r5) +#endif /* Switch to kernel stack and jump to handler. */ PPC_LL r3, HOST_RUN(r1) mr r5, r14 /* intno */ -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] KVM: PPC: Book3E HV: Rework to sync the software state
For more detail please have a look at this :) http://patchwork.ozlabs.org/patch/257974/ http://ns1.yosemitephotos.net/lists/kvm-ppc/msg07430.html Tiejun Chen (2): Revert kvm/ppc/booke64: Fix lazy ee handling in kvmppc_handle_exit() KVM: PPC: Book3E HV: call SOFT_DISABLE_INTS to sync the software state arch/powerpc/kvm/booke.c | 11 --- arch/powerpc/kvm/bookehv_interrupts.S | 10 ++ 2 files changed, 10 insertions(+), 11 deletions(-) Tiejun -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] Revert kvm/ppc/booke64: Fix lazy ee handling in kvmppc_handle_exit()
We should revert this commit to rework. Signed-off-by: Tiejun Chen tiejun.c...@windriver.com --- arch/powerpc/kvm/booke.c | 11 --- 1 file changed, 11 deletions(-) diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 17722d8..7653c9c 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -833,17 +833,6 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, int s; int idx; -#ifdef CONFIG_PPC64 - WARN_ON(local_paca-irq_happened != 0); -#endif - - /* -* We enter with interrupts disabled in hardware, but -* we need to call hard_irq_disable anyway to ensure that -* the software state is kept in sync. -*/ - hard_irq_disable(); - /* update before a new last_exit_type is rewritten */ kvmppc_update_timing_stats(vcpu); -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/6 v3] powerpc: move linux pte/hugepte search to more generic file
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 bharat.bhus...@freescale.com
---
v2-v3
- no change
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
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/6 v3] kvm: powerpc: use caching attributes as per linux pte
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 bharat.bhus...@freescale.com
---
v2-v3
- setting pgdir before kvmppc_fix_ee_before_entry() on vcpu_run
- Aligned as per changes in patch 5/6
- setting WIMG for pfnmap pages also
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| 36 --
4 files changed, 28 insertions(+), 20 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..0d96d50 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -696,8 +696,8 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct
kvm_vcpu *vcpu)
kvmppc_load_guest_fp(vcpu);
#endif
+ vcpu-arch.pgdir = current-mm-pgd;
kvmppc_fix_ee_before_entry();
-
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..001a2b0 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,10 @@ static inline int kvmppc_e500_shadow_map(struct
kvmppc_vcpu_e500 *vcpu_e500,
unsigned long hva;
int pfnmap = 0;
int tsize = BOOK3E_PAGESZ_4K;
+ unsigned long tsize_pages = 0;
+ pte_t *ptep;
+ int wimg = 0;
+ pgd_t *pgdir;
/*
* Translate guest physical to true physical, acquiring
@@ -394,7 +390,7 @@ static inline int kvmppc_e500_shadow_map(struct
kvmppc_vcpu_e500 *vcpu_e500,
*/
for (; tsize BOOK3E_PAGESZ_4K; tsize -= 2) {
- unsigned long gfn_start, gfn_end, tsize_pages;
+
[PATCH 2/6 v3] kvm: powerpc: allow guest control E attribute in mas2
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 bharat.bhus...@freescale.com --- v2-v3 - no change 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 -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/6 v3] kvm: powerpc: allow guest control G attribute in mas2
G bit in MAS2 indicates whether the page is Guarded. There is no reason to stop guest setting G, so allow him. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- v2-v3 - no change 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 -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 5/6 v2] kvm: powerpc: booke: Add linux pte lookup like booke3s
-Original Message-
From: Wood Scott-B07421
Sent: Tuesday, August 06, 2013 12:49 AM
To: Bhushan Bharat-R65777
Cc: Benjamin Herrenschmidt; Wood Scott-B07421; ag...@suse.de; kvm-
p...@vger.kernel.org; k...@vger.kernel.org; linuxppc-...@lists.ozlabs.org
Subject: Re: [PATCH 5/6 v2] kvm: powerpc: booke: Add linux pte lookup like
booke3s
On Mon, 2013-08-05 at 09:27 -0500, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Benjamin Herrenschmidt [mailto:b...@kernel.crashing.org]
Sent: Saturday, August 03, 2013 9:54 AM
To: Bhushan Bharat-R65777
Cc: Wood Scott-B07421; ag...@suse.de; kvm-ppc@vger.kernel.org;
k...@vger.kernel.org; linuxppc-...@lists.ozlabs.org
Subject: Re: [PATCH 5/6 v2] kvm: powerpc: booke: Add linux pte
lookup like booke3s
On Sat, 2013-08-03 at 02:58 +, 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 ?
How about the generic function:
diff --git a/arch/powerpc/include/asm/pgtable-ppc64.h
b/arch/powerpc/include/asm/pgtable-ppc64.h
index d257d98..21daf28 100644
--- a/arch/powerpc/include/asm/pgtable-ppc64.h
+++ b/arch/powerpc/include/asm/pgtable-ppc64.h
@@ -221,6 +221,27 @@ static inline unsigned long pte_update(struct mm_struct
*mm,
return old;
}
+static inline unsigned long pte_read(pte_t *p) { #ifdef
+PTE_ATOMIC_UPDATES
+ pte_t pte;
+ pte_t tmp;
+ __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);
+
+ return pte;
+#else
+ return pte_val(*p);
+#endif
+#endif
+}
static inline int __ptep_test_and_clear_young(struct mm_struct *mm,
unsigned long addr,
pte_t *ptep)
Please leave a blank line between functions.
{
diff --git a/arch/powerpc/include/asm/pgtable.h
b/arch/powerpc/include/asm/pgtable.h
index 690c8c2..dad712c 100644
--- a/arch/powerpc/include/asm/pgtable.h
+++ b/arch/powerpc/include/asm/pgtable.h
@@ -254,6 +254,45 @@ static inline pte_t
*find_linux_pte_or_hugepte(pgd_t *pgdir, unsigned long ea, } #endif
/* !CONFIG_HUGETLB_PAGE */
+static inline pte_t lookup_linux_pte(pgd_t *pgdir, unsigned long hva,
+int writing, unsigned long
+*pte_sizep)
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)?
Scott, I sent the next version of patch based on above idea. Now I think we do
not need to update the pte flags on booke
So we do not need to solve the kvmppc_read_update_linux_pte() stuff of book3s.
-Bharat
Though even then you have the undocumented side effect of locking the PTE on
certain targets.
+{
+ pte_t *ptep;
+ pte_t pte;
+ 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);
+
+#ifdef CONFIG_PPC64
+ /* Lock PTE (set _PAGE_BUSY) and read */
+ pte = pte_read(ptep);
+#else
+ pte = pte_val(*ptep);
+#endif
What about 32-bit platforms that need atomic PTEs?
-Scott
N�r��yb�X��ǧv�^�){.n�+jir)w* jg��� �ݢj/���z�ޖ��2�ޙ�)ߡ�a���� �G���h��j:+v���w��٥
Re: [PATCH 1/2] Revert kvm/ppc/booke64: Fix lazy ee handling in kvmppc_handle_exit()
On Tue, 2013-08-06 at 17:31 +0800, Tiejun Chen wrote: We should revert this commit to rework. Signed-off-by: Tiejun Chen tiejun.c...@windriver.com This breaks bisect. -Scott -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/6 v2] kvm: powerpc: booke: Add linux pte lookup like booke3s
On Tue, Aug 06, 2013 at 07:02:48AM +, 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. -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/6 v2] kvm: powerpc: booke: Add linux pte lookup like booke3s
On Wed, 2013-08-07 at 10:24 +1000, Paul Mackerras wrote: On Tue, Aug 06, 2013 at 07:02:48AM +, 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. 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 -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/6 v2] kvm: powerpc: booke: Add linux pte lookup like booke3s
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: On Tue, Aug 06, 2013 at 07:02:48AM +, 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. 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. 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. -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Revert kvm/ppc/booke64: Fix lazy ee handling in kvmppc_handle_exit()
On 08/07/2013 04:50 AM, Scott Wood wrote: On Tue, 2013-08-06 at 17:31 +0800, Tiejun Chen wrote: We should revert this commit to rework. Signed-off-by: Tiejun Chen tiejun.c...@windriver.com This breaks bisect. Hmm... Maybe I can squash these two patches into one. Tiejun -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 21/23] KVM: PPC: Book3S PR: Use mmu_notifier_retry() in kvmppc_mmu_map_page()
-Original Message-
From: kvm-ow...@vger.kernel.org [mailto:kvm-ow...@vger.kernel.org] On Behalf
Of
Paul Mackerras
Sent: Tuesday, August 06, 2013 9:58 AM
To: Alexander Graf; Benjamin Herrenschmidt
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org
Subject: [PATCH 21/23] KVM: PPC: Book3S PR: Use mmu_notifier_retry() in
kvmppc_mmu_map_page()
When the MM code is invalidating a range of pages, it calls the KVM
kvm_mmu_notifier_invalidate_range_start() notifier function, which calls
kvm_unmap_hva_range(), which arranges to flush all the existing host HPTEs for
guest pages. However, the Linux PTEs for the range being flushed are still
valid at that point. We are not supposed to establish any new references to
pages in the range until the ...range_end() notifier gets called. The PPC-
specific KVM code doesn't get any explicit notification of that; instead, we
are
supposed to use
mmu_notifier_retry() to test whether we are or have been inside a range flush
notifier pair while we have been getting a page and instantiating a host HPTE
for the page.
This therefore adds a call to mmu_notifier_retry inside kvmppc_mmu_map_page().
This call is inside a region locked with
kvm-mmu_lock, which is the same lock that is called by the KVM
MMU notifier functions, thus ensuring that no new notification can proceed
while
we are in the locked region. Inside this region we also create the host HPTE
and link the corresponding hpte_cache structure into the lists used to find it
later. We cannot allocate the hpte_cache structure inside this locked region
because that can lead to deadlock, so we allocate it outside the region and
free
it if we end up not using it.
This also moves the updates of vcpu3s-hpte_cache_count inside the regions
locked with vcpu3s-mmu_lock, and does the increment in
kvmppc_mmu_hpte_cache_map() when the pte is added to the cache rather than
when
it is allocated, in order that the hpte_cache_count is accurate.
Signed-off-by: Paul Mackerras pau...@samba.org
---
arch/powerpc/include/asm/kvm_book3s.h | 1 +
arch/powerpc/kvm/book3s_64_mmu_host.c | 37 ++-
arch/powerpc/kvm/book3s_mmu_hpte.c| 14 +
3 files changed, 39 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_book3s.h
b/arch/powerpc/include/asm/kvm_book3s.h
index 4fe6864..e711e77 100644
--- a/arch/powerpc/include/asm/kvm_book3s.h
+++ b/arch/powerpc/include/asm/kvm_book3s.h
@@ -143,6 +143,7 @@ extern long kvmppc_hv_find_lock_hpte(struct kvm *kvm,
gva_t
eaddr,
extern void kvmppc_mmu_hpte_cache_map(struct kvm_vcpu *vcpu, struct
hpte_cache
*pte); extern struct hpte_cache *kvmppc_mmu_hpte_cache_next(struct kvm_vcpu
*vcpu);
+extern void kvmppc_mmu_hpte_cache_free(struct hpte_cache *pte);
extern void kvmppc_mmu_hpte_destroy(struct kvm_vcpu *vcpu); extern int
kvmppc_mmu_hpte_init(struct kvm_vcpu *vcpu); extern void
kvmppc_mmu_invalidate_pte(struct kvm_vcpu *vcpu, struct hpte_cache *pte);
diff -
-git a/arch/powerpc/kvm/book3s_64_mmu_host.c
b/arch/powerpc/kvm/book3s_64_mmu_host.c
index 7fcf38f..b7e9504 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_host.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_host.c
@@ -93,6 +93,13 @@ int kvmppc_mmu_map_page(struct kvm_vcpu *vcpu, struct
kvmppc_pte *orig_pte,
int r = 0;
int hpsize = MMU_PAGE_4K;
bool writable;
+ unsigned long mmu_seq;
+ struct kvm *kvm = vcpu-kvm;
+ struct hpte_cache *cpte;
+
+ /* used to check for invalidations in progress */
+ mmu_seq = kvm-mmu_notifier_seq;
+ smp_rmb();
Should not the smp_rmb() come before reading kvm-mmu_notifier_seq.
-Bharat
/* Get host physical address for gpa */
hpaddr = kvmppc_gfn_to_pfn(vcpu, orig_pte-raddr PAGE_SHIFT, @@
-143,6
+150,14 @@ int kvmppc_mmu_map_page(struct kvm_vcpu *vcpu, struct kvmppc_pte
*orig_pte,
hash = hpt_hash(vpn, mmu_psize_defs[hpsize].shift, MMU_SEGSIZE_256M);
+ cpte = kvmppc_mmu_hpte_cache_next(vcpu);
+
+ spin_lock(kvm-mmu_lock);
+ if (!cpte || mmu_notifier_retry(kvm, mmu_seq)) {
+ r = -EAGAIN;
+ goto out_unlock;
+ }
+
map_again:
hpteg = ((hash htab_hash_mask) * HPTES_PER_GROUP);
@@ -150,7 +165,7 @@ map_again:
if (attempt 1)
if (ppc_md.hpte_remove(hpteg) 0) {
r = -1;
- goto out;
+ goto out_unlock;
}
ret = ppc_md.hpte_insert(hpteg, vpn, hpaddr, rflags, vflags, @@ -163,8
+178,6 @@ map_again:
attempt++;
goto map_again;
} else {
- struct hpte_cache *pte = kvmppc_mmu_hpte_cache_next(vcpu);
-
trace_kvm_book3s_64_mmu_map(rflags, hpteg,
vpn, hpaddr, orig_pte);
@@ -175,15 +188,21 @@ map_again:
hpteg = ((hash
Re: [PATCH 21/23] KVM: PPC: Book3S PR: Use mmu_notifier_retry() in kvmppc_mmu_map_page()
On Wed, Aug 07, 2013 at 04:13:34AM +, Bhushan Bharat-R65777 wrote: + /* used to check for invalidations in progress */ + mmu_seq = kvm-mmu_notifier_seq; + smp_rmb(); Should not the smp_rmb() come before reading kvm-mmu_notifier_seq. No, it should come after, because it is ordering the read of kvm-mmu_notifier_seq before the read of the Linux PTE. Paul. -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 21/23] KVM: PPC: Book3S PR: Use mmu_notifier_retry() in kvmppc_mmu_map_page()
-Original Message-
From: kvm-ow...@vger.kernel.org [mailto:kvm-ow...@vger.kernel.org] On Behalf
Of
Paul Mackerras
Sent: Tuesday, August 06, 2013 9:58 AM
To: Alexander Graf; Benjamin Herrenschmidt
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org
Subject: [PATCH 21/23] KVM: PPC: Book3S PR: Use mmu_notifier_retry() in
kvmppc_mmu_map_page()
When the MM code is invalidating a range of pages, it calls the KVM
kvm_mmu_notifier_invalidate_range_start() notifier function, which calls
kvm_unmap_hva_range(), which arranges to flush all the existing host
HPTEs for guest pages. However, the Linux PTEs for the range being
flushed are still valid at that point. We are not supposed to establish
any new references to pages in the range until the ...range_end()
notifier gets called. The PPC-specific KVM code doesn't get any
explicit notification of that; instead, we are supposed to use
mmu_notifier_retry() to test whether we are or have been inside a
range flush notifier pair while we have been getting a page and
instantiating a host HPTE for the page.
This therefore adds a call to mmu_notifier_retry inside
kvmppc_mmu_map_page(). This call is inside a region locked with
kvm-mmu_lock, which is the same lock that is called by the KVM
MMU notifier functions, thus ensuring that no new notification can
proceed while we are in the locked region. Inside this region we
also create the host HPTE and link the corresponding hpte_cache
structure into the lists used to find it later. We cannot allocate
the hpte_cache structure inside this locked region because that can
lead to deadlock, so we allocate it outside the region and free it
if we end up not using it.
This also moves the updates of vcpu3s-hpte_cache_count inside the
regions locked with vcpu3s-mmu_lock, and does the increment in
kvmppc_mmu_hpte_cache_map() when the pte is added to the cache
rather than when it is allocated, in order that the hpte_cache_count
is accurate.
Signed-off-by: Paul Mackerras pau...@samba.org
---
arch/powerpc/include/asm/kvm_book3s.h | 1 +
arch/powerpc/kvm/book3s_64_mmu_host.c | 37
++-
arch/powerpc/kvm/book3s_mmu_hpte.c| 14 +
3 files changed, 39 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_book3s.h
b/arch/powerpc/include/asm/kvm_book3s.h
index 4fe6864..e711e77 100644
--- a/arch/powerpc/include/asm/kvm_book3s.h
+++ b/arch/powerpc/include/asm/kvm_book3s.h
@@ -143,6 +143,7 @@ extern long kvmppc_hv_find_lock_hpte(struct kvm *kvm,
gva_t
eaddr,
extern void kvmppc_mmu_hpte_cache_map(struct kvm_vcpu *vcpu, struct
hpte_cache
*pte);
extern struct hpte_cache *kvmppc_mmu_hpte_cache_next(struct kvm_vcpu *vcpu);
+extern void kvmppc_mmu_hpte_cache_free(struct hpte_cache *pte);
extern void kvmppc_mmu_hpte_destroy(struct kvm_vcpu *vcpu);
extern int kvmppc_mmu_hpte_init(struct kvm_vcpu *vcpu);
extern void kvmppc_mmu_invalidate_pte(struct kvm_vcpu *vcpu, struct
hpte_cache
*pte);
diff --git a/arch/powerpc/kvm/book3s_64_mmu_host.c
b/arch/powerpc/kvm/book3s_64_mmu_host.c
index 7fcf38f..b7e9504 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_host.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_host.c
@@ -93,6 +93,13 @@ int kvmppc_mmu_map_page(struct kvm_vcpu *vcpu, struct
kvmppc_pte *orig_pte,
int r = 0;
int hpsize = MMU_PAGE_4K;
bool writable;
+ unsigned long mmu_seq;
+ struct kvm *kvm = vcpu-kvm;
+ struct hpte_cache *cpte;
+
+ /* used to check for invalidations in progress */
+ mmu_seq = kvm-mmu_notifier_seq;
+ smp_rmb();
/* Get host physical address for gpa */
hpaddr = kvmppc_gfn_to_pfn(vcpu, orig_pte-raddr PAGE_SHIFT,
@@ -143,6 +150,14 @@ int kvmppc_mmu_map_page(struct kvm_vcpu *vcpu, struct
kvmppc_pte *orig_pte,
hash = hpt_hash(vpn, mmu_psize_defs[hpsize].shift, MMU_SEGSIZE_256M);
+ cpte = kvmppc_mmu_hpte_cache_next(vcpu);
+
+ spin_lock(kvm-mmu_lock);
+ if (!cpte || mmu_notifier_retry(kvm, mmu_seq)) {
+ r = -EAGAIN;
Pauls, I am trying to understand the flow; does retry mean that we do not
create the mapping and return to guest, which will fault again and then we will
retry?
Thanks
-Bharat
+ goto out_unlock;
+ }
+
map_again:
hpteg = ((hash htab_hash_mask) * HPTES_PER_GROUP);
@@ -150,7 +165,7 @@ map_again:
if (attempt 1)
if (ppc_md.hpte_remove(hpteg) 0) {
r = -1;
- goto out;
+ goto out_unlock;
}
ret = ppc_md.hpte_insert(hpteg, vpn, hpaddr, rflags, vflags,
@@ -163,8 +178,6 @@ map_again:
attempt++;
goto map_again;
} else {
- struct hpte_cache *pte = kvmppc_mmu_hpte_cache_next(vcpu);
-
trace_kvm_book3s_64_mmu_map(rflags, hpteg,
vpn,
RE: [PATCH 21/23] KVM: PPC: Book3S PR: Use mmu_notifier_retry() in kvmppc_mmu_map_page()
-Original Message- From: Paul Mackerras [mailto:pau...@samba.org] Sent: Wednesday, August 07, 2013 9:59 AM To: Bhushan Bharat-R65777 Cc: Alexander Graf; Benjamin Herrenschmidt; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH 21/23] KVM: PPC: Book3S PR: Use mmu_notifier_retry() in kvmppc_mmu_map_page() On Wed, Aug 07, 2013 at 04:13:34AM +, Bhushan Bharat-R65777 wrote: + /* used to check for invalidations in progress */ + mmu_seq = kvm-mmu_notifier_seq; + smp_rmb(); Should not the smp_rmb() come before reading kvm-mmu_notifier_seq. No, it should come after, because it is ordering the read of kvm-mmu_notifier_seq before the read of the Linux PTE. Ahh, ok. Thanks -Bharat Paul. -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
