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

[Aurelien Jarno]

On 2015-07-28 09:52, Mark Cave-Ayland wrote:
> On 27/07/15 23:00, Aurelien Jarno wrote:
> 
> > On 2015-05-22 15:59, John Snow wrote:
> >> From: Mark Cave-Ayland <address@hidden>
> >>
> >> 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 <address@hidden>
> >> Acked-by: John Snow <address@hidden>
> >> Message-id: address@hidden
> >> Signed-off-by: John Snow <address@hidden>
> >> ---
> >>  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?

You need a Linux based image, with a relatively recent kernel (3.2 is
ok). The disk image has to be created with -drive file=...,discard=unmap
for the TRIM feature to be advertised in the guest. Finally you need to
run the command "fstrim /" or mount a partition or the swap with the
option discard.

As a summary:
- wget 
https://people.debian.org/~aurel32/qemu/powerpc/debian_wheezy_powerpc_standard.qcow2
- qemu-system-ppc -snapshot -drive 
file=debian_wheezy_powerpc_standard.qcow2,discard=unmap 
- in the image, login as root (password root) and run fstrim /.

I have looked a bit at the code, and it looks like we have to provide a
function similar to pmac_dma_read/write for the TRIM. I'll look at it a
bit more in details and I will try to provide a patch, but it's probably
won't be before the week-end.

Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
address@hidden                 http://www.aurel32.net