Re: [Qemu-devel] [PULL 18/19] macio: move unaligned DMA write code into separate pmac_dma_write() function

[Kevin Wolf <kwolf@redhat.com>]

Am 28.07.2015 um 10:52 hat Mark Cave-Ayland geschrieben:
> On 27/07/15 23:00, Aurelien Jarno wrote:
> 
> > On 2015-05-22 15:59, John Snow wrote:
> >> From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> >>
> >> Similarly switch the macio IDE routines over to use the new function and
> >> tidy-up the remaining code as required.
> >>
> >> [Maintainer edit: printf format codes adjusted for 32/64bit. --js]
> >>
> >> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> >> Acked-by: John Snow <jsnow@redhat.com>
> >> Message-id: 1425939893-14404-3-git-send-email-mark.cave-ayland@ilande.co.uk
> >> Signed-off-by: John Snow <jsnow@redhat.com>
> >> ---
> >>  hw/ide/macio.c             | 268 +++++++++++++++++++++------------------------
> >>  include/hw/ppc/mac_dbdma.h |   4 -
> >>  2 files changed, 125 insertions(+), 147 deletions(-)
> > 
> > This patch has removed TRIM support without any obvious reason, and
> > without mentioning it in the changelog. As a consequence guests with
> > TRIM enabled now fail to boot:
> > 
> > | [   46.916047] ata1.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x0
> > | [   46.916545] ata1.00: failed command: DATA SET MANAGEMENT
> > | [   46.916794] ata1.00: cmd 06/01:01:00:00:00/00:00:00:00:00/a0 tag 0 dma 512 out
> > | [   46.916794]          res 40/00:01:00:00:00/00:00:00:00:00/e0 Emask 0x20 (host bus error)
> > | [   46.917219] ata1.00: status: { DRDY }
> > | [   51.957389] ata1.00: qc timeout (cmd 0xec)
> > | [   51.958076] ata1.00: failed to IDENTIFY (I/O error, err_mask=0x4)
> > | [   51.958551] ata1.00: revalidation failed (errno=-5)
> > | [   56.996713] ata1: link is slow to respond, please be patient (ready=0)
> > | [   61.981042] ata1: device not ready (errno=-16), forcing hardreset
> > | [   61.981669] ata1: soft resetting link
> > | [   62.137894] ata1.00: configured for MWDMA2
> > | [   62.138294] ata1.00: device reported invalid CHS sector 0
> > | [   62.139045] sd 0:0:0:0: [sda]  
> > | [   62.139128] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
> > | [   62.139243] sd 0:0:0:0: [sda]  
> > | [   62.139346] Sense Key : Aborted Command [current] [descriptor]
> > | [   62.139581] Descriptor sense data with sense descriptors (in hex):
> > | [   62.139670]         72 0b 00 00 00 00 00 0c 00 0a 80 00 00 00 00 00 
> > | [   62.139812]         00 00 00 00 
> > | [   62.139897] sd 0:0:0:0: [sda]  
> > | [   62.140009] Add. Sense: No additional sense information
> > | [   62.140115] sd 0:0:0:0: [sda] CDB: 
> > | [   62.140204] Write same(16): 93 08 00 00 00 00 03 c0 00 48 00 3f ff b8 00 00
> > | [   62.140661] end_request: I/O error, dev sda, sector 62914632
> > | [   62.141270] ata1: EH complete
> 
> Hi Aurelien,
> 
> Thanks for the heads-up. I have a fairly comprehensive suite of various
> OS test images I use for OpenBIOS testing and evidently not a single one
> of them issues a TRIM command as I don't see any regressions here. Can
> you point me towards the particular test image you are using?

It would probably be useful to add qtest cases for this controller so
that you don't have to run all of these test images. Generally, I won't
run your OS test images, but it's unlikely that anything covered by
qtest breaks accidentally as all subsystem maintainers routinely run it.

Kevin