Re: [Qemu-devel] [PATCH v5 0/8] e1000: Various fixes and registers' implementation

[Jason Wang <jasowang@redhat.com>]

On 11/10/2015 07:39 PM, Leonid Bloch wrote:
> On Tue, Nov 10, 2015 at 8:21 AM, Jason Wang <jasowang@redhat.com> 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(-)
>>>