From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34492) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bba7I-0000Fb-JT for qemu-devel@nongnu.org; Sun, 21 Aug 2016 17:17:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bba7H-0000J4-BJ for qemu-devel@nongnu.org; Sun, 21 Aug 2016 17:17:16 -0400 References: <1471513714-11709-1-git-send-email-hpoussin@reactos.org> <20160818142455.GB4147@noname.redhat.com> From: =?UTF-8?Q?Herv=c3=a9_Poussineau?= Message-ID: Date: Sun, 21 Aug 2016 23:16:52 +0200 MIME-Version: 1.0 In-Reply-To: <20160818142455.GB4147@noname.redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [Qemu-block] [PATCH] atapi: allow 0 transfer bytes for read_cd command List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: John Snow , qemu-block@nongnu.org, qemu-devel@nongnu.org Le 18/08/2016 =E0 16:24, Kevin Wolf a =E9crit : > Hm, which of the paths in cmd_read_cd() does this hit? Is it the one > that directly calls ide_atapi_cmd_ok() without doing anything? This is in ide_atapi_cmd, at line: if (cmd->handler && !(cmd->flags & NONDATA)) { handler is cmd_read_cd and flags doesn't contain NONDATA and atapi_byte_c= ount_limit is 0 and atapi_dma is false, so command is aborted. Adding NONDATA flag prevents this command abort. > > I think adding NONDATA is okay, but we may need to add explicit > atapi_byte_count_limit() =3D=3D 0 checks to those paths that do transfe= r > some data. At least at first sight I'm not sure that > ide_atapi_cmd_read() can handle this. > ATAPI packet is: ATAPI limit=3D0x0 packet: be 00 00 00 00 00 00 00 00 00 00 00 Note that byte count limit is 0x0. I also checked that s->packet_dma is false. cmd_read_cd calculates nb_sectors using buf[6], buf[7] and buf[8] =3D> nb= _sectors =3D 0. There is a specific case in cmd_read_cd if nb_sectors =3D=3D 0, which suc= ceeds the command. So, we have four cases: a) byte limit =3D=3D 0 && nb_sectors =3D=3D 0 -> used by NT4, currently i= s aborting the command in ide_atapi_cmd b) byte limit =3D=3D 0 && nb_sectors !=3D 0 -> command is aborted in ide_= atapi_cmd c) byte limit !=3D 0 && nb_sectors =3D=3D 0 -> command succeeds in cmd_re= ad_cd d) byte limit !=3D 0 && nb_sectors !=3D 0 -> usual case, works fine Maybe we should add NONDATA flag for cmd_read_cd command, and add on top = of cmd_read_cd - if nb_sectors =3D=3D 0, succeed command (for cases a and c) - if byte limit =3D=3D 0 && nb_sectors !=3D 0, abort command (for case b) - otherwise, process as usual (for case d) Herv=E9