[SeaBIOS] [PATCH] mpt-scsi: Align MPT request to 8 bytes

Kevin O'Connor kevin at koconnor.net
Sun Dec 2 17:08:14 CET 2018


On Sat, Dec 01, 2018 at 01:46:06AM +0200, Liran Alon wrote:
> > On 30 Nov 2018, at 21:20, Kevin O'Connor <kevin at koconnor.net> wrote:
> > On Wed, Nov 28, 2018 at 05:50:01PM +0200, Liran Alon wrote:
> >> From: Nikita Leshchenko <nikita.leshchenko at 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



More information about the SeaBIOS mailing list