Re: [RFC PATCH 1/1] OvmfPkg: Introduce LSI 53C895A SCSI Controller Driver

["Gary Lin" <glin@suse.com>]

On Mon, Jun 15, 2020 at 01:27:22PM +0200, Laszlo Ersek wrote:
> Hello Gary,
> 
Hi Laszlo,

> On 06/12/20 12:04, Gary Lin wrote:
> > This commit introduces the driver for LSI 53C895A SCSI Controller
> > which, so that OVMF can access the devices attached to the emulated
> > "lsi" SCSI controller.
> >
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> > Signed-off-by: Gary Lin <glin@suse.com>
> > ---
> > Although lsi is now considered obsolete in QEMU, I wrote this driver
> > mainly for learning the UEFI SCSI driver and ... FUN! The majority of
> > the code is actually borrowed from VirtioScsci and MptScsi, and I
> > mainly focus on LsiScsiPassThru().
> >
> > If it's fine to add this driver into OvmfPkg, I'll start to split this
> > patch into smaller pieces to make it easier to review.
> 
> (1) Do we have an official deprecation notice for this SCSI controller
> in QEMU?
> 
> If we do, then (AIUI) the controller will be removed in QEMU in one or
> two releases, so this code would become effectively dead in the mid
> term. I wouldn't like to review and/or carry code that's soon to be
> dead.
> 
I just vaguely remember that virtio-scsi is the new default over lsi and
it's not recommended to use lsi except for the old OS without virtio-scsi
driver.

> (2) If there is no official deprecation notice in QEMU, then I agree it
> makes sense to include this driver. In that case, I have another
> question:
> 
> Do you intend this driver for production purposes? I.e., do you expect
> users or "layered products" (libvirt, proxmox, openstack, ...) to use
> this SCSI controller for some well-defined purpose? (The MPT SCSI and PV
> SCSI drivers had a clear product-oriented modivation:
> 
>   https://edk2.groups.io/g/devel/message/55620
>   http://mid.mail-archive.com/a96b6b74-c35d-e291-2122-9d77f1d5f89c@oracle.com
> )
> 
> (2a) If this driver is not meant for a production environment, then
> LSI_SCSI_ENABLE should be FALSE by default (and I'll do a lighter
> review).
> 
> (2b) If the driver is meant for production, then LSI_SCSI_ENABLE should
> indeed be TRUE, and I'll have to be more diligent in reviewing this.
> 
I kind of wonder if the any serious use for lsi+ovmf. If the OS is so
old and only supports lsi, seabios probably serves it better. Anyway,
I'll try to gather more information from my colleagues to see if they
got any feedback about lsi from our customers. If not, I'll set
LSI_SCSI_ENABLE to FALSE by default.

> For either (2a) or (2b), please do split up the driver into smaller
> patches, and please also add yourself to Maintainers.txt as the
> designated reviewer of the new driver.
> 
Sure. Will do that.

Thanks,

Gary Lin