On Thu, Mar 02, 2017 at 07:37:38PM +0300, Roman Kagan wrote: > On Thu, Mar 02, 2017 at 11:14:37AM -0500, Kevin O'Connor wrote: > > On Wed, Mar 01, 2017 at 01:45:33PM +0300, Roman Kagan wrote: > > > A number of SCSI drivers currently only see luns #0 in their targets. > > > > > > This may be a problem when drives have to be assigned bigger lun > > > numbers, e.g. because the storage controllers don't provide enough > > > target numbers to accomodate all drives. > > > (In particular, I'm about to submit a driver for Hyper-V VMBus SCSI > > > controller which is limited to 2 targets only). > > > > > > This series adds generic SCSI lun enumeration (either via REPORT LUNS > > > command or sequentially trying every lun), and makes the respective > > > drivers use it. > > > > Thanks. Let me make sure I understand this series. Some scsi > > controllers have hardware specific mechanisms for finding the number > > of luns (usb-msc, megasas, pvscsi) and some controllers use a generic > > REPORT LUNS mechanism (virtio-scsi, esp-scsi, usb-uas, mpt-scsi, > > lsi-scsi). > > > > The basic difficulty with implementing REPORT LUNS in seabios is that > > the code needs a "struct drive_s" to issue the REPORT LUNS command, > > but since the drive parameters (or even the number of drives) aren't > > known, a dummy "lun0" drive_s must be created just for REPORT LUNS. > > Thus the series breaks the driver xxx_add_lun() functions into > > xxx_init_lun() and xxx_add_lun() so that a dummy lun0 can be created. > > > > An additional complexity is that the REPORT LUNS mechanism is broken > > in current QEMU on lsi-scsi and mpt-scsi. > > > > Your goal is to add support for "Hyper-V VMBus SCSI" which also > > requires REPORT LUNS. > > > > Is the above correct? > > Absolutely. I couldn't have explained it better. > > One minor nit is that, strictly speaking, the upcoming vmbus scsi driver > doesn't *require* REPORTS LUNS. It's just that it would be too limiting > if the users had to stick with lun #0 only like was currently the case > with other drivers: here the number of available targets was only 2, and > thus the number of BIOS-visible disks would be no more than that. > > So I thought it was a good idea to start with a series that adds generic > lun enumeration to the SCSI layer, that would lift this limitation for > the future vmbus scsi and could benefit other drivers, too.
Thanks. I have a few high-level comments on the series. I wonder if there is a way to reduce the amount of control passing between the generic scsi code and the driver code. Specifically, I wonder if the callback function scsi_add_lun could be simplified. Some thoughts: - instead of scsi_rep_luns_scan() being passed a callback, perhaps introduce a scsi_get_luns() command that returns a malloc'd struct containing the list of luns. The driver code could then iterate over the list. - if REPORT LUNS fails, then I don't think we need to iterate over every possible lun. If this is just to workaround qemu issues, then falling back to just using the first lun should be fine. - instead of calling xxx_init_lun() in each driver's xxx_add_lun() function, I wonder if the code would be simpler if it just memcpy'd the tmpl_drv struct over and modified the lun parameter. Sorry for the delay in responding. -Kevin _______________________________________________ SeaBIOS mailing list [email protected] https://www.coreboot.org/mailman/listinfo/seabios
