The introduction of REPORT LUNS caused a potential regression when many disks are present on the same SCSI target. If LUN0 is processed too late, SeaBIOS might run out of memory and fail to register it.
There are two kinds of problems:
- QEMU presents the results of REPORT LUNS in the opposite order to the "-device" command-line options. It's likely that LUN 0 will be first in the command-line, and thus will be processed last by SeaBIOS.
- QEMU registers all LUNs in each target first. It's thus possible that LUN 0 of target 1 is processed after many LUNs of target 0 and not be able to allocate memory.
To fix both, the SCSI scans are modified to operate in two phases; LUN 0 is scanned before the REPORT LUNS command is sent. A possible future enhancement is to remember the results of the scan in a 32-byte bitmap, and skip the REPORT LUNS command during the second phase.
When multiple SCSI controllers are found, it's still possible to have a regression because they are scanned in parallel.
Signed-off-by: Paolo Bonzini pbonzini@redhat.com --- src/hw/blockcmd.c | 4 ++-- src/hw/esp-scsi.c | 10 ++++++++-- src/hw/lsi-scsi.c | 10 ++++++++-- src/hw/mpt-scsi.c | 10 ++++++++-- src/hw/usb-uas.c | 1 + src/hw/virtio-scsi.c | 13 +++++++++---- 6 files changed, 36 insertions(+), 12 deletions(-)
diff --git a/src/hw/blockcmd.c b/src/hw/blockcmd.c index 324188d..af9cd4d 100644 --- a/src/hw/blockcmd.c +++ b/src/hw/blockcmd.c @@ -254,7 +254,7 @@ int scsi_rep_luns_scan(struct drive_s *tmp_drive, scsi_add_lun add_lun)
for (i = 0, ret = 0; i < nluns; i++) { u64 lun = scsilun2u64(&resp->luns[i]); - if (lun >> 32) + if (lun >> 32 || !lun) continue; ret += !add_lun((u32)lun, tmp_drive); } @@ -270,7 +270,7 @@ int scsi_sequential_scan(struct drive_s *tmp_drive, u32 maxluns, int ret; u32 lun;
- for (lun = 0, ret = 0; lun < maxluns; lun++) + for (lun = 1, ret = 0; lun < maxluns; lun++) ret += !add_lun(lun, tmp_drive); return ret; } diff --git a/src/hw/esp-scsi.c b/src/hw/esp-scsi.c index 57d3832..a2f6cbc 100644 --- a/src/hw/esp-scsi.c +++ b/src/hw/esp-scsi.c @@ -194,11 +194,15 @@ fail: }
static void -esp_scsi_scan_target(struct pci_device *pci, u32 iobase, u8 target) +esp_scsi_scan_target(struct pci_device *pci, u32 iobase, u8 target, u8 report_luns) { struct esp_lun_s llun0;
esp_scsi_init_lun(&llun0, pci, iobase, target, 0); + if (!report_luns) { + esp_scsi_add_lun(0, &llun0.drive); + return; + }
scsi_rep_luns_scan(&llun0.drive, esp_scsi_add_lun); } @@ -219,7 +223,9 @@ init_esp_scsi(void *data)
int i; for (i = 0; i <= 7; i++) - esp_scsi_scan_target(pci, iobase, i); + esp_scsi_scan_target(pci, iobase, i, 0); + for (i = 0; i <= 7; i++) + esp_scsi_scan_target(pci, iobase, i, 1); }
void diff --git a/src/hw/lsi-scsi.c b/src/hw/lsi-scsi.c index 846bb0b..bf2ff76 100644 --- a/src/hw/lsi-scsi.c +++ b/src/hw/lsi-scsi.c @@ -175,11 +175,15 @@ fail: }
static void -lsi_scsi_scan_target(struct pci_device *pci, u32 iobase, u8 target) +lsi_scsi_scan_target(struct pci_device *pci, u32 iobase, u8 target, u8 report_luns) { struct lsi_lun_s llun0;
lsi_scsi_init_lun(&llun0, pci, iobase, target, 0); + if (!report_luns) { + lsi_scsi_add_lun(0, &llun0.drive); + return; + }
if (scsi_rep_luns_scan(&llun0.drive, lsi_scsi_add_lun) < 0) scsi_sequential_scan(&llun0.drive, 8, lsi_scsi_add_lun); @@ -201,7 +205,9 @@ init_lsi_scsi(void *data)
int i; for (i = 0; i < 7; i++) - lsi_scsi_scan_target(pci, iobase, i); + lsi_scsi_scan_target(pci, iobase, i, 0); + for (i = 0; i < 7; i++) + lsi_scsi_scan_target(pci, iobase, i, 1); }
void diff --git a/src/hw/mpt-scsi.c b/src/hw/mpt-scsi.c index 80c6d6b..5a412aa 100644 --- a/src/hw/mpt-scsi.c +++ b/src/hw/mpt-scsi.c @@ -237,11 +237,15 @@ fail: }
static void -mpt_scsi_scan_target(struct pci_device *pci, u32 iobase, u8 target) +mpt_scsi_scan_target(struct pci_device *pci, u32 iobase, u8 target, u8 report_luns) { struct mpt_lun_s llun0;
mpt_scsi_init_lun(&llun0, pci, iobase, target, 0); + if (!report_luns) { + mpt_scsi_add_lun(0, &llun0.drive); + return; + }
if (scsi_rep_luns_scan(&llun0.drive, mpt_scsi_add_lun) < 0) scsi_sequential_scan(&llun0.drive, 8, mpt_scsi_add_lun); @@ -295,7 +299,9 @@ init_mpt_scsi(void *data)
int i; for (i = 0; i < 7; i++) - mpt_scsi_scan_target(pci, iobase, i); + mpt_scsi_scan_target(pci, iobase, i, 0); + for (i = 0; i < 7; i++) + mpt_scsi_scan_target(pci, iobase, i, 1); }
void diff --git a/src/hw/usb-uas.c b/src/hw/usb-uas.c index f00221a..f2eb049 100644 --- a/src/hw/usb-uas.c +++ b/src/hw/usb-uas.c @@ -272,6 +272,7 @@ usb_uas_setup(struct usbdevice_s *usbdev)
struct uasdrive_s lun0; uas_init_lun(&lun0, usbdev, command, status, data_in, data_out, 0); + uas_add_lun(0, &lun0.drive); int ret = scsi_rep_luns_scan(&lun0.drive, uas_add_lun); if (ret <= 0) { dprintf(1, "Unable to configure UAS drive.\n"); diff --git a/src/hw/virtio-scsi.c b/src/hw/virtio-scsi.c index 7490ec0..3e8f13e 100644 --- a/src/hw/virtio-scsi.c +++ b/src/hw/virtio-scsi.c @@ -135,12 +135,14 @@ fail:
static int virtio_scsi_scan_target(struct pci_device *pci, struct vp_device *vp, - struct vring_virtqueue *vq, u16 target) + struct vring_virtqueue *vq, u16 target, u8 report_luns) {
struct virtio_lun_s vlun0;
virtio_scsi_init_lun(&vlun0, pci, vp, vq, target, 0); + if (!report_luns) + return !virtio_scsi_add_lun(0, &vlun0.drive);
int ret = scsi_rep_luns_scan(&vlun0.drive, virtio_scsi_add_lun); return ret < 0 ? 0 : ret; @@ -185,9 +187,12 @@ init_virtio_scsi(void *data) status |= VIRTIO_CONFIG_S_DRIVER_OK; vp_set_status(vp, status);
- int i, tot; - for (tot = 0, i = 0; i < 256; i++) - tot += virtio_scsi_scan_target(pci, vp, vq, i); + int tot = 0; + int i; + for (i = 0; i < 256; i++) + tot += virtio_scsi_scan_target(pci, vp, vq, i, 0); + for (i = 0; i < 256; i++) + tot += virtio_scsi_scan_target(pci, vp, vq, i, 1);
if (!tot) goto fail;