From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52698) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zk9yc-00048M-Qx for qemu-devel@nongnu.org; Thu, 08 Oct 2015 08:07:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zk9yZ-000229-HO for qemu-devel@nongnu.org; Thu, 08 Oct 2015 08:07:14 -0400 Received: from mx-v6.kamp.de ([2a02:248:0:51::16]:37978 helo=mx01.kamp.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zk9yZ-00021j-99 for qemu-devel@nongnu.org; Thu, 08 Oct 2015 08:07:11 -0400 References: <1442838328-23117-1-git-send-email-pl@kamp.de> <1442838328-23117-2-git-send-email-pl@kamp.de> <5612E873.1090503@redhat.com> <56138A76.1030804@kamp.de> <56154B71.1060001@redhat.com> From: Peter Lieven Message-ID: <56165C60.1010806@kamp.de> Date: Thu, 8 Oct 2015 14:06:56 +0200 MIME-Version: 1.0 In-Reply-To: <56154B71.1060001@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/5] ide/atapi: make PIO read requests async List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow , qemu-devel@nongnu.org, qemu-block@nongnu.org Cc: kwolf@redhat.com, stefanha@gmail.com, jcody@redhat.com Hi all, short summary from my side. The whole thing seems to get complicated, let me explain why: 1) During review I found that the code in ide_atapi_cmd_reply_end can't work correctly if the byte_count_limit is not a divider or a multiple of cd_sector_size. The reason is that as soon as we load the next sector we start at io_buffer offset 0 overwriting whatever is left in there for transfer. We also reset the io_buffer_index to 0 which means if we continue with the elementary transfer we always transfer a whole sector (of corrupt data) regardless if we are allowed to transfer that much data. Before we consider fixing this I wonder if it is legal at all to have an unaligned byte_count_limit. It obviously has never caused trouble in practice so maybe its not happening in real life. 2) I found that whatever cool optimization I put in to buffer multiple sectors at once I end up with code that breaks migration because older versions would either not fill the io_buffer as expected or we introduce variables that older versions do not understand. This will lead to problems if we migrate in the middle of a transfer. 3) My current plan to get this patch to a useful state would be to use my initial patch and just change the code to use a sync request if we need to buffer additional sectors in an elementary transfer. I found that in real world operating systems the byte_count_limit seems to be equal to the cd_sector_size. After all its just a PIO transfer an operating system will likely switch to DMA as soon as the kernel ist loaded. Thanks, Peter