From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages Date: Tue, 23 Jul 2013 13:26:43 -0500 Message-ID: <1374604003.15592.32@snotra> References: <6A3DF150A5B70D4F9B66A25E3F7C888D070DEA9B@039-SN2MPN1-013.039d.mgd.msft.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; delsp=Yes; format=Flowed Content-Transfer-Encoding: 8BIT Cc: Wood Scott-B07421 , "kvm-ppc@vger.kernel.org" , "kvm@vger.kernel.org" , "agraf@suse.de" To: Bhushan Bharat-R65777 Return-path: In-Reply-To: <6A3DF150A5B70D4F9B66A25E3F7C888D070DEA9B@039-SN2MPN1-013.039d.mgd.msft.net> (from R65777@freescale.com on Tue Jul 23 06:13:58 2013) Content-Disposition: inline Sender: kvm-ppc-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On 07/23/2013 06:13:58 AM, Bhushan Bharat-R65777 wrote: > > > > -----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 > > > --- > > > 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? __early_init_mmu() sets MAS4[MD], without regard to CONFIG_SMP. However, the TLB miss handler replaces WIMGE with the flags from the PTE, so I guess MAS4 is only relevant for things like indirect PTEs and (for non-FSL chips) linear faults. So never mind about 64-bit. There's still the AMP case, though. -Scott