Re: [Qemu-devel] [PATCH 1/2 v1] blkdrv: Add queue limits parameters for sg block drive
On Wed 22 Aug 2012 10:13:44 PM CST, Paolo Bonzini wrote: Il 22/08/2012 15:13, Stefan Hajnoczi ha scritto: http://lists.gnu.org/archive/html/qemu-devel/2010-12/msg01741.html This is a real problem in practice. IE. the USB CD-ROM on this POWER7 blade limits transfers to 0x1e000 bytes for example and the Linux sr driver on the guest is going to try to give me bigger requests than that if I don't start limiting them, which will cause all sort of errors. It cannot be fixed for emulated SCSI HBAs in general but it can for virtio-scsi. For disks, this should be fixed simply by using scsi-block instead of scsi-generic. CD-ROMs are indeed more complicated because burning CDs cannot be done with syscalls. :/ So, as the problem exist to CD-ROM, I will continue to get these patches move on. As Paolo pointed out, the only limit is max_sectors (the total size of a scatter-list), I will drop the other 2 parameters. Paolo, what's your opinion? Cong Paolo
Re: [Qemu-devel] [PATCH 1/2 v1] blkdrv: Add queue limits parameters for sg block drive
On 08/21/2012 06:14 PM, Paolo Bonzini wrote: Il 21/08/2012 11:52, Stefan Hajnoczi ha scritto: Using /sys/dev/block or /sys/dev/char seems easier, and lets you retrieve the parameters for block devices too. what do you mean with block devices? Using /dev/sda instead of /dev/sg0? Yes. However, I'm worried of the consequences this has for migration. You could have the same physical disk accessed with two different HBAs, with different limits. So I don't know if this can really be solved at all. I know little about qemu migration now. The pending scsi commands will be saved and transfered to remote machine when starting migration? Passthrough is already a migration blocker if both hosts do not have access to the same LUNs. Yes, but requiring the exact same hardware may be too much. I'm trying to understand the problem better before committing to a threefold spec/qemu/kernel change. Cong, what is the limit that the host HBA enforces (and what is the HBA)? What commands see a problem? Is it fixed by using scsi-block instead of scsi-generic (if you can use scsi-block at all, i.e. it's not a tape or similar device)? I don't see real problem caused by the the queue limits actually. It's a bug which Stefan told me. With scsi-generic, QEMU uses a bounce buffer for non-I/O commands to a SCSI passthrough device, so the only problem in that case should be the maximum segment size. This could change in the future, but max_segments and max_sectors should not yet be a problem. about bounce buffer, do you meat the buffer allocated in scsi_send_command() of hw/scsi-generic.c? Cong. With scsi-block, QEMU will use read/write on the block device and the host kernel will then obey the host HBA's block limits. QEMU will still use a bounce buffer for non-I/O commands to a scsi-block device, but the payload is usually small for non-I/O commands. Paolo When both hosts do have access to the same LUNs it's possible to extract the block queue limits (using sysfs) and compare them. Today you can start QEMU with different image files on both hosts. Migration will appear to work but the disk image on the destination host could be junk. This is a similar case, I don't see a problem except that there should be a safety check (maybe at the libvirt level) to make this safe.
[Qemu-devel] [PATCH 1/2 v1] blkdrv: Add queue limits parameters for sg block drive
This patch adds some queue limit parameters into block drive. And inits them
for sg block drive. Some interfaces are also added for accessing them.
Signed-off-by: Cong Meng m...@linux.vnet.ibm.com
---
block/raw-posix.c | 58 +
block_int.h |4 +++
blockdev.c| 15 +
hw/block-common.h |3 ++
4 files changed, 80 insertions(+), 0 deletions(-)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 0dce089..a086f89 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -53,6 +53,7 @@
#include linux/cdrom.h
#include linux/fd.h
#include linux/fs.h
+#include dirent.h
#endif
#ifdef CONFIG_FIEMAP
#include linux/fiemap.h
@@ -829,6 +830,62 @@ static int hdev_probe_device(const char *filename)
return 0;
}
+static void read_queue_limit(char *path, const char *filename, unsigned int
*val)
+{
+FILE *f;
+char *tail = path + strlen(path);
+
+pstrcat(path, MAXPATHLEN, filename);
+f = fopen(path, r);
+if (!f) {
+goto out;
+}
+
+fscanf(f, %u, val);
+fclose(f);
+
+out:
+*tail = 0;
+}
+
+static void sg_get_queue_limits(BlockDriverState *bs, const char *filename)
+{
+DIR *ffs;
+struct dirent *d;
+char path[MAXPATHLEN];
+
+snprintf(path, MAXPATHLEN,
+ /sys/class/scsi_generic/sg%s/device/block/,
+ filename + strlen(/dev/sg));
+
+ffs = opendir(path);
+if (!ffs) {
+return;
+}
+
+for (;;) {
+d = readdir(ffs);
+if (!d) {
+return;
+}
+
+if (strcmp(d-d_name, .) == 0 || strcmp(d-d_name, ..) == 0) {
+continue;
+}
+
+break;
+}
+
+closedir(ffs);
+
+pstrcat(path, MAXPATHLEN, d-d_name);
+pstrcat(path, MAXPATHLEN, /queue/);
+
+read_queue_limit(path, max_sectors_kb, bs-max_sectors);
+read_queue_limit(path, max_segments, bs-max_segments);
+read_queue_limit(path, max_segment_size, bs-max_segment_size);
+}
+
static int hdev_open(BlockDriverState *bs, const char *filename, int flags)
{
BDRVRawState *s = bs-opaque;
@@ -868,6 +925,7 @@ static int hdev_open(BlockDriverState *bs, const char
*filename, int flags)
temp = realpath(filename, resolved_path);
if (temp strstart(temp, /dev/sg, NULL)) {
bs-sg = 1;
+sg_get_queue_limits(bs, temp);
}
}
#endif
diff --git a/block_int.h b/block_int.h
index d72317f..a9d07a2 100644
--- a/block_int.h
+++ b/block_int.h
@@ -333,6 +333,10 @@ struct BlockDriverState {
/* long-running background operation */
BlockJob *job;
+
+unsigned int max_sectors;
+unsigned int max_segments;
+unsigned int max_segment_size;
};
int get_tmp_filename(char *filename, int size);
diff --git a/blockdev.c b/blockdev.c
index 3d75015..e17edbf 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1191,3 +1191,18 @@ BlockJobInfoList *qmp_query_block_jobs(Error **errp)
bdrv_iterate(do_qmp_query_block_jobs_one, prev);
return dummy.next;
}
+
+unsigned int get_queue_max_sectors(BlockDriverState *bs)
+{
+return (bs-file bs-file-sg) ? bs-file-max_sectors : 0;
+}
+
+unsigned int get_queue_max_segments(BlockDriverState *bs)
+{
+return (bs-file bs-file-sg) ? bs-file-max_segments : 0;
+}
+
+unsigned int get_queue_max_segment_size(BlockDriverState *bs)
+{
+return (bs-file bs-file-sg) ? bs-file-max_segment_size : 0;
+}
diff --git a/hw/block-common.h b/hw/block-common.h
index bb808f7..d47fcd7 100644
--- a/hw/block-common.h
+++ b/hw/block-common.h
@@ -76,4 +76,7 @@ void hd_geometry_guess(BlockDriverState *bs,
int *ptrans);
int hd_bios_chs_auto_trans(uint32_t cyls, uint32_t heads, uint32_t secs);
+unsigned int get_queue_max_sectors(BlockDriverState *bs);
+unsigned int get_queue_max_segments(BlockDriverState *bs);
+unsigned int get_queue_max_segment_size(BlockDriverState *bs);
#endif
--
1.7.7.6
[Qemu-devel] [PATCH 2/2 v1] virtio-scsi: set per-LUN queue limits for sg devices
Each virtio scsi HBA has global request queue limits. But the passthrough
LUNs (scsi-generic) come from different host HBAs may have different request
queue limits. If the guest sends commands that exceed the host limits, the
commands will be rejected by host HAB.
To address the issue, this patch responses the newly added virtio control
queue request by returning the per-LUN queue limits.
Signed-off-by: Cong Meng m...@linux.vnet.ibm.com
---
hw/virtio-scsi.c | 65 ++
1 files changed, 65 insertions(+), 0 deletions(-)
diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c
index c4a5b22..3c0bd99 100644
--- a/hw/virtio-scsi.c
+++ b/hw/virtio-scsi.c
@@ -28,6 +28,7 @@
#define VIRTIO_SCSI_F_INOUT0
#define VIRTIO_SCSI_F_HOTPLUG 1
#define VIRTIO_SCSI_F_CHANGE 2
+#define VIRTIO_SCSI_F_LUN_QUERY3
/* Response codes */
#define VIRTIO_SCSI_S_OK 0
@@ -48,6 +49,7 @@
#define VIRTIO_SCSI_T_TMF 0
#define VIRTIO_SCSI_T_AN_QUERY 1
#define VIRTIO_SCSI_T_AN_SUBSCRIBE 2
+#define VIRTIO_SCSI_T_LUN_QUERY3
/* Valid TMF subtypes. */
#define VIRTIO_SCSI_T_TMF_ABORT_TASK 0
@@ -66,6 +68,11 @@
#define VIRTIO_SCSI_T_ASYNC_NOTIFY 2
#define VIRTIO_SCSI_T_PARAM_CHANGE 3
+/* LUN Query */
+#define VIRTIO_SCSI_T_LQ_MAX_SECTORS 0
+#define VIRTIO_SCSI_T_LQ_MAX_SEGMENTS 1
+#define VIRTIO_SCSI_T_LQ_MAX_SEGMENT_SIZE 2
+
/* Reasons for transport reset event */
#define VIRTIO_SCSI_EVT_RESET_HARD 0
#define VIRTIO_SCSI_EVT_RESET_RESCAN 1
@@ -115,6 +122,18 @@ typedef struct {
uint8_t response;
} QEMU_PACKED VirtIOSCSICtrlANResp;
+/* LUN qeury */
+typedef struct {
+uint32_t type;
+uint8_t lun[8];
+uint32_t subtype;
+} QEMU_PACKED VirtIOSCSICtrlLQReq;
+
+typedef struct {
+uint32_t response;
+uint32_t value;
+} QEMU_PACKED VirtIOSCSICtrlLQResp;
+
typedef struct {
uint32_t event;
uint8_t lun[8];
@@ -160,6 +179,7 @@ typedef struct VirtIOSCSIReq {
VirtIOSCSICmdReq *cmd;
VirtIOSCSICtrlTMFReq *tmf;
VirtIOSCSICtrlANReq *an;
+VirtIOSCSICtrlLQReq *lq;
} req;
union {
char *buf;
@@ -167,6 +187,7 @@ typedef struct VirtIOSCSIReq {
VirtIOSCSICtrlTMFResp *tmf;
VirtIOSCSICtrlANResp *an;
VirtIOSCSIEvent *event;
+VirtIOSCSICtrlLQResp *lq;
} resp;
} VirtIOSCSIReq;
@@ -285,6 +306,43 @@ static void *virtio_scsi_load_request(QEMUFile *f,
SCSIRequest *sreq)
return req;
}
+static void virtio_scsi_do_lun_query(VirtIOSCSI *s, VirtIOSCSIReq *req)
+{
+SCSIDevice *d = virtio_scsi_device_find(s, req-req.lq-lun);
+
+if (!d) {
+goto fail;
+}
+
+switch (req-req.lq-subtype) {
+case VIRTIO_SCSI_T_LQ_MAX_SECTORS:
+req-resp.lq-value = get_queue_max_sectors(d-conf.bs);
+if (!req-resp.lq-value) {
+goto fail;
+}
+break;
+case VIRTIO_SCSI_T_LQ_MAX_SEGMENTS:
+req-resp.lq-value = get_queue_max_segments(d-conf.bs);
+if (!req-resp.lq-value) {
+goto fail;
+}
+break;
+case VIRTIO_SCSI_T_LQ_MAX_SEGMENT_SIZE:
+req-resp.lq-value = get_queue_max_segment_size(d-conf.bs);
+if (!req-resp.lq-value) {
+goto fail;
+}
+break;
+default:
+goto fail;
+}
+
+req-resp.lq-response = VIRTIO_SCSI_S_OK;
+return;
+fail:
+req-resp.lq-response = VIRTIO_SCSI_S_FAILURE;
+}
+
static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
{
SCSIDevice *d = virtio_scsi_device_find(s, req-req.tmf-lun);
@@ -414,6 +472,12 @@ static void virtio_scsi_handle_ctrl(VirtIODevice *vdev,
VirtQueue *vq)
}
req-resp.an-event_actual = 0;
req-resp.an-response = VIRTIO_SCSI_S_OK;
+} else if (req-req.lq-type == VIRTIO_SCSI_T_LUN_QUERY) {
+if (out_size sizeof(VirtIOSCSICtrlLQReq) ||
+in_size sizeof(VirtIOSCSICtrlLQResp)) {
+virtio_scsi_bad_req();
+}
+virtio_scsi_do_lun_query(s, req);
}
virtio_scsi_complete_req(req);
}
@@ -557,6 +621,7 @@ static uint32_t virtio_scsi_get_features(VirtIODevice *vdev,
{
requested_features |= (1UL VIRTIO_SCSI_F_HOTPLUG);
requested_features |= (1UL VIRTIO_SCSI_F_CHANGE);
+requested_features |= (1UL VIRTIO_SCSI_F_LUN_QUERY);
return requested_features;
}
--
1.7.7.6
[Qemu-devel] [PATCH v1] virtio-scsi: get and set the queue limits for sg device
Each virtio scsi HBA has global request queue limits. But the passthrough
LUNs (scsi-generic) come from different host HBAs may have different request
queue limits. If the guest sends commands that exceed the host limits, the
commands will be rejected by host HAB.
This patch addresses this issue by getting the per-LUN queue limits via the the
newly added virtio control request, then setting them properly.
Signed-off-by: Cong Meng m...@linux.vnet.ibm.com
---
drivers/scsi/virtio_scsi.c | 113 +--
include/linux/virtio_scsi.h | 18 +++
2 files changed, 116 insertions(+), 15 deletions(-)
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 173cb39..ec5066f 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -35,12 +35,14 @@ struct virtio_scsi_cmd {
struct virtio_scsi_cmd_req cmd;
struct virtio_scsi_ctrl_tmf_req tmf;
struct virtio_scsi_ctrl_an_req an;
+ struct virtio_scsi_ctrl_lq_req lq;
} req;
union {
struct virtio_scsi_cmd_resp cmd;
struct virtio_scsi_ctrl_tmf_resp tmf;
struct virtio_scsi_ctrl_an_resp an;
struct virtio_scsi_event evt;
+ struct virtio_scsi_ctrl_lq_resp lq;
} resp;
} cacheline_aligned_in_smp;
@@ -469,6 +471,46 @@ out:
return ret;
}
+static u32 virtscsi_lun_query(struct scsi_device *sdev, u32 *value, u32
subtype)
+{
+ struct Scsi_Host *shost = sdev-host;
+ struct virtio_scsi *vscsi = shost_priv(shost);
+ DECLARE_COMPLETION_ONSTACK(comp);
+ struct virtio_scsi_cmd *cmd;
+ struct virtio_scsi_target_state *tgt = vscsi-tgt[sdev-id];
+ unsigned int ret = VIRTIO_SCSI_S_FAILURE;
+
+ cmd = mempool_alloc(virtscsi_cmd_pool, GFP_ATOMIC);
+ if (!cmd)
+ goto out;
+
+ memset(cmd, 0, sizeof(*cmd));
+ cmd-comp = comp;
+ cmd-req.lq = (struct virtio_scsi_ctrl_lq_req){
+ .type = VIRTIO_SCSI_T_LUN_QUERY,
+ .subtype = subtype,
+ .lun[0] = 1,
+ .lun[1] = sdev-id,
+ .lun[2] = (sdev-lun 8) | 0x40,
+ .lun[3] = sdev-lun 0xff,
+ };
+
+ if (virtscsi_kick_cmd(tgt, vscsi-ctrl_vq, cmd, sizeof cmd-req.lq,
+ sizeof cmd-resp.lq, GFP_NOIO) 0) {
+ goto out;
+ }
+
+ wait_for_completion(comp);
+
+ ret = cmd-resp.lq.response;
+ if (ret == VIRTIO_SCSI_S_OK) {
+ *value = cmd-resp.lq.value;
+ }
+out:
+ mempool_free(cmd, virtscsi_cmd_pool);
+ return ret;
+}
+
static int virtscsi_device_reset(struct scsi_cmnd *sc)
{
struct virtio_scsi *vscsi = shost_priv(sc-device-host);
@@ -516,20 +558,6 @@ static int virtscsi_abort(struct scsi_cmnd *sc)
return virtscsi_tmf(vscsi, cmd);
}
-static struct scsi_host_template virtscsi_host_template = {
- .module = THIS_MODULE,
- .name = Virtio SCSI HBA,
- .proc_name = virtio_scsi,
- .queuecommand = virtscsi_queuecommand,
- .this_id = -1,
- .eh_abort_handler = virtscsi_abort,
- .eh_device_reset_handler = virtscsi_device_reset,
-
- .can_queue = 1024,
- .dma_boundary = UINT_MAX,
- .use_clustering = ENABLE_CLUSTERING,
-};
-
#define virtscsi_config_get(vdev, fld) \
({ \
typeof(((struct virtio_scsi_config *)0)-fld) __val; \
@@ -547,6 +575,60 @@ static struct scsi_host_template virtscsi_host_template = {
__val, sizeof(__val)); \
})
+static u32 virtscsi_max_sectors(struct scsi_device *sdev, u32 *value)
+{
+ return virtscsi_lun_query(sdev, value, VIRTIO_SCSI_T_LQ_MAX_SECTORS);
+}
+
+static u32 virtscsi_max_segments(struct scsi_device *sdev, u32 *value)
+{
+ return virtscsi_lun_query(sdev, value, VIRTIO_SCSI_T_LQ_MAX_SEGMENTS);
+}
+
+static u32 virtscsi_max_segment_size(struct scsi_device *sdev, u32 *value)
+{
+ return virtscsi_lun_query(sdev, value,
VIRTIO_SCSI_T_LQ_MAX_SEGMENT_SIZE);
+}
+
+static int virtscsi_slave_alloc(struct scsi_device *sdev)
+{
+ struct Scsi_Host *shost = sdev-host;
+ struct virtio_scsi *vscsi = shost_priv(shost);
+ struct virtio_device *vdev = vscsi-vdev;
+ struct request_queue *q = sdev-request_queue;
+ unsigned int max_sectors, max_segments, max_segment_size;
+
+ if (!virtio_has_feature(vdev, VIRTIO_SCSI_F_LUN_QUERY))
+ goto out;
+
+ if (virtscsi_max_sectors(sdev, max_sectors) ||
+ virtscsi_max_segments(sdev, max_segments) ||
+ virtscsi_max_segment_size(sdev, max_segment_size)) {
+ goto out;
+ }
+
+ blk_queue_max_hw_sectors(q, max_sectors);
+ blk_queue_max_segments(q, max_segments - 2);
+ blk_queue_max_segment_size(q, max_segment_size
Re: [Qemu-devel] [PATCH 1/2 v1] blkdrv: Add queue limits parameters for sg block drive
On Tue 21 Aug 2012 04:48:17 PM CST, Paolo Bonzini wrote:
Il 21/08/2012 10:23, Cong Meng ha scritto:
+static void sg_get_queue_limits(BlockDriverState *bs, const char *filename)
+{
+DIR *ffs;
+struct dirent *d;
+char path[MAXPATHLEN];
+
+snprintf(path, MAXPATHLEN,
+ /sys/class/scsi_generic/sg%s/device/block/,
+ filename + strlen(/dev/sg));
+
+ffs = opendir(path);
+if (!ffs) {
+return;
+}
+
+for (;;) {
+d = readdir(ffs);
+if (!d) {
+return;
+}
+
+if (strcmp(d-d_name, .) == 0 || strcmp(d-d_name, ..) == 0) {
+continue;
+}
+
+break;
+}
+
+closedir(ffs);
+
+pstrcat(path, MAXPATHLEN, d-d_name);
+pstrcat(path, MAXPATHLEN, /queue/);
+
+read_queue_limit(path, max_sectors_kb, bs-max_sectors);
+read_queue_limit(path, max_segments, bs-max_segments);
+read_queue_limit(path, max_segment_size, bs-max_segment_size);
+}
Using /sys/dev/block or /sys/dev/char seems easier, and lets you
retrieve the parameters for block devices too.
what do you mean with block devices? Using /dev/sda instead of
/dev/sg0?
However, I'm worried of the consequences this has for migration. You
could have the same physical disk accessed with two different HBAs, with
different limits. So I don't know if this can really be solved at all.
I know little about qemu migration now. The pending scsi commands will
be saved and
transfered to remote machine when starting migration?
Cong.
Paolo
Re: [Qemu-devel] [PATCH v1] virtio-scsi: get and set the queue limits for sg device
On Tue 21 Aug 2012 04:53:59 PM CST, Stefan Hajnoczi wrote:
On Tue, Aug 21, 2012 at 9:26 AM, Cong Meng m...@linux.vnet.ibm.com wrote:
Each virtio scsi HBA has global request queue limits. But the passthrough
LUNs (scsi-generic) come from different host HBAs may have different request
queue limits. If the guest sends commands that exceed the host limits, the
commands will be rejected by host HAB.
This patch addresses this issue by getting the per-LUN queue limits via the the
newly added virtio control request, then setting them properly.
Signed-off-by: Cong Meng m...@linux.vnet.ibm.com
---
drivers/scsi/virtio_scsi.c | 113 +--
include/linux/virtio_scsi.h | 18 +++
2 files changed, 116 insertions(+), 15 deletions(-)
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 173cb39..ec5066f 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -35,12 +35,14 @@ struct virtio_scsi_cmd {
struct virtio_scsi_cmd_req cmd;
struct virtio_scsi_ctrl_tmf_req tmf;
struct virtio_scsi_ctrl_an_req an;
+ struct virtio_scsi_ctrl_lq_req lq;
} req;
union {
struct virtio_scsi_cmd_resp cmd;
struct virtio_scsi_ctrl_tmf_resp tmf;
struct virtio_scsi_ctrl_an_resp an;
struct virtio_scsi_event evt;
+ struct virtio_scsi_ctrl_lq_resp lq;
} resp;
} cacheline_aligned_in_smp;
@@ -469,6 +471,46 @@ out:
return ret;
}
+static u32 virtscsi_lun_query(struct scsi_device *sdev, u32 *value, u32
subtype)
+{
+ struct Scsi_Host *shost = sdev-host;
+ struct virtio_scsi *vscsi = shost_priv(shost);
+ DECLARE_COMPLETION_ONSTACK(comp);
+ struct virtio_scsi_cmd *cmd;
+ struct virtio_scsi_target_state *tgt = vscsi-tgt[sdev-id];
+ unsigned int ret = VIRTIO_SCSI_S_FAILURE;
+
+ cmd = mempool_alloc(virtscsi_cmd_pool, GFP_ATOMIC);
+ if (!cmd)
+ goto out;
+
+ memset(cmd, 0, sizeof(*cmd));
+ cmd-comp = comp;
+ cmd-req.lq = (struct virtio_scsi_ctrl_lq_req){
+ .type = VIRTIO_SCSI_T_LUN_QUERY,
+ .subtype = subtype,
+ .lun[0] = 1,
+ .lun[1] = sdev-id,
+ .lun[2] = (sdev-lun 8) | 0x40,
+ .lun[3] = sdev-lun 0xff,
The LUN addressing code has been duplicated several times now. How
about replacing it with something like
sure. I will include it.
Thnaks.
Cong.
static void virtio_scsi_set_lun(u8 *lun, struct scsi_device *sdev)
{
lun[0] = 1;
lun[1] = sdev-id;
lun[2] = (sdev-lun 8) | 0x40;
lun[3] = sdev-lun 0xff;
lun[4] = lun[5] = lun[6] = lun[7] = 0;
}
[Qemu-devel] [PATCH] virtio-scsi spec: add per-LUN parameter query
Each virtio scsi HBA has global request queue limits. But the passthrough
LUNs (scsi-generic) come from different host HBAs may have different request
queue limits. If the guest sends commands that exceed the host limits, the
commands will be rejected by host HAB.
To address this issue, this patch adds a per-LUN parameter query via the control
queue, and defines some parameter query types. The driver can query and set the
needed per-LUN parameters if the device enables this feature.
Signed-off-by: Cong Meng m...@linux.vnet.ibm.com
--
virtio-spec.lyx | 218 +++
1 files changed, 218 insertions(+), 0 deletions(-)
diff --git a/virtio-spec.lyx b/virtio-spec.lyx
index 7a073f4..6d23ab9 100644
--- a/virtio-spec.lyx
+++ b/virtio-spec.lyx
@@ -58,6 +58,7 @@
\html_be_strict false
\author -608949062 Rusty Russell,,,
\author 1531152142 Paolo Bonzini,,,
+\author 2090695081 Cong Meng,,,
\end_header
\begin_body
@@ -7028,6 +7029,19 @@ VIRTIO_SCSI_F_CHANGE
(2) The host will report changes to LUN parameters via a
VIRTIO_SCSI_T_PARAM_CHA
NGE event.
+\change_inserted 2090695081 1345439613
+
+\end_layout
+
+\begin_layout Description
+
+\change_inserted 2090695081 1345442620
+VIRTIO_SCSI_F_PER_LUN_QUERY
+\begin_inset space ~
+\end_inset
+
+(3) The host will support the per-LUN parameter query.
+
\change_unchanged
\end_layout
@@ -8178,6 +8192,210 @@ response
\begin_layout Standard
No command-specific values are defined for the response byte.
+\change_inserted 2090695081 1345439679
+
+\end_layout
+
+\end_deeper
+\begin_layout Description
+
+\change_inserted 2090695081 1345441715
+Per
+\begin_inset space \space{}
+\end_inset
+
+LUN
+\begin_inset space \space{}
+\end_inset
+
+query
+\begin_inset space ~
+\end_inset
+
+
+\end_layout
+
+\begin_deeper
+\begin_layout Standard
+
+\change_inserted 2090695081 1345440230
+\begin_inset listings
+inline false
+status open
+
+\begin_layout Plain Layout
+
+\change_inserted 2090695081 1345441722
+
+#define VIRTIO_SCSI_T_PER_LUN_QUERY 3
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 2090695081 1345440248
+
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 2090695081 1345441801
+
+/* query type */
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 2090695081 1345441725
+
+#define VIRTIO_SCSI_T_PLQ_MAX_SECTORS 0
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 2090695081 1345441726
+
+#define VIRTIO_SCSI_T_PLQ_MAX_SEGMENTS 1
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 2090695081 1345441727
+
+#define VIRTIO_SCSI_T_PLQ_MAX_SEGMENT_SIZE 2
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 2090695081 1345440248
+
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 2090695081 1345441729
+
+struct virtio_scsi_ctrl_plq {
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 2090695081 1345440248
+
+// Read-only part
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 2090695081 1345440248
+
+u32 type;
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 2090695081 1345441767
+
+u8 lun[8];
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 2090695081 1345441807
+
+u32 query_type;
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 2090695081 1345440248
+
+// Write-only part
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 2090695081 1345440248
+
+u8 response;
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 2090695081 1345445844
+
+u32 parameter;
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 2090695081 1345440248
+
+}
+\end_layout
+
+\end_inset
+
+
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted 2090695081 1345443439
+The type field should be VIRTIO_SCSI_T_PER_LUN_QUERY.
+
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted 2090695081 1345445951
+The query_type field is defined as below:
+\end_layout
+
+\begin_layout Itemize
+
+\change_inserted 2090695081 1345445082
+VIRTIO_SCSI_T_PLQ_MAX_SECTORS: Maximum request size of the LUN's request
+ queue.
+ The unit is KB (1024 bytes).
+\end_layout
+
+\begin_layout Itemize
+
+\change_inserted 2090695081 1345446052
+VIRTIO_SCSI_T_PLQ_MAX_SEGMENTS: Maximum sg list segments of the LUN's request
+ queue.
+\end_layout
+
+\begin_layout Itemize
+
+\change_inserted 2090695081 1345445058
+VIRTIO_SCSI_T_PLQ_MAX_SEGMENT_SIZE: Maximum segment size of the LUN's request
+ queue.
+ The unit is byte.
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted 2090695081 1345445877
+By sending this command, the driver asks the specified LUN to report the
+ parameter specified by the query_type field.
+ The device responds by writing the parameter that it supports into the
+ parameter field, and VIRTIO_SCSI_S_OK into the response field.
+ Otherwise the device should write the corresponding failure response value
[Qemu-devel] [PATCH] add support for ATA_PASSTHROUGH_xx scsi command
Correct the command names of opcode 0x85 and 0xa1, and calculate
their xfer size from CDB.
ChangeLog:
v2: For opcode 0xa1 on TYPE_ROM device, do not calc the xfer size
v3: Complete xfer calculation of all situations
Signed-off-by: Cong Meng m...@linux.vnet.ibm.com
---
hw/scsi-bus.c | 90 ++--
hw/scsi-defs.h |4 +-
2 files changed, 89 insertions(+), 5 deletions(-)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index e4ec19e..553dc8b 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -733,6 +733,74 @@ static int scsi_get_performance_length(int num_desc, int
type, int data_type)
}
}
+static size_t ata_passthrough_xfer_unit(SCSIDevice *dev, uint8_t *buf)
+{
+int byte_block = (buf[2] 2) 0x1;
+int type = (buf[2] 4) 0x1;
+size_t xfer_unit;
+
+if (byte_block) {
+if (type) {
+xfer_unit = dev-blocksize;
+} else {
+xfer_unit = 512;
+}
+} else {
+xfer_unit = 1;
+}
+
+return xfer_unit;
+}
+
+static size_t ata_passthrough_12_xfer_size(SCSIDevice *dev, uint8_t *buf)
+{
+int length = buf[2] 0x3;
+size_t xfer;
+size_t unit = ata_passthrough_xfer_unit(dev, buf);
+
+switch (length) {
+case 0:
+xfer = 0;
+break;
+case 1:
+xfer = buf[3] * unit;
+break;
+case 2:
+xfer = buf[4] * unit;
+break;
+case 3:
+xfer = UINT32_MAX;
+break;
+}
+
+return xfer;
+}
+
+static size_t ata_passthrough_16_xfer_size(SCSIDevice *dev, uint8_t *buf)
+{
+int extend = buf[1] 0x1;
+int length = buf[2] 0x3;
+size_t xfer;
+size_t unit = ata_passthrough_xfer_unit(dev, buf);
+
+switch (length) {
+case 0:
+xfer = 0;
+break;
+case 1:
+xfer = ((buf[3] * extend) 8 | buf[4]) * unit;
+break;
+case 2:
+xfer = ((buf[5] * extend) 8 | buf[6]) * unit;
+break;
+case 3:
+xfer = UINT32_MAX;
+break;
+}
+
+return xfer;
+}
+
static int scsi_req_length(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf)
{
switch (buf[0] 5) {
@@ -867,6 +935,17 @@ static int scsi_req_length(SCSICommand *cmd, SCSIDevice
*dev, uint8_t *buf)
cmd-xfer = buf[9] | (buf[8] 8);
}
break;
+case ATA_PASSTHROUGH_12:
+if (dev-type == TYPE_ROM) {
+/* BLANK command of MMC */
+cmd-xfer = 0;
+} else {
+cmd-xfer = ata_passthrough_12_xfer_size(dev, buf);
+}
+break;
+case ATA_PASSTHROUGH_16:
+cmd-xfer = ata_passthrough_16_xfer_size(dev, buf);
+break;
}
return 0;
}
@@ -996,9 +1075,14 @@ static void scsi_cmd_xfer_mode(SCSICommand *cmd)
case SEND_DVD_STRUCTURE:
case PERSISTENT_RESERVE_OUT:
case MAINTENANCE_OUT:
-case ATA_PASSTHROUGH:
cmd-mode = SCSI_XFER_TO_DEV;
break;
+case ATA_PASSTHROUGH_12:
+case ATA_PASSTHROUGH_16:
+/* T_DIR */
+cmd-mode = (cmd-buf[2] 0x8) ?
+ SCSI_XFER_FROM_DEV : SCSI_XFER_TO_DEV;
+break;
default:
cmd-mode = SCSI_XFER_FROM_DEV;
break;
@@ -1335,7 +1419,7 @@ static const char *scsi_command_name(uint8_t cmd)
[ PERSISTENT_RESERVE_OUT ] = PERSISTENT_RESERVE_OUT,
[ WRITE_FILEMARKS_16 ] = WRITE_FILEMARKS_16,
[ EXTENDED_COPY] = EXTENDED_COPY,
-[ ATA_PASSTHROUGH ] = ATA_PASSTHROUGH,
+[ ATA_PASSTHROUGH_16 ] = ATA_PASSTHROUGH_16,
[ ACCESS_CONTROL_IN] = ACCESS_CONTROL_IN,
[ ACCESS_CONTROL_OUT ] = ACCESS_CONTROL_OUT,
[ READ_16 ] = READ_16,
@@ -1352,7 +1436,7 @@ static const char *scsi_command_name(uint8_t cmd)
[ SERVICE_ACTION_IN_16 ] = SERVICE_ACTION_IN_16,
[ WRITE_LONG_16] = WRITE_LONG_16,
[ REPORT_LUNS ] = REPORT_LUNS,
-[ BLANK] = BLANK,
+[ ATA_PASSTHROUGH_12 ] = ATA_PASSTHROUGH_12,
[ MOVE_MEDIUM ] = MOVE_MEDIUM,
[ EXCHANGE_MEDIUM ] = EXCHANGE MEDIUM,
[ LOAD_UNLOAD ] = LOAD_UNLOAD,
diff --git a/hw/scsi-defs.h b/hw/scsi-defs.h
index 8a73f74..d7a4019 100644
--- a/hw/scsi-defs.h
+++ b/hw/scsi-defs.h
@@ -100,7 +100,7 @@
#define READ_REVERSE_16 0x81
#define ALLOW_OVERWRITE 0x82
#define EXTENDED_COPY 0x83
-#define ATA_PASSTHROUGH 0x85
+#define ATA_PASSTHROUGH_160x85
#define ACCESS_CONTROL_IN 0x86
#define ACCESS_CONTROL_OUT0x87
#define READ_16 0x88
@@ -117,7 +117,7 @@
#define SERVICE_ACTION_IN_16 0x9e
#define WRITE_LONG_16 0x9f
#define REPORT_LUNS 0xa0
-#define BLANK 0xa1
+#define ATA_PASSTHROUGH_120xa1
#define MAINTENANCE_IN0xa3
#define MAINTENANCE_OUT 0xa4
#define MOVE_MEDIUM
[Qemu-devel] [PATCH] add suport for ATA_PASSTHROUGH_xx scsi command
Correct the command names of opcode 0x85 and 0xa1, and calculate
their xfer size from CDB.
Signed-off-by: Cong Meng m...@linux.vnet.ibm.com
---
hw/scsi-bus.c | 17 ++---
hw/scsi-defs.h |4 ++--
2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index e4ec19e..cf9f4fe 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -867,6 +867,16 @@ static int scsi_req_length(SCSICommand *cmd, SCSIDevice
*dev, uint8_t *buf)
cmd-xfer = buf[9] | (buf[8] 8);
}
break;
+case ATA_PASSTHROUGH_12:
+if ((buf[2] 0x3) == 2) {
+cmd-xfer = buf[4] * dev-blocksize;
+}
+break;
+case ATA_PASSTHROUGH_16:
+if ((buf[2] 0x3) == 2) {
+cmd-xfer = ((buf[5] 8) | buf[6]) * dev-blocksize;
+}
+break;
}
return 0;
}
@@ -996,7 +1006,8 @@ static void scsi_cmd_xfer_mode(SCSICommand *cmd)
case SEND_DVD_STRUCTURE:
case PERSISTENT_RESERVE_OUT:
case MAINTENANCE_OUT:
-case ATA_PASSTHROUGH:
+case ATA_PASSTHROUGH_12:
+case ATA_PASSTHROUGH_16:
cmd-mode = SCSI_XFER_TO_DEV;
break;
default:
@@ -1335,7 +1346,7 @@ static const char *scsi_command_name(uint8_t cmd)
[ PERSISTENT_RESERVE_OUT ] = PERSISTENT_RESERVE_OUT,
[ WRITE_FILEMARKS_16 ] = WRITE_FILEMARKS_16,
[ EXTENDED_COPY] = EXTENDED_COPY,
-[ ATA_PASSTHROUGH ] = ATA_PASSTHROUGH,
+[ ATA_PASSTHROUGH_16 ] = ATA_PASSTHROUGH_16,
[ ACCESS_CONTROL_IN] = ACCESS_CONTROL_IN,
[ ACCESS_CONTROL_OUT ] = ACCESS_CONTROL_OUT,
[ READ_16 ] = READ_16,
@@ -1352,7 +1363,7 @@ static const char *scsi_command_name(uint8_t cmd)
[ SERVICE_ACTION_IN_16 ] = SERVICE_ACTION_IN_16,
[ WRITE_LONG_16] = WRITE_LONG_16,
[ REPORT_LUNS ] = REPORT_LUNS,
-[ BLANK] = BLANK,
+[ ATA_PASSTHROUGH_12 ] = ATA_PASSTHROUGH_12,
[ MOVE_MEDIUM ] = MOVE_MEDIUM,
[ EXCHANGE_MEDIUM ] = EXCHANGE MEDIUM,
[ LOAD_UNLOAD ] = LOAD_UNLOAD,
diff --git a/hw/scsi-defs.h b/hw/scsi-defs.h
index 8a73f74..d7a4019 100644
--- a/hw/scsi-defs.h
+++ b/hw/scsi-defs.h
@@ -100,7 +100,7 @@
#define READ_REVERSE_16 0x81
#define ALLOW_OVERWRITE 0x82
#define EXTENDED_COPY 0x83
-#define ATA_PASSTHROUGH 0x85
+#define ATA_PASSTHROUGH_160x85
#define ACCESS_CONTROL_IN 0x86
#define ACCESS_CONTROL_OUT0x87
#define READ_16 0x88
@@ -117,7 +117,7 @@
#define SERVICE_ACTION_IN_16 0x9e
#define WRITE_LONG_16 0x9f
#define REPORT_LUNS 0xa0
-#define BLANK 0xa1
+#define ATA_PASSTHROUGH_120xa1
#define MAINTENANCE_IN0xa3
#define MAINTENANCE_OUT 0xa4
#define MOVE_MEDIUM 0xa5
--
1.7.7.6
Re: [Qemu-devel] [PATCH] add suport for ATA_PASSTHROUGH_xx scsi command
On Wed 01 Aug 2012 04:20:28 PM CST, Paolo Bonzini wrote:
Il 01/08/2012 09:53, Cong Meng ha scritto:
Correct the command names of opcode 0x85 and 0xa1, and calculate
their xfer size from CDB.
Signed-off-by: Cong Meng m...@linux.vnet.ibm.com
---
hw/scsi-bus.c | 17 ++---
hw/scsi-defs.h |4 ++--
2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index e4ec19e..cf9f4fe 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -867,6 +867,16 @@ static int scsi_req_length(SCSICommand *cmd, SCSIDevice
*dev, uint8_t *buf)
cmd-xfer = buf[9] | (buf[8] 8);
}
break;
+case ATA_PASSTHROUGH_12:
Please make this conditional on the type _not_ being TYPE_ROM.
May I ask why cdrom device should be an exception?
Thanks.
Cong
Paolo
+if ((buf[2] 0x3) == 2) {
+cmd-xfer = buf[4] * dev-blocksize;
+}
+break;
+case ATA_PASSTHROUGH_16:
+if ((buf[2] 0x3) == 2) {
+cmd-xfer = ((buf[5] 8) | buf[6]) * dev-blocksize;
+}
+break;
}
return 0;
}
@@ -996,7 +1006,8 @@ static void scsi_cmd_xfer_mode(SCSICommand *cmd)
case SEND_DVD_STRUCTURE:
case PERSISTENT_RESERVE_OUT:
case MAINTENANCE_OUT:
-case ATA_PASSTHROUGH:
+case ATA_PASSTHROUGH_12:
+case ATA_PASSTHROUGH_16:
cmd-mode = SCSI_XFER_TO_DEV;
break;
default:
@@ -1335,7 +1346,7 @@ static const char *scsi_command_name(uint8_t cmd)
[ PERSISTENT_RESERVE_OUT ] = PERSISTENT_RESERVE_OUT,
[ WRITE_FILEMARKS_16 ] = WRITE_FILEMARKS_16,
[ EXTENDED_COPY] = EXTENDED_COPY,
-[ ATA_PASSTHROUGH ] = ATA_PASSTHROUGH,
+[ ATA_PASSTHROUGH_16 ] = ATA_PASSTHROUGH_16,
[ ACCESS_CONTROL_IN] = ACCESS_CONTROL_IN,
[ ACCESS_CONTROL_OUT ] = ACCESS_CONTROL_OUT,
[ READ_16 ] = READ_16,
@@ -1352,7 +1363,7 @@ static const char *scsi_command_name(uint8_t cmd)
[ SERVICE_ACTION_IN_16 ] = SERVICE_ACTION_IN_16,
[ WRITE_LONG_16] = WRITE_LONG_16,
[ REPORT_LUNS ] = REPORT_LUNS,
-[ BLANK] = BLANK,
+[ ATA_PASSTHROUGH_12 ] = ATA_PASSTHROUGH_12,
[ MOVE_MEDIUM ] = MOVE_MEDIUM,
[ EXCHANGE_MEDIUM ] = EXCHANGE MEDIUM,
[ LOAD_UNLOAD ] = LOAD_UNLOAD,
diff --git a/hw/scsi-defs.h b/hw/scsi-defs.h
index 8a73f74..d7a4019 100644
--- a/hw/scsi-defs.h
+++ b/hw/scsi-defs.h
@@ -100,7 +100,7 @@
#define READ_REVERSE_16 0x81
#define ALLOW_OVERWRITE 0x82
#define EXTENDED_COPY 0x83
-#define ATA_PASSTHROUGH 0x85
+#define ATA_PASSTHROUGH_160x85
#define ACCESS_CONTROL_IN 0x86
#define ACCESS_CONTROL_OUT0x87
#define READ_16 0x88
@@ -117,7 +117,7 @@
#define SERVICE_ACTION_IN_16 0x9e
#define WRITE_LONG_16 0x9f
#define REPORT_LUNS 0xa0
-#define BLANK 0xa1
+#define ATA_PASSTHROUGH_120xa1
#define MAINTENANCE_IN0xa3
#define MAINTENANCE_OUT 0xa4
#define MOVE_MEDIUM 0xa5
Re: [Qemu-devel] [PATCH] add suport for ATA_PASSTHROUGH_xx scsi command
I see. thank both. On Wed 01 Aug 2012 04:49:18 PM CST, ronnie sahlberg wrote: Because opcode 0xA1 has a different meaning for CDROM/MMC devices. On such devices 0xA1 means BLANK, so it would collide if QEMU some day adds support for re-writeable mmc device. On Wed, Aug 1, 2012 at 6:45 PM, Paolo Bonzini pbonz...@redhat.com wrote: Il 01/08/2012 10:43, Cong Meng ha scritto: Please make this conditional on the type _not_ being TYPE_ROM. May I ask why cdrom device should be an exception? Because CD-ROM devices do not support ATA PASSTHROUGH (the SAT standard only describes direct access devices---type=0x00). The 0xa1 command is BLANK for CD-ROMs. Paolo
[Qemu-devel] [PATCH v2] add suport for ATA_PASSTHROUGH_xx scsi command
Correct the command names of opcode 0x85 and 0xa1, and calculate
their xfer size from CDB.
ChangeLog:
v2: For opcode 0xa1 on TYPE_ROM device, do not calc the xfer size
Signed-off-by: Cong Meng m...@linux.vnet.ibm.com
---
hw/scsi-bus.c | 19 ---
hw/scsi-defs.h |4 ++--
2 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index e4ec19e..8af1e86 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -867,6 +867,18 @@ static int scsi_req_length(SCSICommand *cmd, SCSIDevice
*dev, uint8_t *buf)
cmd-xfer = buf[9] | (buf[8] 8);
}
break;
+case ATA_PASSTHROUGH_12:
+if (dev-type != TYPE_ROM) {
+if ((buf[2] 0x3) == 2) {
+cmd-xfer = buf[4] * dev-blocksize;
+}
+}
+break;
+case ATA_PASSTHROUGH_16:
+if ((buf[2] 0x3) == 2) {
+cmd-xfer = ((buf[5] 8) | buf[6]) * dev-blocksize;
+}
+break;
}
return 0;
}
@@ -996,7 +1008,8 @@ static void scsi_cmd_xfer_mode(SCSICommand *cmd)
case SEND_DVD_STRUCTURE:
case PERSISTENT_RESERVE_OUT:
case MAINTENANCE_OUT:
-case ATA_PASSTHROUGH:
+case ATA_PASSTHROUGH_12:
+case ATA_PASSTHROUGH_16:
cmd-mode = SCSI_XFER_TO_DEV;
break;
default:
@@ -1335,7 +1348,7 @@ static const char *scsi_command_name(uint8_t cmd)
[ PERSISTENT_RESERVE_OUT ] = PERSISTENT_RESERVE_OUT,
[ WRITE_FILEMARKS_16 ] = WRITE_FILEMARKS_16,
[ EXTENDED_COPY] = EXTENDED_COPY,
-[ ATA_PASSTHROUGH ] = ATA_PASSTHROUGH,
+[ ATA_PASSTHROUGH_16 ] = ATA_PASSTHROUGH_16,
[ ACCESS_CONTROL_IN] = ACCESS_CONTROL_IN,
[ ACCESS_CONTROL_OUT ] = ACCESS_CONTROL_OUT,
[ READ_16 ] = READ_16,
@@ -1352,7 +1365,7 @@ static const char *scsi_command_name(uint8_t cmd)
[ SERVICE_ACTION_IN_16 ] = SERVICE_ACTION_IN_16,
[ WRITE_LONG_16] = WRITE_LONG_16,
[ REPORT_LUNS ] = REPORT_LUNS,
-[ BLANK] = BLANK,
+[ ATA_PASSTHROUGH_12 ] = ATA_PASSTHROUGH_12,
[ MOVE_MEDIUM ] = MOVE_MEDIUM,
[ EXCHANGE_MEDIUM ] = EXCHANGE MEDIUM,
[ LOAD_UNLOAD ] = LOAD_UNLOAD,
diff --git a/hw/scsi-defs.h b/hw/scsi-defs.h
index 8a73f74..d7a4019 100644
--- a/hw/scsi-defs.h
+++ b/hw/scsi-defs.h
@@ -100,7 +100,7 @@
#define READ_REVERSE_16 0x81
#define ALLOW_OVERWRITE 0x82
#define EXTENDED_COPY 0x83
-#define ATA_PASSTHROUGH 0x85
+#define ATA_PASSTHROUGH_160x85
#define ACCESS_CONTROL_IN 0x86
#define ACCESS_CONTROL_OUT0x87
#define READ_16 0x88
@@ -117,7 +117,7 @@
#define SERVICE_ACTION_IN_16 0x9e
#define WRITE_LONG_16 0x9f
#define REPORT_LUNS 0xa0
-#define BLANK 0xa1
+#define ATA_PASSTHROUGH_120xa1
#define MAINTENANCE_IN0xa3
#define MAINTENANCE_OUT 0xa4
#define MOVE_MEDIUM 0xa5
--
1.7.7.6
Re: [Qemu-devel] [PATCH v2] add suport for ATA_PASSTHROUGH_xx scsi command
On Wed 01 Aug 2012 05:42:08 PM CST, Paolo Bonzini wrote:
Il 01/08/2012 11:05, Cong Meng ha scritto:
+case ATA_PASSTHROUGH_12:
+if (dev-type != TYPE_ROM) {
+if ((buf[2] 0x3) == 2) {
+cmd-xfer = buf[4] * dev-blocksize;
+}
+}
+break;
+case ATA_PASSTHROUGH_16:
+if ((buf[2] 0x3) == 2) {
+cmd-xfer = ((buf[5] 8) | buf[6]) * dev-blocksize;
+}
+break;
Hmm, I think you're only handling this partially.
Ah, I will complete it.
Cong.
Four bits of buf[2] count; bits 0..1 are T_LENGTH, bit 2 is BYTE_BLOCK,
bit 4 is T_TYPE:
If buf[2] is xx00, cmd-xfer = 0
else
if buf[2] is x0xx, xfer_unit = 1
else if buf[2] is xxx0x1xx, xfer_unit = 512
else xfer_unit = dev-blocksize (this is when buf[2] is xxx1x1xx)
if buf[2] is xx01, set cmd-xfer to the FEATURES field
if buf[2] is xx10, set cmd-xfer to the SECTOR_COUNT
for ATA_PASSTHROUGH_16, if buf[1] bit 0 is 0, then cmd-xfer = 255;
cmd-xfer *= xfer_unit;
Also we cannot support buf[2] is xx11. Please add a check to
hw/scsi-generic.c, so that the request is failed in this case.
This is better encapsulated in a separate function, of course.
On top of this, the direction is not necessarily TO_DEV (as in the
current code for scsi_cmd_xfer_mode). It is TO_DEV if buf[2] bit 3
(T_DIR) is zero; it is FROM_DEV if buf[2] bit 3 is one.
Do you have a copy of the SAT (SCSI/ATA translation) standard? This is
all in paragraph 12.2.2.2 in my copy.
Paolo
[Qemu-devel] [PATCH 1/2 v2] scsi bus: introduce hotplug() and hot_unplug() interfaces for SCSI bus
Add two interfaces hotplug() and hot_unplug() to scsi bus info.
The embody scsi bus can implement these two interfaces to signal the HBA driver
of guest kernel to add/remove the scsi device in question.
Signed-off-by: Cong Meng m...@linux.vnet.ibm.com
Signed-off-by: Sen Wang senw...@linux.vnet.ibm.com
---
hw/scsi-bus.c | 16 +++-
hw/scsi.h |2 ++
2 files changed, 17 insertions(+), 1 deletions(-)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index dbdb99c..f08900e 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -177,6 +177,10 @@ static int scsi_qdev_init(DeviceState *qdev)
dev);
}
+if (bus-info-hotplug) {
+bus-info-hotplug(bus, dev);
+}
+
err:
return rc;
}
@@ -1539,6 +1543,16 @@ static int get_scsi_requests(QEMUFile *f, void *pv,
size_t size)
return 0;
}
+static int scsi_qdev_unplug(DeviceState *qdev)
+{
+SCSIDevice *dev = SCSI_DEVICE(qdev);
+SCSIBus *bus = scsi_bus_from_device(dev);
+
+if (bus-info-hot_unplug)
+bus-info-hot_unplug(bus, dev);
+return qdev_simple_unplug_cb(qdev);
+}
+
const VMStateInfo vmstate_info_scsi_requests = {
.name = scsi-requests,
.get = get_scsi_requests,
@@ -1575,7 +1589,7 @@ static void scsi_device_class_init(ObjectClass *klass,
void *data)
DeviceClass *k = DEVICE_CLASS(klass);
k-bus_info = scsi_bus_info;
k-init = scsi_qdev_init;
-k-unplug = qdev_simple_unplug_cb;
+k-unplug = scsi_qdev_unplug;
k-exit = scsi_qdev_exit;
}
diff --git a/hw/scsi.h b/hw/scsi.h
index 2eb66f7..5768071 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -130,6 +130,8 @@ struct SCSIBusInfo {
void (*transfer_data)(SCSIRequest *req, uint32_t arg);
void (*complete)(SCSIRequest *req, uint32_t arg, size_t resid);
void (*cancel)(SCSIRequest *req);
+void (*hotplug)(SCSIBus *bus, SCSIDevice *dev);
+void (*hot_unplug)(SCSIBus *bus, SCSIDevice *dev);
QEMUSGList *(*get_sg_list)(SCSIRequest *req);
void (*save_request)(QEMUFile *f, SCSIRequest *req);
--
1.7.7.6
[Qemu-devel] [PATCH 0/2] Hotplug support for virtio-scsi
These patches implement the hotplug support for virtio-scsi. When a new device attaches/detaches to virtio-scsi bus via device_add/device_del commands, the HBA driver in guest kernel will be signaled to add/remove the scsi device. Cong Meng (2): scsi bus: introduce hotplug() and hot_unplug() interfaces for SCSI bus virtio-scsi: Implement hotplug support for virtio-scsi hw/scsi-bus.c| 16 +++- hw/scsi.h|2 + hw/virtio-scsi.c | 72 +++-- 3 files changed, 86 insertions(+), 4 deletions(-) -- 1.7.7
[Qemu-devel] [PATCH 2/2] virtio-scsi: Implement hotplug support for virtio-scsi
Implement the hotplug() and hot_unplug() interfaces in virtio-scsi, by signal
the virtio_scsi.ko in guest kernel via event virtual queue.
The counterpart patch of virtio_scsi.ko will be sent soon in another thread.
Signed-off-by: Cong Meng m...@linux.vnet.ibm.com
Signed-off-by: Sen Wang senw...@linux.vnet.ibm.com
---
hw/virtio-scsi.c | 72 +++--
1 files changed, 69 insertions(+), 3 deletions(-)
diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c
index e8328f4..626ec5f 100644
--- a/hw/virtio-scsi.c
+++ b/hw/virtio-scsi.c
@@ -24,6 +24,10 @@
#define VIRTIO_SCSI_MAX_TARGET 255
#define VIRTIO_SCSI_MAX_LUN 16383
+/* Feature Bits */
+#define VIRTIO_SCSI_F_INOUT0
+#define VIRTIO_SCSI_F_HOTPLUG 1
+
/* Response codes */
#define VIRTIO_SCSI_S_OK 0
#define VIRTIO_SCSI_S_OVERRUN 1
@@ -60,6 +64,11 @@
#define VIRTIO_SCSI_T_TRANSPORT_RESET 1
#define VIRTIO_SCSI_T_ASYNC_NOTIFY 2
+/* Reasons of transport reset event */
+#define VIRTIO_SCSI_EVT_RESET_HARD 0
+#define VIRTIO_SCSI_EVT_RESET_RESCAN 1
+#define VIRTIO_SCSI_EVT_RESET_REMOVED 2
+
/* SCSI command request, followed by data-out */
typedef struct {
uint8_t lun[8]; /* Logical Unit Number */
@@ -206,11 +215,12 @@ static void qemu_sgl_init_external(QEMUSGList *qsgl,
struct iovec *sg,
static void virtio_scsi_parse_req(VirtIOSCSI *s, VirtQueue *vq,
VirtIOSCSIReq *req)
{
-assert(req-elem.out_num req-elem.in_num);
+assert(req-elem.in_num);
req-vq = vq;
req-dev = s;
req-sreq = NULL;
-req-req.buf = req-elem.out_sg[0].iov_base;
+if (req-elem.out_num)
+req-req.buf = req-elem.out_sg[0].iov_base;
req-resp.buf = req-elem.in_sg[0].iov_base;
if (req-elem.out_num 1) {
@@ -405,6 +415,10 @@ static void virtio_scsi_handle_ctrl(VirtIODevice *vdev,
VirtQueue *vq)
}
}
+static void virtio_scsi_handle_event(VirtIODevice *vdev, VirtQueue *vq)
+{
+}
+
static void virtio_scsi_command_complete(SCSIRequest *r, uint32_t status,
size_t resid)
{
@@ -541,6 +555,7 @@ static void virtio_scsi_set_config(VirtIODevice *vdev,
static uint32_t virtio_scsi_get_features(VirtIODevice *vdev,
uint32_t requested_features)
{
+requested_features |= (1UL VIRTIO_SCSI_F_HOTPLUG);
return requested_features;
}
@@ -568,6 +583,55 @@ static int virtio_scsi_load(QEMUFile *f, void *opaque, int
version_id)
return 0;
}
+static void virtio_scsi_push_event(VirtIOSCSI *s, uint32_t target, uint32_t
lun,
+ uint32_t event, uint32_t reason)
+{
+VirtIOSCSIReq *req;
+VirtIOSCSIEvent *evt;
+
+if ((req = virtio_scsi_pop_req(s, s-event_vq))) {
+int in_size;
+if (req-elem.out_num || req-elem.in_num != 1) {
+virtio_scsi_bad_req();
+}
+
+in_size = req-elem.in_sg[0].iov_len;
+if (in_size sizeof(VirtIOSCSIEvent)) {
+virtio_scsi_bad_req();
+}
+
+evt = req-resp.event;
+evt-event = event;
+evt-reason = reason;
+evt-lun[0] = 0;
+evt-lun[1] = target;
+evt-lun[2] = (lun 8);
+evt-lun[3] = lun 0xFF;
+virtio_scsi_complete_req(req);
+}
+}
+
+static void virtio_scsi_hotplug(SCSIBus *bus, SCSIDevice *dev)
+{
+VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus);
+
+if (((s-vdev.guest_features VIRTIO_SCSI_F_HOTPLUG) 1)
+(s-vdev.status VIRTIO_CONFIG_S_DRIVER_OK)) {
+virtio_scsi_push_event(s, dev-id, dev-lun,
+VIRTIO_SCSI_T_TRANSPORT_RESET, VIRTIO_SCSI_EVT_RESET_RESCAN);
+}
+}
+
+static void virtio_scsi_hot_unplug(SCSIBus *bus, SCSIDevice *dev)
+{
+VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus);
+
+if ((s-vdev.guest_features VIRTIO_SCSI_F_HOTPLUG) 1) {
+virtio_scsi_push_event(s, dev-id, dev-lun,
+VIRTIO_SCSI_T_TRANSPORT_RESET, VIRTIO_SCSI_EVT_RESET_REMOVED);
+}
+}
+
static struct SCSIBusInfo virtio_scsi_scsi_info = {
.tcq = true,
.max_channel = VIRTIO_SCSI_MAX_CHANNEL,
@@ -576,6 +640,8 @@ static struct SCSIBusInfo virtio_scsi_scsi_info = {
.complete = virtio_scsi_command_complete,
.cancel = virtio_scsi_request_cancelled,
+.hotplug = virtio_scsi_hotplug,
+.hot_unplug = virtio_scsi_hot_unplug,
.get_sg_list = virtio_scsi_get_sg_list,
.save_request = virtio_scsi_save_request,
.load_request = virtio_scsi_load_request,
@@ -604,7 +670,7 @@ VirtIODevice *virtio_scsi_init(DeviceState *dev,
VirtIOSCSIConf *proxyconf)
s-ctrl_vq = virtio_add_queue(s-vdev, VIRTIO_SCSI_VQ_SIZE,
virtio_scsi_handle_ctrl);
s-event_vq = virtio_add_queue(s-vdev, VIRTIO_SCSI_VQ_SIZE
[Qemu-devel] [PATCH 1/2] scsi bus: introduce hotplug() and hot_unplug() interfaces for SCSI bus
Add two interfaces hotplug() and hot_unplug() to scsi bus info.
The embody scsi bus can implement these two interfaces to signal the HBA driver
of guest kernel to add/remove the scsi device in question.
Signed-off-by: Cong Meng m...@linux.vnet.ibm.com
Signed-off-by: Sen Wang senw...@linux.vnet.ibm.com
---
hw/scsi-bus.c | 16 +++-
hw/scsi.h |2 ++
2 files changed, 17 insertions(+), 1 deletions(-)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index dbdb99c..cc3ec75 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -177,6 +177,10 @@ static int scsi_qdev_init(DeviceState *qdev)
dev);
}
+if (bus-info-hotplug) {
+ bus-info-hotplug(bus, dev);
+}
+
err:
return rc;
}
@@ -1539,6 +1543,16 @@ static int get_scsi_requests(QEMUFile *f, void *pv,
size_t size)
return 0;
}
+static int scsi_qdev_unplug(DeviceState *qdev)
+{
+SCSIDevice *dev = SCSI_DEVICE(qdev);
+SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, dev-qdev.parent_bus);
+
+if (bus-info-hot_unplug)
+ bus-info-hot_unplug(bus, dev);
+return qdev_simple_unplug_cb(qdev);
+}
+
const VMStateInfo vmstate_info_scsi_requests = {
.name = scsi-requests,
.get = get_scsi_requests,
@@ -1575,7 +1589,7 @@ static void scsi_device_class_init(ObjectClass *klass,
void *data)
DeviceClass *k = DEVICE_CLASS(klass);
k-bus_info = scsi_bus_info;
k-init = scsi_qdev_init;
-k-unplug = qdev_simple_unplug_cb;
+k-unplug = scsi_qdev_unplug;
k-exit = scsi_qdev_exit;
}
diff --git a/hw/scsi.h b/hw/scsi.h
index 2eb66f7..5768071 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -130,6 +130,8 @@ struct SCSIBusInfo {
void (*transfer_data)(SCSIRequest *req, uint32_t arg);
void (*complete)(SCSIRequest *req, uint32_t arg, size_t resid);
void (*cancel)(SCSIRequest *req);
+void (*hotplug)(SCSIBus *bus, SCSIDevice *dev);
+void (*hot_unplug)(SCSIBus *bus, SCSIDevice *dev);
QEMUSGList *(*get_sg_list)(SCSIRequest *req);
void (*save_request)(QEMUFile *f, SCSIRequest *req);
--
1.7.7
Re: [Qemu-devel] IO performance test on the tcm-vhost scsi
On Thu, 2012-06-14 at 09:30 +0100, Stefan Hajnoczi wrote: On Wed, Jun 13, 2012 at 11:13 AM, mengcong m...@linux.vnet.ibm.com wrote: seq-readseq-write rand-read rand-write 8k 256k 8k 256k 8k 256k 8k 256k bare-metal 67951 6980267064 670751758 292841969 26360 tcm-vhost-iblock61501 6657551775 678721011 225331851 28216 tcm-vhost-pscsi 66479 6819150873 675471008 225231818 28304 virtio-blk 26284 6673723373 657351724 289621805 27774 scsi-disk 36013 6028946222 625271663 129921804 27670 unit: KB/s seq-read/write = sequential read/write rand-read/write = random read/write 8k,256k are blocksize of the IO What strikes me is how virtio-blk performs significantly worse than bare metal and tcm_vhost for seq-read/seq-write 8k. The good tcm_vhost results suggest that the overhead is not the virtio interface itself, since tcm_vhost implements virtio-scsi. To drill down on the tcm_vhost vs userspace performance gap we need virtio-scsi userspace results. QEMU needs to use the same block device as the tcm-vhost-iblock benchmark. Cong: Is it possible to collect the virtio-scsi userspace results using the same block device as tcm-vhost-iblock and -drive format=raw,aio=native,cache=none? virtio-scsi-raw 43065 6972952052 673781757 294192024 28135 qemu \ -drive file=/dev/sdb,format=raw,if=none,id=sdb,cache=none,aio=native \ -device virtio-scsi-pci,id=mcbus \ -device scsi-disk,drive=sdb there is only one scsi HBA. /dev/sdb is the disk on which all tests have been done. Is this what you want? Cong Meng Stefan
Re: [Qemu-devel] IO performance test on the tcm-vhost scsi
On Wed, 2012-06-13 at 12:08 -0700, Nicholas A. Bellinger wrote: On Wed, 2012-06-13 at 18:13 +0800, mengcong wrote: Hi folks, I did an IO performance test on the tcm-vhost scsi. I want to share the test result data here. seq-readseq-write rand-read rand-write 8k 256k 8k 256k 8k 256k 8k 256k bare-metal 67951 6980267064 670751758 292841969 26360 tcm-vhost-iblock61501 6657551775 678721011 225331851 28216 tcm-vhost-pscsi 66479 6819150873 675471008 225231818 28304 virtio-blk 26284 6673723373 657351724 289621805 27774 scsi-disk 36013 6028946222 625271663 129921804 27670 unit: KB/s seq-read/write = sequential read/write rand-read/write = random read/write 8k,256k are blocksize of the IO In tcm-vhost-iblock test, the emulate_write_cache attr was enabled. In virtio-blk test, cache=none,aio=native were set. In scsi-disk test, cache=none,aio=native were set, and LSI HBA was used. I also tried to do the test with a scsi-generic LUN (pass through the physical partition /dev/sgX device). But I couldn't setup it successfully. It's a pity. Benchmark tool: fio, with ioengine=aio,direct=1,iodepth=8 set for all tests. kvm vm: 2 cpus and 2G ram These initial performance results look quite promising for virtio-scsi. I'd be really interested to see how a raw flash block device backend that locally can do ~100K 4k mixed R/W random IOPs compares with virtio-scsi guest performance as the random small block fio workload increases.. flash block == Solid state disk? I have no one on hand. Also note there is a bottleneck wrt to random small block I/O performance (per LUN) on the Linux/SCSI initiator side that is effecting things here. We've run into this limitation numerous times with using SCSI LLDs as backend TCM devices, and I usually recommend using iblock export with raw block flash backends for achieving the best small block random I/O performance results. A number of high performance flash storage folks do something similar with raw block access (Jen's CC'ed) As per Stefan's earlier question, how does virtio-scsi to QEMU SCSI userspace compare with these results..? Is there a reason why these where not included in the initial results..? This should be a mistake I made. I will do this pattern later. Thanks Meng! --nab
