Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages

[Scott Wood <scottwood@freescale.com>]

On 07/23/2013 11:50:35 AM, Bhushan Bharat-R65777 wrote:
> 
> 
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Tuesday, July 23, 2013 10:15 PM
> > To: Bhushan Bharat-R65777
> > Cc: Wood Scott-B07421; Alexander Graf; kvm-ppc@vger.kernel.org;
> > kvm@vger.kernel.org
> > Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only  
> for kernel
> > managed pages
> >
> > On 07/22/2013 10:39:16 PM, Bhushan Bharat-R65777 wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Wood Scott-B07421
> > > > Sent: Tuesday, July 23, 2013 12:18 AM
> > > > To: Bhushan Bharat-R65777
> > > > Cc: Wood Scott-B07421; Alexander Graf; kvm-ppc@vger.kernel.org;
> > > > kvm@vger.kernel.org
> > > > Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency  
> only
> > > for kernel
> > > > managed pages
> > > >
> > > > On 07/21/2013 11:39:45 PM, Bhushan Bharat-R65777 wrote:
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Wood Scott-B07421
> > > > > > Sent: Thursday, July 18, 2013 11:09 PM
> > > > > > To: Alexander Graf
> > > > > > Cc: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org;
> > > > > kvm@vger.kernel.org; Bhushan
> > > > > > Bharat-R65777
> > > > > > Subject: Re: [PATCH 2/2 v2] kvm: powerpc: set cache  
> coherency
> > > only
> > > > > for kernel
> > > > > > managed pages
> > > > > >
> > > > > > On 07/18/2013 12:32:18 PM, Alexander Graf wrote:
> > > > > > >
> > > > > > > On 18.07.2013, at 19:17, Scott Wood wrote:
> > > > > > >
> > > > > > > > On 07/18/2013 08:19:03 AM, Bharat Bhushan wrote:
> > > > > > > > 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.  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.
> > > > > > >
> > > > > > > How about we always set M then for RAM?
> > > > > >
> > > > > > M is like I in that bad things happen if you mix them.
> > > > >
> > > > > I am trying to list the invalid mixing of WIMG:
> > > > >
> > > > >  1) I & M
> > > > >  2) W & I
> > > > >  3) W & M (Scott mentioned that he observed issues when   
> mixing
> > > these
> > > > > two)
> > > > >  4) is there any other?
> > > >
> > > > That's not what I was talking about (and I don't think I  
> mentioned
> > > W at all,
> > > > though it is also potentially problematic).
> > >
> > > Here is cut paste of your one response:
> > > "The architecture makes it illegal to mix cacheable and
> > > cache-inhibited mappings to the same physical page.  Mixing W or M
> > > bits is generally bad as well.  I've seen it cause machine checks,
> > > error interrupts, etc.
> > > -- not just corrupting the page in question."
> > >
> > > So I added not mixing W & M. But at that time I missed to  
> understood
> > > why mixing M & I for same physical address can be issue :).
> >
> > "W or M", not "W and M".  I meant that each one, separately, is in  
> a similar
> > situation as the I bit.
> >
> > None of this is about invalid combinations of attributes on a  
> single TLB entry
> > (though there are architectural restrictions there as well).
> 
> Ok, I misread again :(. The second part of comment was (looks like  
> you missed so copy pasted below)
> 
> "
> When we say all RAM (page_is_ram() is true) will be having "M" bit,  
> then same RAM physical address will not have "M" mixed with any  
> other, right?
> 
> Similarly, For IO (which is not RAM), we will set "I+G", so "I" will  
> not be mixed with "M". Is not that?
> "

I didn't miss it; it just seemed moot given the earlier confusion.  But  
yes, for now we will set all RAM to M, and all I/O to I+G.  Eventually  
that will change if/when we do vfio for QMan portals or other devices  
that require cacheable I/O.

-Scott