> On 2 Dec 2018, at 18:08, Kevin O'Connor <ke...@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 <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
OK. Thanks for your time reviewing this. Your opinion is valuable and appreciated. -Liran _______________________________________________ SeaBIOS mailing list SeaBIOS@seabios.org https://mail.coreboot.org/mailman/listinfo/seabios
