Hello Gary, 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.jus...@intel.com> > Cc: Laszlo Ersek <ler...@redhat.com> > Cc: Ard Biesheuvel <ard.biesheu...@arm.com> > Signed-off-by: Gary Lin <g...@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. (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. 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. Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#61286): https://edk2.groups.io/g/devel/message/61286 Mute This Topic: https://groups.io/mt/74836137/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
