From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35101) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zw8Ym-0002nx-HJ for qemu-devel@nongnu.org; Tue, 10 Nov 2015 08:02:05 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zw8Yg-00019Y-JM for qemu-devel@nongnu.org; Tue, 10 Nov 2015 08:02:04 -0500 Received: from mx1.redhat.com ([209.132.183.28]:56360) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zw8Yg-00019F-Aj for qemu-devel@nongnu.org; Tue, 10 Nov 2015 08:01:58 -0500 References: <1447081170-11614-1-git-send-email-leonid.bloch@ravellosystems.com> <56418CE8.5080704@redhat.com> From: Jason Wang Message-ID: <5641EAC0.1020805@redhat.com> Date: Tue, 10 Nov 2015 21:01:52 +0800 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 0/8] e1000: Various fixes and registers' implementation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Leonid Bloch Cc: Dmitry Fleytman , Leonid Bloch , qemu-devel@nongnu.org, Shmulik Ladkani On 11/10/2015 07:39 PM, Leonid Bloch wrote: > On Tue, Nov 10, 2015 at 8:21 AM, Jason Wang wrote: >> >> On 11/09/2015 10:59 PM, Leonid Bloch wrote: >>> This series fixes issues with packet/octet counting in e1000's Statistic >>> registers, fixes a bug in the packet address filtering procedure, and >>> implements many MAC registers that were absent before, some Statistic >>> counters among them. >>> >>> Besides this, the series introduces a parameter which, if set to "on" >>> (default), will cause the entire MAC registers' array to migrate during >>> live migration (please see patch #2 for details). The rational behind >>> this is the ability to implement additional MAC registers in the future, >>> without worrying about migration compatibility between future versions. >>> For compatibility with previous versions, the above mentioned parameter >>> can be set to "off". >>> >>> Also, a new array is introduced to control the access to the various MAC >>> registers. This takes care of situations when a MAC register requires a >>> certain parameter to be accessed, or is partially implemented, and >>> requires a debug warning to be printed on access attempts. >>> >>> Additionally, several cosmetic changes are made. >>> >>> Differences v1-2: >>> -------------------- >>> * Wording of several commit messages corrected. >>> * For trivially implemented Diagnostic registers, a debug message is >>> added on read/write attempts, alerting of incomplete implementation. >>> * Following testing on a physical device, only the lower 16 bits can now >>> be read from AIT, and only the lower 4 - from FFMT*. >>> * The grow_8reg_if_not_full function is rewritten. >>> * inc_tx_bcast_or_mcast_count and increase_size_stats are now called >>> from within e1000_send_packet, to avoid code duplication. >>> >>> Differences v2-3: >>> -------------------- >>> * Minor rewordings of some commit messages (0002, 0003). >>> * Live migration capability is added to the newly implemented registers. >>> >>> Differences v3-4: >>> -------------------- >>> * Introduction of the "full_mac_registers" parameter (see above). >>> * Reversion of the live migration handling introduced in v3. >>> * Small alignment changes in patch #1 to correspond with the following >>> patches. >>> >>> Differences v4-v5: >>> -------------------- >>> * Introduction of an array to control the access to the MAC registers. >>> * Removal of the specific functions that warned of partial >>> implementation on read/write from patch 4. >>> * Adequate changes to patches 4 and 8: mainly adding the registers >>> introduced there to the new array. >>> >>> The majority of these changes result from Jason Wang's review - thank >>> you, Jason! >> Thanks a lot for the patches. Almost done with two minor concerns: >> >> 1) to unbreak bisection we'd better enable the extra_mac_registers (and >> compatibility stuffs) in patch 8 or patch 9 > Do you mean by that changing patch 2, so that the compatibility would > be "on" by default, and setting it to "off" by default only in patch > 8, or an additional patch 9? I mean do not introduce the property "extra_mac_registers" until patch 8 and 9. In this case all function will be enabled completely at that time instead of partially patch by patch in this series. >> 2) looks like we could save some lines of codes in patch 3, see the >> comment in that patch >> >> Since we're near to soft freeze (12th), want to ask whether or not you >> want to send a v6 or I can fix 1 my self. (if 2 is correct, we can do >> optimizations on top). > Will send a v6 with a fix to 2 today. Regarding 1 - awaiting your answer. > > Thanks, > Leonid. >>> Leonid Bloch (8): >>> e1000: Cosmetic and alignment fixes >>> e1000: Add support for migrating the entire MAC registers' array >>> e1000: Introduced an array to control the access to the MAC registers >>> e1000: Trivial implementation of various MAC registers >>> e1000: Fixing the received/transmitted packets' counters >>> e1000: Fixing the received/transmitted octets' counters >>> e1000: Fixing the packet address filtering procedure >>> e1000: Implementing various counters >>> >>> hw/net/e1000.c | 503 +++++++++++++++++++++++++++++++++++++++++----------- >>> hw/net/e1000_regs.h | 8 +- >>> include/hw/compat.h | 4 + >>> 3 files changed, 406 insertions(+), 109 deletions(-) >>>