From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:50457) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SzsN9-0006JR-W5 for qemu-devel@nongnu.org; Fri, 10 Aug 2012 12:47:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SzsN7-00083P-B4 for qemu-devel@nongnu.org; Fri, 10 Aug 2012 12:47:39 -0400 Received: from mx1.redhat.com ([209.132.183.28]:29455) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SzsN7-00083F-3T for qemu-devel@nongnu.org; Fri, 10 Aug 2012 12:47:37 -0400 From: Kevin Wolf Date: Fri, 10 Aug 2012 18:47:20 +0200 Message-Id: <1344617249-6620-3-git-send-email-kwolf@redhat.com> In-Reply-To: <1344617249-6620-1-git-send-email-kwolf@redhat.com> References: <1344617249-6620-1-git-send-email-kwolf@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: [Qemu-devel] [PATCH 02/11] ahci: Fix ahci cdrom read corruptions for reads > 128k List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: anthony@codemonkey.ws Cc: kwolf@redhat.com, qemu-devel@nongnu.org From: Jason Baron While testing q35, which has its cdrom attached to the ahci controller, I= found that the Fedora 17 install would panic on boot. The panic occurs while squashfs is trying to read from the cdrom. The errors are: [ 8.622711] SQUASHFS error: xz_dec_run error, data probably corrupt [ 8.625180] SQUASHFS error: squashfs_read_data failed to read block 0x20be48a I was also able to produce corrupt data reads using an installed piix bas= ed qemu machine, using 'dd'. I found that the corruptions were only occuring= when then read size was greater than 128k. For example, the following command results in corrupted reads: dd if=3D/dev/sr0 of=3D/tmp/blah bs=3D256k iflag=3Ddirect The > 128k size reads exercise a different code path than 128k and below.= In ide_atapi_cmd_read_dma_cb() s->io_buffer_size is capped at 128k. Thus, ide_atapi_cmd_read_dma_cb() is called a second time when the read is > 12= 8k. However, ahci_dma_rw_buf() restart the read from offset 0, instead of at = 128k. Thus, resulting in a corrupted read. To fix this, I've introduced 'io_buffer_offset' field in IDEState to keep track of the offset. I've also modified ahci_populate_sglist() to take a = new 3rd offset argument, so that the sglist is property initialized. I've tested this patch using 'dd' testing, and Fedora 17 now correctly bo= ots and installs on q35 with the cdrom ahci controller. Signed-off-by: Jason Baron Tested-by: Andreas F=C3=A4rber Signed-off-by: Kevin Wolf --- hw/ide/ahci.c | 41 ++++++++++++++++++++++++++++++++++------- hw/ide/internal.h | 1 + 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index efea93f..de580a6 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -636,7 +636,7 @@ static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_= t *cmd_fis) } } =20 -static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList *sglist) +static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList *sglist, int = offset) { AHCICmdHdr *cmd =3D ad->cur_cmd; uint32_t opts =3D le32_to_cpu(cmd->opts); @@ -647,6 +647,10 @@ static int ahci_populate_sglist(AHCIDevice *ad, QEMU= SGList *sglist) uint8_t *prdt; int i; int r =3D 0; + int sum =3D 0; + int off_idx =3D -1; + int off_pos =3D -1; + int tbl_entry_size; =20 if (!sglist_alloc_hint) { DPRINTF(ad->port_no, "no sg list given by guest: 0x%08x\n", opts= ); @@ -669,10 +673,31 @@ static int ahci_populate_sglist(AHCIDevice *ad, QEM= USGList *sglist) /* Get entries in the PRDT, init a qemu sglist accordingly */ if (sglist_alloc_hint > 0) { AHCI_SG *tbl =3D (AHCI_SG *)prdt; - - qemu_sglist_init(sglist, sglist_alloc_hint, ad->hba->dma); + sum =3D 0; for (i =3D 0; i < sglist_alloc_hint; i++) { /* flags_size is zero-based */ + tbl_entry_size =3D (le32_to_cpu(tbl[i].flags_size) + 1); + if (offset <=3D (sum + tbl_entry_size)) { + off_idx =3D i; + off_pos =3D offset - sum; + break; + } + sum +=3D tbl_entry_size; + } + if ((off_idx =3D=3D -1) || (off_pos < 0) || (off_pos > tbl_entry= _size)) { + DPRINTF(ad->port_no, "%s: Incorrect offset! " + "off_idx: %d, off_pos: %d\n", + __func__, off_idx, off_pos); + r =3D -1; + goto out; + } + + qemu_sglist_init(sglist, (sglist_alloc_hint - off_idx), ad->hba-= >dma); + qemu_sglist_add(sglist, le64_to_cpu(tbl[off_idx].addr + off_pos)= , + le32_to_cpu(tbl[off_idx].flags_size) + 1 - off_p= os); + + for (i =3D off_idx + 1; i < sglist_alloc_hint; i++) { + /* flags_size is zero-based */ qemu_sglist_add(sglist, le64_to_cpu(tbl[i].addr), le32_to_cpu(tbl[i].flags_size) + 1); } @@ -745,7 +770,7 @@ static void process_ncq_command(AHCIState *s, int por= t, uint8_t *cmd_fis, ncq_tfs->lba, ncq_tfs->lba + ncq_tfs->sector_count - 2, s->dev[port].port.ifs[0].nb_sectors - 1); =20 - ahci_populate_sglist(&s->dev[port], &ncq_tfs->sglist); + ahci_populate_sglist(&s->dev[port], &ncq_tfs->sglist, 0); ncq_tfs->tag =3D tag; =20 switch(ncq_fis->command) { @@ -970,7 +995,7 @@ static int ahci_start_transfer(IDEDMA *dma) goto out; } =20 - if (!ahci_populate_sglist(ad, &s->sg)) { + if (!ahci_populate_sglist(ad, &s->sg, 0)) { has_sglist =3D 1; } =20 @@ -1015,6 +1040,7 @@ static void ahci_start_dma(IDEDMA *dma, IDEState *s= , DPRINTF(ad->port_no, "\n"); ad->dma_cb =3D dma_cb; ad->dma_status |=3D BM_STATUS_DMAING; + s->io_buffer_offset =3D 0; dma_cb(s, 0); } =20 @@ -1023,7 +1049,7 @@ static int ahci_dma_prepare_buf(IDEDMA *dma, int is= _write) AHCIDevice *ad =3D DO_UPCAST(AHCIDevice, dma, dma); IDEState *s =3D &ad->port.ifs[0]; =20 - ahci_populate_sglist(ad, &s->sg); + ahci_populate_sglist(ad, &s->sg, 0); s->io_buffer_size =3D s->sg.size; =20 DPRINTF(ad->port_no, "len=3D%#x\n", s->io_buffer_size); @@ -1037,7 +1063,7 @@ static int ahci_dma_rw_buf(IDEDMA *dma, int is_writ= e) uint8_t *p =3D s->io_buffer + s->io_buffer_index; int l =3D s->io_buffer_size - s->io_buffer_index; =20 - if (ahci_populate_sglist(ad, &s->sg)) { + if (ahci_populate_sglist(ad, &s->sg, s->io_buffer_offset)) { return 0; } =20 @@ -1050,6 +1076,7 @@ static int ahci_dma_rw_buf(IDEDMA *dma, int is_writ= e) /* update number of transferred bytes */ ad->cur_cmd->status =3D cpu_to_le32(le32_to_cpu(ad->cur_cmd->status)= + l); s->io_buffer_index +=3D l; + s->io_buffer_offset +=3D l; =20 DPRINTF(ad->port_no, "len=3D%#x\n", l); =20 diff --git a/hw/ide/internal.h b/hw/ide/internal.h index 7170bd9..bf7d313 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -393,6 +393,7 @@ struct IDEState { struct iovec iov; QEMUIOVector qiov; /* ATA DMA state */ + int io_buffer_offset; int io_buffer_size; QEMUSGList sg; /* PIO transfer handling */ --=20 1.7.6.5