svn commit: r260165 - head/sys/dev/ahci

[Ian Lepore]

On Fri, 2014-01-03 at 20:21 +0200, Konstantin Belousov wrote:
> On Fri, Jan 03, 2014 at 09:49:12AM -0700, Ian Lepore wrote:
> > On Wed, 2014-01-01 at 22:32 +0200, Konstantin Belousov wrote:
> > > On Wed, Jan 01, 2014 at 08:26:08PM +0000, Zbigniew Bodek wrote:
> > > > Author: zbb
> > > > Date: Wed Jan  1 20:26:08 2014
> > > > New Revision: 260165
> > > > URL: http://svnweb.freebsd.org/changeset/base/260165
> > > > 
> > > > Log:
> > > >   Use only mapped BIOs on ARM
> > > >   
> > > >   Using unmapped BIOs causes failure inside bus_dmamap_sync, since
> > > >   this function requires valid MVA address, which is not present
> > > >   if mapping is not set up.
> > > >   
> > > >   Submitted by:	Wojciech Macek <wma at semihalf.com>
> > > >   Obtained from:	Semihalf
> > > > 
> > > > Modified:
> > > >   head/sys/dev/ahci/ahci.c
> > > > 
> > > > Modified: head/sys/dev/ahci/ahci.c
> > > > ==============================================================================
> > > > --- head/sys/dev/ahci/ahci.c	Wed Jan  1 20:22:29 2014	(r260164)
> > > > +++ head/sys/dev/ahci/ahci.c	Wed Jan  1 20:26:08 2014	(r260165)
> > > > @@ -3066,7 +3066,15 @@ ahciaction(struct cam_sim *sim, union cc
> > > >  		if (ch->caps & AHCI_CAP_SPM)
> > > >  			cpi->hba_inquiry |= PI_SATAPM;
> > > >  		cpi->target_sprt = 0;
> > > > +#ifdef __arm__
> > > > +		/*
> > > > +		 * Do not use unmapped buffers on ARM. Doing so will cause
> > > > +		 * failure inside bus_dmamap_sync due to lack of VA.
> > > > +		 */
> > > > +		cpi->hba_misc = PIM_SEQSCAN;
> > > > +#else
> > > >  		cpi->hba_misc = PIM_SEQSCAN | PIM_UNMAPPED;
> > > > +#endif
> > > >  		cpi->hba_eng_cnt = 0;
> > > >  		if (ch->caps & AHCI_CAP_SPM)
> > > >  			cpi->max_target = 15;
> > > 
> > > I think this is wrong. If bus_dmamap_sync(9) is not functional on arm,
> > > then unmapped io should be disabled on arm unconditionally, using
> > > unmapped_buf_allowed.  Why ahci(4) is special in this regard, leaving
> > > other controllers broken for arm ?
> > 
> > I think this entire concept is wrong, and the fix should be in armv6
> > busdma or pmap code.  An unmapped page is, by definition, not cached.
> > The only way data becomes cached is because flags in a mapping indicated
> > the memory is cacheable.  If we have data from unmapped pages in the
> > cache, that's a bug in the armv6 pmap implementation, I think.
> 
> I agree with the statement that the fix must be in busdma MD code,
> possibly with some cooperation with pmap code. On the other hand, I do
> not understand later claims.
> 
> BIO unmapped attribute only means that bio does not carry a pointer to
> (some) mapping of the bio_ma pages. In other words, unmapped BIO does
> not imply globally unmapped page, and CPU cache might contain the lines
> from the BIO pages.

Oh, I didn't realize that.  I thought with unmapped IO it was implicit
that there were no mappings of the page in the kernel or any userland
vmspace.  If the page can be mapped, that does indeed imply that
unmapped IO needs to be globally disabled for ARM -- the hardware
performs cache maintenance by virtual address.

Hmm, or we could plumb up some new interface between arm busdma and pmap
implementations, so that busdma can get any virtual addresses associated
with the page and do the cache flushes.  It looks like pmap has the
required info in the pv_table, easily accessible by physical address.  I
wonder if any benefits from unmapped IO will get lost in the extra work
required at the busdma level?

-- Ian