[PATCH 0/5] virtio-scsi: Add max_target and max_lun knobs

Karolina Stolarek posted 5 patches 3 weeks, 5 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/cover.1777890280.git.karolina.stolarek@oracle.com
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Fam Zheng <fam@euphon.net>, Richard Henderson <richard.henderson@linaro.org>, Helge Deller <deller@gmx.de>, Nicholas Piggin <npiggin@gmail.com>, Harsh Prateek Bora <harshpb@linux.ibm.com>, "Michael S. Tsirkin" <mst@redhat.com>, Stefano Garzarella <sgarzare@redhat.com>, Raphael Norwitz <rnorwitz@nvidia.com>, Dmitry Fleytman <dmitry.fleytman@gmail.com>, Jeuk Kim <jeuk20.kim@samsung.com>, Chao Liu <chao.liu.zevorn@gmail.com>
hw/scsi/esp-pci.c               |  7 ++--
hw/scsi/esp.c                   |  7 ++--
hw/scsi/lasi_ncr710.c           |  6 ++--
hw/scsi/lsi53c895a.c            |  6 ++--
hw/scsi/megasas.c               |  7 ++--
hw/scsi/mptsas.c                |  9 +++--
hw/scsi/ncr53c710.c             |  6 ++--
hw/scsi/scsi-bus.c              | 64 +++++++++++++++++----------------
hw/scsi/scsi-disk.c             |  2 +-
hw/scsi/spapr_vscsi.c           |  7 ++--
hw/scsi/vhost-scsi.c            |  4 +++
hw/scsi/vhost-user-scsi.c       |  4 +++
hw/scsi/virtio-scsi.c           | 51 ++++++++++++++++++--------
hw/scsi/vmw_pvscsi.c            |  7 ++--
hw/ufs/lu.c                     |  8 +++--
hw/usb/dev-storage-bot.c        |  7 ++--
hw/usb/dev-storage-classic.c    |  6 ++--
hw/usb/dev-uas.c                |  7 ++--
include/hw/scsi/scsi.h          | 22 ++++++++----
include/hw/virtio/virtio-scsi.h |  4 +++
scripts/checkpatch.pl           |  2 +-
21 files changed, 161 insertions(+), 82 deletions(-)
[PATCH 0/5] virtio-scsi: Add max_target and max_lun knobs
Posted by Karolina Stolarek 3 weeks, 5 days ago
This series introduces an option to override the maximum number of
targets and LUNs, now set to VIRTIO_SCSI_MAX_TARGET and
VIRTIO_SCSI_MAX_LUN respectively, via max_target and max_lun
properties in virtio-scsi and vhost-scsi devices.

When probing a device, the driver walks through all possible targets
to discover what is available on the bus. In the process, it sends
INQUIRY commands, including to devices that do not exist. The issue
can be observed in a guest with a single scsi-hd device, with verbose
SCSI logging enabled:

scsi 0:0:0:0: scsi scan: REPORT LUN scan
scsi 0:0:0:0: scsi scan: device exists on 0:0:0:0
scsi 0:0:1:0: scsi scan: INQUIRY pass 1 length 36
scsi 0:0:1:0: scsi scan: INQUIRY failed with code 0x40000
(...)
scsi 0:0:255:0: scsi scan: INQUIRY pass 1 length 36
scsi 0:0:255:0: scsi scan: INQUIRY failed with code 0x40000

The issue is more noticeable with the vhost-scsi backend.
On hypervisors without the patch [2], each INQUIRY queued up
for a non-existent target triggers vq_error(), registered by
vhost_virtqueue_error_notifier(), resulting in a flood of
"vhost vring error in virtqueue X" messages when booting a VM.

Even with [2] applied, we still send commands to phantom targets
and have no way to stop the scan earlier, spending more time
in virtscsi_probe() than necessary.

The fourth patch in the series addresses this issue by introducing
the "max_target" property for virtio-scsi, vhost-scsi and vhost-user-scsi
devices. This provides an option to specify how many targets are used
by the guest, and to stop scanning before hitting VIRTIO_SCSI_MAX_TARGET.

A similar issue can be seen when scanning LUNs on existing targets.
For example, the OVMF VirtioScsiDxe driver queues up INQUIRY for each
0..7 LUN, even if only LUN 0 is available. This can be easily observed
when using a vhost-scsi backend, as such messages appear in the hypervisor
dmesg when booting up a guest:

TARGET_CORE[vhost]: Detected NON_EXISTENT_LUN Access for 0x00000001 from
naa.5001405277e02c68
TARGET_CORE[vhost]: Detected NON_EXISTENT_LUN Access for 0x00000002 from
naa.5001405277e02c68
TARGET_CORE[vhost]: Detected NON_EXISTENT_LUN Access for 0x00000003 from
naa.5001405277e02c68
TARGET_CORE[vhost]: Detected NON_EXISTENT_LUN Access for 0x00000004 from
naa.5001405277e02c68
TARGET_CORE[vhost]: Detected NON_EXISTENT_LUN Access for 0x00000005 from
naa.5001405277e02c68
TARGET_CORE[vhost]: Detected NON_EXISTENT_LUN Access for 0x00000006 from
naa.5001405277e02c68
TARGET_CORE[vhost]: Detected NON_EXISTENT_LUN Access for 0x00000007 from
naa.5001405277e02c68

The last patch of the series provides a way to specify how many LUNs
are actually present by setting the "max_lun" property in virtio-scsi,
vhost-scsi and vhost-user-scsi devices.

If neither property is specified, max_target and max_lun are set to
their default values.

The series overview:

- Patches 1-2 - refactoring of SCSIBusInfo suggested by Alex Bennée
                in [1] that splits the struct into two: one for bus
                callbacks, SCSIBusOps, the other for bus configuration,
                SCSIBusConfig
- Patch 3     - use errp in virtio_scsi_device_realize() instead of
                a local error variable
- Patch 4     - introduction of the max_target parameter
- Patch 5     - introduction of the max_lun parameter

This is a complete rewrite of the [3] series, so I decided to submit
it as a new patchset, instead of labeling it v2.

Testing: In addition to running make check, I ran a guest with multiple
         virtio-scsi-pci and vhost-scsi controllers using different
         max_target values, verified that the properties are set, and
         tried hotplugging virtio-scsi devices to see if the custom
         max_target setting is respected.

----
[1] - https://lore.kernel.org/all/87h5u1hye5.fsf@draig.linaro.org/
[2] -
https://lore.kernel.org/virtualization/20250607171815.111030-1-michael.christie@oracle.com/T/#u
[3] -
https://lore.kernel.org/all/cover.1763999544.git.karolina.stolarek@oracle.com

Karolina Stolarek (5):
  scsi: Split SCSIBusInfo struct
  scsi: Rename SCSIBusInfo to SCSIBusOps
  virtio-scsi: Use errp in virtio_scsi_device_realize
  virtio-scsi: Make max_target parameter configurable
  virtio-scsi: Make max_lun parameter configurable

 hw/scsi/esp-pci.c               |  7 ++--
 hw/scsi/esp.c                   |  7 ++--
 hw/scsi/lasi_ncr710.c           |  6 ++--
 hw/scsi/lsi53c895a.c            |  6 ++--
 hw/scsi/megasas.c               |  7 ++--
 hw/scsi/mptsas.c                |  9 +++--
 hw/scsi/ncr53c710.c             |  6 ++--
 hw/scsi/scsi-bus.c              | 64 +++++++++++++++++----------------
 hw/scsi/scsi-disk.c             |  2 +-
 hw/scsi/spapr_vscsi.c           |  7 ++--
 hw/scsi/vhost-scsi.c            |  4 +++
 hw/scsi/vhost-user-scsi.c       |  4 +++
 hw/scsi/virtio-scsi.c           | 51 ++++++++++++++++++--------
 hw/scsi/vmw_pvscsi.c            |  7 ++--
 hw/ufs/lu.c                     |  8 +++--
 hw/usb/dev-storage-bot.c        |  7 ++--
 hw/usb/dev-storage-classic.c    |  6 ++--
 hw/usb/dev-uas.c                |  7 ++--
 include/hw/scsi/scsi.h          | 22 ++++++++----
 include/hw/virtio/virtio-scsi.h |  4 +++
 scripts/checkpatch.pl           |  2 +-
 21 files changed, 161 insertions(+), 82 deletions(-)

-- 
2.47.1