On Tue, Mar 14, 2017 at 07:13:19PM +0100, Paolo Bonzini wrote:
On 14/03/2017 16:16, Roman Kagan wrote:
- 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.
Perhaps. As it was trivial to code scsi_sequential_scan in addition to scsi_rep_luns_scan, I went ahead and did it. Yes the two places where it's used in the patchset are the ones where REPORT LUNS is known to fail due to QEMU issues. At least for mpt-scsi it increases the number of drives supported by SeaBIOS with the existing QEMU (it supports 2 luns per target). And no, I don't see it as very important.
At least for mpt-scsi we should fix it. lsi-scsi might be a SeaBIOS bug too, but I don't know the hardware too well.
Do you mean fixing in SeaBIOS or in QEMU?
OK let me sum up the limitations in various places:
- mpt-scsi:
* in SeaBIOS:
1) only lun #0 is scanned 2) mpt_scsi_cmd() bails out on lun != 0 3) the code assumes uint8_t per lun #
The patchset removes 2), and scans with REPORT LUNS and, failing that, sequentially luns ##0-7.
* in QEMU:
1) mptsas_scsi_info.max_lun = 1 (i.e. one can only create 2 luns per target) 2) the code assumes that lun# fits in uint8_t
For reference, mptsas driver in Linux allows up to 16895 luns (the limit is tunable via module parameter).
- lsi-scsi:
* in SeaBIOS:
1) only lun #0 is scanned 2) the code assumes 3 bits per lun #
The patchset scans with REPORT LUNS and, failing that, sequentially luns ##0-7; besides, it fixes recovery from failed requests (like REPORT LUNS or INQUIRY on an inactive lun).
* in QEMU:
1) lsi_scsi_info.max_lun = 0 with a comment "LUN support is buggy" 2) the code assumes 3 bits per lun #
For reference, sym53c8xx driver in Linux allows up to 64 luns but has a comment that "target that implements more than 7 logical units are pretty rare".
So, when used with existing QEMU versions, scanning sequentially 8 luns is indeed unnecessary for both drivers: for the former 2 is enough, for the latter only lun #0 is possible. Once REPORT LUNS for these devices is fixed in QEMU, this won't matter any longer.
Roman.