All of lore.kernel.org
 help / color / mirror / Atom feed
* make SCSI passthrough support optional
@ 2017-01-28  8:32 Christoph Hellwig
  2017-01-28  8:32 ` [PATCH 1/5] nvme/scsi: don't rely on BLK_MAX_CDB Christoph Hellwig
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Christoph Hellwig @ 2017-01-28  8:32 UTC (permalink / raw)
  To: Jens Axboe, mst, jasowang; +Cc: linux-block, virtualization

Hi all,

this series builds on my previous changes in Jens' for-4.11/rq-refactor
branch that split out the BLOCK_PC fields from struct request into a new
struct scsi_request, and makes support for struct scsi_request and the
SCSI passthrough ioctls optional.  It is now only enabled by drivers that
need it.

In addition I've made SCSI passthrough support in the virtio_blk driver an
optional compile time feature, as it's not actually needed for most
common setups.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/5] nvme/scsi: don't rely on BLK_MAX_CDB
  2017-01-28  8:32 make SCSI passthrough support optional Christoph Hellwig
@ 2017-01-28  8:32 ` Christoph Hellwig
  2017-01-28  8:32 ` [PATCH 2/5] skd: implement trivial scsi ioctls directly Christoph Hellwig
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2017-01-28  8:32 UTC (permalink / raw)
  To: Jens Axboe, mst, jasowang; +Cc: linux-block, virtualization

The NVMe SCSI emulation doesn't use BLOCK_PC requests, so BLK_MAX_CDB
doesn't have a meaning for it.  Instead opencode the value of 16
and refactor the code a bit so that related checks are next to each
other and we only need to use the value in one place.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/nvme/host/scsi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/host/scsi.c b/drivers/nvme/host/scsi.c
index a5c09e7..0a5687a 100644
--- a/drivers/nvme/host/scsi.c
+++ b/drivers/nvme/host/scsi.c
@@ -2347,12 +2347,14 @@ static int nvme_trans_unmap(struct nvme_ns *ns, struct sg_io_hdr *hdr,
 
 static int nvme_scsi_translate(struct nvme_ns *ns, struct sg_io_hdr *hdr)
 {
-	u8 cmd[BLK_MAX_CDB];
+	u8 cmd[16];
 	int retcode;
 	unsigned int opcode;
 
 	if (hdr->cmdp == NULL)
 		return -EMSGSIZE;
+	if (hdr->cmd_len > sizeof(cmd))
+		return -EINVAL;
 	if (copy_from_user(cmd, hdr->cmdp, hdr->cmd_len))
 		return -EFAULT;
 
@@ -2451,8 +2453,6 @@ int nvme_sg_io(struct nvme_ns *ns, struct sg_io_hdr __user *u_hdr)
 		return -EFAULT;
 	if (hdr.interface_id != 'S')
 		return -EINVAL;
-	if (hdr.cmd_len > BLK_MAX_CDB)
-		return -EINVAL;
 
 	/*
 	 * A positive return code means a NVMe status, which has been
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/5] skd: implement trivial scsi ioctls directly
  2017-01-28  8:32 make SCSI passthrough support optional Christoph Hellwig
  2017-01-28  8:32 ` [PATCH 1/5] nvme/scsi: don't rely on BLK_MAX_CDB Christoph Hellwig
@ 2017-01-28  8:32 ` Christoph Hellwig
  2017-01-28  8:32 ` [PATCH 3/5] block: make scsi_request and scsi ioctl support optional Christoph Hellwig
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2017-01-28  8:32 UTC (permalink / raw)
  To: Jens Axboe, mst, jasowang; +Cc: linux-block, virtualization

This way there is no need to drag in a dependency on the
BLOCK_PC code, which is going to become optional.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/skd_main.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/block/skd_main.c b/drivers/block/skd_main.c
index abf805e..27833e4 100644
--- a/drivers/block/skd_main.c
+++ b/drivers/block/skd_main.c
@@ -1204,10 +1204,11 @@ static void skd_complete_special(struct skd_device *skdev,
 static int skd_bdev_ioctl(struct block_device *bdev, fmode_t mode,
 			  uint cmd_in, ulong arg)
 {
-	int rc = 0;
+	static const int sg_version_num = 30527;
+	int rc = 0, timeout;
 	struct gendisk *disk = bdev->bd_disk;
 	struct skd_device *skdev = disk->private_data;
-	void __user *p = (void *)arg;
+	int __user *p = (int __user *)arg;
 
 	pr_debug("%s:%s:%d %s: CMD[%s] ioctl  mode 0x%x, cmd 0x%x arg %0lx\n",
 		 skdev->name, __func__, __LINE__,
@@ -1218,12 +1219,18 @@ static int skd_bdev_ioctl(struct block_device *bdev, fmode_t mode,
 
 	switch (cmd_in) {
 	case SG_SET_TIMEOUT:
+		rc = get_user(timeout, p);
+		if (!rc)
+			disk->queue->sg_timeout = clock_t_to_jiffies(timeout);
+		break;
 	case SG_GET_TIMEOUT:
+		rc = jiffies_to_clock_t(disk->queue->sg_timeout);
+		break;
 	case SG_GET_VERSION_NUM:
-		rc = scsi_cmd_ioctl(disk->queue, disk, mode, cmd_in, p);
+		rc = put_user(sg_version_num, p);
 		break;
 	case SG_IO:
-		rc = skd_ioctl_sg_io(skdev, mode, p);
+		rc = skd_ioctl_sg_io(skdev, mode, (void __user *)arg);
 		break;
 
 	default:
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/5] block: make scsi_request and scsi ioctl support optional
  2017-01-28  8:32 make SCSI passthrough support optional Christoph Hellwig
  2017-01-28  8:32 ` [PATCH 1/5] nvme/scsi: don't rely on BLK_MAX_CDB Christoph Hellwig
  2017-01-28  8:32 ` [PATCH 2/5] skd: implement trivial scsi ioctls directly Christoph Hellwig
@ 2017-01-28  8:32 ` Christoph Hellwig
  2017-01-28  8:32 ` [PATCH 4/5] virtio_blk: remove struct request backpointer from virtblk_req Christoph Hellwig
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2017-01-28  8:32 UTC (permalink / raw)
  To: Jens Axboe, mst, jasowang; +Cc: linux-block, virtualization

We only need this code to support scsi, ide, cciss and virtio.  And at
least for virtio it's a deprecated feature to start with.

This should shrink the kernel size for embedded device that only use,
say eMMC a bit.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/Kconfig                | 5 +++++
 block/Makefile               | 5 +++--
 drivers/block/Kconfig        | 4 ++++
 drivers/block/paride/Kconfig | 1 +
 drivers/ide/Kconfig          | 1 +
 drivers/scsi/Kconfig         | 1 +
 drivers/target/Kconfig       | 1 +
 fs/nfsd/Kconfig              | 1 +
 8 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/block/Kconfig b/block/Kconfig
index 8bf114a..9791bb8 100644
--- a/block/Kconfig
+++ b/block/Kconfig
@@ -49,9 +49,13 @@ config LBDAF
 
 	  If unsure, say Y.
 
+config BLK_SCSI_REQUEST
+	bool
+
 config BLK_DEV_BSG
 	bool "Block layer SG support v4"
 	default y
+	select BLK_SCSI_REQUEST
 	help
 	  Saying Y here will enable generic SG (SCSI generic) v4 support
 	  for any block device.
@@ -71,6 +75,7 @@ config BLK_DEV_BSGLIB
 	bool "Block layer SG support v4 helper lib"
 	default n
 	select BLK_DEV_BSG
+	select BLK_SCSI_REQUEST
 	help
 	  Subsystems will normally enable this if needed. Users will not
 	  normally need to manually enable this.
diff --git a/block/Makefile b/block/Makefile
index 6cabe6b..6d7de29 100644
--- a/block/Makefile
+++ b/block/Makefile
@@ -7,10 +7,11 @@ obj-$(CONFIG_BLOCK) := bio.o elevator.o blk-core.o blk-tag.o blk-sysfs.o \
 			blk-exec.o blk-merge.o blk-softirq.o blk-timeout.o \
 			blk-lib.o blk-mq.o blk-mq-tag.o blk-stat.o \
 			blk-mq-sysfs.o blk-mq-cpumap.o blk-mq-sched.o ioctl.o \
-			genhd.o scsi_ioctl.o partition-generic.o ioprio.o \
+			genhd.o partition-generic.o ioprio.o \
 			badblocks.o partitions/
 
-obj-$(CONFIG_BOUNCE)	+= bounce.o
+obj-$(CONFIG_BOUNCE)		+= bounce.o
+obj-$(CONFIG_BLK_SCSI_REQUEST)	+= scsi_ioctl.o
 obj-$(CONFIG_BLK_DEV_BSG)	+= bsg.o
 obj-$(CONFIG_BLK_DEV_BSGLIB)	+= bsg-lib.o
 obj-$(CONFIG_BLK_CGROUP)	+= blk-cgroup.o
diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
index 223ff2f..7a4c787 100644
--- a/drivers/block/Kconfig
+++ b/drivers/block/Kconfig
@@ -69,6 +69,7 @@ config AMIGA_Z2RAM
 config GDROM
 	tristate "SEGA Dreamcast GD-ROM drive"
 	depends on SH_DREAMCAST
+	select BLK_SCSI_REQUEST # only for the generic cdrom code
 	help
 	  A standard SEGA Dreamcast comes with a modified CD ROM drive called a
 	  "GD-ROM" by SEGA to signify it is capable of reading special disks
@@ -114,6 +115,7 @@ config BLK_CPQ_CISS_DA
 	tristate "Compaq Smart Array 5xxx support"
 	depends on PCI
 	select CHECK_SIGNATURE
+	select BLK_SCSI_REQUEST
 	help
 	  This is the driver for Compaq Smart Array 5xxx controllers.
 	  Everyone using these boards should say Y here.
@@ -386,6 +388,7 @@ config BLK_DEV_RAM_DAX
 config CDROM_PKTCDVD
 	tristate "Packet writing on CD/DVD media (DEPRECATED)"
 	depends on !UML
+	select BLK_SCSI_REQUEST
 	help
 	  Note: This driver is deprecated and will be removed from the
 	  kernel in the near future!
@@ -497,6 +500,7 @@ config XEN_BLKDEV_BACKEND
 config VIRTIO_BLK
 	tristate "Virtio block driver"
 	depends on VIRTIO
+	select BLK_SCSI_REQUEST
 	---help---
 	  This is the virtual block driver for virtio.  It can be used with
           lguest or QEMU based VMMs (like KVM or Xen).  Say Y or M.
diff --git a/drivers/block/paride/Kconfig b/drivers/block/paride/Kconfig
index efefb5a..3a15247 100644
--- a/drivers/block/paride/Kconfig
+++ b/drivers/block/paride/Kconfig
@@ -25,6 +25,7 @@ config PARIDE_PD
 config PARIDE_PCD
 	tristate "Parallel port ATAPI CD-ROMs"
 	depends on PARIDE
+	select BLK_SCSI_REQUEST # only for the generic cdrom code
 	---help---
 	  This option enables the high-level driver for ATAPI CD-ROM devices
 	  connected through a parallel port. If you chose to build PARIDE
diff --git a/drivers/ide/Kconfig b/drivers/ide/Kconfig
index 39ea67f..c99a25c 100644
--- a/drivers/ide/Kconfig
+++ b/drivers/ide/Kconfig
@@ -10,6 +10,7 @@ menuconfig IDE
 	tristate "ATA/ATAPI/MFM/RLL support (DEPRECATED)"
 	depends on HAVE_IDE
 	depends on BLOCK
+	select BLK_SCSI_REQUEST
 	---help---
 	  If you say Y here, your kernel will be able to manage ATA/(E)IDE and
 	  ATAPI units. The most common cases are IDE hard drives and ATAPI
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index a4f6b0d..d4023bf 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -18,6 +18,7 @@ config SCSI
 	depends on BLOCK
 	select SCSI_DMA if HAS_DMA
 	select SG_POOL
+	select BLK_SCSI_REQUEST
 	---help---
 	  If you want to use a SCSI hard disk, SCSI tape drive, SCSI CD-ROM or
 	  any other SCSI device under Linux, say Y and make sure that you know
diff --git a/drivers/target/Kconfig b/drivers/target/Kconfig
index 2573612..e2bc999 100644
--- a/drivers/target/Kconfig
+++ b/drivers/target/Kconfig
@@ -4,6 +4,7 @@ menuconfig TARGET_CORE
 	depends on SCSI && BLOCK
 	select CONFIGFS_FS
 	select CRC_T10DIF
+	select BLK_SCSI_REQUEST # only for scsi_command_size_tbl..
 	default n
 	help
 	Say Y or M here to enable the TCM Storage Engine and ConfigFS enabled
diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
index 47febcf..20b1c17 100644
--- a/fs/nfsd/Kconfig
+++ b/fs/nfsd/Kconfig
@@ -104,6 +104,7 @@ config NFSD_SCSILAYOUT
 	depends on NFSD_V4 && BLOCK
 	select NFSD_PNFS
 	select EXPORTFS_BLOCK_OPS
+	select BLK_SCSI_REQUEST
 	help
 	  This option enables support for the exporting pNFS SCSI layouts
 	  in the kernel's NFS server. The pNFS SCSI layout enables NFS
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 4/5] virtio_blk: remove struct request backpointer from virtblk_req
  2017-01-28  8:32 make SCSI passthrough support optional Christoph Hellwig
                   ` (2 preceding siblings ...)
  2017-01-28  8:32 ` [PATCH 3/5] block: make scsi_request and scsi ioctl support optional Christoph Hellwig
@ 2017-01-28  8:32 ` Christoph Hellwig
  2017-01-28  8:32 ` [PATCH 5/5] virtio_blk: make SCSI passthrough support configurable Christoph Hellwig
  2017-01-31 17:56 ` make SCSI passthrough support optional Jens Axboe
  5 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2017-01-28  8:32 UTC (permalink / raw)
  To: Jens Axboe, mst, jasowang; +Cc: linux-block, virtualization

We can simply use blk_mq_rq_from_pdu to get back at the request at
I/O completion time.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/virtio_blk.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 3027d2e..7c730e2 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -53,7 +53,6 @@ struct virtio_blk {
 
 struct virtblk_req {
 	struct scsi_request sreq;	/* for SCSI passthrough */
-	struct request *req;
 	struct virtio_blk_outhdr out_hdr;
 	struct virtio_scsi_inhdr in_hdr;
 	u8 status;
@@ -148,7 +147,9 @@ static void virtblk_done(struct virtqueue *vq)
 	do {
 		virtqueue_disable_cb(vq);
 		while ((vbr = virtqueue_get_buf(vblk->vqs[qid].vq, &len)) != NULL) {
-			blk_mq_complete_request(vbr->req, vbr->req->errors);
+			struct request *req = blk_mq_rq_from_pdu(vbr);
+
+			blk_mq_complete_request(req, req->errors);
 			req_done = true;
 		}
 		if (unlikely(virtqueue_is_broken(vq)))
@@ -175,27 +176,26 @@ static int virtio_queue_rq(struct blk_mq_hw_ctx *hctx,
 
 	BUG_ON(req->nr_phys_segments + 2 > vblk->sg_elems);
 
-	vbr->req = req;
 	if (req_op(req) == REQ_OP_FLUSH) {
 		vbr->out_hdr.type = cpu_to_virtio32(vblk->vdev, VIRTIO_BLK_T_FLUSH);
 		vbr->out_hdr.sector = 0;
-		vbr->out_hdr.ioprio = cpu_to_virtio32(vblk->vdev, req_get_ioprio(vbr->req));
+		vbr->out_hdr.ioprio = cpu_to_virtio32(vblk->vdev, req_get_ioprio(req));
 	} else {
 		switch (req->cmd_type) {
 		case REQ_TYPE_FS:
 			vbr->out_hdr.type = 0;
-			vbr->out_hdr.sector = cpu_to_virtio64(vblk->vdev, blk_rq_pos(vbr->req));
-			vbr->out_hdr.ioprio = cpu_to_virtio32(vblk->vdev, req_get_ioprio(vbr->req));
+			vbr->out_hdr.sector = cpu_to_virtio64(vblk->vdev, blk_rq_pos(req));
+			vbr->out_hdr.ioprio = cpu_to_virtio32(vblk->vdev, req_get_ioprio(req));
 			break;
 		case REQ_TYPE_BLOCK_PC:
 			vbr->out_hdr.type = cpu_to_virtio32(vblk->vdev, VIRTIO_BLK_T_SCSI_CMD);
 			vbr->out_hdr.sector = 0;
-			vbr->out_hdr.ioprio = cpu_to_virtio32(vblk->vdev, req_get_ioprio(vbr->req));
+			vbr->out_hdr.ioprio = cpu_to_virtio32(vblk->vdev, req_get_ioprio(req));
 			break;
 		case REQ_TYPE_DRV_PRIV:
 			vbr->out_hdr.type = cpu_to_virtio32(vblk->vdev, VIRTIO_BLK_T_GET_ID);
 			vbr->out_hdr.sector = 0;
-			vbr->out_hdr.ioprio = cpu_to_virtio32(vblk->vdev, req_get_ioprio(vbr->req));
+			vbr->out_hdr.ioprio = cpu_to_virtio32(vblk->vdev, req_get_ioprio(req));
 			break;
 		default:
 			/* We don't put anything else in the queue. */
@@ -205,9 +205,9 @@ static int virtio_queue_rq(struct blk_mq_hw_ctx *hctx,
 
 	blk_mq_start_request(req);
 
-	num = blk_rq_map_sg(hctx->queue, vbr->req, vbr->sg);
+	num = blk_rq_map_sg(hctx->queue, req, vbr->sg);
 	if (num) {
-		if (rq_data_dir(vbr->req) == WRITE)
+		if (rq_data_dir(req) == WRITE)
 			vbr->out_hdr.type |= cpu_to_virtio32(vblk->vdev, VIRTIO_BLK_T_OUT);
 		else
 			vbr->out_hdr.type |= cpu_to_virtio32(vblk->vdev, VIRTIO_BLK_T_IN);
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 5/5] virtio_blk: make SCSI passthrough support configurable
  2017-01-28  8:32 make SCSI passthrough support optional Christoph Hellwig
                   ` (3 preceding siblings ...)
  2017-01-28  8:32 ` [PATCH 4/5] virtio_blk: remove struct request backpointer from virtblk_req Christoph Hellwig
@ 2017-01-28  8:32 ` Christoph Hellwig
  2017-01-31 17:56 ` make SCSI passthrough support optional Jens Axboe
  5 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2017-01-28  8:32 UTC (permalink / raw)
  To: Jens Axboe, mst, jasowang; +Cc: linux-block, virtualization

The SCSI passthrough idea was a a bad idea to start with (guess who came
up with it?), and has been removed from the virtio 1.O spec, and is not
enabled by defauly by any host I know of.  Add a separate config option
for it so that we don't need to enable it for most setups.  That way
any bugs related to it (like the one recently fixed for vmapped stacks)
do not affect other users, and the size of the virtblk_req structure
also shrinks significantly.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/Kconfig      |  11 +++-
 drivers/block/virtio_blk.c | 143 +++++++++++++++++++++++++++++----------------
 2 files changed, 104 insertions(+), 50 deletions(-)

diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
index 7a4c787..f744de7 100644
--- a/drivers/block/Kconfig
+++ b/drivers/block/Kconfig
@@ -500,11 +500,20 @@ config XEN_BLKDEV_BACKEND
 config VIRTIO_BLK
 	tristate "Virtio block driver"
 	depends on VIRTIO
-	select BLK_SCSI_REQUEST
 	---help---
 	  This is the virtual block driver for virtio.  It can be used with
           lguest or QEMU based VMMs (like KVM or Xen).  Say Y or M.
 
+config VIRTIO_BLK_SCSI
+	bool "SCSI passthrough request for the Virtio block driver"
+	depends on VIRTIO_BLK
+	select BLK_SCSI_REQUEST
+	---help---
+	  Enable support for SCSI passthrough (e.g. the SG_IO ioctl) on
+	  virtio-blk devices.  This is only supported for the legacy
+	  virtio protocol and not enabled by default by any hypervisor.
+	  Your probably want to virtio-scsi instead.
+
 config BLK_DEV_HD
 	bool "Very old hard disk (MFM/RLL/IDE) driver"
 	depends on HAVE_IDE
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 7c730e2..d9491bd 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -52,11 +52,13 @@ struct virtio_blk {
 };
 
 struct virtblk_req {
-	struct scsi_request sreq;	/* for SCSI passthrough */
-	struct virtio_blk_outhdr out_hdr;
+#ifdef CONFIG_VIRTIO_BLK_SCSI
+	struct scsi_request sreq;	/* for SCSI passthrough, must be first */
+	u8 sense[SCSI_SENSE_BUFFERSIZE];
 	struct virtio_scsi_inhdr in_hdr;
+#endif
+	struct virtio_blk_outhdr out_hdr;
 	u8 status;
-	u8 sense[SCSI_SENSE_BUFFERSIZE];
 	struct scatterlist sg[];
 };
 
@@ -72,28 +74,88 @@ static inline int virtblk_result(struct virtblk_req *vbr)
 	}
 }
 
-static int __virtblk_add_req(struct virtqueue *vq,
-			     struct virtblk_req *vbr,
-			     struct scatterlist *data_sg,
-			     bool have_data)
+/*
+ * If this is a packet command we need a couple of additional headers.  Behind
+ * the normal outhdr we put a segment with the scsi command block, and before
+ * the normal inhdr we put the sense data and the inhdr with additional status
+ * information.
+ */
+#ifdef CONFIG_VIRTIO_BLK_SCSI
+static int virtblk_add_req_scsi(struct virtqueue *vq, struct virtblk_req *vbr,
+		struct scatterlist *data_sg, bool have_data)
 {
 	struct scatterlist hdr, status, cmd, sense, inhdr, *sgs[6];
 	unsigned int num_out = 0, num_in = 0;
-	__virtio32 type = vbr->out_hdr.type & ~cpu_to_virtio32(vq->vdev, VIRTIO_BLK_T_OUT);
 
 	sg_init_one(&hdr, &vbr->out_hdr, sizeof(vbr->out_hdr));
 	sgs[num_out++] = &hdr;
+	sg_init_one(&cmd, vbr->sreq.cmd, vbr->sreq.cmd_len);
+	sgs[num_out++] = &cmd;
+
+	if (have_data) {
+		if (vbr->out_hdr.type & cpu_to_virtio32(vq->vdev, VIRTIO_BLK_T_OUT))
+			sgs[num_out++] = data_sg;
+		else
+			sgs[num_out + num_in++] = data_sg;
+	}
+
+	sg_init_one(&sense, vbr->sense, SCSI_SENSE_BUFFERSIZE);
+	sgs[num_out + num_in++] = &sense;
+	sg_init_one(&inhdr, &vbr->in_hdr, sizeof(vbr->in_hdr));
+	sgs[num_out + num_in++] = &inhdr;
+	sg_init_one(&status, &vbr->status, sizeof(vbr->status));
+	sgs[num_out + num_in++] = &status;
+
+	return virtqueue_add_sgs(vq, sgs, num_out, num_in, vbr, GFP_ATOMIC);
+}
+
+static inline void virtblk_scsi_reques_done(struct request *req)
+{
+	struct virtblk_req *vbr = blk_mq_rq_to_pdu(req);
+	struct virtio_blk *vblk = req->q->queuedata;
+	struct scsi_request *sreq = &vbr->sreq;
+
+	sreq->resid_len = virtio32_to_cpu(vblk->vdev, vbr->in_hdr.residual);
+	sreq->sense_len = virtio32_to_cpu(vblk->vdev, vbr->in_hdr.sense_len);
+	req->errors = virtio32_to_cpu(vblk->vdev, vbr->in_hdr.errors);
+}
+
+static int virtblk_ioctl(struct block_device *bdev, fmode_t mode,
+			     unsigned int cmd, unsigned long data)
+{
+	struct gendisk *disk = bdev->bd_disk;
+	struct virtio_blk *vblk = disk->private_data;
 
 	/*
-	 * If this is a packet command we need a couple of additional headers.
-	 * Behind the normal outhdr we put a segment with the scsi command
-	 * block, and before the normal inhdr we put the sense data and the
-	 * inhdr with additional status information.
+	 * Only allow the generic SCSI ioctls if the host can support it.
 	 */
-	if (type == cpu_to_virtio32(vq->vdev, VIRTIO_BLK_T_SCSI_CMD)) {
-		sg_init_one(&cmd, vbr->sreq.cmd, vbr->sreq.cmd_len);
-		sgs[num_out++] = &cmd;
-	}
+	if (!virtio_has_feature(vblk->vdev, VIRTIO_BLK_F_SCSI))
+		return -ENOTTY;
+
+	return scsi_cmd_blk_ioctl(bdev, mode, cmd,
+				  (void __user *)data);
+}
+#else
+static inline int virtblk_add_req_scsi(struct virtqueue *vq,
+		struct virtblk_req *vbr, struct scatterlist *data_sg,
+		bool have_data)
+{
+	return -EIO;
+}
+static inline void virtblk_scsi_reques_done(struct request *req)
+{
+}
+#define virtblk_ioctl	NULL
+#endif /* CONFIG_VIRTIO_BLK_SCSI */
+
+static int virtblk_add_req(struct virtqueue *vq, struct virtblk_req *vbr,
+		struct scatterlist *data_sg, bool have_data)
+{
+	struct scatterlist hdr, status, *sgs[3];
+	unsigned int num_out = 0, num_in = 0;
+
+	sg_init_one(&hdr, &vbr->out_hdr, sizeof(vbr->out_hdr));
+	sgs[num_out++] = &hdr;
 
 	if (have_data) {
 		if (vbr->out_hdr.type & cpu_to_virtio32(vq->vdev, VIRTIO_BLK_T_OUT))
@@ -102,13 +164,6 @@ static int __virtblk_add_req(struct virtqueue *vq,
 			sgs[num_out + num_in++] = data_sg;
 	}
 
-	if (type == cpu_to_virtio32(vq->vdev, VIRTIO_BLK_T_SCSI_CMD)) {
-		sg_init_one(&sense, vbr->sense, SCSI_SENSE_BUFFERSIZE);
-		sgs[num_out + num_in++] = &sense;
-		sg_init_one(&inhdr, &vbr->in_hdr, sizeof(vbr->in_hdr));
-		sgs[num_out + num_in++] = &inhdr;
-	}
-
 	sg_init_one(&status, &vbr->status, sizeof(vbr->status));
 	sgs[num_out + num_in++] = &status;
 
@@ -118,17 +173,15 @@ static int __virtblk_add_req(struct virtqueue *vq,
 static inline void virtblk_request_done(struct request *req)
 {
 	struct virtblk_req *vbr = blk_mq_rq_to_pdu(req);
-	struct virtio_blk *vblk = req->q->queuedata;
 	int error = virtblk_result(vbr);
 
-	if (req->cmd_type == REQ_TYPE_BLOCK_PC) {
-		scsi_req(req)->resid_len =
-			virtio32_to_cpu(vblk->vdev, vbr->in_hdr.residual);
-		vbr->sreq.sense_len =
-			virtio32_to_cpu(vblk->vdev, vbr->in_hdr.sense_len);
-		req->errors = virtio32_to_cpu(vblk->vdev, vbr->in_hdr.errors);
-	} else if (req->cmd_type == REQ_TYPE_DRV_PRIV) {
+	switch (req->cmd_type) {
+	case REQ_TYPE_BLOCK_PC:
+		virtblk_scsi_reques_done(req);
+		break;
+	case REQ_TYPE_DRV_PRIV:
 		req->errors = (error != 0);
+		break;
 	}
 
 	blk_mq_end_request(req, error);
@@ -214,7 +267,10 @@ static int virtio_queue_rq(struct blk_mq_hw_ctx *hctx,
 	}
 
 	spin_lock_irqsave(&vblk->vqs[qid].lock, flags);
-	err = __virtblk_add_req(vblk->vqs[qid].vq, vbr, vbr->sg, num);
+	if (req->cmd_type == REQ_TYPE_BLOCK_PC)
+		err = virtblk_add_req_scsi(vblk->vqs[qid].vq, vbr, vbr->sg, num);
+	else
+		err = virtblk_add_req(vblk->vqs[qid].vq, vbr, vbr->sg, num);
 	if (err) {
 		virtqueue_kick(vblk->vqs[qid].vq);
 		blk_mq_stop_hw_queue(hctx);
@@ -259,22 +315,6 @@ static int virtblk_get_id(struct gendisk *disk, char *id_str)
 	return err;
 }
 
-static int virtblk_ioctl(struct block_device *bdev, fmode_t mode,
-			     unsigned int cmd, unsigned long data)
-{
-	struct gendisk *disk = bdev->bd_disk;
-	struct virtio_blk *vblk = disk->private_data;
-
-	/*
-	 * Only allow the generic SCSI ioctls if the host can support it.
-	 */
-	if (!virtio_has_feature(vblk->vdev, VIRTIO_BLK_F_SCSI))
-		return -ENOTTY;
-
-	return scsi_cmd_blk_ioctl(bdev, mode, cmd,
-				  (void __user *)data);
-}
-
 /* We provide getgeo only to please some old bootloader/partitioning tools */
 static int virtblk_getgeo(struct block_device *bd, struct hd_geometry *geo)
 {
@@ -540,7 +580,9 @@ static int virtblk_init_request(void *data, struct request *rq,
 	struct virtio_blk *vblk = data;
 	struct virtblk_req *vbr = blk_mq_rq_to_pdu(rq);
 
+#ifdef CONFIG_VIRTIO_BLK_SCSI
 	vbr->sreq.sense = vbr->sense;
+#endif
 	sg_init_table(vbr->sg, vblk->sg_elems);
 	return 0;
 }
@@ -824,7 +866,10 @@ static const struct virtio_device_id id_table[] = {
 
 static unsigned int features_legacy[] = {
 	VIRTIO_BLK_F_SEG_MAX, VIRTIO_BLK_F_SIZE_MAX, VIRTIO_BLK_F_GEOMETRY,
-	VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE, VIRTIO_BLK_F_SCSI,
+	VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE,
+#ifdef CONFIG_VIRTIO_BLK_SCSI
+	VIRTIO_BLK_F_SCSI,
+#endif
 	VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
 	VIRTIO_BLK_F_MQ,
 }
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: make SCSI passthrough support optional
  2017-01-28  8:32 make SCSI passthrough support optional Christoph Hellwig
                   ` (4 preceding siblings ...)
  2017-01-28  8:32 ` [PATCH 5/5] virtio_blk: make SCSI passthrough support configurable Christoph Hellwig
@ 2017-01-31 17:56 ` Jens Axboe
  5 siblings, 0 replies; 7+ messages in thread
From: Jens Axboe @ 2017-01-31 17:56 UTC (permalink / raw)
  To: Christoph Hellwig, mst, jasowang; +Cc: linux-block, virtualization

On 01/28/2017 12:32 AM, Christoph Hellwig wrote:
> Hi all,
> 
> this series builds on my previous changes in Jens' for-4.11/rq-refactor
> branch that split out the BLOCK_PC fields from struct request into a new
> struct scsi_request, and makes support for struct scsi_request and the
> SCSI passthrough ioctls optional.  It is now only enabled by drivers that
> need it.
> 
> In addition I've made SCSI passthrough support in the virtio_blk driver an
> optional compile time feature, as it's not actually needed for most
> common setups.

Appled.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-01-31 17:56 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-28  8:32 make SCSI passthrough support optional Christoph Hellwig
2017-01-28  8:32 ` [PATCH 1/5] nvme/scsi: don't rely on BLK_MAX_CDB Christoph Hellwig
2017-01-28  8:32 ` [PATCH 2/5] skd: implement trivial scsi ioctls directly Christoph Hellwig
2017-01-28  8:32 ` [PATCH 3/5] block: make scsi_request and scsi ioctl support optional Christoph Hellwig
2017-01-28  8:32 ` [PATCH 4/5] virtio_blk: remove struct request backpointer from virtblk_req Christoph Hellwig
2017-01-28  8:32 ` [PATCH 5/5] virtio_blk: make SCSI passthrough support configurable Christoph Hellwig
2017-01-31 17:56 ` make SCSI passthrough support optional Jens Axboe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.