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

[Liran Alon]

> On 2 Dec 2018, at 18:08, Kevin O'Connor <kevin at koconnor.net> wrote:
> 
> 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

OK.
Thanks for your time reviewing this. Your opinion is valuable and appreciated.

-Liran