Re: [SeaBIOS] [PATCH 2/2] lsi53c895a boot support

[Kevin O'Connor]

On Thu, Jul 19, 2012 at 06:15:03PM +0200, Gerd Hoffmann wrote:
> This patch adds support for the lsi53c895a scsi host adapter,
> allowing seabios to boot from scsi disks and cdroms connected
> to the lsi scsi hba emulated by qemu.
>
> This driver was written by looking at the expectations of qemu's
> lsi emulation.  I have no idea idea how close this is to work on
> real hardware, and I somehow doubt anyone cares given the age of
> physical lsi scsi cards.  It depends on !COREBOOT for that reason.

Thanks.  Looks good to me.  I definitely agree on marking it as
!COREBOOT.  Some minor comments below.

[...]
> --- a/src/Kconfig
> +++ b/src/Kconfig
> @@ -119,6 +119,12 @@ menu "Hardware support"
>          default y
>          help
>              Support boot from virtio-scsi storage.
> config LSI_SCSI
>    depends on DRIVES && !COREBOOT
>    bool "lsi53c895a scsi controllers"
>    default y
>    help
>        Support boot from lsi53c895a scsi storage.

It's probably worthwhile to point out in the help that this is for
QEMU emulation only.

[...]
> +static int
> +lsi_scsi_cmd(struct lsi_lun_s *llun, struct disk_op_s *op,
>         void *cdbcmd, u16 target, u16 lun, u16 blocksize)
>
> +{
> u32 iobase = GET_GLOBAL(llun->iobase);
> u32 dma = ((cdb_is_read(cdbcmd, blocksize) ? 0x01000000 : 0x00000000) |
>           (op->count * blocksize));
> u8 msgout[] = {
>    0x80 | lun,                 // select lun
>    0x08,
> };
> u8 status = 0xff;
> u8 msgin_tmp[2];
> u8 msgin = 0xff;
> /* FIXME: probably not the best idea to have this on the stack ... */
> u32 script[] = {

Several places in the code DMA to the stack already (usb-ehci,
usb-uhci, ata).

[...]
> --- /dev/null
> +++ b/src/lsi-scsi.h
> @@ -0,0 +1,2 @@
> +void lsi_scsi_setup(void);
> +int lsi_scsi_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize);

Should have a header guard.

-Kevin