svn commit: r295557 - head/sys/dev/uart

[Bruce Evans]

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