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

login
register
mail settings
Submitter Bharat Bhushan
Date July 26, 2013, 5:46 a.m.
Message ID <1374817613-20169-4-git-send-email-Bharat.Bhushan@freescale.com>
Download mbox | patch
Permalink /patch/262063/
State Superseded
Headers show

Comments

Bharat Bhushan - July 26, 2013, 5:46 a.m.
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 <bharat.bhushan@freescale.com>
---
 arch/powerpc/kvm/e500_mmu_host.c |   24 +++++++++++++++++++-----
 1 files changed, 19 insertions(+), 5 deletions(-)
Benjamin Herrenschmidt - July 26, 2013, 8:26 a.m.
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 <bharat.bhushan@freescale.com>
> ---
>  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);
>
Alexander Graf - July 26, 2013, 8:50 a.m.
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
Bharat Bhushan - July 26, 2013, 8:51 a.m.
> -----Original Message-----
> From: Benjamin Herrenschmidt [mailto:benh@kernel.crashing.org]
> Sent: Friday, July 26, 2013 1:57 PM
> To: Bhushan Bharat-R65777
> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;
> agraf@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 <bharat.bhushan@freescale.com>
> > ---
> >  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);
> >
> 
>
Bharat Bhushan - July 26, 2013, 8:52 a.m.
> -----Original Message-----
> From: kvm-ppc-owner@vger.kernel.org [mailto:kvm-ppc-owner@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; kvm@vger.kernel.org;
> linuxppc-dev@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 majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
Bharat Bhushan - July 26, 2013, 3:03 p.m.
> -----Original Message-----
> From: kvm-ppc-owner@vger.kernel.org [mailto:kvm-ppc-owner@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; kvm@vger.kernel.org;
> linuxppc-dev@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
Benjamin Herrenschmidt - July 26, 2013, 10:26 p.m.
On Fri, 2013-07-26 at 15:03 +0000, 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

Patch

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);