VPD Block Limits emulation implementation
v2 cover VPD Block Limits emulation implementation
v2:
- series overhauled - first 2 patches are cleanups (split in 2 for easier review), patch 3 contains the whole logic
- previous version link:
https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg02289.html
When using SCSI passthrough and running in Linux, QEMU edits the reply of the SCSI Inquiry VPD Block Limits message with the value of the /sys/bus/<dev>/queue/max_sectors_kb parameter the device has in the host. Doing so allows the Linux guest to proper setup the device.
But the Block Limits message is optional, and its absence can cause the device to be unusable by the guest. An example can be seen at https://bugzilla.redhat.com/show_bug.cgi?id=1566195.
This series implements an approach to solve it.
Daniel Henrique Barboza (3):
hw/scsi: cleanups before VPD BL emulation hw/scsi: centralize SG_IO calls into single function hw/scsi: add VPD Block Limits emulation
hw/scsi/scsi-disk.c | 427 +++++++++++++++++++++-------------------- hw/scsi/scsi-generic.c | 246 ++++++++++++++++++------ include/hw/scsi/scsi.h | 6 +- 3 files changed, 401 insertions(+), 278 deletions(-)
Signature · 1 line
2.17.1
PATCH v2 1/3 hw/scsi: cleanups before VPD BL emulation
To add support for the emulation of Block Limits VPD page for passthrough devices, a few adjustments in the current code base is required to avoid repetition and improve clarity.
In scsi-generic.c, detach the Inquiry handling from scsi_read_complete and put it into a new function called scsi_handle_inquiry_reply. This change aims to avoid cluttering of scsi_read_complete when we more logic in the Inquiry response handling is added in the next patches, centralizing the changes in the new function.
In scsi-disk.c, take the build of all emulated VPD pages from scsi_disk_emulate_inquiry and make it available to other files into a non-static function called scsi_disk_emulate_vpd_page. Making it public will allow the future VPD BL emulation code for passthrough devices to use it from scsi-generic.c, avoiding copy/pasting this code solely for that purpose. It also has the advantage of providing emulation of all VPD pages in case we need to emulate other pages in other scenarios. As a bonus, scsi_disk_emulate_inquiry got tidier.
--- hw/scsi/scsi-disk.c | 407 +++++++++++++++++++++-------------------- hw/scsi/scsi-generic.c | 71 +++---- include/hw/scsi/scsi.h | 1 + 3 files changed, 249 insertions(+), 230 deletions(-)
hw/scsi/scsi-disk.c +210 -197
@@ -585,219 +585,228 @@ static uint8_t *scsi_get_buf(SCSIRequest *req)return (uint8_t *)r->iov.iov_base; } -static int scsi_disk_emulate_inquiry (SCSIRequest *req,uint8_t *outbuf) +int scsi_disk_emulate_vpd_page (SCSIRequest *req,uint8_t *outbuf) {SCSIDiskState *s =DO_UPCAST (SCSIDiskState, qdev, req->dev); -int buflen =0 ; -int start; - -if (req->cmd.buf[1 ] &0x1 ) { -/* Vital product data */ -uint8_t page_code = req->cmd.buf[2 ]; - - outbuf[buflen++] = s->qdev.type &0x1f ; - outbuf[buflen++] = page_code ;// this page - outbuf[buflen++] =0x00 ; - outbuf[buflen++] =0x00 ; - start = buflen; - -switch (page_code) { -case 0x00 :/* Supported page codes, mandatory */ - { -DPRINTF ("Inquiry EVPD[Supported pages] " -"buffer size %zd\n" , req->cmd.xfer); - outbuf[buflen++] =0x00 ;// list of supported pages (this page) -if (s->serial) { - outbuf[buflen++] =0x80 ;// unit serial number - } - outbuf[buflen++] =0x83 ;// device identification -if (s->qdev.type ==TYPE_DISK ) { - outbuf[buflen++] =0xb0 ;// block limits - outbuf[buflen++] =0xb1 ;/* block device characteristics */ - outbuf[buflen++] =0xb2 ;// thin provisioning - } -break ; - } -case 0x80 :/* Device serial number, optional */ - { -int l; +uint8_t page_code = req->cmd.buf[2 ]; +int start, buflen =0 ; -if (!s->serial) { -DPRINTF ("Inquiry (EVPD[Serial number] not supported\n" ); -return -1 ; - } + outbuf[buflen++] = s->qdev.type &0x1f ; + outbuf[buflen++] = page_code; + outbuf[buflen++] =0x00 ; + outbuf[buflen++] =0x00 ; + start = buflen; - l =strlen (s->serial); -if (l >36 ) { - l =36 ; - } +switch (page_code) { +case 0x00 :/* Supported page codes, mandatory */ + { +DPRINTF ("Inquiry EVPD[Supported pages] " +"buffer size %zd\n" , req->cmd.xfer); + outbuf[buflen++] =0x00 ;/* list of supported pages (this page) */ +if (s->serial) { + outbuf[buflen++] =0x80 ;/* unit serial number */ + } + outbuf[buflen++] =0x83 ;/* device identification */ +if (s->qdev.type ==TYPE_DISK ) { + outbuf[buflen++] =0xb0 ;/* block limits */ + outbuf[buflen++] =0xb1 ;/* block device characteristics */ + outbuf[buflen++] =0xb2 ;/* thin provisioning */ + } +break ; + } +case 0x80 :/* Device serial number, optional */ + { +int l; -DPRINTF ("Inquiry EVPD[Serial number] " -"buffer size %zd\n" , req->cmd.xfer); -memcpy (outbuf+buflen, s->serial, l); - buflen += l; -break ; +if (!s->serial) { +DPRINTF ("Inquiry (EVPD[Serial number] not supported\n" ); +return -1 ; } -case 0x83 :/* Device identification page, mandatory */ - { -const char *str = s->serial ?:blk_name (s->qdev.conf.blk); -int max_len = s->serial ?20 :255 -8 ; -int id_len =strlen (str); + l =strlen (s->serial); +if (l >36 ) { + l =36 ; + } -if (id_len > max_len) { - id_len = max_len; - } -DPRINTF ("Inquiry EVPD[Device identification] " -"buffer size %zd\n" , req->cmd.xfer); - - outbuf[buflen++] =0x2 ;// ASCII - outbuf[buflen++] =0 ;// not officially assigned - outbuf[buflen++] =0 ;// reserved - outbuf[buflen++] = id_len;// length of data following -memcpy (outbuf+buflen, str, id_len); - buflen += id_len; - -if (s->qdev.wwn) { - outbuf[buflen++] =0x1 ;// Binary - outbuf[buflen++] =0x3 ;// NAA - outbuf[buflen++] =0 ;// reserved - outbuf[buflen++] =8 ; -stq_be_p (&outbuf[buflen], s->qdev.wwn); - buflen +=8 ; - } +DPRINTF ("Inquiry EVPD[Serial number] " +"buffer size %zd\n" , req->cmd.xfer); +memcpy (outbuf + buflen, s->serial, l); + buflen += l; +break ; + } -if (s->qdev.port_wwn) { - outbuf[buflen++] =0x61 ;// SAS / Binary - outbuf[buflen++] =0x93 ;// PIV / Target port / NAA - outbuf[buflen++] =0 ;// reserved - outbuf[buflen++] =8 ; -stq_be_p (&outbuf[buflen], s->qdev.port_wwn); - buflen +=8 ; - } +case 0x83 :/* Device identification page, mandatory */ + { +const char *str = s->serial ?:blk_name (s->qdev.conf.blk); +int max_len = s->serial ?20 :255 -8 ; +int id_len =strlen (str); -if (s->port_index) { - outbuf[buflen++] =0x61 ;// SAS / Binary - outbuf[buflen++] =0x94 ;// PIV / Target port / relative target port - outbuf[buflen++] =0 ;// reserved - outbuf[buflen++] =4 ; -stw_be_p (&outbuf[buflen +2 ], s->port_index); - buflen +=4 ; - } -break ; +if (id_len > max_len) { + id_len = max_len; } -case 0xb0 :/* block limits */ - { -unsigned int unmap_sectors = - s->qdev.conf.discard_granularity / s->qdev.blocksize; -unsigned int min_io_size = - s->qdev.conf.min_io_size / s->qdev.blocksize; -unsigned int opt_io_size = - s->qdev.conf.opt_io_size / s->qdev.blocksize; -unsigned int max_unmap_sectors = - s->max_unmap_size / s->qdev.blocksize; -unsigned int max_io_sectors = - s->max_io_size / s->qdev.blocksize; - -if (s->qdev.type ==TYPE_ROM ) { -DPRINTF ("Inquiry (EVPD[%02X] not supported for CDROM\n" , - page_code); -return -1 ; - } -if (s->qdev.type ==TYPE_DISK ) { -int max_transfer_blk =blk_get_max_transfer (s->qdev.conf.blk); -int max_io_sectors_blk = - max_transfer_blk / s->qdev.blocksize; - - max_io_sectors = -MIN_NON_ZERO (max_io_sectors_blk, max_io_sectors); - -/* min_io_size and opt_io_size can't be greater than -* max_io_sectors */ -if (min_io_size) { - min_io_size =MIN (min_io_size, max_io_sectors); - } -if (opt_io_size) { - opt_io_size =MIN (opt_io_size, max_io_sectors); - } - } -/* required VPD size with unmap support */ - buflen =0x40 ; -memset (outbuf +4 ,0 , buflen -4 ); - - outbuf[4 ] =0x1 ;/* wsnz */ - -/* optimal transfer length granularity */ - outbuf[6 ] = (min_io_size >>8 ) &0xff ; - outbuf[7 ] = min_io_size &0xff ; - -/* maximum transfer length */ - outbuf[8 ] = (max_io_sectors >>24 ) &0xff ; - outbuf[9 ] = (max_io_sectors >>16 ) &0xff ; - outbuf[10 ] = (max_io_sectors >>8 ) &0xff ; - outbuf[11 ] = max_io_sectors &0xff ; - -/* optimal transfer length */ - outbuf[12 ] = (opt_io_size >>24 ) &0xff ; - outbuf[13 ] = (opt_io_size >>16 ) &0xff ; - outbuf[14 ] = (opt_io_size >>8 ) &0xff ; - outbuf[15 ] = opt_io_size &0xff ; - -/* max unmap LBA count, default is 1GB */ - outbuf[20 ] = (max_unmap_sectors >>24 ) &0xff ; - outbuf[21 ] = (max_unmap_sectors >>16 ) &0xff ; - outbuf[22 ] = (max_unmap_sectors >>8 ) &0xff ; - outbuf[23 ] = max_unmap_sectors &0xff ; - -/* max unmap descriptors, 255 fit in 4 kb with an 8-byte header. */ - outbuf[24 ] =0 ; - outbuf[25 ] =0 ; - outbuf[26 ] =0 ; - outbuf[27 ] =255 ; - -/* optimal unmap granularity */ - outbuf[28 ] = (unmap_sectors >>24 ) &0xff ; - outbuf[29 ] = (unmap_sectors >>16 ) &0xff ; - outbuf[30 ] = (unmap_sectors >>8 ) &0xff ; - outbuf[31 ] = unmap_sectors &0xff ; - -/* max write same size */ - outbuf[36 ] =0 ; - outbuf[37 ] =0 ; - outbuf[38 ] =0 ; - outbuf[39 ] =0 ; - - outbuf[40 ] = (max_io_sectors >>24 ) &0xff ; - outbuf[41 ] = (max_io_sectors >>16 ) &0xff ; - outbuf[42 ] = (max_io_sectors >>8 ) &0xff ; - outbuf[43 ] = max_io_sectors &0xff ; -break ; +DPRINTF ("Inquiry EVPD[Device identification] " +"buffer size %zd\n" , req->cmd.xfer); + + outbuf[buflen++] =0x2 ;/* ASCII */ + outbuf[buflen++] =0 ;/* not officially assigned */ + outbuf[buflen++] =0 ;/* reserved */ + outbuf[buflen++] = id_len;/* length of data following */ +memcpy (outbuf + buflen, str, id_len); + buflen += id_len; + +if (s->qdev.wwn) { + outbuf[buflen++] =0x1 ;/* Binary */ + outbuf[buflen++] =0x3 ;/* NAA */ + outbuf[buflen++] =0 ;/* reserved */ + outbuf[buflen++] =8 ; +stq_be_p (&outbuf[buflen], s->qdev.wwn); + buflen +=8 ; } -case 0xb1 :/* block device characteristics */ - { - buflen =8 ; - outbuf[4 ] = (s->rotation_rate >>8 ) &0xff ; - outbuf[5 ] = s->rotation_rate &0xff ; - outbuf[6 ] =0 ; - outbuf[7 ] =0 ; -break ; + +if (s->qdev.port_wwn) { + outbuf[buflen++] =0x61 ;/* SAS / Binary */ + outbuf[buflen++] =0x93 ;/* PIV / Target port / NAA */ + outbuf[buflen++] =0 ;/* reserved */ + outbuf[buflen++] =8 ; +stq_be_p (&outbuf[buflen], s->qdev.port_wwn); + buflen +=8 ; } -case 0xb2 :/* thin provisioning */ - { - buflen =8 ; - outbuf[4 ] =0 ; - outbuf[5 ] =0xe0 ;/* unmap & write_same 10/16 all supported */ - outbuf[6 ] = s->qdev.conf.discard_granularity ?2 :1 ; - outbuf[7 ] =0 ; -break ; + +if (s->port_index) { + outbuf[buflen++] =0x61 ;/* SAS / Binary */ + +/* PIV/Target port/relative target port */ + outbuf[buflen++] =0x94 ; + + outbuf[buflen++] =0 ;/* reserved */ + outbuf[buflen++] =4 ; +stw_be_p (&outbuf[buflen +2 ], s->port_index); + buflen +=4 ; } -default : +break ; + } +case 0xb0 :/* block limits */ + { +unsigned int unmap_sectors = + s->qdev.conf.discard_granularity / s->qdev.blocksize; +unsigned int min_io_size = + s->qdev.conf.min_io_size / s->qdev.blocksize; +unsigned int opt_io_size = + s->qdev.conf.opt_io_size / s->qdev.blocksize; +unsigned int max_unmap_sectors = + s->max_unmap_size / s->qdev.blocksize; +unsigned int max_io_sectors = + s->max_io_size / s->qdev.blocksize; + +if (s->qdev.type ==TYPE_ROM ) { +DPRINTF ("Inquiry (EVPD[%02X] not supported for CDROM\n" , + page_code);return -1 ; } -/* done with EVPD */ -assert (buflen - start <=255 ); - outbuf[start -1 ] = buflen - start; -return buflen; +if (s->qdev.type ==TYPE_DISK ) { +int max_transfer_blk =blk_get_max_transfer (s->qdev.conf.blk); +int max_io_sectors_blk = + max_transfer_blk / s->qdev.blocksize; + + max_io_sectors = +MIN_NON_ZERO (max_io_sectors_blk, max_io_sectors); + +/* min_io_size and opt_io_size can't be greater than +* max_io_sectors */ +if (min_io_size) { + min_io_size =MIN (min_io_size, max_io_sectors); + } +if (opt_io_size) { + opt_io_size =MIN (opt_io_size, max_io_sectors); + } + } +/* required VPD size with unmap support */ + buflen =0x40 ; +memset (outbuf +4 ,0 , buflen -4 ); + + outbuf[4 ] =0x1 ;/* wsnz */ + +/* optimal transfer length granularity */ + outbuf[6 ] = (min_io_size >>8 ) &0xff ; + outbuf[7 ] = min_io_size &0xff ; + +/* maximum transfer length */ + outbuf[8 ] = (max_io_sectors >>24 ) &0xff ; + outbuf[9 ] = (max_io_sectors >>16 ) &0xff ; + outbuf[10 ] = (max_io_sectors >>8 ) &0xff ; + outbuf[11 ] = max_io_sectors &0xff ; + +/* optimal transfer length */ + outbuf[12 ] = (opt_io_size >>24 ) &0xff ; + outbuf[13 ] = (opt_io_size >>16 ) &0xff ; + outbuf[14 ] = (opt_io_size >>8 ) &0xff ; + outbuf[15 ] = opt_io_size &0xff ; + +/* max unmap LBA count, default is 1GB */ + outbuf[20 ] = (max_unmap_sectors >>24 ) &0xff ; + outbuf[21 ] = (max_unmap_sectors >>16 ) &0xff ; + outbuf[22 ] = (max_unmap_sectors >>8 ) &0xff ; + outbuf[23 ] = max_unmap_sectors &0xff ; + +/* max unmap descriptors, 255 fit in 4 kb with an 8-byte header */ + outbuf[24 ] =0 ; + outbuf[25 ] =0 ; + outbuf[26 ] =0 ; + outbuf[27 ] =255 ; + +/* optimal unmap granularity */ + outbuf[28 ] = (unmap_sectors >>24 ) &0xff ; + outbuf[29 ] = (unmap_sectors >>16 ) &0xff ; + outbuf[30 ] = (unmap_sectors >>8 ) &0xff ; + outbuf[31 ] = unmap_sectors &0xff ; + +/* max write same size */ + outbuf[36 ] =0 ; + outbuf[37 ] =0 ; + outbuf[38 ] =0 ; + outbuf[39 ] =0 ; + + outbuf[40 ] = (max_io_sectors >>24 ) &0xff ; + outbuf[41 ] = (max_io_sectors >>16 ) &0xff ; + outbuf[42 ] = (max_io_sectors >>8 ) &0xff ; + outbuf[43 ] = max_io_sectors &0xff ; +break ; + } +case 0xb1 :/* block device characteristics */ + { + buflen =8 ; + outbuf[4 ] = (s->rotation_rate >>8 ) &0xff ; + outbuf[5 ] = s->rotation_rate &0xff ; + outbuf[6 ] =0 ; + outbuf[7 ] =0 ; +break ; + } +case 0xb2 :/* thin provisioning */ + { + buflen =8 ; + outbuf[4 ] =0 ; + outbuf[5 ] =0xe0 ;/* unmap & write_same 10/16 all supported */ + outbuf[6 ] = s->qdev.conf.discard_granularity ?2 :1 ; + outbuf[7 ] =0 ; +break ; + } +default : +return -1 ; + } +/* done with EVPD */ +assert (buflen - start <=255 ); + outbuf[start -1 ] = buflen - start; +return buflen; +} + +static int scsi_disk_emulate_inquiry (SCSIRequest *req,uint8_t *outbuf) +{ +SCSIDiskState *s =DO_UPCAST (SCSIDiskState, qdev, req->dev); +int buflen =0 ; + +if (req->cmd.buf[1 ] &0x1 ) { +/* Vital product data */ +return scsi_disk_emulate_vpd_page (req, outbuf); }/* Standard INQUIRY data */
@@ -3039,6 +3048,10 @@ static Property scsi_block_properties[] = {DEFINE_PROP_DRIVE ("drive" , SCSIDiskState, qdev.conf.blk),DEFINE_PROP_BOOL ("share-rw" , SCSIDiskState, qdev.conf.share_rw, false),DEFINE_PROP_UINT16 ("rotation_rate" , SCSIDiskState, rotation_rate,0 ), +DEFINE_PROP_UINT64 ("max_unmap_size" , SCSIDiskState, max_unmap_size, +DEFAULT_MAX_UNMAP_SIZE ), +DEFINE_PROP_UINT64 ("max_io_size" , SCSIDiskState, max_io_size, +DEFAULT_MAX_IO_SIZE ),DEFINE_PROP_INT32 ("scsi_version" , SCSIDiskState, qdev.default_scsi_version,-1 ), DEFINE_PROP_END_OF_LIST(),
hw/scsi/scsi-generic.c +38 -33
83 lines not on this page · raw message
include/hw/scsi/scsi.h +1 -0
@@ -186,6 +186,7 @@ void scsi_device_report_change(SCSIDevice *dev, SCSISense sense);void scsi_device_unit_attention_reported (SCSIDevice *dev);void scsi_generic_read_device_identification (SCSIDevice *dev);int scsi_device_get_sense (SCSIDevice *dev,uint8_t *buf,int len,bool fixed); +int scsi_disk_emulate_vpd_page (SCSIRequest *req,uint8_t *outbuf);SCSIDevice *scsi_device_find (SCSIBus *bus,int channel,int target,int lun);/* scsi-generic.c. */
Signature · 1 line
2.17.1
PATCH v2 2/3 hw/scsi: centralize SG_IO calls into single function
In reply to Daniel Henrique BarbozaFor the VPD Block Limits emulation with SCSI passthrough, we'll issue an Inquiry request with EVPD set to retrieve the available VPD pages of the device. This would be done in a way similar of what scsi_generic_read_device_identification
does: create a SCSI command and a reply buffer, fill in the sg_io_hdr_t structure, call blk_ioctl, check if an error occurred, process the response.
This same process is done in other 2 functions, get_device_type and get_stream_blocksize. They differ in the command/reply buffer and post-processing, everything else is almost a copy/paste.
Instead of adding a forth copy/pasted-ish code when adding the passthrough VPD BL emulation, this patch extirpates this repetition of those 3 functions and put it into a new one called scsi_SG_IO_FROM_DEV. Any future code that wants to execute an SG_DXFER_FROM_DEV to the device can use it, avoiding filling sg_io_hdr_t again and et cetera.
--- hw/scsi/scsi-disk.c | 18 +++---------- hw/scsi/scsi-generic.c | 61 +++++++++++++++++++++--------------------- include/hw/scsi/scsi.h | 2 ++ 3 files changed, 36 insertions(+), 45 deletions(-)
hw/scsi/scsi-disk.c +3 -15
@@ -2578,8 +2578,6 @@ static int get_device_type(SCSIDiskState *s) {uint8_t cmd[16 ];uint8_t buf[36 ]; -uint8_t sensebuf[8 ]; -sg_io_hdr_t io_header;int ret;memset (cmd,0 ,sizeof (cmd));
@@ -2587,19 +2585,9 @@ static int get_device_type(SCSIDiskState *s) cmd[0 ] =INQUIRY ; cmd[4 ] =sizeof (buf); -memset (&io_header,0 ,sizeof (io_header)); - io_header.interface_id ='S' ; - io_header.dxfer_direction =SG_DXFER_FROM_DEV ; - io_header.dxfer_len =sizeof (buf); - io_header.dxferp = buf; - io_header.cmdp = cmd; - io_header.cmd_len =sizeof (cmd); - io_header.mx_sb_len =sizeof (sensebuf); - io_header.sbp = sensebuf; - io_header.timeout =6000 ;/* XXX */ - - ret =blk_ioctl (s->qdev.conf.blk,SG_IO , &io_header); -if (ret <0 || io_header.driver_status || io_header.host_status) { + ret =scsi_SG_IO_FROM_DEV (s->qdev.conf.blk, cmd,sizeof (cmd), + buf,sizeof (buf)); +if (ret <0 ) {return -1 ; } s->qdev.type = buf[0 ];
hw/scsi/scsi-generic.c +31 -30
95 lines not on this page · raw message
include/hw/scsi/scsi.h +2 -0
@@ -187,6 +187,8 @@ void scsi_device_unit_attention_reported(SCSIDevice *dev);void scsi_generic_read_device_identification (SCSIDevice *dev);int scsi_device_get_sense (SCSIDevice *dev,uint8_t *buf,int len,bool fixed);int scsi_disk_emulate_vpd_page (SCSIRequest *req,uint8_t *outbuf); +int scsi_SG_IO_FROM_DEV (BlockBackend *blk,uint8_t *cmd,uint8_t cmd_size, +uint8_t *buf,uint8_t buf_size);SCSIDevice *scsi_device_find (SCSIBus *bus,int channel,int target,int lun);/* scsi-generic.c. */
Signature · 1 line
2.17.1
PATCH v2 3/3 hw/scsi: add VPD Block Limits emulation
In reply to Daniel Henrique BarbozaThe VPD Block Limits Inquiry page is optional, allowing SCSI devices to not implement it. This is the case for devices like the MegaRAID SAS 9361-8i and Microsemi PM8069.
In case of SCSI passthrough, the response of this request is used by the QEMU SCSI layer to set the max_io_sectors that the guest device will support, based on the value of the max_sectors_kb that the device has set in the host at that time. Without this response, the guest kernel is free to assume any value of max_io_sectors for the SCSI device. If this value is greater than the value from the host, SCSI Sense errors will occur because the guest will send read/write requests that are larger than the underlying host device is configured to support. An example of this behavior can be seen in [1].
A workaround is to set the max_sectors_kb host value back in the guest kernel (a process that can be automated using rc.local startup scripts and the like), but this has several drawbacks:
- it can be troublesome if the guest has many passthrough devices that needs this tuning;
- if a change in max_sectors_kb is made in the host side, manual change in the guests will also be required;
- during an OS install it is difficult, and sometimes not possible, to go to a terminal and change the max_sectors_kb prior to the installation. This means that the disk can't be used during the install process. The easiest alternative here is to roll back to scsi-hd, install the guest and then go back to SCSI passthrough when the installation is done and max_sectors_kb can be set.
An easier way would be to QEMU handle the absence of the Block Limits VPD device response, setting max_io_sectors accordingly and allowing the guest to use the device without the hassle.
This patch adds emulation of the Block Limits VPD response for SCSI passthrough devices of type TYPE_DISK that doesn't support it. The following changes were made:
- scsi_handle_inquiry_reply will now check the available VPD pages from the Inquiry EVPD reply. In case the device does not
- a new function called scsi_generic_set_vpd_bl_emulation, that is called during device realize, was created to set a new flag 'needs_vpd_bl_emulation' of the device. This function retrieves the Inquiry EVPD response of the device to check for VPD BL support.
- scsi_handle_inquiry_reply will now check the available VPD pages from the Inquiry EVPD reply in case the device needs VPD BL emulation, adding the Block Limits page (0xb0) to the list. This will make the guest kernel aware of the support that we're now providing by emulation.
- a new function scsi_emulate_block_limits creates the emulated Block Limits response. This function is called inside scsi_read_complete in case the device requires Block Limits VPD emulation and we detected a SCSI Sense error in the VPD Block Limits reply that was issued from the guest kernel to the device. This error is
expected: we're reporting support from our side, but the device isn't aware of it.
With this patch, the guest now queries the Block Limits page during the device configuration because it is being advertised in the Supported Pages response. It will either receive the Block Limits page from the hardware, if it supports it, or will receive an emulated response from QEMU. At any rate, the guest now has the information to set the max_sectors_kb parameter accordingly, sparing the user of SCSI sense errors that would happen without the emulated response and in the absence of Block Limits support from the hardware.
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1566195
--- hw/scsi/scsi-disk.c | 2 +- hw/scsi/scsi-generic.c | 132 +++++++++++++++++++++++++++++++++++++---- include/hw/scsi/scsi.h | 3 +- 3 files changed, 125 insertions(+), 12 deletions(-)
hw/scsi/scsi-disk.c +1 -1
@@ -2645,7 +2645,7 @@ static void scsi_block_realize(SCSIDevice *dev, Error **errp) s->features |= (1 <<SCSI_DISK_F_NO_REMOVABLE_DEVOPS );scsi_realize (&s->qdev, errp); -scsi_generic_read_device_identification (&s->qdev); +scsi_generic_read_device_inquiry (&s->qdev); } typedef struct SCSIBlockReq {
hw/scsi/scsi-generic.c +122 -10
182 lines not on this page · raw message
include/hw/scsi/scsi.h +2 -1
@@ -87,6 +87,7 @@ struct SCSIDeviceuint64_t port_wwn;int scsi_version;int default_scsi_version; +bool needs_vpd_bl_emulation; };extern const VMStateDescription vmstate_scsi_device;
@@ -184,7 +185,7 @@ void scsi_device_purge_requests(SCSIDevice *sdev, SCSISense sense);void scsi_device_set_ua (SCSIDevice *sdev,SCSISense sense);void scsi_device_report_change (SCSIDevice *dev,SCSISense sense);void scsi_device_unit_attention_reported (SCSIDevice *dev); -void scsi_generic_read_device_identification (SCSIDevice *dev); +void scsi_generic_read_device_inquiry (SCSIDevice *dev);int scsi_device_get_sense (SCSIDevice *dev,uint8_t *buf,int len,bool fixed);int scsi_disk_emulate_vpd_page (SCSIRequest *req,uint8_t *outbuf); int scsi_SG_IO_FROM_DEV(BlockBackend *blk, uint8_t *cmd, uint8_t cmd_size,
Signature · 1 line
2.17.1
Paolo Bonzini
AppliedQuoted 13 lines · Daniel Henrique Barboza
v2:
- series overhauled - first 2 patches are cleanups (split in 2 for easier review), patch 3 contains the whole logic
- previous version link:
https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg02289.htmlWhen using SCSI passthrough and running in Linux, QEMU edits the reply of the SCSI Inquiry VPD Block Limits message with the value of the /sys/bus/<dev>/queue/max_sectors_kb parameter the device has in the host. Doing so allows the Linux guest to proper setup the device.
Quoted 5 lines
But the Block Limits message is optional, and its absence can cause the device to be unusable by the guest. An example can be seen at https://bugzilla.redhat.com/show_bug.cgi?id=1566195.
This series implements an approach to solve it.
Queued, thanks.
Paolo