Subject: Re: Busspace sanity ...
To: Jason Thorpe <thorpej@nas.nasa.gov>
From: Stefan Grefen <grefen@hprc.tandem.com>
List: port-i386
Date: 06/09/1998 21:32:44
In message <199806091749.KAA08315@lestat.nas.nasa.gov> Jason Thorpe wrote:
> On Tue, 09 Jun 1998 16:27:08 +0200
> Stefan Grefen <grefen@hprc.tandem.com> wrote:
>
> > My serial console showed me that there is a new check in
> > bus_space_write_XXX to catch unaligned busspace accesses.
> > If this happens in the ethernet driver of a diskless machine, this
> > gets your attention :-))
>
> That was the point :-)
I understand that :-))
>
> > I think it is a little to aggressive.
> > Forcing alignment to access-size may help getting more drivers real MI it
> > is annoying for people that check already for propper alignment for
> > the actual architecture.
>
> I don't think so, because those checks are enabled w/ DEBUG, which is
> "more expensive kernel consistency checks, and possibly noisy output".
>
> The truth of the matter is that drivers that do unaligned access are
> broken, even on platforms which do not require strict alignment. Those
> drivers should be fixed.
I consider a driver fixed if it checks for proper alignment for a given
architecture and acts on misalignment.
>
> > Eg. a check with ALIGNED_POINTER(p,unit) should be sufficient to avoid
> > the reminder from bus_space_XXXX.
>
> Exactly... the reminder from bus_space_*() is designed to point out
> drivers which DO NOT do alignment fixups! Once the drivers are fixed,
> the messages go away.
My point is that even if ALIGNED_POINTER() is true (always on a i386),
__BUS_SPACE_ALIGNED_ADDRESS() maybe false.
The code in question does a ALIGNED_POINTER() and a pullup if that fails.
I consider that not broken.
>
> > There could be an option like BUSSPACE_DEBUG_ALIGNMENT with
> > stricter rules for MI-testing, but for the normal kernel (with DEBUG)
> > the __BUS_SPACE_ALIGNED_ADDRESS macro should be equal to ALIGNED_POINTER.
>
> No... that is not what the checks were designed to do. The checks are
> specifically designed to enfore _MI_ alignment requirements, as documented
> in the bus_space(9) manual page.
I can't find anything there that mandates that data-pointers have to be
aligned differently than in 'normal' code.
I think the __BUS_SPACE_ADDRESS_SANITY checks for the data pointer should
use ALIGNED_POINTER (but than the CPU will find those anyway :-)) ).
An option for stricter checking would allow to test if a driver would
run on an architecture with different alignment restrictions (and I
think that is not covered by DEBUG).
I would hate to have to copy the mbuf data on a slow i386SX running
diskless, because some fast RISC ships can't do unaligned access :-)))
>
> Can you collect the messages and submit a bug report against the device
> driver in question? Then it will get fixed, and you won't have to see
> the messages anymore :-)
It is a driver for the CS8900 lan driver, I ported from the SHARK tree to
i386. It's work in progress because it has to be tested again in the
SHARK tree (I'm waiting for hardware).
I uploaded the current version sometime ago to ftp.netbsd.org incoming.
Stefan
PS.
Code sniplet:
if(!ALIGNED_POINTER(p,u_int16_t)) {
/*
* THIS IS UGLY
*/
m0=m_pullup(m,len);
continue;
}
...
bus_space_write_multi_2(sc->sc_iot,sc->sc_ioh,
PORT_RXTX_DATA,p,(len>>1) );
Console messages:
buffer 0xf025b495 not aligned to 2 bytes ../../../../dev/ic/cs8900.c:3067
....
>
> Jason R. Thorpe thorpej@nas.nasa.gov
> NASA Ames Research Center Home: +1 408 866 1912
> NAS: M/S 258-5 Work: +1 650 604 0935
> Moffett Field, CA 94035 Pager: +1 650 428 6939
--
Stefan Grefen Tandem Computers Europe Inc.
grefen@hprc.tandem.com High Performance Research Center
--- Hacking's just another word for nothing left to kludge. ---