From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56075) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vk2Ok-0002uX-Bx for qemu-devel@nongnu.org; Fri, 22 Nov 2013 20:52:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vk2Oe-0007IL-0Y for qemu-devel@nongnu.org; Fri, 22 Nov 2013 20:52:38 -0500 Received: from mx1.redhat.com ([209.132.183.28]:19475) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vk2Od-0007ID-Ow for qemu-devel@nongnu.org; Fri, 22 Nov 2013 20:52:31 -0500 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rAN1qUqS016397 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 22 Nov 2013 20:52:30 -0500 Date: Sat, 23 Nov 2013 09:52:27 +0800 From: Amos Kong Message-ID: <20131123012533.GA8206@amosk.info> References: <1385046834-11929-1-git-send-email-mst@redhat.com> <1385046834-11929-6-git-send-email-mst@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1385046834-11929-6-git-send-email-mst@redhat.com> Subject: Re: [Qemu-devel] [PULL for-1.7 5/5] Revert "e1000/rtl8139: update HMP NIC when every bit is written" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: Vlad Yasevich , qemu-devel@nongnu.org On Thu, Nov 21, 2013 at 05:14:36PM +0200, Michael S. Tsirkin wrote: > This reverts commit cd5be5829c1ce87aa6b3a7806524fac07ac9a757. > Digging into hardware specs shows this does not > actually make QEMU behave more like hardware: > > There are valid arguments backed by the spec to indicate why the version > of e1000 prior to cd5be582 was more correct: the high byte actually > includes a valid bit, this is why all guests write it last. > > For rtl8139 there's actually a separate undocumented valid bit, but we > don't implement it yet. > > To summarize all the drivers we know about behave in one way > that allows us to make an assumption about write order and avoid > spurious, incorrect mac address updates to the monitor. > > Let's stick to the tried heuristic for 1.7 and > possibly revisit for 1.8. > > Reported-by: Vlad Yasevich > Reviewed-by: Vlad Yasevich > Cc: Amos Kong > Signed-off-by: Michael S. Tsirkin Reviewed-by: Amos Kong > --- > hw/net/e1000.c | 2 +- > hw/net/rtl8139.c | 5 ++++- > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/hw/net/e1000.c b/hw/net/e1000.c > index ae63591..8387443 100644 > --- a/hw/net/e1000.c > +++ b/hw/net/e1000.c > @@ -1106,7 +1106,7 @@ mac_writereg(E1000State *s, int index, uint32_t val) > > s->mac_reg[index] = val; > > - if (index == RA || index == RA + 1) { > + if (index == RA + 1) { > macaddr[0] = cpu_to_le32(s->mac_reg[RA]); > macaddr[1] = cpu_to_le32(s->mac_reg[RA + 1]); > qemu_format_nic_info_str(qemu_get_queue(s->nic), (uint8_t *)macaddr); > diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c > index 7f2b4db..5329f44 100644 > --- a/hw/net/rtl8139.c > +++ b/hw/net/rtl8139.c > @@ -2741,7 +2741,10 @@ static void rtl8139_io_writeb(void *opaque, uint8_t addr, uint32_t val) > > switch (addr) > { > - case MAC0 ... MAC0+5: > + case MAC0 ... MAC0+4: > + s->phys[addr - MAC0] = val; > + break; > + case MAC0+5: > s->phys[addr - MAC0] = val; > qemu_format_nic_info_str(qemu_get_queue(s->nic), s->phys); > break; > -- > MST > -- Amos.