| Message ID | 1374153543-10734-2-git-send-email-Bharat.Bhushan@freescale.com |
|---|---|
| State | New, archived |
| Headers | show |
On 18.07.2013, at 15:19, Bharat Bhushan wrote: > If there is a struct page for the requested mapping then it's > normal RAM and the mapping is set to "M" bit (coherent, cacheable) > otherwise this is treated as I/O and we set "I + G" (cache inhibited, guarded) > > This helps setting proper TLB mapping for direct assigned device > > Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> > --- > v2: some cleanup and added comment > - > arch/powerpc/kvm/e500_mmu_host.c | 23 ++++++++++++++++++----- > 1 files changed, 18 insertions(+), 5 deletions(-) > > diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c > index 1c6a9d7..02eb973 100644 > --- a/arch/powerpc/kvm/e500_mmu_host.c > +++ b/arch/powerpc/kvm/e500_mmu_host.c > @@ -64,13 +64,26 @@ 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; > + > + /* > + * RAM is always mappable on e500 systems, so this is identical > + * to kvm_is_mmio_pfn(), just without its overhead. > + */ > + if (!pfn_valid(pfn)) { > + /* Pages not managed by Kernel are treated as I/O, set I + G */ Please also document the intermediate thought that I/O should be mapped non-cached. > + mas2_attr |= MAS2_I | MAS2_G; > #ifdef CONFIG_SMP Please separate the SMP case out of the branch. > - return (mas2 & MAS2_ATTRIB_MASK) | MAS2_M; > -#else > - return mas2 & MAS2_ATTRIB_MASK; > + } else { > + /* Kernel managed pages are actually RAM so set "M" */ This comment doesn't tell me why M can be set ;). Alex > + mas2_attr |= MAS2_M; > #endif > + } > + return mas2_attr; > } > > /* > @@ -313,7 +326,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 majordomo@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: kvm-ppc-owner@vger.kernel.org [mailto:kvm-ppc-owner@vger.kernel.org] On > Behalf Of Alexander Graf > Sent: Thursday, July 18, 2013 8:23 PM > To: Bhushan Bharat-R65777 > Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Wood Scott-B07421; Bhushan > Bharat-R65777 > Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel > managed pages > > > On 18.07.2013, at 15:19, Bharat Bhushan wrote: > > > If there is a struct page for the requested mapping then it's normal > > RAM and the mapping is set to "M" bit (coherent, cacheable) otherwise > > this is treated as I/O and we set "I + G" (cache inhibited, guarded) > > > > This helps setting proper TLB mapping for direct assigned device > > > > Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> > > --- > > v2: some cleanup and added comment > > - > > arch/powerpc/kvm/e500_mmu_host.c | 23 ++++++++++++++++++----- > > 1 files changed, 18 insertions(+), 5 deletions(-) > > > > diff --git a/arch/powerpc/kvm/e500_mmu_host.c > > b/arch/powerpc/kvm/e500_mmu_host.c > > index 1c6a9d7..02eb973 100644 > > --- a/arch/powerpc/kvm/e500_mmu_host.c > > +++ b/arch/powerpc/kvm/e500_mmu_host.c > > @@ -64,13 +64,26 @@ 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; > > + > > + /* > > + * RAM is always mappable on e500 systems, so this is identical > > + * to kvm_is_mmio_pfn(), just without its overhead. > > + */ > > + if (!pfn_valid(pfn)) { > > + /* Pages not managed by Kernel are treated as I/O, set I + G */ > > Please also document the intermediate thought that I/O should be mapped non- > cached. I did not get what you mean to document? > > > + mas2_attr |= MAS2_I | MAS2_G; > > #ifdef CONFIG_SMP > > Please separate the SMP case out of the branch. Really :) this was looking simple to me. > > > - return (mas2 & MAS2_ATTRIB_MASK) | MAS2_M; > > -#else > > - return mas2 & MAS2_ATTRIB_MASK; > > + } else { > > + /* Kernel managed pages are actually RAM so set "M" */ > > This comment doesn't tell me why M can be set ;). RAM in SMP, so setting coherent, is not that obvious? -Bharat > > > Alex > > > + mas2_attr |= MAS2_M; > > #endif > > + } > > + return mas2_attr; > > } > > > > /* > > @@ -313,7 +326,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 majordomo@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 majordomo@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 18.07.2013, at 17:15, Bhushan Bharat-R65777 wrote: > > >> -----Original Message----- >> From: kvm-ppc-owner@vger.kernel.org [mailto:kvm-ppc-owner@vger.kernel.org] On >> Behalf Of Alexander Graf >> Sent: Thursday, July 18, 2013 8:23 PM >> To: Bhushan Bharat-R65777 >> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Wood Scott-B07421; Bhushan >> Bharat-R65777 >> Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel >> managed pages >> >> >> On 18.07.2013, at 15:19, Bharat Bhushan wrote: >> >>> If there is a struct page for the requested mapping then it's normal >>> RAM and the mapping is set to "M" bit (coherent, cacheable) otherwise >>> this is treated as I/O and we set "I + G" (cache inhibited, guarded) >>> >>> This helps setting proper TLB mapping for direct assigned device >>> >>> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> >>> --- >>> v2: some cleanup and added comment >>> - >>> arch/powerpc/kvm/e500_mmu_host.c | 23 ++++++++++++++++++----- >>> 1 files changed, 18 insertions(+), 5 deletions(-) >>> >>> diff --git a/arch/powerpc/kvm/e500_mmu_host.c >>> b/arch/powerpc/kvm/e500_mmu_host.c >>> index 1c6a9d7..02eb973 100644 >>> --- a/arch/powerpc/kvm/e500_mmu_host.c >>> +++ b/arch/powerpc/kvm/e500_mmu_host.c >>> @@ -64,13 +64,26 @@ 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; >>> + >>> + /* >>> + * RAM is always mappable on e500 systems, so this is identical >>> + * to kvm_is_mmio_pfn(), just without its overhead. >>> + */ >>> + if (!pfn_valid(pfn)) { >>> + /* Pages not managed by Kernel are treated as I/O, set I + G */ >> >> Please also document the intermediate thought that I/O should be mapped non- >> cached. > > I did not get what you mean to document? Page snot managed by the kernel are treated as I/O. Map it with disabled cache. > >> >>> + mas2_attr |= MAS2_I | MAS2_G; >>> #ifdef CONFIG_SMP >> >> Please separate the SMP case out of the branch. > > Really :) this was looking simple to me. Two branches intertwined never look simple :). > >> >>> - return (mas2 & MAS2_ATTRIB_MASK) | MAS2_M; >>> -#else >>> - return mas2 & MAS2_ATTRIB_MASK; >>> + } else { >>> + /* Kernel managed pages are actually RAM so set "M" */ >> >> This comment doesn't tell me why M can be set ;). > > RAM in SMP, so setting coherent, is not that obvious? No. 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
> -----Original Message----- > From: Wood Scott-B07421 > Sent: Thursday, July 18, 2013 10:48 PM > To: Bhushan Bharat-R65777 > Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; agraf@suse.de; Bhushan Bharat- > R65777 > Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel > managed pages > > On 07/18/2013 08:19:03 AM, Bharat Bhushan wrote: > > If there is a struct page for the requested mapping then it's normal > > RAM and the mapping is set to "M" bit (coherent, cacheable) otherwise > > this is treated as I/O and we set "I + G" (cache inhibited, guarded) > > > > This helps setting proper TLB mapping for direct assigned device > > > > Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> > > --- > > v2: some cleanup and added comment > > - > > arch/powerpc/kvm/e500_mmu_host.c | 23 ++++++++++++++++++----- > > 1 files changed, 18 insertions(+), 5 deletions(-) > > > > diff --git a/arch/powerpc/kvm/e500_mmu_host.c > > b/arch/powerpc/kvm/e500_mmu_host.c > > index 1c6a9d7..02eb973 100644 > > --- a/arch/powerpc/kvm/e500_mmu_host.c > > +++ b/arch/powerpc/kvm/e500_mmu_host.c > > @@ -64,13 +64,26 @@ 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; > > + > > + /* > > + * RAM is always mappable on e500 systems, so this is identical > > + * to kvm_is_mmio_pfn(), just without its overhead. > > + */ > > + if (!pfn_valid(pfn)) { > > Please use page_is_ram(), which is what gets used when setting the WIMG for the > host userspace mapping. We want to make sure the two are consistent. > > > + /* Pages not managed by Kernel are treated as I/O, set > > I + G */ > > + mas2_attr |= MAS2_I | MAS2_G; > > #ifdef CONFIG_SMP > > - return (mas2 & MAS2_ATTRIB_MASK) | MAS2_M; > > -#else > > - return mas2 & MAS2_ATTRIB_MASK; > > + } else { > > + /* Kernel managed pages are actually RAM so set "M" */ > > + mas2_attr |= MAS2_M; > > #endif > > Likewise, we want to make sure this matches the host entry. > Unfortunately, this is a bit of a mess already. 64-bit booke appears to always > set MAS2_M for TLB0 mappings. Scott, can you please point to the code where MAS2_M is always set for TLB0? -Bharat > The initial KERNELBASE mapping on boot uses > M_IF_SMP, and the settlbcam() that (IIRC) replaces it uses _PAGE_COHERENT. 32- > bit always uses _PAGE_COHERENT, except that initial KERNELBASE mapping. > _PAGE_COHERENT appears to be set based on CONFIG_SMP || CONFIG_PPC_STD_MMU (the > latter config clears _PAGE_COHERENT in the non-CPU_FTR_NEED_COHERENT case). > > As for what we actually want to happen, there are cases when we want M to be set > for non-SMP. One such case is AMP, where CPUs may be sharing memory even if the > Linux instance only runs on one CPU (this is not hypothetical, BTW). It's also > possible that we encounter a hardware bug that requires MAS2_M, similar to what > some of our non-booke chips require. > > -Scott -- 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
diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c index 1c6a9d7..02eb973 100644 --- a/arch/powerpc/kvm/e500_mmu_host.c +++ b/arch/powerpc/kvm/e500_mmu_host.c @@ -64,13 +64,26 @@ 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; + + /* + * RAM is always mappable on e500 systems, so this is identical + * to kvm_is_mmio_pfn(), just without its overhead. + */ + if (!pfn_valid(pfn)) { + /* Pages not managed by Kernel are treated as I/O, set I + G */ + mas2_attr |= MAS2_I | MAS2_G; #ifdef CONFIG_SMP - return (mas2 & MAS2_ATTRIB_MASK) | MAS2_M; -#else - return mas2 & MAS2_ATTRIB_MASK; + } else { + /* Kernel managed pages are actually RAM so set "M" */ + mas2_attr |= MAS2_M; #endif + } + return mas2_attr; } /* @@ -313,7 +326,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);
If there is a struct page for the requested mapping then it's normal RAM and the mapping is set to "M" bit (coherent, cacheable) otherwise this is treated as I/O and we set "I + G" (cache inhibited, guarded) This helps setting proper TLB mapping for direct assigned device Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com> --- v2: some cleanup and added comment - arch/powerpc/kvm/e500_mmu_host.c | 23 ++++++++++++++++++----- 1 files changed, 18 insertions(+), 5 deletions(-)