On Fri, Jan 03, 2014 at 12:08:01PM -0700, Ian Lepore wrote: > 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 <w...@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?
Yes, I started mumbling about MI busdma code and pmap helper from the very beginning. The benefits can be only measured, it is impossible to speculate about it. Still, the normal reads for non-mmaped pages should pass the buffer pages which are not mapped anywhere, and unmaped code would be still a win. For arm, since we do not support SMP AFAIK, mapping and unmapping buffer pages is not as costly as for the large SMP machines. Keeping the feature working would be benefitial in future.
pgp6OtGt_wWhz.pgp
Description: PGP signature
