From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:54301)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1TOkaM-0006hG-8x
	for qemu-devel@nongnu.org; Thu, 18 Oct 2012 03:32:10 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1TOkaF-0000qY-4r
	for qemu-devel@nongnu.org; Thu, 18 Oct 2012 03:32:06 -0400
Received: from mail-bk0-f45.google.com ([209.85.214.45]:46925)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1TOkaE-0000qO-S0
	for qemu-devel@nongnu.org; Thu, 18 Oct 2012 03:31:59 -0400
Received: by mail-bk0-f45.google.com with SMTP id jf3so3364535bkc.4
	for <qemu-devel@nongnu.org>; Thu, 18 Oct 2012 00:31:58 -0700 (PDT)
Date: Thu, 18 Oct 2012 09:31:56 +0200
From: Stefan Hajnoczi <stefanha@gmail.com>
Message-ID: <20121018073156.GE22425@stefanha-thinkpad.redhat.com>
References: <1350498707-6749-1-git-send-email-dmitry@daynix.com>
	<1350498707-6749-2-git-send-email-dmitry@daynix.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1350498707-6749-2-git-send-email-dmitry@daynix.com>
Subject: Re: [Qemu-devel] [PATCH 1/2] Ignore RX tail kicks when RX disabled.
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Dmitry Fleytman <dmitry@daynix.com>
Cc: Yan Vugenfirer <yan@daynix.com>, Chris Webb <chris.webb@elastichosts.com>, qemu-devel@nongnu.org, Richard Davies <richard.davies@elastichosts.com>

On Wed, Oct 17, 2012 at 08:31:46PM +0200, Dmitry Fleytman wrote:
> Device RX initization from driver's side consists of following steps:
>   1. Initialize head and tail of RX ring to 0
>   2. Enable Rx (set bit in RCTL register)
>   3. Allocate buffers, fill descriptors
>   4. Write ring tail
> 
> Forth operation signals hardware that RX buffers available
> and it may start packets indication.
> 
> Current implementation treats first operation (write 0 to ring tail)
> as signal of buffers availability and starts data transfers as soon
> as RX enable indicaton arrives.
> 
> This is not correct because there is a chance that ring is still
> empty (third action not performed yet) and then memory corruption
> occures.

The existing code tries to prevent this:

e1000_receive(NetClientState *nc, const uint8_t *buf, size_t size)
{
    [...]

    if (!(s->mac_reg[RCTL] & E1000_RCTL_EN))
        return -1;

    [...]
    total_size = size + fcs_len(s);
    if (!e1000_has_rxbufs(s, total_size)) {
            set_ics(s, 0, E1000_ICS_RXO);
            return -1;
    }

Why are these checks not enough?

Which memory gets corrupted?

Stefan