imx6 sata cdrom driver issue

[Fabio Estevam]

On Tue, May 19, 2015 at 4:18 PM, Kevin Groeneveld
<KGroeneveld at lenbrook.com> wrote:
> -----Original Message-----
> From: Jonathan Bagg
> Sent: May-19-15 3:14 PM
>
>>Adding my co-worker Kevin who has done a few experiments in the kernel
>
> I am one of Jonathan's co-workers who has been helping to diagnose and
> fix the SATA issue we are experiencing.  Most of my testing has been
> with an optical drive connected directly to the Freescale Sabre board.
>
> Using the debug patch I wrote that Jon posted earlier I can see that the
> response to the ATAPI 0x4A command (Get Event Status Notification)
> during initial boot is always random. For example:
>
> [    2.237783] CDB rbuf: [4a 01 00 00 10 00 00 00 08]
> [    2.237789] CDB rbuf: 00000000: 7e 31 f7 fd ff f5 7d fd
> ~1....}.
>
> Running the same test on an iMX53 EVK board the response is always:
>
> [    2.452303] CDB rbuf: [4a 01 00 00 10 00 00 00 08]
> [    2.452308] CDB rbuf: 00000000: 00 06 04 56 02 02 00 00
> ...V....
>
> We have rented a Lecroy STX-231 SATA protocol analyzer. Using this tool
> I have verified that the response from the optical drive is always
> correct in this case but for some reason the data is corrupt by the time
> the debug code I added to atapi_qc_complete dumps the response.
>
> I have added debug code to dump the AHCI command header and table before
> issuing a command. I have not seen anything out of the ordinary here. I
> also added debug code to dump the received FIS on an interrupt. I
> haven't studied the received FIS extensively but I did not see anything
> out of the ordinary here either.
>
> One thing I suspected and confirmed with the Lecroy analyzer is that the
> response to the previous command during boot (ATAPI command 0x5A - Mode
> Sense) is always 64 bytes even though the kernel is requesting 128
> bytes. This exact behaviour is probably drive dependant.  On a hunch I
> modified the get_capabilities function in drivers/scsi/sr.c to only
> request 64 bytes. With that change the response to the Get Event Status
> Notification command during boot is always valid.  Although running
> cdparanoia or trying to keep using the system in other ways still
> results in issues.
>
> Using the Lecroy analyzer I found some other commands where cdparanoia
> requests lengths longer than the drive responds with.  Comparing the
> debug dumps in the kernel to the data captured by the analyzer I can see
> problems always start after this happens.
>
> To try and characterize this further I started messing around with the
> request lengths.  I noticed in the AHCI command header dumps I added
> that many of the ATAPI commands have a second SG DMA buffer. I found the
> second buffer is added when atapi_drain_needed returns true in
> libata-scsi.c.  I started hacking the DMA buffer sizes by manipulating
> the lengths written to the AHCI command table in the ahci_fill_sg
> function in libata.c.  I also hacked atap_drain_needed to sometimes
> return false to force only one DMA buffer for ATAPI commands.
>
> What I found is that if there are two DMA buffers and the response
> length is less than the first buffer size then the response to the next
> command is always corrupted.  Every other scenario I tried worked okay.
> Filling the first buffer and under, over or exactly using the second
> buffer does not trigger the issue.  If there is only one buffer under,
> over or exactly using the one buffer does not trigger the issue.
>
> One other thing I noticed is that if the response under runs the first
> buffer and the second ATAPI drain buffer is smaller than the next
> command response then the response to the next command is missing the
> begging of the response equal to the length of the second buffer from
> the previous command. I decided to add some debug code to print out the
> DMA buffer used for the previous command. What I found is that the
> missing part of the response is written to the DMA buffer from the
> previous command.  This is very bad as that memory could be dynamically
> allocated and subsequently freed and now in use by something else!
>
> Since overruns never seem to cause an issue in this setup I decided to
> simply change the atap_drain_needed function to always return false.
> With this change I can run cdparanoia successfully and rip audio tracks.
> I can also add a port multiplier back to the setup and connect both an
> HDD and the optical drive and experience no readily apparent issues.
>
> So it seems I have found a workaround for the most severe symptoms we
> were experiencing. But this still leaves me with several questions and
> concerns:
>
> 1. Is the root cause some obscure bug in the kernel I have not found
> yet? Or is it a hardware bug in the iMX6 AHCI SATA host? Or maybe
> something else silly we have been doing?
>
> 2. When cdparanoia is ripping the actual data it uses large reads which
> use multiple SG DMA buffers. What happens if the response is short in
> this case? My atapi_drain_needed hack won't help in this case.
> cdparanoia could probably be modified to only do smaller reads that fit
> in one page to avoid the issue.
>
> 3. Can the issue happen with non ATAPI commands?  If for any reason an
> ATA command to a HDD has a short response could all future commands (and
> possibly even system memory) be corrupted similar to what I saw with
> ATAPI commands? I wonder if the kernel could be easily modified to limit
> the size of ATA commands to avoid this scenario.
>
> 4. The Lecroy analyzer can also emulate a device. Using this mode I
> tried to trigger the issue with ATA commands. I couldn't figure out how
> to trigger the exact scenario I wanted.  I was able to cause the
> response to be too short, just not short enough to underrun anything but
> the last DMA buffer which doesn't seem to cause a problem.  What I did
> notice though was that this didn't seem to trigger an error anywhere in
> the kernel or user space, the data would just simply be corrupted. I
> would expect the system should flag the response as being invalid and an
> IO error propagated all the way to user space.

Just tested this and I could reproduce this bug on a mx6 board running
kernel 4.0.4.

Adding some more folks in case anyone has some ideas about this DMA
buffer corruption.

Regards,

Fabio Estevam