From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46773) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VggZ1-0005ol-0b for qemu-devel@nongnu.org; Wed, 13 Nov 2013 14:57:29 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VggYv-0000md-0Z for qemu-devel@nongnu.org; Wed, 13 Nov 2013 14:57:22 -0500 Received: from mx1.redhat.com ([209.132.183.28]:14492) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VggYu-0000mX-OO for qemu-devel@nongnu.org; Wed, 13 Nov 2013 14:57:16 -0500 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rADJvFo3022770 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 13 Nov 2013 14:57:15 -0500 Date: Wed, 13 Nov 2013 22:00:14 +0200 From: "Michael S. Tsirkin" Message-ID: <20131113200014.GA8005@redhat.com> References: <1383650238-16015-1-git-send-email-akong@redhat.com> <1383939747-10158-1-git-send-email-vyasevic@redhat.com> <20131110121151.GA5908@redhat.com> <5283A6FE.60205@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5283A6FE.60205@redhat.com> Subject: Re: [Qemu-devel] [PATCH] e1000/rtl8139: update HMP NIC when every bit is written List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vlad Yasevich Cc: jasowang@redhat.com, akong@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com On Wed, Nov 13, 2013 at 11:21:18AM -0500, Vlad Yasevich wrote: > On 11/10/2013 07:11 AM, Michael S. Tsirkin wrote: > >On Fri, Nov 08, 2013 at 02:42:27PM -0500, Vlad Yasevich wrote: > >>What about this approach? This only updates the monitory when all the > >>bits have been written to. > >> > >>-vlad > > > > > >Thanks! > >Some comments below. > > > >>-- >8 -- > >>Subject: [PATCH] e1000/rtl8139: update HMP NIC when every bit is written > >> > >>We currently just update the HMP NIC info when the last bit of macaddr > >>is written. This assumes that guest driver will write all the macaddr > >>from bit 0 to bit 5 when it changes the macaddr, this is the current > >>behavior of linux driver (e1000/rtl8139cp), but we can't do this > >>assumption. > > > >I would rather say "it seems better not to make this assumption". > >This does look somewhat safer than what Amos proposed. > > > >> > >>The macaddr that is used for rx-filter will be updated when every bit > >>is changed. This patch updates the e1000/rtl8139 nic to update HMP NIC > >>info when every bit has been changed. It will be same as virtio-net. > >> > >>Signed-off-by: Vlad Yasevich > >>--- > >> hw/net/e1000.c | 17 ++++++++++++++++- > >> hw/net/rtl8139.c | 14 +++++++++----- > >> 2 files changed, 25 insertions(+), 6 deletions(-) > >> > >>diff --git a/hw/net/e1000.c b/hw/net/e1000.c > >>index 8387443..a5967ed 100644 > >>--- a/hw/net/e1000.c > >>+++ b/hw/net/e1000.c > >>@@ -149,6 +149,10 @@ typedef struct E1000State_st { > >> #define E1000_FLAG_AUTONEG (1 << E1000_FLAG_AUTONEG_BIT) > >> #define E1000_FLAG_MIT (1 << E1000_FLAG_MIT_BIT) > >> uint32_t compat_flags; > >>+ uint32_t mac_changed; > > > >Hmm why uint32_t? uint8_t is enough here isn't? > > > >This new state has to be migrated then, and > >we need to fallback to old behaviour if migrating to/from > >an old version (see compat_flags for one way to > >detect this compatibility mode). > > > > Hi Michael > > I started looking at migrating this thing and I now starting to question > the whole approach. > > The only reason to migrate this is if we can migrate between writes to > the mac address registers. Absolutely. For some reason below you only discuss cross version migration but it needs to be migrated for same version migration too. > We can fairly easily solve the issue of > migrating from net to old versions. I'm not sure it's easier, but it needs to happen anymore. > The more interesting question > is migrating from old to new versions. > > If someone is migrating from an older version (without this feature) > to a newer version and doing so between writes, the bitmap state will > have no idea that a partial write has already happened. The completing > write will just set one of the bits and notifications that we are > looking for do not happen. > > -vlad Yep, that's a problem too. For 1.8 just send the bitmap across. For cross version migration I would say we should just detect -M 1.7 and older and just emulate old behaviour, disregard the bitmap completely. Don't do special tricks just for migration. -- MST