From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:41721) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RRO7c-0004CX-QQ for qemu-devel@nongnu.org; Fri, 18 Nov 2011 08:04:52 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RRO7W-0001YD-Nr for qemu-devel@nongnu.org; Fri, 18 Nov 2011 08:04:48 -0500 Received: from mx1.redhat.com ([209.132.183.28]:52073) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RRO7W-0001Y7-Bp for qemu-devel@nongnu.org; Fri, 18 Nov 2011 08:04:42 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id pAID4f8W021090 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 18 Nov 2011 08:04:41 -0500 Message-ID: <4EC658A4.5000404@redhat.com> Date: Fri, 18 Nov 2011 14:07:48 +0100 From: Kevin Wolf MIME-Version: 1.0 References: <1321277512-9414-1-git-send-email-pbonzini@redhat.com> <1321277512-9414-4-git-send-email-pbonzini@redhat.com> In-Reply-To: <1321277512-9414-4-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 3/6] scsi: fix parsing of allocation length field List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org Am 14.11.2011 14:31, schrieb Paolo Bonzini: > - several MMC commands were parsed wrong by QEMU because their allocation > length/parameter list length is placed in a non-standard position in > the CDB (i.e. it is different from most commands with the same value in > bits 5-7). > > - SEND VOLUME TAG length was multiplied by 40 which is not in SMC. The > parameter list length is between 32 and 40 bytes. Same for MEDIUM SCAN > (spec found at http://ldkelley.com/SCSI2/SCSI2-16.html but not in any of > the PDFs I have here). > > - READ_POSITION (SSC) conflicts with PRE_FETCH (SBC). READ_POSITION's > transfer length is not hardcoded to 20 in SSC; for PRE_FETCH cmd->xfer > should be 0. Both fixed. > > - FORMAT MEDIUM (the SSC name for FORMAT UNIT) was missing. The FORMAT > UNIT command is still somewhat broken for block devices because its > parameter list length is not in the CDB. However it works for CD/DVD > drives, which mandate the length of the payload. > > - fixed wrong sign-extensions for 32-bit fields (for the LBA field, > this affects disks >1 TB). > > - several other SBC or SSC commands were missing or parsed wrong. > > - some commands were not in the list of "write" commands. > > Reported-by: Thomas Schmitt > Tested-by: Thomas Schmitt (MMC bits only) > Signed-off-by: Paolo Bonzini > --- > hw/scsi-bus.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++----- > 1 files changed, 86 insertions(+), 9 deletions(-) > @@ -671,11 +696,11 @@ static int scsi_req_length(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf) > cmd->len = 10; > break; > case 4: > - cmd->xfer = ldl_be_p(&buf[10]); > + cmd->xfer = ldl_be_p(&buf[10]) & 0xffffffffULL; Makes me wonder why we don't have an unsigned version of ldl_be_p... I'll apply this anyway, we can still clean it up on top if we like. Kevin