From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35633) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zw9ve-0001Tm-N2 for qemu-devel@nongnu.org; Tue, 10 Nov 2015 09:29:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zw9vd-0001W0-Ck for qemu-devel@nongnu.org; Tue, 10 Nov 2015 09:29:46 -0500 Received: from mail-lb0-x22c.google.com ([2a00:1450:4010:c04::22c]:33863) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zw9vd-0001Vu-16 for qemu-devel@nongnu.org; Tue, 10 Nov 2015 09:29:45 -0500 Received: by lbbcs9 with SMTP id cs9so15471820lbb.1 for ; Tue, 10 Nov 2015 06:29:44 -0800 (PST) MIME-Version: 1.0 In-Reply-To: References: <1447081170-11614-1-git-send-email-leonid.bloch@ravellosystems.com> <56418CE8.5080704@redhat.com> <5641EAC0.1020805@redhat.com> Date: Tue, 10 Nov 2015 16:29:44 +0200 Message-ID: From: Leonid Bloch Content-Type: text/plain; charset=UTF-8 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: Jason Wang Cc: Dmitry Fleytman , Leonid Bloch , qemu-devel@nongnu.org, Shmulik Ladkani On Tue, Nov 10, 2015 at 3:19 PM, Leonid Bloch wrote: > On Tue, Nov 10, 2015 at 3:01 PM, Jason Wang wrote: >> >> >> 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. > > But won't there be compatibility issues between the patches if done > like that? Why not to prepare the ground for compatibility, and only > then introduce the new registers (as it is done now)? >> >>>> 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). I have a v6 with #2 fixed ready. Awaiting your answer to my question regarding #1 before sending the series. >>> 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(-) >>>>> >>