From patchwork Tue Dec 20 21:11:40 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrey Smirnov X-Patchwork-Id: 9482259 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 5863060237 for ; Tue, 20 Dec 2016 21:12:25 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4C77B28342 for ; Tue, 20 Dec 2016 21:12:25 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 410A6283D4; Tue, 20 Dec 2016 21:12:25 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 18A6028342 for ; Tue, 20 Dec 2016 21:12:23 +0000 (UTC) Received: from localhost ([::1]:53482 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cJRhu-0001jx-1U for patchwork-qemu-devel@patchwork.kernel.org; Tue, 20 Dec 2016 16:12:22 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43611) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cJRhW-0001i9-8x for qemu-devel@nongnu.org; Tue, 20 Dec 2016 16:11:59 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cJRhV-0005En-2N for qemu-devel@nongnu.org; Tue, 20 Dec 2016 16:11:58 -0500 Received: from mail-pf0-x243.google.com ([2607:f8b0:400e:c00::243]:33731) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1cJRhQ-0005Al-Ba; Tue, 20 Dec 2016 16:11:52 -0500 Received: by mail-pf0-x243.google.com with SMTP id 144so9690122pfv.0; Tue, 20 Dec 2016 13:11:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=C0TgJupRj7L7TeylFtDa4TkbYV7+bpfwdTjvI7LS/qc=; b=ouOfEsTHMK7tiYlQ8JqrFxGbGGfa/1K75oFbx6EsKD6E1xZS9BN+VRLVfliT7ltXah hCymzDwZC/R/vgaZKiO5hHzu4/idoMEqdmmZa31YKi6pqOiSMe4td3tFsrSjz4zJnebt DssFGKBgN2tt09SVzAZlCcabRNlxNBYvKMU3y8D/zCByjdE5Y99cr/LuGnTSmDid5ZDX u/E4/qOYH8F4d0D1ajzWjHDEv4aq9y7Yek0a3PQFRmgLc4zutZ2BR1rqoQlvAJpTkol5 YqNIO8OVi/Fp171dbaYeJxafceS/eKhSE64vABneqR8NF2oXuxfARQg3nUKCg1v7KByF y9bg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=C0TgJupRj7L7TeylFtDa4TkbYV7+bpfwdTjvI7LS/qc=; b=lPmFI2mp2NCnWFfsa4zMZRyVkXSf5wiLCZAd4UE9VOnSkPMGkZXDLotj7whtYwPqNf DWOtKj4TAnT00Rb3SSQZQjz/sbsotnEKgwePnkvSkZRguUWvVCctKmQFlUAX9OilDj12 geZU6eFYi77ES55X6dY11dy7qaYpCf66IB9a6aIj6ynh2xMc8yIf8wslDaCpwlB5HYaw +1r6CTvBeCMWINgjzMuiV9wep9mt8ETO4yIruEEu8MQi0IzSTD+afdZHXwJpvSCFwLvm 35ZXFyJWFZ0cuXb2lJ8UjYxVBU7Pirwtj4nKqtkEBAyoRrk939d76OEkhK6KBBAH7l4J seJQ== X-Gm-Message-State: AIkVDXK8LSVNDt2se0Fq6GR7c05wRJnGSyLIXrCZ9XA92bggkdCOZ0w7lem7PR08ajPaiA== X-Received: by 10.84.171.1 with SMTP id k1mr2255351plb.169.1482268310008; Tue, 20 Dec 2016 13:11:50 -0800 (PST) Received: from squirtle.localdomain.andrews-space.com ([173.226.206.194]) by smtp.gmail.com with ESMTPSA id i124sm41580357pgd.15.2016.12.20.13.11.47 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 20 Dec 2016 13:11:49 -0800 (PST) From: Andrey Smirnov To: qemu-ppc@nongnu.org Date: Tue, 20 Dec 2016 13:11:40 -0800 Message-Id: <1482268300-10082-1-git-send-email-andrew.smirnov@gmail.com> X-Mailer: git-send-email 2.5.5 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2607:f8b0:400e:c00::243 Subject: [Qemu-devel] [PATCH] fsl_etsec: Fix Tx BD ring wrapping handling X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Scott Wood , Andrey Smirnov , Jason Wang , Alexander Graf , qemu-devel@nongnu.org Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP Current code that handles Tx buffer desciprtor ring scanning employs the following algorithm: 1. Restore current buffer descriptor pointer from TBPTRn 2. Process current descriptor 3. If current descriptor has BD_WRAP flag set set current descriptor pointer to start of the descriptor ring 4. If current descriptor points to start of the ring exit the loop, otherwise increment current descriptor pointer and go to #2 5. Store current descriptor in TBPTRn As it can be seen the way the code is implemented results in buffer descriptor ring being scanned starting at offset/descriptor #0. While covering proverbial "99%" of the cases, this algorithm becomes problematic for a number of edge cases. Consider the following scenario: guest OS driver initializes descriptor ring to N individual descriptors and starts sending data out. Depending on the volume of traffic and probably guest OS driver implementation it is possible that an edge case where a packet, spread across 2 descriptors is placed in descriptors N - 1 and 0 in that order(it is easy to imagine similar examples involving more than 2 descriptors). What happens then is aforementioned algorithm starts at descriptor 0, sees a descriptor marked as BD_LAST, which it happily sends out as a separate packet(very much malformed at this point) then the iteration continues and the first part of the original packet is tacked to the next transmission which ends up being bogus as well. This behvaiour can be pretty reliably observed when scp'ing data from a guest OS via TAP interface for files larger than 160K (every time for 700K+). This patch changes the scanning algorithm to do the following: 1. Restore "current" and "start" buffer descriptor pointer from TBPTRn 2. If "current" descriptor has BD_WRAP flag set "next" descriptor pointer to start of the descriptor ring otherwise set "next" to descriptor right after "current" 3. Process current descriptor 4. If current descriptore has BD_LAST(end of a packet) set save "next" descriptor pointer in TBPTRn 5. Set "current" descriptor pointer to "next" 6. If "current" descriptor points to "start" (from #1) exit the loop loop, otherwise go to #2 This way emulation code always keeps track where guest OS driver was driving data to last while still going full "loop" over every descriptor in a ring, which, hopefully, should fix any potential "wrapping" issues. Signed-off-by: Andrey Smirnov --- hw/net/fsl_etsec/rings.c | 40 +++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/hw/net/fsl_etsec/rings.c b/hw/net/fsl_etsec/rings.c index 54c0127..3aebd59 100644 --- a/hw/net/fsl_etsec/rings.c +++ b/hw/net/fsl_etsec/rings.c @@ -332,7 +332,7 @@ static void process_tx_bd(eTSEC *etsec, void etsec_walk_tx_ring(eTSEC *etsec, int ring_nbr) { hwaddr ring_base = 0; - hwaddr bd_addr = 0; + hwaddr bd_addr, bd_addr_start, bd_addr_next; eTSEC_rxtx_bd bd; uint16_t bd_flags; @@ -343,7 +343,7 @@ void etsec_walk_tx_ring(eTSEC *etsec, int ring_nbr) ring_base = (hwaddr)(etsec->regs[TBASEH].value & 0xF) << 32; ring_base += etsec->regs[TBASE0 + ring_nbr].value & ~0x7; - bd_addr = etsec->regs[TBPTR0 + ring_nbr].value & ~0x7; + bd_addr_start = bd_addr = etsec->regs[TBPTR0 + ring_nbr].value & ~0x7; do { read_buffer_descriptor(etsec, bd_addr, &bd); @@ -358,26 +358,36 @@ void etsec_walk_tx_ring(eTSEC *etsec, int ring_nbr) /* Save flags before BD update */ bd_flags = bd.flags; - if (bd_flags & BD_TX_READY) { - process_tx_bd(etsec, &bd); - - /* Write back BD after update */ - write_buffer_descriptor(etsec, bd_addr, &bd); - } - /* Wrap or next BD */ if (bd_flags & BD_WRAP) { - bd_addr = ring_base; + bd_addr_next = ring_base; } else { - bd_addr += sizeof(eTSEC_rxtx_bd); + bd_addr_next = bd_addr + sizeof(eTSEC_rxtx_bd); } - } while (bd_addr != ring_base); + if (bd_flags & BD_TX_READY) { + if (bd_flags & BD_LAST) { + /* If we encounter a descriptor marking end of a + * packet - save a pointer to descriptor after that as + * a place to resume descriptor processing for next + * time. + * + * As we iterate through a ring we progressively move + * this point forward and at the end of one cycle end + * up with the position right after the last packet we + * encountered. + */ + etsec->regs[TBPTR0 + ring_nbr].value = bd_addr_next; + } + + process_tx_bd(etsec, &bd); - bd_addr = ring_base; + /* Write back BD after update */ + write_buffer_descriptor(etsec, bd_addr, &bd); + } - /* Save the Buffer Descriptor Pointers to current bd */ - etsec->regs[TBPTR0 + ring_nbr].value = bd_addr; + bd_addr = bd_addr_next; + } while (bd_addr != bd_addr_start); /* Set transmit halt THLTx */ etsec->regs[TSTAT].value |= 1 << (31 - ring_nbr);