All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexey Makhalov <amakhalov@vmware.com>
To: linux-scsi@vger.kernel.org
Cc: namit@vmware.com, srivatsa@csail.mit.edu,
	shmulik.ladkani@gmail.com, Alexey Makhalov <amakhalov@vmware.com>,
	Matt Wang <wwentao@vmware.com>,
	"Martin K . Petersen" <martin.petersen@oracle.com>,
	Vishal Bhakta <vbhakta@vmware.com>,
	VMware PV-Drivers <pv-drivers@vmware.com>,
	"James E . J . Bottomley" <jejb@linux.ibm.com>,
	stable@vger.kernel.org
Subject: [PATCH] scsi: vmw_pvscsi: Set residual data length conditionally
Date: Mon, 20 Dec 2021 11:05:14 -0800	[thread overview]
Message-ID: <20211220190514.55935-1-amakhalov@vmware.com> (raw)

PVSCSI implementation in VMware hypervisor under specific configuration
("SCSI Bus Sharing" set to "Physical") returns zero dataLen in completion
descriptor for read_capacity_16. As a result, the kernel can not detect proper
disk geometry. It can be recognized by the kernel message:
[    0.776588] sd 1:0:0:0: [sdb] Sector size 0 reported, assuming 512.

PVSCSI implementation in QEMU does not set dataLen at all keeping it zeroed,
leading to the boot hang, as was reported by Shmulik Ladkani.

It is likely that the controller returns the garbage at the end of the buffer.
Residual length should be set by the driver in that case. scsi_lib layer will
erase corresponding data. See commit bdb2b8cab439 ("[SCSI] erase invalid data
returned by device") for details.

Commit e662502b3a78 ("scsi: vmw_pvscsi: Set correct residual data length")
introduced the issue by setting residual length unconditionally causing
scsi_lib layer to erase the useful payload beyond dataLen in the mentioned
above cases.

Considering existing issues in implementations of PVSCSI controllers, we
do not want to call scsi_set_resid() when dataLen == 0. Calling scsi_set_resid()
has no effect if dataLen equals buffer length.

Fixes: e662502b3a78 ("scsi: vmw_pvscsi: Set correct residual data length")
Reported-and-suggested-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Link: https://lore.kernel.org/lkml/20210824120028.30d9c071@blondie/
Cc: Matt Wang <wwentao@vmware.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Vishal Bhakta <vbhakta@vmware.com>
Cc: VMware PV-Drivers <pv-drivers@vmware.com>
Cc: James E.J. Bottomley <jejb@linux.ibm.com>
Cc: linux-scsi@vger.kernel.org
Cc: stable@vger.kernel.org
Signed-off-by: Alexey Makhalov <amakhalov@vmware.com>
---
 drivers/scsi/vmw_pvscsi.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/vmw_pvscsi.c b/drivers/scsi/vmw_pvscsi.c
index ce1ba1b93..9419d6d1d 100644
--- a/drivers/scsi/vmw_pvscsi.c
+++ b/drivers/scsi/vmw_pvscsi.c
@@ -586,9 +586,12 @@ static void pvscsi_complete_request(struct pvscsi_adapter *adapter,
 			 * Commands like INQUIRY may transfer less data than
 			 * requested by the initiator via bufflen. Set residual
 			 * count to make upper layer aware of the actual amount
-			 * of data returned.
+			 * of data returned. There are cases when controller
+			 * returns zero dataLen with non zero data - do not set
+			 * residual count in that case.
 			 */
-			scsi_set_resid(cmd, scsi_bufflen(cmd) - e->dataLen);
+			if (e->dataLen && (e->dataLen < scsi_bufflen(cmd)))
+				scsi_set_resid(cmd, scsi_bufflen(cmd) - e->dataLen);
 			cmd->result = (DID_OK << 16);
 			break;
 
-- 
2.30.0


             reply	other threads:[~2021-12-20 19:05 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-20 19:05 Alexey Makhalov [this message]
2021-12-23  5:08 ` [PATCH] scsi: vmw_pvscsi: Set residual data length conditionally Martin K. Petersen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211220190514.55935-1-amakhalov@vmware.com \
    --to=amakhalov@vmware.com \
    --cc=jejb@linux.ibm.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=namit@vmware.com \
    --cc=pv-drivers@vmware.com \
    --cc=shmulik.ladkani@gmail.com \
    --cc=srivatsa@csail.mit.edu \
    --cc=stable@vger.kernel.org \
    --cc=vbhakta@vmware.com \
    --cc=wwentao@vmware.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.