From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:60187) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SUE2S-00058X-Rg for qemu-devel@nongnu.org; Tue, 15 May 2012 05:27:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SUE2L-0005WH-If for qemu-devel@nongnu.org; Tue, 15 May 2012 05:27:28 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58926) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SUE2L-0005Vm-AM for qemu-devel@nongnu.org; Tue, 15 May 2012 05:27:21 -0400 Message-ID: <4FB22171.7050104@redhat.com> Date: Tue, 15 May 2012 11:27:13 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1337073445-9679-1-git-send-email-zhihuili@linux.vnet.ibm.com> <1337073445-9679-3-git-send-email-zhihuili@linux.vnet.ibm.com> In-Reply-To: <1337073445-9679-3-git-send-email-zhihuili@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 2/3 v6] Replace bdrv_* to bdrv_aio_* functions in DMA mode in fdc.c List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Li Zhi Hui Cc: =?ISO-8859-15?Q?Herv=E9_Poussineau?= , qemu-devel@nongnu.org Il 15/05/2012 11:17, Li Zhi Hui ha scritto: > Signed-off-by: Paolo Bonzini > Signed-off-by: Li Zhi Hui > --- > hw/fdc.c | 313 +++++++++++++++++++++++++++++++++++++++++-------------= ------- > 1 files changed, 210 insertions(+), 103 deletions(-) To reviewers, this is obviously missing migration support. :) It is also missing a good commit message. It should say that the support in the existing code for scan commands has a lot of "weirdness", for example: - if ((ret < 0 && fdctrl->data_dir =3D=3D FD_DIR_SCANL) || - (ret > 0 && fdctrl->data_dir =3D=3D FD_DIR_SCANH)) { - status2 =3D 0x00; - goto end_transfer; ... - end_transfer: - len =3D fdctrl->data_pos - start_pos; - FLOPPY_DPRINTF("end transfer %d %d %d\n", - fdctrl->data_pos, len, fdctrl->data_len); - if (fdctrl->data_dir =3D=3D FD_DIR_SCANE || - fdctrl->data_dir =3D=3D FD_DIR_SCANL || - fdctrl->data_dir =3D=3D FD_DIR_SCANH) - status2 =3D FD_SR2_SEH; which blindly overwrites status2. Hence the new code was not written based on it. However, the new code is untested as far as I know. > + if (fdctrl->data_dir =3D=3D FD_DIR_SCANE || > + fdctrl->data_dir =3D=3D FD_DIR_SCANL || > + fdctrl->data_dir =3D=3D FD_DIR_SCANH) { > + fdctrl->dma_status2 =3D FD_SR2_SEH; > + } This should be FD_SR2_SNS (Spec for the FDC is at http://www.cepece.info/amstrad/docs/i8272/8272sp.htm: "If the conditions for scan are not met between the the starting sector (as specified by R) and the last sector on the cylinder (EOT), then the FDC sets the SN (Scan Not Satisfied) flag of Status Register 2 to a 1 (high), and terminates the Scan Command"). I'm not sure whether scan commands should be kept at all though. Herv=E9= , do you know anything that uses them? Paolo