> Date: Thu, 9 Feb 2012 17:14:41 +0000
> From: Miod Vallat <[email protected]>
>
> Could people with ahc(4) SCSI controllers give the following diff a try
> and report whether this causes any difference in behaviour? This diff
> seems to do wonders on alpha, but I'd like reports from other platforms
> (such as i386, amd64 and macppc) before considering putting it in.
I'm not sure this is a good idea. Writes on the PCI bus may be
posted. The ahc_inb() call makes sure that any preceding (posted)
writes have indeed completed. By replacing this with a
bus_space_barrier() you no longer guarantee completion of the writes,
but instead guarantee that the CPU (or the compiler) doesn't reorder
the writes. So it's not the same thing.
I don't know what the ahc(4) driver/hardware requires here. Could be
that posted writes are not an issue at all. But if they are, we
probably need to keep that ahc_inb() call around. So it would be
safer to just add the bus_space_barrier() call before the ahc_inb()
call.
Now it can be argued that bus_space_barrier() should guarantee
completion of posted writes. Unfortunately that's difficult to
achieve since doing a read on the PCI bus is the only way to achieve
this, and doing a read typically has side-effects.
I've never fully understood the issues with posted writes. I think
issues will only show up for devices that are sitting behind a PCI-PCI
bridge. All I know the Linux people tend to make a big fuss about
them and add seemingly redundant reads all over the place to deal with
them.
> Index: aic7xxx_openbsd.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/ic/aic7xxx_openbsd.h,v
> retrieving revision 1.21
> diff -u -p -r1.21 aic7xxx_openbsd.h
> --- aic7xxx_openbsd.h 17 Jul 2011 22:46:48 -0000 1.21
> +++ aic7xxx_openbsd.h 8 Feb 2012 21:48:48 -0000
> @@ -193,8 +193,8 @@ static __inline void ahc_flush_device_wr
> static __inline void
> ahc_flush_device_writes(struct ahc_softc *ahc)
> {
> - /* XXX Is this sufficient for all architectures??? */
> - ahc_inb(ahc, INTSTAT);
> + bus_space_barrier(ahc->tag, ahc->bsh, 0, 0x100,
> + BUS_SPACE_BARRIER_READ | BUS_SPACE_BARRIER_WRITE);
> }
>
> /**************************** Locking Primitives
> ******************************/
