On 11/04/2015 11:44 PM, Leonid Bloch wrote: > On Wed, Nov 4, 2015 at 4:46 AM, Jason Wang <jasow...@redhat.com> wrote: >> >> On 11/03/2015 07:14 PM, Leonid Bloch wrote: >>> This implements the following Statistic registers (various counters) >>> according to Intel's specs: >>> >>> TSCTC GOTCL GOTCH GORCL GORCH MPRC BPRC RUC ROC >>> BPTC MPTC PTC... PRC... >>> >>> Signed-off-by: Leonid Bloch <leonid.bl...@ravellosystems.com> >>> Signed-off-by: Dmitry Fleytman <dmitry.fleyt...@ravellosystems.com> >>> --- >>> hw/net/e1000.c | 78 >>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++---- >>> 1 file changed, 73 insertions(+), 5 deletions(-) >>> >>> diff --git a/hw/net/e1000.c b/hw/net/e1000.c >>> index af97e8a..fbda0d1 100644 >>> --- a/hw/net/e1000.c >>> +++ b/hw/net/e1000.c >>> @@ -37,6 +37,8 @@ >>> >>> #include "e1000_regs.h" >>> >> [...] >> >>> @@ -1111,6 +1164,7 @@ e1000_receive_iov(NetClientState *nc, const struct >>> iovec *iov, int iovcnt) >>> } >>> } while (desc_offset < total_size); >>> >>> + increase_size_stats(s, PRCregs, total_size); >>> inc_reg_if_not_full(s, TPR); >>> s->mac_reg[GPRC] = s->mac_reg[TPR]; >>> /* TOR - Total Octets Received: >>> @@ -1119,6 +1173,8 @@ e1000_receive_iov(NetClientState *nc, const struct >>> iovec *iov, int iovcnt) >>> * Always include FCS length (4) in size. >>> */ >>> grow_8reg_if_not_full(s, TORL, size+4); >>> + s->mac_reg[GORCL] = s->mac_reg[TORL]; >>> + s->mac_reg[GORCH] = s->mac_reg[TORH]; >>> >>> n = E1000_ICS_RXT0; >>> if ((rdt = s->mac_reg[RDT]) < s->mac_reg[RDH]) >>> @@ -1307,11 +1363,23 @@ static uint32_t (*macreg_readops[])(E1000State *, >>> int) = { >>> getreg(TNCRS), getreg(SEC), getreg(CEXTERR), getreg(RLEC), >>> getreg(XONRXC), getreg(XONTXC), getreg(XOFFRXC), getreg(XOFFTXC), >>> getreg(RFC), getreg(RJC), getreg(RNBC), getreg(TSCTFC), >>> - getreg(MGTPRC), getreg(MGTPDC), getreg(MGTPTC), >>> + getreg(MGTPRC), getreg(MGTPDC), getreg(MGTPTC), getreg(GORCL), >>> + getreg(GOTCL), >>> >>> [TOTH] = mac_read_clr8, [TORH] = mac_read_clr8, >>> + [GOTCH] = mac_read_clr8, [GORCH] = mac_read_clr8, >>> + [PRC64] = mac_read_clr4, [PRC127] = mac_read_clr4, >>> + [PRC255] = mac_read_clr4, [PRC511] = mac_read_clr4, >>> + [PRC1023] = mac_read_clr4, [PRC1522] = mac_read_clr4, >>> + [PTC64] = mac_read_clr4, [PTC127] = mac_read_clr4, >>> + [PTC255] = mac_read_clr4, [PTC511] = mac_read_clr4, >>> + [PTC1023] = mac_read_clr4, [PTC1522] = mac_read_clr4, >>> [GPRC] = mac_read_clr4, [GPTC] = mac_read_clr4, >>> [TPT] = mac_read_clr4, [TPR] = mac_read_clr4, >>> + [RUC] = mac_read_clr4, [ROC] = mac_read_clr4, >>> + [BPRC] = mac_read_clr4, [MPRC] = mac_read_clr4, >>> + [TSCTC] = mac_read_clr4, [BPTC] = mac_read_clr4, >>> + [MPTC] = mac_read_clr4, >>> [ICR] = mac_icr_read, [EECD] = get_eecd, >>> [EERD] = flash_eerd_read, >>> [RDFH] = mac_low13_read_prt, [RDFT] = mac_low13_read_prt, >> Same issue with patch 3. Need limit the function of those registers >> works only for 2.5 and post 2.5. > Contrary to the registers in patch 3, these registers do have a > functionality in the device - they are counters. But they have a > simple logic and they are distributed throughout the code. > Contrary to that, for example, the registers that were implemented in > the patch that added interrupt mitigation support (e9845f098), are > concentrated in a single location, and a single "if" statement checks > for a flag: if it is set, some non-trivial logic begins (which is > skipped if these registers are not needed anyway). > In the current case, some 10 "if"s are needed to enable a simple logic > in many places. That may influence performance, and will certainly > make the code more bulky.
I agree, but this is the price of compatibility. (And which can make our user happy). For performance, I doubt a single condition will cause noticeable difference. > On the other hand, if these registers will function always, absolutely > no harm will be done if migrating to an older version: these registers > will simply be inaccessible, as they were so far. Same as patch 3, reading to those registers will have a zero value. For the issue of bulky, how about something like this? - introduce another array mac_regcap[] (which is like phy_regcap). - store the compat flags required for the function of the registers in this array, zero means no requirement - check the enabled compat flag again this in e1000_mmio_write() and e1000_mmio_read() And this method would be even useful for future extension for e1000.