From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55288) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XjGmy-0005eN-RC for qemu-devel@nongnu.org; Tue, 28 Oct 2014 20:07:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XjGmu-0005mF-0L for qemu-devel@nongnu.org; Tue, 28 Oct 2014 20:07:00 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40483) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XjGmt-0005mB-Of for qemu-devel@nongnu.org; Tue, 28 Oct 2014 20:06:55 -0400 Message-ID: <54502F9B.2070901@redhat.com> Date: Tue, 28 Oct 2014 20:06:51 -0400 From: John Snow MIME-Version: 1.0 References: <1412204151-18117-1-git-send-email-jsnow@redhat.com> <20141028135151.GC22805@stefanha-thinkpad.redhat.com> <54502CA1.1040802@redhat.com> <54502EE4.3090704@redhat.com> In-Reply-To: <54502EE4.3090704@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 0/6] AHCI Device Fixes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , Stefan Hajnoczi Cc: kwolf@redhat.com, mst@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, armbru@redhat.com On 10/28/2014 08:03 PM, Paolo Bonzini wrote: > > > On 10/29/2014 12:54 AM, John Snow wrote: >> >> >> On 10/28/2014 09:51 AM, Stefan Hajnoczi wrote: >>> On Wed, Oct 01, 2014 at 06:55:45PM -0400, John Snow wrote: >>>> Based off of feedback from the RFC of the same name, >>>> this series batches together a group of fixes that >>>> improve the AHCI device to fix a number of bugs. >>>> >>>> A number of fixes included in the RFC that provide more >>>> radical changes are omitted for now in favor of a smaller, >>>> more easily reviewable set for QEMU 2.2. >>>> >>>> In summary: >>>> >>>> Patch #1 and #6 correct the format of FIS packet responses >>>> that are available to the guest operating system upon interrupt. >>>> >>>> Patch #2 corrects an oversight where we do not inform the >>>> guest how many bytes we've transferred. This is relied upon >>>> for non-NCQ modes and in some early bootup and shutdown code. >>>> >>>> Patch #5 corrects cases with malformed scatter-gather lists that >>>> may cause leaks, or cause QEMU to hang in an allocation loop. >>>> >>>> Patch #4 attempts to continue minimizing the divergence of the >>>> multiple pathways through the AHCI device by re-using existing >>>> callbacks. >>>> >>>> Taken together, these patches should allow non-ncq operation >>>> for Windows hosts, as well as enable hibernation for Windows 7. >>>> >>>> Hibernation for Windows 8 and AHCI remains non-functional. >>>> >>>> John Snow (6): >>>> ahci: Correct PIO/D2H FIS responses >>>> ahci: Update byte count after DMA completion >>>> ide: repair PIO transfers for cases where nsector > 1 >>>> ahci: unify sglist preparation >>>> ide: Correct handling of malformed/short PRDTs >>>> ahci: Fix SDB FIS Construction >>>> >>>> dma-helpers.c | 3 ++ >>>> hw/ide/ahci.c | 113 >>>> ++++++++++++++++++++++++++++++++---------------------- >>>> hw/ide/ahci.h | 8 ++++ >>>> hw/ide/core.c | 17 ++++++-- >>>> hw/ide/internal.h | 2 + >>>> hw/ide/pci.c | 5 ++- >>>> 6 files changed, 97 insertions(+), 51 deletions(-) >>> >>> Dropped the broken assert that Paolo spotted. >>> >>> Thanks, applied to my block tree: >>> https://github.com/stefanha/qemu/commits/block >>> >>> Stefan >> >> Drop the PIO patch for now (#3), I ran into a regression with it with >> CDROM drives. I believe the others are still fine. > > Yeah, I was wondering if any commands could have <512 bytes response... > I sort of convinced myself that the answer was no for ATA commands, but > stupidly forgot about packet (SCSI) commands. Their results are > obviously shorter than 512 bytes. > > Paolo > Are you referencing the sglist underflow patch? (#5 instead of #3) There were cases in the code already where we /assumed/ that having any bytes implied we had at least a sector's worth. If there are valid cases for the sglist to have less than a sector's worth (SCSI) then I'll need to touch that again as well and update all the assumptions in the IDE code to look for numbytes instead of numsectors.