From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35635) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X1aT5-0006oj-Jo for qemu-devel@nongnu.org; Mon, 30 Jun 2014 08:14:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X1aSy-00036T-M6 for qemu-devel@nongnu.org; Mon, 30 Jun 2014 08:13:55 -0400 Received: from cantor2.suse.de ([195.135.220.15]:50950 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X1aSy-00036M-AF for qemu-devel@nongnu.org; Mon, 30 Jun 2014 08:13:48 -0400 Message-ID: <53B1547B.5030904@suse.de> Date: Mon, 30 Jun 2014 14:13:47 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1404066076-32859-1-git-send-email-reza.jelveh@tuhh.de> <1404066076-32859-2-git-send-email-reza.jelveh@tuhh.de> In-Reply-To: <1404066076-32859-2-git-send-email-reza.jelveh@tuhh.de> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] ahci.c: mask unused flags when reading size PRDT DBC List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: reza.jelveh@gmail.com, qemu-devel@nongnu.org Cc: pbonzini@redhat.com, jsnow@redhat.com, Reza Jelveh On 29.06.14 20:21, reza.jelveh@gmail.com wrote: > From: Reza Jelveh This is a hint that your git configuration isn't fully correct. If the email address git thinks you want to use is the same as the From: email address, it will not print this line. I suppose the problem is with the difference in @gmail.com and @tuhh.de? > The data byte count(DBC) read from the description information is defined for bits > 21:00. Bits > 30:22 are reserved and bit > 31 is the Interrupt on Completion (I) flag. > > Interrupt is not implemented in QEMU. They are implemented in QEMU, but incorrectly. We trigger a completion interrupt after every transaction, not every time we see the I bit in the PRDT. > tbl_entry_size is a signed integer and > improperly reading the DBC leads to a negative offset that causes sglist > allocation to fail. > > Signed-off-by: Reza Jelveh > --- > hw/ide/ahci.c | 12 +++++++++--- > hw/ide/ahci.h | 2 ++ > 2 files changed, 11 insertions(+), 3 deletions(-) > > diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c > index 9bae22e..93aa981 100644 > --- a/hw/ide/ahci.c > +++ b/hw/ide/ahci.c > @@ -639,6 +639,12 @@ static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t *cmd_fis) > } > } > > +static int prdt_tbl_entry_size(const AHCI_SG tbl) The argument should still be a pointer. > +{ > + return (le32_to_cpu(tbl.flags_size) & AHCI_PRDT_SIZE_MASK) + 1; > +} > + > + There is still one whitespace line too much :). Alex > static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList *sglist, int offset) > { > AHCICmdHdr *cmd = ad->cur_cmd; > @@ -681,7 +687,7 @@ static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList *sglist, int offset) > sum = 0; > for (i = 0; i < sglist_alloc_hint; i++) { > /* flags_size is zero-based */ > - tbl_entry_size = (le32_to_cpu(tbl[i].flags_size) + 1); > + tbl_entry_size = prdt_tbl_entry_size(tbl[i]); > if (offset <= (sum + tbl_entry_size)) { > off_idx = i; > off_pos = offset - sum; > @@ -700,12 +706,12 @@ static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList *sglist, int offset) > qemu_sglist_init(sglist, qbus->parent, (sglist_alloc_hint - off_idx), > ad->hba->as); > qemu_sglist_add(sglist, le64_to_cpu(tbl[off_idx].addr + off_pos), > - le32_to_cpu(tbl[off_idx].flags_size) + 1 - off_pos); > + prdt_tbl_entry_size(tbl[off_idx]) - off_pos); > > for (i = 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); > + prdt_tbl_entry_size(tbl[i])); > } > } > > diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h > index 9a4064f..f418b30 100644 > --- a/hw/ide/ahci.h > +++ b/hw/ide/ahci.h > @@ -201,6 +201,8 @@ > > #define AHCI_COMMAND_TABLE_ACMD 0x40 > > +#define AHCI_PRDT_SIZE_MASK 0x3fffff > + > #define IDE_FEATURE_DMA 1 > > #define READ_FPDMA_QUEUED 0x60