Re: [PATCH] add delay between port write and port read

From: Mikulas Patocka (mpatocka@xxxxxxxxxx)
Date: Thu Feb 21 2019 - 14:31:32 EST


Do you think that we should fix this by identifying hardware that needs
the delays and adding the delays there?

In my opinion, adding mb() to the port accessing functions is safer - it
is 6 line patch.

Reading all the hardware manuals is time consuming and hardly anyone will
do it for 25 years old hardware.

Mikulas



On Wed, 20 Feb 2019, Maciej W. Rozycki wrote:

> On Wed, 20 Feb 2019, Mikulas Patocka wrote:
>
> > > Will Deacon is in the process of sanitizing our documentation for this,
> > > and this part is still under discussion.
> > >
> > > I personally don't see a problem with having different barrier
> > > semantics between outb() and iowrite8(), the reasoning being that
> > > any caller of iowriteX() would already have to be careful about
> > > posted writes when iowriteX is backed by ioremapped memory
> > > space rather than port I/O.
> > >
> > > I think the more important question we have to figure out here
> > > however is why exactly the barrier is needed for alpha, as I still
> > > don't fully understand the issue:
> >
> > Port delays were common on MS-DOS with the ISA bus. On the ISA bus, the
> > device cannot reject a transaction, so the programmer has to wait until
> > the device is ready before talking to it.
> >
> > My hypothesis is that the engineers who built this Alpha Avanti machine
> > simply bought some crappy serial line and real-time clock logic from some
> > ISA bus vendor. And so it requires delays. The PCI stuff on the same
> > machine doesn't require any extra delays.
>
> What you call crap was in the day, i.e. back in 1994, a state-of-the art
> ISA super I/O chip (the NS87332) and a standard RTC chip (the BQ3287).
> There was no LPC in existence yet when the Avanti line was built, the
> south bridge used was the i82378 SIO, additionally wiring two actual ISA
> slots.
>
> Datasheets fo all these pieces should be available; I'm fairly sure I
> have at least some of them myself. Also full engineering documentation is
> available for the Avanti. So it can all be verified quite easily.
>
> If any of these devices need actual ISA access delays, then `outX_p' and
> `inX_p' accessors should be used by the respective drivers, and we need to
> implement these accordingly; that would be no different to a standard x86
> PC with say an ISA serial port (I still have such hardware BTW).
>
> BTW, I have recently got an Avanti system myself, an AS/250 I believe,
> although for an unknown reason placed in an AS/300 enclosure. The main
> board of the two systems is identical and the only difference is the
> maximum amount of RAM supported anyway. I have't wired it properly yet
> though, so I can't do any verification ATM, but I will try sometime later.
>
> > > a) maybe the barrier here is only needed to provide the
> > > non-posted PCI write behavior required by outb(), in that
> > > case I would suggest adding the barriers to outX() but
> > > perhaps not iowriteX()
> >
> > Except for serial line and real-time-clock, the machine works OK. So
> > there's no need to slow down regural PCI accesses that use iowriteX.
>
> If actual delays are required rather than strong ordering only, then the
> respective drivers need to be fixed, however platform code still has to
> enforce the ordering semantics we put into the respective internal
> interfaces.
>
> > > b) or the barriers are in fact needed for /any/ I/O operation
> > > on alpha, to ensure that a store is ordered with regard to
> > > a following load. If this is the case, we need the barriers
> > > in all three families: outX(), writeX() and iowriteX().
> >
> > My understanding is that multiple accesses to the same PCI space are
> > ordered.
>
> I believe they are not; there's not such thing as "a PCI space" from the
> CPU's point of view, its just another MMIO location, and it's the CPU that
> is weakly ordered, not the chipset.
>
> I'll come back with the relevant citations from the architecture manual,
> ISTR I quoted them before already.
>
> Maciej
>