Date: Tue, 16 Feb 2016 11:01:21 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Ian Lepore <ian@freebsd.org> Cc: Bruce Evans <brde@optusnet.com.au>, Michal Meloun <mmel@freebsd.org>, src-committers@freebsd.org, Marius Strobl <marius@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r295557 - head/sys/dev/uart Message-ID: <20160216103914.F1693@besplex.bde.org> In-Reply-To: <1455579466.12873.23.camel@freebsd.org> References: <201602120514.u1C5EwWt053622@repo.freebsd.org> <20160212164755.GC4980@alchemy.franken.de> <20160213041246.V1974@besplex.bde.org> <20160213005801.GF15359@alchemy.franken.de> <56C1BDE2.8090300@freebsd.org> <20160216080249.F1233@besplex.bde.org> <1455579466.12873.23.camel@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 15 Feb 2016, Ian Lepore wrote: > On Tue, 2016-02-16 at 09:28 +1100, Bruce Evans wrote: >> On Mon, 15 Feb 2016, Michal Meloun wrote: >> >>> [...] >>> Please note that ARM architecture does not have vectored interrupts, >>> CPU must read actual interrupt source from external interrupt >>> controller (GIC) register. This register contain predefined value if >>> none of interrupts are active. >>> >>> 1 - CPU1: enters ns8250_bus_transmit() and sets IER_ETXRDY. >>> 2 - HW: UART interrupt is asserted, processed by GIC and signaled >>> to CPU2. >>> 3 - CPU2: enters interrupt service. >> >> It is blocked by uart_lock(), right? >> >>> 4 - CPU1: writes character to into REG_DATA register. >>> 5 - HW: UART clear its interrupt request >>> 6 - CPU2: reads interrupt source register. No active interrupt is >>> found, spurious interrupt is signaled, and CPU leaves interrupted >>> state. >>> 7 - CPU1: executes uart_barrier(). This function is not empty on ARM, >>> and can be slow in some cases. >> >> It is not empty even on x86, although it probably should be. >> >> BTW, if arm needs the barrier, then how does it work with >> bus_space_barrier() referenced in just 25 files in all of /sys/dev? > > With a hack, of course. In the arm interrupt-controller drivers we > always call bus_space_barrier() right before doing an EOI. It's not a > 100% solution, but in practice it seems to work pretty well. I thought about the x86 behaviour a bit more and now see that it does need barriers but not the ones given by bus_space_barrier(). All (?) interrupt handlers use mutexes (if not driver ones, then higher-level ones). These might give stronger or different ordering than given by bus_space_barrier(). On x86, they use the same memory bus lock as the bus_space_barrier(). This is needed to give ordering across CPUs. But for accessing a single device, you only need program order for a single CPU. This is automatic on x86 provided a mutex is used to prevent other CPUs accessing the same device. And if you don't use a mutex, then bus_space_barrier() cannot give the necessary ordering since if cannot prevent other CPUs interfering. So how does bus_space_barrier() before EOI make much difference? It doesn't affect the order for a bunch of accesses on a single CPU. It must do more than a mutex to do something good across CPUs. Arguably, it is a bug in mutexes is they don't gives synchronization for device memory. > ... > The hack code does a drain-write-buffer which doesn't g'tee that the > slow peripheral write has made it all the way to the device, but it > does at least g'tee that the write to the bus the perhiperal is on has > been posted and ack'd by any bus<->bus bridge, and that seems to be > good enough in practice. (If there were multiple bridged busses > downstream it probably wouldn't be, but so far things aren't that > complicated inside the socs we support.) Hmm, so there is some automatic strong ordering but mutexes don't work for device memory? > The g'teed way is to do a readback of the register written-to, or some > nearby register if it's write-only. That's a hard thing to do in a > bus_space_barrier implementation that has no knowledge of things like > which registers in a device might be write-only. > > And of course, then you're still left with the problem of hundreds of > drivers that don't do any barrier calls at all. Mostly unimportant ones for fast NICs :-). These could be made even slower using lots of barriers. But the fast ones already have to use special interfaces DMA since single-word bus accesses are too slow. >> [...] >> >>> Also, at this time, UART driver is last one known to generate spurious >>> interrupts in ARM world. >>> >>> So, whats now? I can #ifdef __arm__ change made in r295557 (for maximum >>> safety), if you want this. Or we can just wait to see if someone reports >>> a problem ... >> >> Use better methods. > > How about a dev.uart.<unit>.broken_txrdy tunable sysctl that defaults > to the new behavior but can be turned on by anyone with the buggy > hardware? It's difficult to know that such knobs even exist. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20160216103914.F1693>