From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:37517) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SwVrT-0002zP-Q4 for qemu-devel@nongnu.org; Wed, 01 Aug 2012 06:09:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SwVrN-0006jq-PH for qemu-devel@nongnu.org; Wed, 01 Aug 2012 06:09:03 -0400 Received: from e28smtp06.in.ibm.com ([122.248.162.6]:60201) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SwVrN-0006ja-0a for qemu-devel@nongnu.org; Wed, 01 Aug 2012 06:08:57 -0400 Received: from /spool/local by e28smtp06.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 1 Aug 2012 15:38:53 +0530 Received: from d28av02.in.ibm.com (d28av02.in.ibm.com [9.184.220.64]) by d28relay03.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q71A8m4H11337970 for ; Wed, 1 Aug 2012 15:38:49 +0530 Received: from d28av02.in.ibm.com (loopback [127.0.0.1]) by d28av02.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q71A8lpi025491 for ; Wed, 1 Aug 2012 20:08:48 +1000 Message-ID: <5019002C.9020103@linux.vnet.ibm.com> Date: Wed, 01 Aug 2012 18:08:44 +0800 From: Cong Meng MIME-Version: 1.0 References: <1343811943-3972-1-git-send-email-mc@linux.vnet.ibm.com> <5018F9F0.9020206@redhat.com> In-Reply-To: <5018F9F0.9020206@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] add suport for ATA_PASSTHROUGH_xx scsi command List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: zwanp@cn.ibm.com, linuxram@us.ibm.com, stefanha@linux.vnet.ibm.com, qemu-devel@nongnu.org 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 xxxxxx00, cmd->xfer = 0 > > else > > if buf[2] is xxxxx0xx, 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 xxxxxx01, set cmd->xfer to the FEATURES field > if buf[2] is xxxxxx10, 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 xxxxxx11. 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 >