On Sat, Dec 01, 2018 at 01:46:06AM +0200, Liran Alon wrote: > > On 30 Nov 2018, at 21:20, Kevin O'Connor <ke...@koconnor.net> wrote: > > On Wed, Nov 28, 2018 at 05:50:01PM +0200, Liran Alon wrote: > >> From: Nikita Leshchenko <nikita.leshche...@oracle.com> > >> > >> When mpt-scsi receives a SCSI message, it wraps it in a MPT request > >> message and writes it's address to an IO port to be added to the > >> request queue. > >> > >> This MPT request is allocated on the stack. Previous to this commit, > >> the request is aligned to 4 bytes. However, VirtualBox LSI53c1030 > >> device emulation aligns the request address to 8 bytes. > >> Therefore, this commit change alignment of request to 8 bytes. > >> > >> VirtualBox source code which handles this is at > >> Devices/Storage/DevLsiLogicSCSI.cpp. lsilogicRegisterWrite() > >> LSILOGIC_REG_REQUEST_QUEUE handler adds the request to the > >> queue (pRequestQueueBase). lsilogicR3Worker() reads request from > >> pRequestQueueBase and aligns it to 8 bytes > >> (u32RequestMessageFrameDesc & ~0x07). > > > > Thanks. Is this change done to match virtualbox, or because it fixes > > some type of problem? > > It can be seen from QEMU’s mptsas_mmio_write() MPI_REQUEST_POST_FIFO_OFFSET > handler > that QEMU aligns this value to 4 bytes which matches current SeaBIOS code. > > However QEMU only emulates LSISAS1068 and not LSILOGIC53C1030. > This mpt-scsi.c SeaBIOS driver is suppose to handle both devices. > > Therefore, we thought that maybe LSILOGIC53C1030 does requires value to be > aligned to 8 bytes. > In contrast to LSISAS1068. Deduced from VirtualBox device emulation.
Okay, it sounds like this is premptive and is not known to fix a problem then? I'm not sure aligned(8) actually works on stack allocations with gcc and SeaBIOS. Last I checked, in order for gcc to do proper stack alignment (of greater than 4) it requires the x86 C stack alignment calling convention to be fully followed. I'm not sure that SeaBIOS always does this. The USB drivers (eg, src/hw/ohci.c) perform manual stack alignment, for example. If this isn't known to fix an actual problem and there isn't a spec that explicitly requires 8 bytes, then I'd say it's probably not worth the effort. -Kevin _______________________________________________ SeaBIOS mailing list SeaBIOS@seabios.org https://mail.coreboot.org/mailman/listinfo/seabios
