RE: [PATCH 4/4] kvm: powerpc: set cache coherency only for RAM pages

2013-07-30 Thread Bhushan Bharat-R65777


> -Original Message-
> From: Wood Scott-B07421
> Sent: Wednesday, July 31, 2013 12:19 AM
> To: Bhushan Bharat-R65777
> Cc: Benjamin Herrenschmidt; Alexander Graf; kvm-ppc@vger.kernel.org;
> k...@vger.kernel.org; linuxppc-...@lists.ozlabs.org; Wood Scott-B07421
> Subject: Re: [PATCH 4/4] kvm: powerpc: set cache coherency only for RAM pages
> 
> On 07/30/2013 11:22:54 AM, Bhushan Bharat-R65777 wrote:
> > diff --git a/arch/powerpc/kvm/e500_mmu_host.c
> > b/arch/powerpc/kvm/e500_mmu_host.c
> > index 5cbdc8f..a48c13f 100644
> > --- a/arch/powerpc/kvm/e500_mmu_host.c
> > +++ b/arch/powerpc/kvm/e500_mmu_host.c
> > @@ -40,6 +40,84 @@
> >
> >  static struct kvmppc_e500_tlb_params host_tlb_params[E500_TLB_NUM];
> >
> > +/*
> > + * 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;
> > +}
> 
> How is this specific to KVM or e500?
> 
> > +#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 */
> 
> This is already declared in asm/pgtable.h.  If we need a non-hugepage
> alternative, that should also go in asm/pgtable.h.
> 
> > +/*
> > + * 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 = pte_val(*p);
> > +
> > +   if (pte_present(pte)) {
> > +   pte = pte_mkyoung(pte);
> > +   if (writing && pte_write(pte))
> > +   pte = pte_mkdirty(pte);
> > +   }
> > +
> > +   *p = pte;
> > +
> > +   return pte;
> > +}
> > +
> > +static 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); }
> > +
> 
> None of this belongs in this file either.
> 
> > @@ -326,8 +405,8 @@ 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, pfn);
> > +   stlbe->mas2 = (gvaddr & MAS2_EPN) | (ref->flags &
> > E500_TLB_WIMGE_MASK);
> > +//   e500_shadow_mas2_attrib(gtlbe->mas2, pfn);
> 
> MAS2_E and MAS2_G should be safe to come from the guest.

This is handled when setting WIMGE in ref->flags.

> 
> How does this work for TLB1?  One ref corresponds to one guest entry, which 
> may
> correspond to multiple host entries, potentially each with different WIM
> settings.

Yes, one ref corresponds to one guest e

Re: [PATCH 4/4] kvm: powerpc: set cache coherency only for RAM pages

2013-07-30 Thread Scott Wood

On 07/30/2013 11:22:54 AM, Bhushan Bharat-R65777 wrote:
diff --git a/arch/powerpc/kvm/e500_mmu_host.c  
b/arch/powerpc/kvm/e500_mmu_host.c

index 5cbdc8f..a48c13f 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -40,6 +40,84 @@

 static struct kvmppc_e500_tlb_params host_tlb_params[E500_TLB_NUM];

+/*
+ * 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;
+}


How is this specific to KVM or e500?


+#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 */


This is already declared in asm/pgtable.h.  If we need a non-hugepage  
alternative, that should also go in asm/pgtable.h.



+/*
+ * 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 = pte_val(*p);
+
+   if (pte_present(pte)) {
+   pte = pte_mkyoung(pte);
+   if (writing && pte_write(pte))
+   pte = pte_mkdirty(pte);
+   }
+
+   *p = pte;
+
+   return pte;
+}
+
+static 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);
+}
+


None of this belongs in this file either.


@@ -326,8 +405,8 @@ 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, pfn);
+   stlbe->mas2 = (gvaddr & MAS2_EPN) | (ref->flags &  
E500_TLB_WIMGE_MASK);

+//   e500_shadow_mas2_attrib(gtlbe->mas2, pfn);


MAS2_E and MAS2_G should be safe to come from the guest.

How does this work for TLB1?  One ref corresponds to one guest entry,  
which may correspond to multiple host entries, potentially each with  
different WIM settings.



stlbe->mas7_3 = ((u64)pfn << PAGE_SHIFT) |
e500_shadow_mas3_attrib(gtlbe->mas7_3, pr);

@@ -346,6 +425,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
@@ -451,6 +532,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",
@@ -461,9 +544,15 @@ 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
+   wimg = MAS2_I | MAS2_G;


If the PTE is not present, then we can't map it, right?  So why I+G?

RE: [PATCH 4/4] kvm: powerpc: set cache coherency only for RAM pages

2013-07-30 Thread Bhushan Bharat-R65777


> -Original Message-
> From: Benjamin Herrenschmidt [mailto:b...@kernel.crashing.org]
> Sent: Saturday, July 27, 2013 3:57 AM
> To: Bhushan Bharat-R65777
> Cc: Alexander Graf; kvm-ppc@vger.kernel.org; k...@vger.kernel.org; linuxppc-
> d...@lists.ozlabs.org; Wood Scott-B07421
> Subject: Re: [PATCH 4/4] kvm: powerpc: set cache coherency only for RAM pages
> 
> On Fri, 2013-07-26 at 15:03 +, Bhushan Bharat-R65777 wrote:
> > Will not searching the Linux PTE is a overkill?
> 
> That's the best approach. Also we are searching it already to resolve the page
> fault. That does mean we search twice but on the other hand that also means 
> it's
> hot in the cache.


Below is early git diff (not a proper cleanup patch), to be sure that this is 
what we want on PowerPC and take early feedback. Also I run some benchmark to 
understand the overhead if any. 

Using kvm_is_mmio_pfn(); what the current patch does:   

Real: 0m46.616s + 0m49.517s + 0m49.510s + 0m46.936s + 0m46.889s + 0m46.684s = 
Avg; 47.692s
User: 0m31.636s + 0m31.816s + 0m31.456s + 0m31.752s + 0m32.028s + 0m31.848s = 
Avg; 31.756s
Sys:  0m11.596s + 0m11.868s + 0m12.244s + 0m11.672s + 0m11.356s + 0m11.432s = 
Avg; 11.695s


Using kernel page table search (below changes):
Real: 0m46.431s + 0m50.269s + 0m46.724s + 0m46.645s + 0m46.670s + 0m50.259s = 
Avg; 47.833s
User: 0m31.568s + 0m31.816s + 0m31.444s + 0m31.808s + 0m31.312s + 0m31.740s = 
Avg; 31.614s
Sys:  0m11.516s + 0m12.060s + 0m11.872s + 0m11.476s + 0m12.000s + 0m12.152s = 
Avg; 11.846s

--
diff --git a/arch/powerpc/include/asm/kvm_host.h 
b/arch/powerpc/include/asm/kvm_host.h
index 3328353..d6d0dac 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -532,6 +532,7 @@ struct kvm_vcpu_arch {
u32 epr;
u32 crit_save;
struct kvmppc_booke_debug_reg dbg_reg;
+   pgd_t *pgdir;
 #endif
gpa_t paddr_accessed;
gva_t vaddr_accessed;
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 17722d8..eb2 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 5cbdc8f..a48c13f 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -40,6 +40,84 @@
 
 static struct kvmppc_e500_tlb_params host_tlb_params[E500_TLB_NUM];
 
+/*
+ * 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 */
+
+/*
+ * 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 kvmp

Re: [PATCH 4/4] kvm: powerpc: set cache coherency only for RAM pages

2013-07-26 Thread Benjamin Herrenschmidt
On Fri, 2013-07-26 at 15:03 +, Bhushan Bharat-R65777 wrote:
> Will not searching the Linux PTE is a overkill?

That's the best approach. Also we are searching it already to resolve
the page fault. That does mean we search twice but on the other hand
that also means it's hot in the cache.

Cheers,
Ben


--
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 4/4] kvm: powerpc: set cache coherency only for RAM pages

2013-07-26 Thread Bhushan Bharat-R65777


> -Original Message-
> From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc-ow...@vger.kernel.org] On
> Behalf Of Alexander Graf
> Sent: Friday, July 26, 2013 2:20 PM
> To: Benjamin Herrenschmidt
> Cc: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org; k...@vger.kernel.org;
> linuxppc-...@lists.ozlabs.org; Wood Scott-B07421; Bhushan Bharat-R65777
> Subject: Re: [PATCH 4/4] kvm: powerpc: set cache coherency only for RAM pages
> 
> 
> On 26.07.2013, at 10:26, Benjamin Herrenschmidt wrote:
> 
> > On Fri, 2013-07-26 at 11:16 +0530, Bharat Bhushan wrote:
> >> If the page is RAM then map this as cacheable and coherent (set "M"
> >> bit) otherwise this page is treated as I/O and map this as cache
> >> inhibited and guarded (set  "I + G")
> >>
> >> This helps setting proper MMU mapping for direct assigned device.
> >>
> >> NOTE: There can be devices that require cacheable mapping, which is not yet
> supported.
> >
> > Why don't you do like server instead and enforce the use of the same I
> > and M bits as the corresponding qemu PTE ?
> 
> Specifically, Ben is talking about this code:
> 
> 
> /* Translate to host virtual address */
> hva = __gfn_to_hva_memslot(memslot, gfn);
> 
> /* Look up the Linux PTE for the backing page */
> pte_size = psize;
> pte = lookup_linux_pte(pgdir, hva, writing, &pte_size);
> if (pte_present(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;
> }
> 

Will not searching the Linux PTE is a overkill?

=Bharat



--
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 4/4] kvm: powerpc: set cache coherency only for RAM pages

2013-07-26 Thread Bhushan Bharat-R65777


> -Original Message-
> From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc-ow...@vger.kernel.org] On
> Behalf Of Alexander Graf
> Sent: Friday, July 26, 2013 2:20 PM
> To: Benjamin Herrenschmidt
> Cc: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org; k...@vger.kernel.org;
> linuxppc-...@lists.ozlabs.org; Wood Scott-B07421; Bhushan Bharat-R65777
> Subject: Re: [PATCH 4/4] kvm: powerpc: set cache coherency only for RAM pages
> 
> 
> On 26.07.2013, at 10:26, Benjamin Herrenschmidt wrote:
> 
> > On Fri, 2013-07-26 at 11:16 +0530, Bharat Bhushan wrote:
> >> If the page is RAM then map this as cacheable and coherent (set "M"
> >> bit) otherwise this page is treated as I/O and map this as cache
> >> inhibited and guarded (set  "I + G")
> >>
> >> This helps setting proper MMU mapping for direct assigned device.
> >>
> >> NOTE: There can be devices that require cacheable mapping, which is not yet
> supported.
> >
> > Why don't you do like server instead and enforce the use of the same I
> > and M bits as the corresponding qemu PTE ?
> 
> Specifically, Ben is talking about this code:
> 
> 
> /* Translate to host virtual address */
> hva = __gfn_to_hva_memslot(memslot, gfn);
> 
> /* Look up the Linux PTE for the backing page */
> pte_size = psize;
> pte = lookup_linux_pte(pgdir, hva, writing, &pte_size);
> if (pte_present(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;
> }
> 

Ok

Thanks
-Bharat


> 
> Alex
> 
> --
> 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


--
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 4/4] kvm: powerpc: set cache coherency only for RAM pages

2013-07-26 Thread Bhushan Bharat-R65777


> -Original Message-
> From: Benjamin Herrenschmidt [mailto:b...@kernel.crashing.org]
> Sent: Friday, July 26, 2013 1:57 PM
> To: Bhushan Bharat-R65777
> Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; 
> linuxppc-...@lists.ozlabs.org;
> ag...@suse.de; Wood Scott-B07421; Bhushan Bharat-R65777
> Subject: Re: [PATCH 4/4] kvm: powerpc: set cache coherency only for RAM pages
> 
> On Fri, 2013-07-26 at 11:16 +0530, Bharat Bhushan wrote:
> > If the page is RAM then map this as cacheable and coherent (set "M"
> > bit) otherwise this page is treated as I/O and map this as cache
> > inhibited and guarded (set  "I + G")
> >
> > This helps setting proper MMU mapping for direct assigned device.
> >
> > NOTE: There can be devices that require cacheable mapping, which is not yet
> supported.
> 
> Why don't you do like server instead and enforce the use of the same I and M
> bits as the corresponding qemu PTE ?

Ben/Alex, I will look into the code. Can you please describe how this is 
handled on server?

Thanks
-Bharat

> 
> Cheers,
> Ben.
> 
> > Signed-off-by: Bharat Bhushan 
> > ---
> >  arch/powerpc/kvm/e500_mmu_host.c |   24 +++-
> >  1 files changed, 19 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/powerpc/kvm/e500_mmu_host.c
> > b/arch/powerpc/kvm/e500_mmu_host.c
> > index 1c6a9d7..5cbdc8f 100644
> > --- a/arch/powerpc/kvm/e500_mmu_host.c
> > +++ b/arch/powerpc/kvm/e500_mmu_host.c
> > @@ -64,13 +64,27 @@ static inline u32 e500_shadow_mas3_attrib(u32 mas3, int
> usermode)
> > return mas3;
> >  }
> >
> > -static inline u32 e500_shadow_mas2_attrib(u32 mas2, int usermode)
> > +static inline u32 e500_shadow_mas2_attrib(u32 mas2, pfn_t pfn)
> >  {
> > +   u32 mas2_attr;
> > +
> > +   mas2_attr = mas2 & MAS2_ATTRIB_MASK;
> > +
> > +   if (kvm_is_mmio_pfn(pfn)) {
> > +   /*
> > +* If page is not RAM then it is treated as I/O page.
> > +* Map it with cache inhibited and guarded (set "I" + "G").
> > +*/
> > +   mas2_attr |= MAS2_I | MAS2_G;
> > +   return mas2_attr;
> > +   }
> > +
> > +   /* Map RAM pages as cacheable (Not setting "I" in MAS2) */
> >  #ifdef CONFIG_SMP
> > -   return (mas2 & MAS2_ATTRIB_MASK) | MAS2_M;
> > -#else
> > -   return mas2 & MAS2_ATTRIB_MASK;
> > +   /* Also map as coherent (set "M") in SMP */
> > +   mas2_attr |= MAS2_M;
> >  #endif
> > +   return mas2_attr;
> >  }
> >
> >  /*
> > @@ -313,7 +327,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);
> > + e500_shadow_mas2_attrib(gtlbe->mas2, pfn);
> > stlbe->mas7_3 = ((u64)pfn << PAGE_SHIFT) |
> > e500_shadow_mas3_attrib(gtlbe->mas7_3, pr);
> >
> 
> 



Re: [PATCH 4/4] kvm: powerpc: set cache coherency only for RAM pages

2013-07-26 Thread Alexander Graf

On 26.07.2013, at 10:26, Benjamin Herrenschmidt wrote:

> On Fri, 2013-07-26 at 11:16 +0530, Bharat Bhushan wrote:
>> If the page is RAM then map this as cacheable and coherent (set "M" bit)
>> otherwise this page is treated as I/O and map this as cache inhibited
>> and guarded (set  "I + G")
>> 
>> This helps setting proper MMU mapping for direct assigned device.
>> 
>> NOTE: There can be devices that require cacheable mapping, which is not yet 
>> supported.
> 
> Why don't you do like server instead and enforce the use of the same I
> and M bits as the corresponding qemu PTE ?

Specifically, Ben is talking about this code:


/* Translate to host virtual address */
hva = __gfn_to_hva_memslot(memslot, gfn);

/* Look up the Linux PTE for the backing page */
pte_size = psize;
pte = lookup_linux_pte(pgdir, hva, writing, &pte_size);
if (pte_present(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;
}


Alex

--
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 4/4] kvm: powerpc: set cache coherency only for RAM pages

2013-07-26 Thread Benjamin Herrenschmidt
On Fri, 2013-07-26 at 11:16 +0530, Bharat Bhushan wrote:
> If the page is RAM then map this as cacheable and coherent (set "M" bit)
> otherwise this page is treated as I/O and map this as cache inhibited
> and guarded (set  "I + G")
> 
> This helps setting proper MMU mapping for direct assigned device.
> 
> NOTE: There can be devices that require cacheable mapping, which is not yet 
> supported.

Why don't you do like server instead and enforce the use of the same I
and M bits as the corresponding qemu PTE ?

Cheers,
Ben.

> Signed-off-by: Bharat Bhushan 
> ---
>  arch/powerpc/kvm/e500_mmu_host.c |   24 +++-
>  1 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/e500_mmu_host.c 
> b/arch/powerpc/kvm/e500_mmu_host.c
> index 1c6a9d7..5cbdc8f 100644
> --- a/arch/powerpc/kvm/e500_mmu_host.c
> +++ b/arch/powerpc/kvm/e500_mmu_host.c
> @@ -64,13 +64,27 @@ static inline u32 e500_shadow_mas3_attrib(u32 mas3, int 
> usermode)
>   return mas3;
>  }
>  
> -static inline u32 e500_shadow_mas2_attrib(u32 mas2, int usermode)
> +static inline u32 e500_shadow_mas2_attrib(u32 mas2, pfn_t pfn)
>  {
> + u32 mas2_attr;
> +
> + mas2_attr = mas2 & MAS2_ATTRIB_MASK;
> +
> + if (kvm_is_mmio_pfn(pfn)) {
> + /*
> +  * If page is not RAM then it is treated as I/O page.
> +  * Map it with cache inhibited and guarded (set "I" + "G").
> +  */
> + mas2_attr |= MAS2_I | MAS2_G;
> + return mas2_attr;
> + }
> +
> + /* Map RAM pages as cacheable (Not setting "I" in MAS2) */
>  #ifdef CONFIG_SMP
> - return (mas2 & MAS2_ATTRIB_MASK) | MAS2_M;
> -#else
> - return mas2 & MAS2_ATTRIB_MASK;
> + /* Also map as coherent (set "M") in SMP */
> + mas2_attr |= MAS2_M;
>  #endif
> + return mas2_attr;
>  }
>  
>  /*
> @@ -313,7 +327,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);
> +   e500_shadow_mas2_attrib(gtlbe->mas2, pfn);
>   stlbe->mas7_3 = ((u64)pfn << PAGE_SHIFT) |
>   e500_shadow_mas3_attrib(gtlbe->mas7_3, pr);
>  


--
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/4] kvm: powerpc: set cache coherency only for RAM pages

2013-07-25 Thread Bharat Bhushan
If the page is RAM then map this as cacheable and coherent (set "M" bit)
otherwise this page is treated as I/O and map this as cache inhibited
and guarded (set  "I + G")

This helps setting proper MMU mapping for direct assigned device.

NOTE: There can be devices that require cacheable mapping, which is not yet 
supported.

Signed-off-by: Bharat Bhushan 
---
 arch/powerpc/kvm/e500_mmu_host.c |   24 +++-
 1 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..5cbdc8f 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -64,13 +64,27 @@ static inline u32 e500_shadow_mas3_attrib(u32 mas3, int 
usermode)
return mas3;
 }
 
-static inline u32 e500_shadow_mas2_attrib(u32 mas2, int usermode)
+static inline u32 e500_shadow_mas2_attrib(u32 mas2, pfn_t pfn)
 {
+   u32 mas2_attr;
+
+   mas2_attr = mas2 & MAS2_ATTRIB_MASK;
+
+   if (kvm_is_mmio_pfn(pfn)) {
+   /*
+* If page is not RAM then it is treated as I/O page.
+* Map it with cache inhibited and guarded (set "I" + "G").
+*/
+   mas2_attr |= MAS2_I | MAS2_G;
+   return mas2_attr;
+   }
+
+   /* Map RAM pages as cacheable (Not setting "I" in MAS2) */
 #ifdef CONFIG_SMP
-   return (mas2 & MAS2_ATTRIB_MASK) | MAS2_M;
-#else
-   return mas2 & MAS2_ATTRIB_MASK;
+   /* Also map as coherent (set "M") in SMP */
+   mas2_attr |= MAS2_M;
 #endif
+   return mas2_attr;
 }
 
 /*
@@ -313,7 +327,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);
+ e500_shadow_mas2_attrib(gtlbe->mas2, pfn);
stlbe->mas7_3 = ((u64)pfn << PAGE_SHIFT) |
e500_shadow_mas3_attrib(gtlbe->mas7_3, pr);
 
-- 
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