* [Qemu-devel] [PATCH v5 0/8] e1000: Various fixes and registers' implementation
@ 2015-11-09 14:59 Leonid Bloch
2015-11-09 14:59 ` [Qemu-devel] [PATCH v5 1/8] e1000: Cosmetic and alignment fixes Leonid Bloch
` (8 more replies)
0 siblings, 9 replies; 19+ messages in thread
From: Leonid Bloch @ 2015-11-09 14:59 UTC (permalink / raw)
To: qemu-devel; +Cc: Dmitry Fleytman, Jason Wang, Leonid Bloch, Shmulik Ladkani
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!
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(-)
--
2.4.3
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v5 1/8] e1000: Cosmetic and alignment fixes
2015-11-09 14:59 [Qemu-devel] [PATCH v5 0/8] e1000: Various fixes and registers' implementation Leonid Bloch
@ 2015-11-09 14:59 ` Leonid Bloch
2015-11-09 14:59 ` [Qemu-devel] [PATCH v5 2/8] e1000: Add support for migrating the entire MAC registers' array Leonid Bloch
` (7 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: Leonid Bloch @ 2015-11-09 14:59 UTC (permalink / raw)
To: qemu-devel; +Cc: Dmitry Fleytman, Jason Wang, Leonid Bloch, Shmulik Ladkani
This fixes some alignment and cosmetic issues. The changes are made
in order that the following patches in this series will look like
integral parts of the code surrounding them, while conforming to the
coding style. Although some changes in unrelated areas are also made.
Signed-off-by: Leonid Bloch <leonid.bloch@ravellosystems.com>
Signed-off-by: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
---
hw/net/e1000.c | 166 ++++++++++++++++++++++++++++------------------------
hw/net/e1000_regs.h | 2 +-
2 files changed, 89 insertions(+), 79 deletions(-)
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 910de3a..da72776 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -41,20 +41,20 @@
#ifdef E1000_DEBUG
enum {
- DEBUG_GENERAL, DEBUG_IO, DEBUG_MMIO, DEBUG_INTERRUPT,
- DEBUG_RX, DEBUG_TX, DEBUG_MDIC, DEBUG_EEPROM,
- DEBUG_UNKNOWN, DEBUG_TXSUM, DEBUG_TXERR, DEBUG_RXERR,
+ DEBUG_GENERAL, DEBUG_IO, DEBUG_MMIO, DEBUG_INTERRUPT,
+ DEBUG_RX, DEBUG_TX, DEBUG_MDIC, DEBUG_EEPROM,
+ DEBUG_UNKNOWN, DEBUG_TXSUM, DEBUG_TXERR, DEBUG_RXERR,
DEBUG_RXFILTER, DEBUG_PHY, DEBUG_NOTYET,
};
-#define DBGBIT(x) (1<<DEBUG_##x)
+#define DBGBIT(x) (1<<DEBUG_##x)
static int debugflags = DBGBIT(TXERR) | DBGBIT(GENERAL);
-#define DBGOUT(what, fmt, ...) do { \
+#define DBGOUT(what, fmt, ...) do { \
if (debugflags & DBGBIT(what)) \
fprintf(stderr, "e1000: " fmt, ## __VA_ARGS__); \
} while (0)
#else
-#define DBGOUT(what, fmt, ...) do {} while (0)
+#define DBGOUT(what, fmt, ...) do {} while (0)
#endif
#define IOPORT_SIZE 0x40
@@ -118,7 +118,7 @@ typedef struct E1000State_st {
} tx;
struct {
- uint32_t val_in; // shifted in from guest driver
+ uint32_t val_in; /* shifted in from guest driver */
uint16_t bitnum_in;
uint16_t bitnum_out;
uint16_t reading;
@@ -155,19 +155,19 @@ typedef struct E1000BaseClass {
#define E1000_DEVICE_GET_CLASS(obj) \
OBJECT_GET_CLASS(E1000BaseClass, (obj), TYPE_E1000_BASE)
-#define defreg(x) x = (E1000_##x>>2)
+#define defreg(x) x = (E1000_##x>>2)
enum {
- defreg(CTRL), defreg(EECD), defreg(EERD), defreg(GPRC),
- defreg(GPTC), defreg(ICR), defreg(ICS), defreg(IMC),
- defreg(IMS), defreg(LEDCTL), defreg(MANC), defreg(MDIC),
- defreg(MPC), defreg(PBA), defreg(RCTL), defreg(RDBAH),
- defreg(RDBAL), defreg(RDH), defreg(RDLEN), defreg(RDT),
- defreg(STATUS), defreg(SWSM), defreg(TCTL), defreg(TDBAH),
- defreg(TDBAL), defreg(TDH), defreg(TDLEN), defreg(TDT),
- defreg(TORH), defreg(TORL), defreg(TOTH), defreg(TOTL),
- defreg(TPR), defreg(TPT), defreg(TXDCTL), defreg(WUFC),
- defreg(RA), defreg(MTA), defreg(CRCERRS),defreg(VFTA),
- defreg(VET), defreg(RDTR), defreg(RADV), defreg(TADV),
+ defreg(CTRL), defreg(EECD), defreg(EERD), defreg(GPRC),
+ defreg(GPTC), defreg(ICR), defreg(ICS), defreg(IMC),
+ defreg(IMS), defreg(LEDCTL), defreg(MANC), defreg(MDIC),
+ defreg(MPC), defreg(PBA), defreg(RCTL), defreg(RDBAH),
+ defreg(RDBAL), defreg(RDH), defreg(RDLEN), defreg(RDT),
+ defreg(STATUS), defreg(SWSM), defreg(TCTL), defreg(TDBAH),
+ defreg(TDBAL), defreg(TDH), defreg(TDLEN), defreg(TDT),
+ defreg(TORH), defreg(TORL), defreg(TOTH), defreg(TOTL),
+ defreg(TPR), defreg(TPT), defreg(TXDCTL), defreg(WUFC),
+ defreg(RA), defreg(MTA), defreg(CRCERRS), defreg(VFTA),
+ defreg(VET), defreg(RDTR), defreg(RADV), defreg(TADV),
defreg(ITR),
};
@@ -226,18 +226,18 @@ enum { NPHYWRITEOPS = ARRAY_SIZE(phyreg_writeops) };
enum { PHY_R = 1, PHY_W = 2, PHY_RW = PHY_R | PHY_W };
static const char phy_regcap[0x20] = {
- [PHY_STATUS] = PHY_R, [M88E1000_EXT_PHY_SPEC_CTRL] = PHY_RW,
- [PHY_ID1] = PHY_R, [M88E1000_PHY_SPEC_CTRL] = PHY_RW,
- [PHY_CTRL] = PHY_RW, [PHY_1000T_CTRL] = PHY_RW,
- [PHY_LP_ABILITY] = PHY_R, [PHY_1000T_STATUS] = PHY_R,
- [PHY_AUTONEG_ADV] = PHY_RW, [M88E1000_RX_ERR_CNTR] = PHY_R,
- [PHY_ID2] = PHY_R, [M88E1000_PHY_SPEC_STATUS] = PHY_R,
+ [PHY_STATUS] = PHY_R, [M88E1000_EXT_PHY_SPEC_CTRL] = PHY_RW,
+ [PHY_ID1] = PHY_R, [M88E1000_PHY_SPEC_CTRL] = PHY_RW,
+ [PHY_CTRL] = PHY_RW, [PHY_1000T_CTRL] = PHY_RW,
+ [PHY_LP_ABILITY] = PHY_R, [PHY_1000T_STATUS] = PHY_R,
+ [PHY_AUTONEG_ADV] = PHY_RW, [M88E1000_RX_ERR_CNTR] = PHY_R,
+ [PHY_ID2] = PHY_R, [M88E1000_PHY_SPEC_STATUS] = PHY_R,
[PHY_AUTONEG_EXP] = PHY_R,
};
/* PHY_ID2 documented in 8254x_GBe_SDM.pdf, pp. 250 */
static const uint16_t phy_reg_init[] = {
- [PHY_CTRL] = MII_CR_SPEED_SELECT_MSB |
+ [PHY_CTRL] = MII_CR_SPEED_SELECT_MSB |
MII_CR_FULL_DUPLEX |
MII_CR_AUTO_NEG_EN,
@@ -264,15 +264,15 @@ static const uint16_t phy_reg_init[] = {
};
static const uint32_t mac_reg_init[] = {
- [PBA] = 0x00100030,
- [LEDCTL] = 0x602,
- [CTRL] = E1000_CTRL_SWDPIN2 | E1000_CTRL_SWDPIN0 |
+ [PBA] = 0x00100030,
+ [LEDCTL] = 0x602,
+ [CTRL] = E1000_CTRL_SWDPIN2 | E1000_CTRL_SWDPIN0 |
E1000_CTRL_SPD_1000 | E1000_CTRL_SLU,
- [STATUS] = 0x80000000 | E1000_STATUS_GIO_MASTER_ENABLE |
+ [STATUS] = 0x80000000 | E1000_STATUS_GIO_MASTER_ENABLE |
E1000_STATUS_ASDV | E1000_STATUS_MTXCKOK |
E1000_STATUS_SPEED_1000 | E1000_STATUS_FD |
E1000_STATUS_LU,
- [MANC] = E1000_MANC_EN_MNG2HOST | E1000_MANC_RCV_TCO_EN |
+ [MANC] = E1000_MANC_EN_MNG2HOST | E1000_MANC_RCV_TCO_EN |
E1000_MANC_ARP_EN | E1000_MANC_0298_EN |
E1000_MANC_RMCP_EN,
};
@@ -510,17 +510,19 @@ set_eecd(E1000State *s, int index, uint32_t val)
s->eecd_state.old_eecd = val & (E1000_EECD_SK | E1000_EECD_CS |
E1000_EECD_DI|E1000_EECD_FWE_MASK|E1000_EECD_REQ);
- if (!(E1000_EECD_CS & val)) // CS inactive; nothing to do
- return;
- if (E1000_EECD_CS & (val ^ oldval)) { // CS rise edge; reset state
- s->eecd_state.val_in = 0;
- s->eecd_state.bitnum_in = 0;
- s->eecd_state.bitnum_out = 0;
- s->eecd_state.reading = 0;
+ if (!(E1000_EECD_CS & val)) { /* CS inactive; nothing to do */
+ return;
+ }
+ if (E1000_EECD_CS & (val ^ oldval)) { /* CS rise edge; reset state */
+ s->eecd_state.val_in = 0;
+ s->eecd_state.bitnum_in = 0;
+ s->eecd_state.bitnum_out = 0;
+ s->eecd_state.reading = 0;
}
- if (!(E1000_EECD_SK & (val ^ oldval))) // no clock edge
+ if (!(E1000_EECD_SK & (val ^ oldval))) { /* no clock edge */
return;
- if (!(E1000_EECD_SK & val)) { // falling edge
+ }
+ if (!(E1000_EECD_SK & val)) { /* falling edge */
s->eecd_state.bitnum_out++;
return;
}
@@ -621,12 +623,13 @@ xmit_seg(E1000State *s)
css = tp->ipcss;
DBGOUT(TXSUM, "frames %d size %d ipcss %d\n",
frames, tp->size, css);
- if (tp->ip) { // IPv4
+ if (tp->ip) { /* IPv4 */
stw_be_p(tp->data+css+2, tp->size - css);
stw_be_p(tp->data+css+4,
- be16_to_cpup((uint16_t *)(tp->data+css+4))+frames);
- } else // IPv6
+ be16_to_cpup((uint16_t *)(tp->data+css+4))+frames);
+ } else { /* IPv6 */
stw_be_p(tp->data+css+4, tp->size - css);
+ }
css = tp->tucss;
len = tp->size - css;
DBGOUT(TXSUM, "tcp %d tucss %d len %d\n", tp->tcp, css, len);
@@ -634,8 +637,8 @@ xmit_seg(E1000State *s)
sofar = frames * tp->mss;
stl_be_p(tp->data+css+4, ldl_be_p(tp->data+css+4)+sofar); /* seq */
if (tp->paylen - sofar > tp->mss)
- tp->data[css + 13] &= ~9; // PSH, FIN
- } else // UDP
+ tp->data[css + 13] &= ~9; /* PSH, FIN */
+ } else /* UDP */
stw_be_p(tp->data+css+4, len);
if (tp->sum_needed & E1000_TXD_POPTS_TXSM) {
unsigned int phsum;
@@ -657,8 +660,10 @@ xmit_seg(E1000State *s)
memmove(tp->data, tp->data + 4, 8);
memcpy(tp->data + 8, tp->vlan_header, 4);
e1000_send_packet(s, tp->vlan, tp->size + 4);
- } else
+ } else {
e1000_send_packet(s, tp->data, tp->size);
+ }
+
s->mac_reg[TPT]++;
s->mac_reg[GPTC]++;
n = s->mac_reg[TOTL];
@@ -679,7 +684,7 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
struct e1000_tx *tp = &s->tx;
s->mit_ide |= (txd_lower & E1000_TXD_CMD_IDE);
- if (dtype == E1000_TXD_CMD_DEXT) { // context descriptor
+ if (dtype == E1000_TXD_CMD_DEXT) { /* context descriptor */
op = le32_to_cpu(xp->cmd_and_length);
tp->ipcss = xp->lower_setup.ip_fields.ipcss;
tp->ipcso = xp->lower_setup.ip_fields.ipcso;
@@ -694,7 +699,7 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
tp->tcp = (op & E1000_TXD_CMD_TCP) ? 1 : 0;
tp->tse = (op & E1000_TXD_CMD_TSE) ? 1 : 0;
tp->tso_frames = 0;
- if (tp->tucso == 0) { // this is probably wrong
+ if (tp->tucso == 0) { /* this is probably wrong */
DBGOUT(TXSUM, "TCP/UDP: cso 0!\n");
tp->tucso = tp->tucss + (tp->tcp ? 16 : 6);
}
@@ -718,7 +723,7 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
stw_be_p(tp->vlan_header + 2,
le16_to_cpu(dp->upper.fields.special));
}
-
+
addr = le64_to_cpu(dp->buffer_addr);
if (tp->tse && tp->cptse) {
msh = tp->hdr_len + tp->mss;
@@ -1206,41 +1211,46 @@ set_ims(E1000State *s, int index, uint32_t val)
set_ics(s, 0, 0);
}
-#define getreg(x) [x] = mac_readreg
+#define getreg(x) [x] = mac_readreg
static uint32_t (*macreg_readops[])(E1000State *, int) = {
- getreg(PBA), getreg(RCTL), getreg(TDH), getreg(TXDCTL),
- getreg(WUFC), getreg(TDT), getreg(CTRL), getreg(LEDCTL),
- getreg(MANC), getreg(MDIC), getreg(SWSM), getreg(STATUS),
- getreg(TORL), getreg(TOTL), getreg(IMS), getreg(TCTL),
- getreg(RDH), getreg(RDT), getreg(VET), getreg(ICS),
- getreg(TDBAL), getreg(TDBAH), getreg(RDBAH), getreg(RDBAL),
- getreg(TDLEN), getreg(RDLEN), getreg(RDTR), getreg(RADV),
- getreg(TADV), getreg(ITR),
-
- [TOTH] = mac_read_clr8, [TORH] = mac_read_clr8, [GPRC] = mac_read_clr4,
- [GPTC] = mac_read_clr4, [TPR] = mac_read_clr4, [TPT] = mac_read_clr4,
- [ICR] = mac_icr_read, [EECD] = get_eecd, [EERD] = flash_eerd_read,
- [CRCERRS ... MPC] = &mac_readreg,
- [RA ... RA+31] = &mac_readreg,
- [MTA ... MTA+127] = &mac_readreg,
+ getreg(PBA), getreg(RCTL), getreg(TDH), getreg(TXDCTL),
+ getreg(WUFC), getreg(TDT), getreg(CTRL), getreg(LEDCTL),
+ getreg(MANC), getreg(MDIC), getreg(SWSM), getreg(STATUS),
+ getreg(TORL), getreg(TOTL), getreg(IMS), getreg(TCTL),
+ getreg(RDH), getreg(RDT), getreg(VET), getreg(ICS),
+ getreg(TDBAL), getreg(TDBAH), getreg(RDBAH), getreg(RDBAL),
+ getreg(TDLEN), getreg(RDLEN), getreg(RDTR), getreg(RADV),
+ getreg(TADV), getreg(ITR),
+
+ [TOTH] = mac_read_clr8, [TORH] = mac_read_clr8,
+ [GPRC] = mac_read_clr4, [GPTC] = mac_read_clr4,
+ [TPT] = mac_read_clr4, [TPR] = mac_read_clr4,
+ [ICR] = mac_icr_read, [EECD] = get_eecd,
+ [EERD] = flash_eerd_read,
+
+ [CRCERRS ... MPC] = &mac_readreg,
+ [RA ... RA+31] = &mac_readreg,
+ [MTA ... MTA+127] = &mac_readreg,
[VFTA ... VFTA+127] = &mac_readreg,
};
enum { NREADOPS = ARRAY_SIZE(macreg_readops) };
-#define putreg(x) [x] = mac_writereg
+#define putreg(x) [x] = mac_writereg
static void (*macreg_writeops[])(E1000State *, int, uint32_t) = {
- putreg(PBA), putreg(EERD), putreg(SWSM), putreg(WUFC),
- putreg(TDBAL), putreg(TDBAH), putreg(TXDCTL), putreg(RDBAH),
- putreg(RDBAL), putreg(LEDCTL), putreg(VET),
- [TDLEN] = set_dlen, [RDLEN] = set_dlen, [TCTL] = set_tctl,
- [TDT] = set_tctl, [MDIC] = set_mdic, [ICS] = set_ics,
- [TDH] = set_16bit, [RDH] = set_16bit, [RDT] = set_rdt,
- [IMC] = set_imc, [IMS] = set_ims, [ICR] = set_icr,
- [EECD] = set_eecd, [RCTL] = set_rx_control, [CTRL] = set_ctrl,
- [RDTR] = set_16bit, [RADV] = set_16bit, [TADV] = set_16bit,
- [ITR] = set_16bit,
- [RA ... RA+31] = &mac_writereg,
- [MTA ... MTA+127] = &mac_writereg,
+ putreg(PBA), putreg(EERD), putreg(SWSM), putreg(WUFC),
+ putreg(TDBAL), putreg(TDBAH), putreg(TXDCTL), putreg(RDBAH),
+ putreg(RDBAL), putreg(LEDCTL), putreg(VET),
+
+ [TDLEN] = set_dlen, [RDLEN] = set_dlen, [TCTL] = set_tctl,
+ [TDT] = set_tctl, [MDIC] = set_mdic, [ICS] = set_ics,
+ [TDH] = set_16bit, [RDH] = set_16bit, [RDT] = set_rdt,
+ [IMC] = set_imc, [IMS] = set_ims, [ICR] = set_icr,
+ [EECD] = set_eecd, [RCTL] = set_rx_control, [CTRL] = set_ctrl,
+ [RDTR] = set_16bit, [RADV] = set_16bit, [TADV] = set_16bit,
+ [ITR] = set_16bit,
+
+ [RA ... RA+31] = &mac_writereg,
+ [MTA ... MTA+127] = &mac_writereg,
[VFTA ... VFTA+127] = &mac_writereg,
};
diff --git a/hw/net/e1000_regs.h b/hw/net/e1000_regs.h
index 60b96aa..afd81cc 100644
--- a/hw/net/e1000_regs.h
+++ b/hw/net/e1000_regs.h
@@ -158,7 +158,7 @@
#define E1000_PHY_CTRL 0x00F10 /* PHY Control Register in CSR */
#define FEXTNVM_SW_CONFIG 0x0001
#define E1000_PBA 0x01000 /* Packet Buffer Allocation - RW */
-#define E1000_PBS 0x01008 /* Packet Buffer Size */
+#define E1000_PBS 0x01008 /* Packet Buffer Size - RW */
#define E1000_EEMNGCTL 0x01010 /* MNG EEprom Control */
#define E1000_FLASH_UPDATES 1000
#define E1000_EEARBC 0x01024 /* EEPROM Auto Read Bus Control */
--
2.4.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v5 2/8] e1000: Add support for migrating the entire MAC registers' array
2015-11-09 14:59 [Qemu-devel] [PATCH v5 0/8] e1000: Various fixes and registers' implementation Leonid Bloch
2015-11-09 14:59 ` [Qemu-devel] [PATCH v5 1/8] e1000: Cosmetic and alignment fixes Leonid Bloch
@ 2015-11-09 14:59 ` Leonid Bloch
2015-11-09 14:59 ` [Qemu-devel] [PATCH v5 3/8] e1000: Introduced an array to control the access to the MAC registers Leonid Bloch
` (6 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: Leonid Bloch @ 2015-11-09 14:59 UTC (permalink / raw)
To: qemu-devel; +Cc: Dmitry Fleytman, Jason Wang, Leonid Bloch, Shmulik Ladkani
This patch enables the migration of the entire array of MAC registers
during live migration. The entire array is just 128 KB long, so
practically no penalty should be felt when transmitting it, additionally
to the previously transmitted individual registers. The advantage here is
eliminating the need to introduce new vmstate subsections in the future,
when additional MAC registers will be implemented.
Backward compatibility is preserved by introducing the e1000-specific
boolean parameter "extra_mac_registers", which is on by default. Setting
it to off will enable migration to older versions of QEMU.
Additionally, this parameter can be used to control the implementation of
extra MAC registers in the future. I.e. new MAC registers may be set to
behave differently, or not to be active at all, if "extra_mac_registers"
will be set to "off", e.g.:
qemu-system-x86_64 -device e1000,extra_mac_registers=off,... ...
As mentioned above, the default value is "on".
Signed-off-by: Leonid Bloch <leonid.bloch@ravellosystems.com>
Signed-off-by: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
---
hw/net/e1000.c | 23 +++++++++++++++++++++++
include/hw/compat.h | 4 ++++
2 files changed, 27 insertions(+)
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index da72776..0e00afa 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -135,8 +135,10 @@ typedef struct E1000State_st {
/* Compatibility flags for migration to/from qemu 1.3.0 and older */
#define E1000_FLAG_AUTONEG_BIT 0
#define E1000_FLAG_MIT_BIT 1
+#define E1000_FLAG_MAC_BIT 2
#define E1000_FLAG_AUTONEG (1 << E1000_FLAG_AUTONEG_BIT)
#define E1000_FLAG_MIT (1 << E1000_FLAG_MIT_BIT)
+#define E1000_FLAG_MAC (1 << E1000_FLAG_MAC_BIT)
uint32_t compat_flags;
} E1000State;
@@ -1380,6 +1382,13 @@ static bool e1000_mit_state_needed(void *opaque)
return s->compat_flags & E1000_FLAG_MIT;
}
+static bool e1000_full_mac_needed(void *opaque)
+{
+ E1000State *s = opaque;
+
+ return s->compat_flags & E1000_FLAG_MAC;
+}
+
static const VMStateDescription vmstate_e1000_mit_state = {
.name = "e1000/mit_state",
.version_id = 1,
@@ -1395,6 +1404,17 @@ static const VMStateDescription vmstate_e1000_mit_state = {
}
};
+static const VMStateDescription vmstate_e1000_full_mac_state = {
+ .name = "e1000/full_mac_state",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .needed = e1000_full_mac_needed,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT32_ARRAY(mac_reg, E1000State, 0x8000),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
static const VMStateDescription vmstate_e1000 = {
.name = "e1000",
.version_id = 2,
@@ -1474,6 +1494,7 @@ static const VMStateDescription vmstate_e1000 = {
},
.subsections = (const VMStateDescription*[]) {
&vmstate_e1000_mit_state,
+ &vmstate_e1000_full_mac_state,
NULL
}
};
@@ -1606,6 +1627,8 @@ static Property e1000_properties[] = {
compat_flags, E1000_FLAG_AUTONEG_BIT, true),
DEFINE_PROP_BIT("mitigation", E1000State,
compat_flags, E1000_FLAG_MIT_BIT, true),
+ DEFINE_PROP_BIT("extra_mac_registers", E1000State,
+ compat_flags, E1000_FLAG_MAC_BIT, true),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/include/hw/compat.h b/include/hw/compat.h
index 93e71af..c394593 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -6,6 +6,10 @@
.driver = "virtio-blk-device",\
.property = "scsi",\
.value = "true",\
+ },{\
+ .driver = "e1000",\
+ .property = "extra_mac_registers",\
+ .value = "off",\
},
#define HW_COMPAT_2_3 \
--
2.4.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v5 3/8] e1000: Introduced an array to control the access to the MAC registers
2015-11-09 14:59 [Qemu-devel] [PATCH v5 0/8] e1000: Various fixes and registers' implementation Leonid Bloch
2015-11-09 14:59 ` [Qemu-devel] [PATCH v5 1/8] e1000: Cosmetic and alignment fixes Leonid Bloch
2015-11-09 14:59 ` [Qemu-devel] [PATCH v5 2/8] e1000: Add support for migrating the entire MAC registers' array Leonid Bloch
@ 2015-11-09 14:59 ` Leonid Bloch
2015-11-10 5:37 ` Jason Wang
2015-11-09 14:59 ` [Qemu-devel] [PATCH v5 4/8] e1000: Trivial implementation of various " Leonid Bloch
` (5 subsequent siblings)
8 siblings, 1 reply; 19+ messages in thread
From: Leonid Bloch @ 2015-11-09 14:59 UTC (permalink / raw)
To: qemu-devel; +Cc: Dmitry Fleytman, Jason Wang, Leonid Bloch, Shmulik Ladkani
The array of uint8_t's which is introduced here, contains useful metadata
about the MAC registers: if a register should be always accessible, or if
it is accessible, but partly implemented, or if the register requires a
certain compatibility flag to be accessed. Currently, 5 hypothetical flags
are supported (3 exist for e1000 so far) but if in the future more than 5
flags will be needed, the datatype of this array can simply be swapped for
a larger one.
This patch is intended to solve the following current problems:
1) On migration between different versions of QEMU, which differ by the
MAC registers implemented in them, some registers need not to be active if
a compatibility flag is set, in order to preserve the machine's state
perfectly for the older version. Checking this for each register
individually, would create a lot of clutter in the code.
2) Some registers are (or may be) only partly implemented (e.g.
placeholders that allow reading and writing, but lack other functions).
In such cases it is better to print a debug warning on read/write attempts.
As above, dealing with this functionality on a per-register level, would
require longer and more messy code.
Signed-off-by: Leonid Bloch <leonid.bloch@ravellosystems.com>
Signed-off-by: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
---
hw/net/e1000.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 73 insertions(+), 12 deletions(-)
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 0e00afa..2bc533f 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -142,6 +142,8 @@ typedef struct E1000State_st {
uint32_t compat_flags;
} E1000State;
+#define chkflag(x) (s->compat_flags & E1000_FLAG_##x)
+
typedef struct E1000BaseClass {
PCIDeviceClass parent_class;
uint16_t phy_id2;
@@ -195,8 +197,7 @@ e1000_link_up(E1000State *s)
static bool
have_autoneg(E1000State *s)
{
- return (s->compat_flags & E1000_FLAG_AUTONEG) &&
- (s->phy_reg[PHY_CTRL] & MII_CR_AUTO_NEG_EN);
+ return chkflag(AUTONEG) && (s->phy_reg[PHY_CTRL] & MII_CR_AUTO_NEG_EN);
}
static void
@@ -321,7 +322,7 @@ set_interrupt_cause(E1000State *s, int index, uint32_t val)
if (s->mit_timer_on) {
return;
}
- if (s->compat_flags & E1000_FLAG_MIT) {
+ if (chkflag(MIT)) {
/* Compute the next mitigation delay according to pending
* interrupts and the current values of RADV (provided
* RDTR!=0), TADV and ITR.
@@ -1258,6 +1259,43 @@ static void (*macreg_writeops[])(E1000State *, int, uint32_t) = {
enum { NWRITEOPS = ARRAY_SIZE(macreg_writeops) };
+enum { MAC_ACCESS_ALWAYS = 1, MAC_ACCESS_PARTIAL = 2,
+ MAC_ACCESS_FLAG_NEEDED = 4 };
+
+#define markflag(x) ((E1000_FLAG_##x << 3) | MAC_ACCESS_FLAG_NEEDED)
+/* In the array below the meaning of the bits is: [f|f|f|f|f|n|p|a]
+ * f - flag bits (up to 5 possible flags)
+ * n - flag needed
+ * p - partially implenented
+ * a - access enabled always
+ * n=p=a=0 - not implemented or unknown */
+static const uint8_t mac_reg_chk[0x8000] = {
+ [CTRL] = MAC_ACCESS_ALWAYS, [EECD] = MAC_ACCESS_ALWAYS,
+ [GPTC] = MAC_ACCESS_ALWAYS, [ICR] = MAC_ACCESS_ALWAYS,
+ [IMS] = MAC_ACCESS_ALWAYS, [LEDCTL] = MAC_ACCESS_ALWAYS,
+ [MPC] = MAC_ACCESS_ALWAYS, [PBA] = MAC_ACCESS_ALWAYS,
+ [RDBAL] = MAC_ACCESS_ALWAYS, [RDH] = MAC_ACCESS_ALWAYS,
+ [STATUS] = MAC_ACCESS_ALWAYS, [SWSM] = MAC_ACCESS_ALWAYS,
+ [TDBAL] = MAC_ACCESS_ALWAYS, [TDH] = MAC_ACCESS_ALWAYS,
+ [TORH] = MAC_ACCESS_ALWAYS, [TORL] = MAC_ACCESS_ALWAYS,
+ [TPR] = MAC_ACCESS_ALWAYS, [TPT] = MAC_ACCESS_ALWAYS,
+ [RA] = MAC_ACCESS_ALWAYS, [MTA] = MAC_ACCESS_ALWAYS,
+ [EERD] = MAC_ACCESS_ALWAYS, [GPRC] = MAC_ACCESS_ALWAYS,
+ [ICS] = MAC_ACCESS_ALWAYS, [IMC] = MAC_ACCESS_ALWAYS,
+ [MANC] = MAC_ACCESS_ALWAYS, [MDIC] = MAC_ACCESS_ALWAYS,
+ [RCTL] = MAC_ACCESS_ALWAYS, [RDBAH] = MAC_ACCESS_ALWAYS,
+ [RDLEN] = MAC_ACCESS_ALWAYS, [RDT] = MAC_ACCESS_ALWAYS,
+ [TCTL] = MAC_ACCESS_ALWAYS, [TDBAH] = MAC_ACCESS_ALWAYS,
+ [TDLEN] = MAC_ACCESS_ALWAYS, [TDT] = MAC_ACCESS_ALWAYS,
+ [TOTH] = MAC_ACCESS_ALWAYS, [TOTL] = MAC_ACCESS_ALWAYS,
+ [TXDCTL] = MAC_ACCESS_ALWAYS, [WUFC] = MAC_ACCESS_ALWAYS,
+ [CRCERRS] = MAC_ACCESS_ALWAYS, [VFTA] = MAC_ACCESS_ALWAYS,
+ [VET] = MAC_ACCESS_ALWAYS,
+
+ [RDTR] = markflag(MIT), [TADV] = markflag(MIT),
+ [RADV] = markflag(MIT), [ITR] = markflag(MIT),
+};
+
static void
e1000_mmio_write(void *opaque, hwaddr addr, uint64_t val,
unsigned size)
@@ -1266,9 +1304,21 @@ e1000_mmio_write(void *opaque, hwaddr addr, uint64_t val,
unsigned int index = (addr & 0x1ffff) >> 2;
if (index < NWRITEOPS && macreg_writeops[index]) {
- macreg_writeops[index](s, index, val);
+ if ((mac_reg_chk[index] & MAC_ACCESS_ALWAYS)
+ || ((mac_reg_chk[index] & MAC_ACCESS_FLAG_NEEDED)
+ && (s->compat_flags & (mac_reg_chk[index] >> 3)))) {
+ if (mac_reg_chk[index] & MAC_ACCESS_PARTIAL) {
+ DBGOUT(GENERAL, "Writing to register at offset: 0x%08x. "
+ "It is not fully implemented.\n", index<<2);
+ }
+ macreg_writeops[index](s, index, val);
+ } else if (mac_reg_chk[index] & MAC_ACCESS_FLAG_NEEDED) {
+ DBGOUT(MMIO, "MMIO write attempt to disabled reg. addr=0x%08x\n",
+ index<<2);
+ }
} else if (index < NREADOPS && macreg_readops[index]) {
- DBGOUT(MMIO, "e1000_mmio_writel RO %x: 0x%04"PRIx64"\n", index<<2, val);
+ DBGOUT(MMIO, "e1000_mmio_writel RO %x: 0x%04"PRIx64"\n",
+ index<<2, val);
} else {
DBGOUT(UNKNOWN, "MMIO unknown write addr=0x%08x,val=0x%08"PRIx64"\n",
index<<2, val);
@@ -1281,11 +1331,22 @@ e1000_mmio_read(void *opaque, hwaddr addr, unsigned size)
E1000State *s = opaque;
unsigned int index = (addr & 0x1ffff) >> 2;
- if (index < NREADOPS && macreg_readops[index])
- {
- return macreg_readops[index](s, index);
+ if (index < NREADOPS && macreg_readops[index]) {
+ if ((mac_reg_chk[index] & MAC_ACCESS_ALWAYS)
+ || ((mac_reg_chk[index] & MAC_ACCESS_FLAG_NEEDED)
+ && (s->compat_flags & (mac_reg_chk[index] >> 3)))) {
+ if (mac_reg_chk[index] & MAC_ACCESS_PARTIAL) {
+ DBGOUT(GENERAL, "Reading register at offset: 0x%08x. "
+ "It is not fully implemented.\n", index<<2);
+ }
+ return macreg_readops[index](s, index);
+ } else if (mac_reg_chk[index] & MAC_ACCESS_FLAG_NEEDED) {
+ DBGOUT(MMIO, "MMIO read attempt of disabled reg. addr=0x%08x\n",
+ index<<2);
+ }
+ } else {
+ DBGOUT(UNKNOWN, "MMIO unknown read addr=0x%08x\n", index<<2);
}
- DBGOUT(UNKNOWN, "MMIO unknown read addr=0x%08x\n", index<<2);
return 0;
}
@@ -1352,7 +1413,7 @@ static int e1000_post_load(void *opaque, int version_id)
E1000State *s = opaque;
NetClientState *nc = qemu_get_queue(s->nic);
- if (!(s->compat_flags & E1000_FLAG_MIT)) {
+ if (!chkflag(MIT)) {
s->mac_reg[ITR] = s->mac_reg[RDTR] = s->mac_reg[RADV] =
s->mac_reg[TADV] = 0;
s->mit_irq_level = false;
@@ -1379,14 +1440,14 @@ static bool e1000_mit_state_needed(void *opaque)
{
E1000State *s = opaque;
- return s->compat_flags & E1000_FLAG_MIT;
+ return chkflag(MIT);
}
static bool e1000_full_mac_needed(void *opaque)
{
E1000State *s = opaque;
- return s->compat_flags & E1000_FLAG_MAC;
+ return chkflag(MAC);
}
static const VMStateDescription vmstate_e1000_mit_state = {
--
2.4.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v5 4/8] e1000: Trivial implementation of various MAC registers
2015-11-09 14:59 [Qemu-devel] [PATCH v5 0/8] e1000: Various fixes and registers' implementation Leonid Bloch
` (2 preceding siblings ...)
2015-11-09 14:59 ` [Qemu-devel] [PATCH v5 3/8] e1000: Introduced an array to control the access to the MAC registers Leonid Bloch
@ 2015-11-09 14:59 ` Leonid Bloch
2015-11-09 14:59 ` [Qemu-devel] [PATCH v5 5/8] e1000: Fixing the received/transmitted packets' counters Leonid Bloch
` (4 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: Leonid Bloch @ 2015-11-09 14:59 UTC (permalink / raw)
To: qemu-devel; +Cc: Dmitry Fleytman, Jason Wang, Leonid Bloch, Shmulik Ladkani
These registers appear in Intel's specs, but were not implemented.
These registers are now implemented trivially, i.e. they are initiated
with zero values, and if they are RW, they can be written or read by the
driver, or read only if they are R (essentially retaining their zero
values). For these registers no other procedures are performed.
For the trivially implemented Diagnostic registers, a debug warning is
produced on read/write attempts.
The registers implemented here are:
Transmit:
RW: AIT
Management:
RW: WUC WUS IPAV IP6AT* IP4AT* FFLT* WUPM* FFMT* FFVT*
Diagnostic:
RW: RDFH RDFT RDFHS RDFTS RDFPC PBM* TDFH TDFT TDFHS
TDFTS TDFPC
Statistic:
RW: FCRUC
R: RNBC TSCTFC MGTPRC MGTPDC MGTPTC RFC RJC SCC ECOL
LATECOL MCC COLC DC TNCRS SEC CEXTERR RLEC XONRXC
XONTXC XOFFRXC XOFFTXC
Signed-off-by: Leonid Bloch <leonid.bloch@ravellosystems.com>
Signed-off-by: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
---
hw/net/e1000.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++--
hw/net/e1000_regs.h | 6 ++++
2 files changed, 100 insertions(+), 3 deletions(-)
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 2bc533f..28fa59c 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -172,7 +172,17 @@ enum {
defreg(TPR), defreg(TPT), defreg(TXDCTL), defreg(WUFC),
defreg(RA), defreg(MTA), defreg(CRCERRS), defreg(VFTA),
defreg(VET), defreg(RDTR), defreg(RADV), defreg(TADV),
- defreg(ITR),
+ defreg(ITR), defreg(FCRUC), defreg(TDFH), defreg(TDFT),
+ defreg(TDFHS), defreg(TDFTS), defreg(TDFPC), defreg(RDFH),
+ defreg(RDFT), defreg(RDFHS), defreg(RDFTS), defreg(RDFPC),
+ defreg(IPAV), defreg(WUC), defreg(WUS), defreg(AIT),
+ defreg(IP6AT), defreg(IP4AT), defreg(FFLT), defreg(FFMT),
+ defreg(FFVT), defreg(WUPM), defreg(PBM), defreg(SCC),
+ defreg(ECOL), defreg(MCC), defreg(LATECOL), defreg(COLC),
+ defreg(DC), defreg(TNCRS), defreg(SEC), defreg(CEXTERR),
+ defreg(RLEC), defreg(XONRXC), defreg(XONTXC), defreg(XOFFRXC),
+ defreg(XOFFTXC), defreg(RFC), defreg(RJC), defreg(RNBC),
+ defreg(TSCTFC), defreg(MGTPRC), defreg(MGTPDC), defreg(MGTPTC)
};
static void
@@ -1122,6 +1132,30 @@ mac_readreg(E1000State *s, int index)
}
static uint32_t
+mac_low4_read(E1000State *s, int index)
+{
+ return s->mac_reg[index] & 0xf;
+}
+
+static uint32_t
+mac_low11_read(E1000State *s, int index)
+{
+ return s->mac_reg[index] & 0x7ff;
+}
+
+static uint32_t
+mac_low13_read(E1000State *s, int index)
+{
+ return s->mac_reg[index] & 0x1fff;
+}
+
+static uint32_t
+mac_low16_read(E1000State *s, int index)
+{
+ return s->mac_reg[index] & 0xffff;
+}
+
+static uint32_t
mac_icr_read(E1000State *s, int index)
{
uint32_t ret = s->mac_reg[ICR];
@@ -1223,18 +1257,37 @@ static uint32_t (*macreg_readops[])(E1000State *, int) = {
getreg(RDH), getreg(RDT), getreg(VET), getreg(ICS),
getreg(TDBAL), getreg(TDBAH), getreg(RDBAH), getreg(RDBAL),
getreg(TDLEN), getreg(RDLEN), getreg(RDTR), getreg(RADV),
- getreg(TADV), getreg(ITR),
+ getreg(TADV), getreg(ITR), getreg(FCRUC), getreg(IPAV),
+ getreg(WUC), getreg(WUS), getreg(SCC), getreg(ECOL),
+ getreg(MCC), getreg(LATECOL), getreg(COLC), getreg(DC),
+ 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),
[TOTH] = mac_read_clr8, [TORH] = mac_read_clr8,
[GPRC] = mac_read_clr4, [GPTC] = mac_read_clr4,
[TPT] = mac_read_clr4, [TPR] = mac_read_clr4,
[ICR] = mac_icr_read, [EECD] = get_eecd,
[EERD] = flash_eerd_read,
+ [RDFH] = mac_low13_read, [RDFT] = mac_low13_read,
+ [RDFHS] = mac_low13_read, [RDFTS] = mac_low13_read,
+ [RDFPC] = mac_low13_read,
+ [TDFH] = mac_low11_read, [TDFT] = mac_low11_read,
+ [TDFHS] = mac_low13_read, [TDFTS] = mac_low13_read,
+ [TDFPC] = mac_low13_read,
+ [AIT] = mac_low16_read,
[CRCERRS ... MPC] = &mac_readreg,
+ [IP6AT ... IP6AT+3] = &mac_readreg, [IP4AT ... IP4AT+6] = &mac_readreg,
+ [FFLT ... FFLT+6] = &mac_low11_read,
[RA ... RA+31] = &mac_readreg,
+ [WUPM ... WUPM+31] = &mac_readreg,
[MTA ... MTA+127] = &mac_readreg,
[VFTA ... VFTA+127] = &mac_readreg,
+ [FFMT ... FFMT+254] = &mac_low4_read,
+ [FFVT ... FFVT+254] = &mac_readreg,
+ [PBM ... PBM+16383] = &mac_readreg,
};
enum { NREADOPS = ARRAY_SIZE(macreg_readops) };
@@ -1242,7 +1295,11 @@ enum { NREADOPS = ARRAY_SIZE(macreg_readops) };
static void (*macreg_writeops[])(E1000State *, int, uint32_t) = {
putreg(PBA), putreg(EERD), putreg(SWSM), putreg(WUFC),
putreg(TDBAL), putreg(TDBAH), putreg(TXDCTL), putreg(RDBAH),
- putreg(RDBAL), putreg(LEDCTL), putreg(VET),
+ putreg(RDBAL), putreg(LEDCTL), putreg(VET), putreg(FCRUC),
+ putreg(TDFH), putreg(TDFT), putreg(TDFHS), putreg(TDFTS),
+ putreg(TDFPC), putreg(RDFH), putreg(RDFT), putreg(RDFHS),
+ putreg(RDFTS), putreg(RDFPC), putreg(IPAV), putreg(WUC),
+ putreg(WUS), putreg(AIT),
[TDLEN] = set_dlen, [RDLEN] = set_dlen, [TCTL] = set_tctl,
[TDT] = set_tctl, [MDIC] = set_mdic, [ICS] = set_ics,
@@ -1252,9 +1309,14 @@ static void (*macreg_writeops[])(E1000State *, int, uint32_t) = {
[RDTR] = set_16bit, [RADV] = set_16bit, [TADV] = set_16bit,
[ITR] = set_16bit,
+ [IP6AT ... IP6AT+3] = &mac_writereg, [IP4AT ... IP4AT+6] = &mac_writereg,
+ [FFLT ... FFLT+6] = &mac_writereg,
[RA ... RA+31] = &mac_writereg,
+ [WUPM ... WUPM+31] = &mac_writereg,
[MTA ... MTA+127] = &mac_writereg,
[VFTA ... VFTA+127] = &mac_writereg,
+ [FFMT ... FFMT+254] = &mac_writereg, [FFVT ... FFVT+254] = &mac_writereg,
+ [PBM ... PBM+16383] = &mac_writereg,
};
enum { NWRITEOPS = ARRAY_SIZE(macreg_writeops) };
@@ -1294,6 +1356,35 @@ static const uint8_t mac_reg_chk[0x8000] = {
[RDTR] = markflag(MIT), [TADV] = markflag(MIT),
[RADV] = markflag(MIT), [ITR] = markflag(MIT),
+
+ [IPAV] = markflag(MAC), [WUC] = markflag(MAC),
+ [IP6AT] = markflag(MAC), [IP4AT] = markflag(MAC),
+ [FFVT] = markflag(MAC), [WUPM] = markflag(MAC),
+ [ECOL] = markflag(MAC), [MCC] = markflag(MAC),
+ [DC] = markflag(MAC), [TNCRS] = markflag(MAC),
+ [RLEC] = markflag(MAC), [XONRXC] = markflag(MAC),
+ [XOFFTXC] = markflag(MAC), [RFC] = markflag(MAC),
+ [TSCTFC] = markflag(MAC), [MGTPRC] = markflag(MAC),
+ [WUS] = markflag(MAC), [AIT] = markflag(MAC),
+ [FFLT] = markflag(MAC), [FFMT] = markflag(MAC),
+ [SCC] = markflag(MAC), [FCRUC] = markflag(MAC),
+ [LATECOL] = markflag(MAC), [COLC] = markflag(MAC),
+ [SEC] = markflag(MAC), [CEXTERR] = markflag(MAC),
+ [XONTXC] = markflag(MAC), [XOFFRXC] = markflag(MAC),
+ [RJC] = markflag(MAC), [RNBC] = markflag(MAC),
+ [MGTPDC] = markflag(MAC), [MGTPTC] = markflag(MAC),
+
+ [TDFH] = markflag(MAC) | MAC_ACCESS_PARTIAL,
+ [TDFT] = markflag(MAC) | MAC_ACCESS_PARTIAL,
+ [TDFHS] = markflag(MAC) | MAC_ACCESS_PARTIAL,
+ [TDFTS] = markflag(MAC) | MAC_ACCESS_PARTIAL,
+ [TDFPC] = markflag(MAC) | MAC_ACCESS_PARTIAL,
+ [RDFH] = markflag(MAC) | MAC_ACCESS_PARTIAL,
+ [RDFT] = markflag(MAC) | MAC_ACCESS_PARTIAL,
+ [RDFHS] = markflag(MAC) | MAC_ACCESS_PARTIAL,
+ [RDFTS] = markflag(MAC) | MAC_ACCESS_PARTIAL,
+ [RDFPC] = markflag(MAC) | MAC_ACCESS_PARTIAL,
+ [PBM] = markflag(MAC) | MAC_ACCESS_PARTIAL,
};
static void
diff --git a/hw/net/e1000_regs.h b/hw/net/e1000_regs.h
index afd81cc..1c40244 100644
--- a/hw/net/e1000_regs.h
+++ b/hw/net/e1000_regs.h
@@ -158,6 +158,7 @@
#define E1000_PHY_CTRL 0x00F10 /* PHY Control Register in CSR */
#define FEXTNVM_SW_CONFIG 0x0001
#define E1000_PBA 0x01000 /* Packet Buffer Allocation - RW */
+#define E1000_PBM 0x10000 /* Packet Buffer Memory - RW */
#define E1000_PBS 0x01008 /* Packet Buffer Size - RW */
#define E1000_EEMNGCTL 0x01010 /* MNG EEprom Control */
#define E1000_FLASH_UPDATES 1000
@@ -191,6 +192,11 @@
#define E1000_RAID 0x02C08 /* Receive Ack Interrupt Delay - RW */
#define E1000_TXDMAC 0x03000 /* TX DMA Control - RW */
#define E1000_KABGTXD 0x03004 /* AFE Band Gap Transmit Ref Data */
+#define E1000_RDFH 0x02410 /* Receive Data FIFO Head Register - RW */
+#define E1000_RDFT 0x02418 /* Receive Data FIFO Tail Register - RW */
+#define E1000_RDFHS 0x02420 /* Receive Data FIFO Head Saved Register - RW */
+#define E1000_RDFTS 0x02428 /* Receive Data FIFO Tail Saved Register - RW */
+#define E1000_RDFPC 0x02430 /* Receive Data FIFO Packet Count - RW */
#define E1000_TDFH 0x03410 /* TX Data FIFO Head - RW */
#define E1000_TDFT 0x03418 /* TX Data FIFO Tail - RW */
#define E1000_TDFHS 0x03420 /* TX Data FIFO Head Saved - RW */
--
2.4.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v5 5/8] e1000: Fixing the received/transmitted packets' counters
2015-11-09 14:59 [Qemu-devel] [PATCH v5 0/8] e1000: Various fixes and registers' implementation Leonid Bloch
` (3 preceding siblings ...)
2015-11-09 14:59 ` [Qemu-devel] [PATCH v5 4/8] e1000: Trivial implementation of various " Leonid Bloch
@ 2015-11-09 14:59 ` Leonid Bloch
2015-11-09 14:59 ` [Qemu-devel] [PATCH v5 6/8] e1000: Fixing the received/transmitted octets' counters Leonid Bloch
` (3 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: Leonid Bloch @ 2015-11-09 14:59 UTC (permalink / raw)
To: qemu-devel; +Cc: Dmitry Fleytman, Jason Wang, Leonid Bloch, Shmulik Ladkani
According to Intel's specs, these counters (as the other Statistic
registers) stick at 0xffffffff when this maximal value is reached.
Previously, they would reset after the max. value.
Signed-off-by: Leonid Bloch <leonid.bloch@ravellosystems.com>
Signed-off-by: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
---
hw/net/e1000.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 28fa59c..c301637 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -580,6 +580,14 @@ putsum(uint8_t *data, uint32_t n, uint32_t sloc, uint32_t css, uint32_t cse)
}
}
+static inline void
+inc_reg_if_not_full(E1000State *s, int index)
+{
+ if (s->mac_reg[index] != 0xffffffff) {
+ s->mac_reg[index]++;
+ }
+}
+
static inline int
vlan_enabled(E1000State *s)
{
@@ -677,8 +685,8 @@ xmit_seg(E1000State *s)
e1000_send_packet(s, tp->data, tp->size);
}
- s->mac_reg[TPT]++;
- s->mac_reg[GPTC]++;
+ inc_reg_if_not_full(s, TPT);
+ s->mac_reg[GPTC] = s->mac_reg[TPT];
n = s->mac_reg[TOTL];
if ((s->mac_reg[TOTL] += s->tx.size) < n)
s->mac_reg[TOTH]++;
@@ -1091,8 +1099,8 @@ e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt)
}
} while (desc_offset < total_size);
- s->mac_reg[GPRC]++;
- s->mac_reg[TPR]++;
+ inc_reg_if_not_full(s, TPR);
+ s->mac_reg[GPRC] = s->mac_reg[TPR];
/* TOR - Total Octets Received:
* This register includes bytes received in a packet from the <Destination
* Address> field through the <CRC> field, inclusively.
--
2.4.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v5 6/8] e1000: Fixing the received/transmitted octets' counters
2015-11-09 14:59 [Qemu-devel] [PATCH v5 0/8] e1000: Various fixes and registers' implementation Leonid Bloch
` (4 preceding siblings ...)
2015-11-09 14:59 ` [Qemu-devel] [PATCH v5 5/8] e1000: Fixing the received/transmitted packets' counters Leonid Bloch
@ 2015-11-09 14:59 ` Leonid Bloch
2015-11-09 14:59 ` [Qemu-devel] [PATCH v5 7/8] e1000: Fixing the packet address filtering procedure Leonid Bloch
` (2 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: Leonid Bloch @ 2015-11-09 14:59 UTC (permalink / raw)
To: qemu-devel; +Cc: Dmitry Fleytman, Jason Wang, Leonid Bloch, Shmulik Ladkani
Previously, these 64-bit registers did not stick at their maximal
values when (and if) they reached them, as they should do, according to
the specs.
This patch introduces a function that takes care of such registers,
avoiding code duplication, making the relevant parts more compatible
with the QEMU coding style, while ensuring that in the unlikely case
of reaching the maximal value, the counter will stick there, as it
supposed to.
Signed-off-by: Leonid Bloch <leonid.bloch@ravellosystems.com>
Signed-off-by: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
---
hw/net/e1000.c | 26 ++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index c301637..c262663 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -588,6 +588,20 @@ inc_reg_if_not_full(E1000State *s, int index)
}
}
+static void
+grow_8reg_if_not_full(E1000State *s, int index, int size)
+{
+ uint64_t sum = s->mac_reg[index] | (uint64_t)s->mac_reg[index+1] << 32;
+
+ if (sum + size < sum) {
+ sum = ~0ULL;
+ } else {
+ sum += size;
+ }
+ s->mac_reg[index] = sum;
+ s->mac_reg[index+1] = sum >> 32;
+}
+
static inline int
vlan_enabled(E1000State *s)
{
@@ -637,7 +651,7 @@ static void
xmit_seg(E1000State *s)
{
uint16_t len, *sp;
- unsigned int frames = s->tx.tso_frames, css, sofar, n;
+ unsigned int frames = s->tx.tso_frames, css, sofar;
struct e1000_tx *tp = &s->tx;
if (tp->tse && tp->cptse) {
@@ -686,10 +700,8 @@ xmit_seg(E1000State *s)
}
inc_reg_if_not_full(s, TPT);
+ grow_8reg_if_not_full(s, TOTL, s->tx.size);
s->mac_reg[GPTC] = s->mac_reg[TPT];
- n = s->mac_reg[TOTL];
- if ((s->mac_reg[TOTL] += s->tx.size) < n)
- s->mac_reg[TOTH]++;
}
static void
@@ -1104,11 +1116,9 @@ e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt)
/* TOR - Total Octets Received:
* This register includes bytes received in a packet from the <Destination
* Address> field through the <CRC> field, inclusively.
+ * Always include FCS length (4) in size.
*/
- n = s->mac_reg[TORL] + size + /* Always include FCS length. */ 4;
- if (n < s->mac_reg[TORL])
- s->mac_reg[TORH]++;
- s->mac_reg[TORL] = n;
+ grow_8reg_if_not_full(s, TORL, size+4);
n = E1000_ICS_RXT0;
if ((rdt = s->mac_reg[RDT]) < s->mac_reg[RDH])
--
2.4.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v5 7/8] e1000: Fixing the packet address filtering procedure
2015-11-09 14:59 [Qemu-devel] [PATCH v5 0/8] e1000: Various fixes and registers' implementation Leonid Bloch
` (5 preceding siblings ...)
2015-11-09 14:59 ` [Qemu-devel] [PATCH v5 6/8] e1000: Fixing the received/transmitted octets' counters Leonid Bloch
@ 2015-11-09 14:59 ` Leonid Bloch
2015-11-09 14:59 ` [Qemu-devel] [PATCH v5 8/8] e1000: Implementing various counters Leonid Bloch
2015-11-10 6:21 ` [Qemu-devel] [PATCH v5 0/8] e1000: Various fixes and registers' implementation Jason Wang
8 siblings, 0 replies; 19+ messages in thread
From: Leonid Bloch @ 2015-11-09 14:59 UTC (permalink / raw)
To: qemu-devel; +Cc: Dmitry Fleytman, Jason Wang, Leonid Bloch, Shmulik Ladkani
Previously, if promiscuous unicast was enabled, a packet was received
straight away, even if it was a multicast or a broadcast packet. This
patch fixes that behavior, while making the filtering procedure a bit
more human-readable.
Signed-off-by: Leonid Bloch <leonid.bloch@ravellosystems.com>
Signed-off-by: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
---
hw/net/e1000.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index c262663..75347b0 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -872,6 +872,7 @@ receive_filter(E1000State *s, const uint8_t *buf, int size)
static const uint8_t bcast[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
static const int mta_shift[] = {4, 3, 2, 0};
uint32_t f, rctl = s->mac_reg[RCTL], ra[2], *rp;
+ int isbcast = !memcmp(buf, bcast, sizeof bcast), ismcast = (buf[0] & 1);
if (is_vlan_packet(s, buf) && vlan_rx_filter_enabled(s)) {
uint16_t vid = be16_to_cpup((uint16_t *)(buf + 14));
@@ -881,14 +882,17 @@ receive_filter(E1000State *s, const uint8_t *buf, int size)
return 0;
}
- if (rctl & E1000_RCTL_UPE) // promiscuous
+ if (!isbcast && !ismcast && (rctl & E1000_RCTL_UPE)) { /* promiscuous ucast */
return 1;
+ }
- if ((buf[0] & 1) && (rctl & E1000_RCTL_MPE)) // promiscuous mcast
+ if (ismcast && (rctl & E1000_RCTL_MPE)) { /* promiscuous mcast */
return 1;
+ }
- if ((rctl & E1000_RCTL_BAM) && !memcmp(buf, bcast, sizeof bcast))
+ if (isbcast && (rctl & E1000_RCTL_BAM)) { /* broadcast enabled */
return 1;
+ }
for (rp = s->mac_reg + RA; rp < s->mac_reg + RA + 32; rp += 2) {
if (!(rp[1] & E1000_RAH_AV))
--
2.4.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v5 8/8] e1000: Implementing various counters
2015-11-09 14:59 [Qemu-devel] [PATCH v5 0/8] e1000: Various fixes and registers' implementation Leonid Bloch
` (6 preceding siblings ...)
2015-11-09 14:59 ` [Qemu-devel] [PATCH v5 7/8] e1000: Fixing the packet address filtering procedure Leonid Bloch
@ 2015-11-09 14:59 ` Leonid Bloch
2015-11-10 6:21 ` [Qemu-devel] [PATCH v5 0/8] e1000: Various fixes and registers' implementation Jason Wang
8 siblings, 0 replies; 19+ messages in thread
From: Leonid Bloch @ 2015-11-09 14:59 UTC (permalink / raw)
To: qemu-devel; +Cc: Dmitry Fleytman, Jason Wang, Leonid Bloch, Shmulik Ladkani
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.bloch@ravellosystems.com>
Signed-off-by: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
---
hw/net/e1000.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 85 insertions(+), 5 deletions(-)
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 75347b0..ef42326 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -37,6 +37,8 @@
#include "e1000_regs.h"
+static const uint8_t bcast[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
+
#define E1000_DEBUG
#ifdef E1000_DEBUG
@@ -182,7 +184,13 @@ enum {
defreg(DC), defreg(TNCRS), defreg(SEC), defreg(CEXTERR),
defreg(RLEC), defreg(XONRXC), defreg(XONTXC), defreg(XOFFRXC),
defreg(XOFFTXC), defreg(RFC), defreg(RJC), defreg(RNBC),
- defreg(TSCTFC), defreg(MGTPRC), defreg(MGTPDC), defreg(MGTPTC)
+ defreg(TSCTFC), defreg(MGTPRC), defreg(MGTPDC), defreg(MGTPTC),
+ defreg(RUC), defreg(ROC), defreg(GORCL), defreg(GORCH),
+ defreg(GOTCL), defreg(GOTCH), defreg(BPRC), defreg(MPRC),
+ defreg(TSCTC), defreg(PRC64), defreg(PRC127), defreg(PRC255),
+ defreg(PRC511), defreg(PRC1023), defreg(PRC1522), defreg(PTC64),
+ defreg(PTC127), defreg(PTC255), defreg(PTC511), defreg(PTC1023),
+ defreg(PTC1522), defreg(MPTC), defreg(BPTC)
};
static void
@@ -588,6 +596,16 @@ inc_reg_if_not_full(E1000State *s, int index)
}
}
+static inline void
+inc_tx_bcast_or_mcast_count(E1000State *s, const unsigned char *arr)
+{
+ if (!memcmp(arr, bcast, sizeof bcast)) {
+ inc_reg_if_not_full(s, BPTC);
+ } else if (arr[0] & 1) {
+ inc_reg_if_not_full(s, MPTC);
+ }
+}
+
static void
grow_8reg_if_not_full(E1000State *s, int index, int size)
{
@@ -602,6 +620,24 @@ grow_8reg_if_not_full(E1000State *s, int index, int size)
s->mac_reg[index+1] = sum >> 32;
}
+static void
+increase_size_stats(E1000State *s, const int *size_regs, int size)
+{
+ if (size > 1023) {
+ inc_reg_if_not_full(s, size_regs[5]);
+ } else if (size > 511) {
+ inc_reg_if_not_full(s, size_regs[4]);
+ } else if (size > 255) {
+ inc_reg_if_not_full(s, size_regs[3]);
+ } else if (size > 127) {
+ inc_reg_if_not_full(s, size_regs[2]);
+ } else if (size > 64) {
+ inc_reg_if_not_full(s, size_regs[1]);
+ } else if (size == 64) {
+ inc_reg_if_not_full(s, size_regs[0]);
+ }
+}
+
static inline int
vlan_enabled(E1000State *s)
{
@@ -639,12 +675,17 @@ fcs_len(E1000State *s)
static void
e1000_send_packet(E1000State *s, const uint8_t *buf, int size)
{
+ static const int PTCregs[6] = { PTC64, PTC127, PTC255, PTC511,
+ PTC1023, PTC1522 };
+
NetClientState *nc = qemu_get_queue(s->nic);
if (s->phy_reg[PHY_CTRL] & MII_CR_LOOPBACK) {
nc->info->receive(nc, buf, size);
} else {
qemu_send_packet(nc, buf, size);
}
+ inc_tx_bcast_or_mcast_count(s, buf);
+ increase_size_stats(s, PTCregs, size);
}
static void
@@ -671,8 +712,11 @@ xmit_seg(E1000State *s)
if (tp->tcp) {
sofar = frames * tp->mss;
stl_be_p(tp->data+css+4, ldl_be_p(tp->data+css+4)+sofar); /* seq */
- if (tp->paylen - sofar > tp->mss)
+ if (tp->paylen - sofar > tp->mss) {
tp->data[css + 13] &= ~9; /* PSH, FIN */
+ } else if (frames) {
+ inc_reg_if_not_full(s, TSCTC);
+ }
} else /* UDP */
stw_be_p(tp->data+css+4, len);
if (tp->sum_needed & E1000_TXD_POPTS_TXSM) {
@@ -702,6 +746,8 @@ xmit_seg(E1000State *s)
inc_reg_if_not_full(s, TPT);
grow_8reg_if_not_full(s, TOTL, s->tx.size);
s->mac_reg[GPTC] = s->mac_reg[TPT];
+ s->mac_reg[GOTCL] = s->mac_reg[TOTL];
+ s->mac_reg[GOTCH] = s->mac_reg[TOTH];
}
static void
@@ -869,7 +915,6 @@ start_xmit(E1000State *s)
static int
receive_filter(E1000State *s, const uint8_t *buf, int size)
{
- static const uint8_t bcast[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
static const int mta_shift[] = {4, 3, 2, 0};
uint32_t f, rctl = s->mac_reg[RCTL], ra[2], *rp;
int isbcast = !memcmp(buf, bcast, sizeof bcast), ismcast = (buf[0] & 1);
@@ -887,10 +932,12 @@ receive_filter(E1000State *s, const uint8_t *buf, int size)
}
if (ismcast && (rctl & E1000_RCTL_MPE)) { /* promiscuous mcast */
+ inc_reg_if_not_full(s, MPRC);
return 1;
}
if (isbcast && (rctl & E1000_RCTL_BAM)) { /* broadcast enabled */
+ inc_reg_if_not_full(s, BPRC);
return 1;
}
@@ -912,8 +959,10 @@ receive_filter(E1000State *s, const uint8_t *buf, int size)
f = mta_shift[(rctl >> E1000_RCTL_MO_SHIFT) & 3];
f = (((buf[5] << 8) | buf[4]) >> f) & 0xfff;
- if (s->mac_reg[MTA + (f >> 5)] & (1 << (f & 0x1f)))
+ if (s->mac_reg[MTA + (f >> 5)] & (1 << (f & 0x1f))) {
+ inc_reg_if_not_full(s, MPRC);
return 1;
+ }
DBGOUT(RXFILTER,
"dropping, inexact filter mismatch: %02x:%02x:%02x:%02x:%02x:%02x MO %d MTA[%d] %x\n",
buf[0], buf[1], buf[2], buf[3], buf[4], buf[5],
@@ -1002,6 +1051,8 @@ e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt)
size_t desc_offset;
size_t desc_size;
size_t total_size;
+ static const int PRCregs[6] = { PRC64, PRC127, PRC255, PRC511,
+ PRC1023, PRC1522 };
if (!(s->mac_reg[STATUS] & E1000_STATUS_LU)) {
return -1;
@@ -1015,6 +1066,7 @@ e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt)
if (size < sizeof(min_buf)) {
iov_to_buf(iov, iovcnt, 0, min_buf, size);
memset(&min_buf[size], 0, sizeof(min_buf) - size);
+ inc_reg_if_not_full(s, RUC);
min_iov.iov_base = filter_buf = min_buf;
min_iov.iov_len = size = sizeof(min_buf);
iovcnt = 1;
@@ -1030,6 +1082,7 @@ e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt)
(size > MAXIMUM_ETHERNET_VLAN_SIZE
&& !(s->mac_reg[RCTL] & E1000_RCTL_LPE)))
&& !(s->mac_reg[RCTL] & E1000_RCTL_SBP)) {
+ inc_reg_if_not_full(s, ROC);
return size;
}
@@ -1115,6 +1168,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:
@@ -1123,6 +1177,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])
@@ -1285,11 +1341,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, [RDFT] = mac_low13_read,
@@ -1395,6 +1463,18 @@ static const uint8_t mac_reg_chk[0x8000] = {
[XONTXC] = markflag(MAC), [XOFFRXC] = markflag(MAC),
[RJC] = markflag(MAC), [RNBC] = markflag(MAC),
[MGTPDC] = markflag(MAC), [MGTPTC] = markflag(MAC),
+ [RUC] = markflag(MAC), [ROC] = markflag(MAC),
+ [GORCL] = markflag(MAC), [GORCH] = markflag(MAC),
+ [GOTCL] = markflag(MAC), [GOTCH] = markflag(MAC),
+ [BPRC] = markflag(MAC), [MPRC] = markflag(MAC),
+ [TSCTC] = markflag(MAC), [PRC64] = markflag(MAC),
+ [PRC127] = markflag(MAC), [PRC255] = markflag(MAC),
+ [PRC511] = markflag(MAC), [PRC1023] = markflag(MAC),
+ [PRC1522] = markflag(MAC), [PTC64] = markflag(MAC),
+ [PTC127] = markflag(MAC), [PTC255] = markflag(MAC),
+ [PTC511] = markflag(MAC), [PTC1023] = markflag(MAC),
+ [PTC1522] = markflag(MAC), [MPTC] = markflag(MAC),
+ [BPTC] = markflag(MAC),
[TDFH] = markflag(MAC) | MAC_ACCESS_PARTIAL,
[TDFT] = markflag(MAC) | MAC_ACCESS_PARTIAL,
--
2.4.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v5 3/8] e1000: Introduced an array to control the access to the MAC registers
2015-11-09 14:59 ` [Qemu-devel] [PATCH v5 3/8] e1000: Introduced an array to control the access to the MAC registers Leonid Bloch
@ 2015-11-10 5:37 ` Jason Wang
2015-11-10 11:19 ` Leonid Bloch
0 siblings, 1 reply; 19+ messages in thread
From: Jason Wang @ 2015-11-10 5:37 UTC (permalink / raw)
To: Leonid Bloch, qemu-devel; +Cc: Dmitry Fleytman, Leonid Bloch, Shmulik Ladkani
On 11/09/2015 10:59 PM, Leonid Bloch wrote:
> The array of uint8_t's which is introduced here, contains useful metadata
> about the MAC registers: if a register should be always accessible, or if
> it is accessible, but partly implemented, or if the register requires a
> certain compatibility flag to be accessed. Currently, 5 hypothetical flags
> are supported (3 exist for e1000 so far) but if in the future more than 5
> flags will be needed, the datatype of this array can simply be swapped for
> a larger one.
>
> This patch is intended to solve the following current problems:
>
> 1) On migration between different versions of QEMU, which differ by the
> MAC registers implemented in them, some registers need not to be active if
> a compatibility flag is set, in order to preserve the machine's state
> perfectly for the older version. Checking this for each register
> individually, would create a lot of clutter in the code.
>
> 2) Some registers are (or may be) only partly implemented (e.g.
> placeholders that allow reading and writing, but lack other functions).
> In such cases it is better to print a debug warning on read/write attempts.
> As above, dealing with this functionality on a per-register level, would
> require longer and more messy code.
>
> Signed-off-by: Leonid Bloch <leonid.bloch@ravellosystems.com>
> Signed-off-by: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
> ---
> hw/net/e1000.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 73 insertions(+), 12 deletions(-)
>
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index 0e00afa..2bc533f 100644
> --- a/hw/net/e1000.c
> +++ b/hw/net/e1000.c
> @@ -142,6 +142,8 @@ typedef struct E1000State_st {
> uint32_t compat_flags;
> } E1000State;
>
> +#define chkflag(x) (s->compat_flags & E1000_FLAG_##x)
> +
> typedef struct E1000BaseClass {
> PCIDeviceClass parent_class;
> uint16_t phy_id2;
> @@ -195,8 +197,7 @@ e1000_link_up(E1000State *s)
> static bool
> have_autoneg(E1000State *s)
> {
> - return (s->compat_flags & E1000_FLAG_AUTONEG) &&
> - (s->phy_reg[PHY_CTRL] & MII_CR_AUTO_NEG_EN);
> + return chkflag(AUTONEG) && (s->phy_reg[PHY_CTRL] & MII_CR_AUTO_NEG_EN);
> }
>
> static void
> @@ -321,7 +322,7 @@ set_interrupt_cause(E1000State *s, int index, uint32_t val)
> if (s->mit_timer_on) {
> return;
> }
> - if (s->compat_flags & E1000_FLAG_MIT) {
> + if (chkflag(MIT)) {
> /* Compute the next mitigation delay according to pending
> * interrupts and the current values of RADV (provided
> * RDTR!=0), TADV and ITR.
> @@ -1258,6 +1259,43 @@ static void (*macreg_writeops[])(E1000State *, int, uint32_t) = {
>
> enum { NWRITEOPS = ARRAY_SIZE(macreg_writeops) };
>
> +enum { MAC_ACCESS_ALWAYS = 1, MAC_ACCESS_PARTIAL = 2,
> + MAC_ACCESS_FLAG_NEEDED = 4 };
> +
> +#define markflag(x) ((E1000_FLAG_##x << 3) | MAC_ACCESS_FLAG_NEEDED)
> +/* In the array below the meaning of the bits is: [f|f|f|f|f|n|p|a]
> + * f - flag bits (up to 5 possible flags)
> + * n - flag needed
> + * p - partially implenented
> + * a - access enabled always
> + * n=p=a=0 - not implemented or unknown */
Looks like n=p=0 implies a=0? If yes we can probably get rid of bit 'a'
and save lots of lines below?
[...]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v5 0/8] e1000: Various fixes and registers' implementation
2015-11-09 14:59 [Qemu-devel] [PATCH v5 0/8] e1000: Various fixes and registers' implementation Leonid Bloch
` (7 preceding siblings ...)
2015-11-09 14:59 ` [Qemu-devel] [PATCH v5 8/8] e1000: Implementing various counters Leonid Bloch
@ 2015-11-10 6:21 ` Jason Wang
2015-11-10 11:39 ` Leonid Bloch
8 siblings, 1 reply; 19+ messages in thread
From: Jason Wang @ 2015-11-10 6:21 UTC (permalink / raw)
To: Leonid Bloch, qemu-devel; +Cc: Dmitry Fleytman, Leonid Bloch, Shmulik Ladkani
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
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).
>
> 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(-)
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v5 3/8] e1000: Introduced an array to control the access to the MAC registers
2015-11-10 5:37 ` Jason Wang
@ 2015-11-10 11:19 ` Leonid Bloch
0 siblings, 0 replies; 19+ messages in thread
From: Leonid Bloch @ 2015-11-10 11:19 UTC (permalink / raw)
To: Jason Wang; +Cc: Dmitry Fleytman, Leonid Bloch, qemu-devel, Shmulik Ladkani
On Tue, Nov 10, 2015 at 7:37 AM, Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 11/09/2015 10:59 PM, Leonid Bloch wrote:
>> The array of uint8_t's which is introduced here, contains useful metadata
>> about the MAC registers: if a register should be always accessible, or if
>> it is accessible, but partly implemented, or if the register requires a
>> certain compatibility flag to be accessed. Currently, 5 hypothetical flags
>> are supported (3 exist for e1000 so far) but if in the future more than 5
>> flags will be needed, the datatype of this array can simply be swapped for
>> a larger one.
>>
>> This patch is intended to solve the following current problems:
>>
>> 1) On migration between different versions of QEMU, which differ by the
>> MAC registers implemented in them, some registers need not to be active if
>> a compatibility flag is set, in order to preserve the machine's state
>> perfectly for the older version. Checking this for each register
>> individually, would create a lot of clutter in the code.
>>
>> 2) Some registers are (or may be) only partly implemented (e.g.
>> placeholders that allow reading and writing, but lack other functions).
>> In such cases it is better to print a debug warning on read/write attempts.
>> As above, dealing with this functionality on a per-register level, would
>> require longer and more messy code.
>>
>> Signed-off-by: Leonid Bloch <leonid.bloch@ravellosystems.com>
>> Signed-off-by: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
>> ---
>> hw/net/e1000.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++---------
>> 1 file changed, 73 insertions(+), 12 deletions(-)
>>
>> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
>> index 0e00afa..2bc533f 100644
>> --- a/hw/net/e1000.c
>> +++ b/hw/net/e1000.c
>> @@ -142,6 +142,8 @@ typedef struct E1000State_st {
>> uint32_t compat_flags;
>> } E1000State;
>>
>> +#define chkflag(x) (s->compat_flags & E1000_FLAG_##x)
>> +
>> typedef struct E1000BaseClass {
>> PCIDeviceClass parent_class;
>> uint16_t phy_id2;
>> @@ -195,8 +197,7 @@ e1000_link_up(E1000State *s)
>> static bool
>> have_autoneg(E1000State *s)
>> {
>> - return (s->compat_flags & E1000_FLAG_AUTONEG) &&
>> - (s->phy_reg[PHY_CTRL] & MII_CR_AUTO_NEG_EN);
>> + return chkflag(AUTONEG) && (s->phy_reg[PHY_CTRL] & MII_CR_AUTO_NEG_EN);
>> }
>>
>> static void
>> @@ -321,7 +322,7 @@ set_interrupt_cause(E1000State *s, int index, uint32_t val)
>> if (s->mit_timer_on) {
>> return;
>> }
>> - if (s->compat_flags & E1000_FLAG_MIT) {
>> + if (chkflag(MIT)) {
>> /* Compute the next mitigation delay according to pending
>> * interrupts and the current values of RADV (provided
>> * RDTR!=0), TADV and ITR.
>> @@ -1258,6 +1259,43 @@ static void (*macreg_writeops[])(E1000State *, int, uint32_t) = {
>>
>> enum { NWRITEOPS = ARRAY_SIZE(macreg_writeops) };
>>
>> +enum { MAC_ACCESS_ALWAYS = 1, MAC_ACCESS_PARTIAL = 2,
>> + MAC_ACCESS_FLAG_NEEDED = 4 };
>> +
>> +#define markflag(x) ((E1000_FLAG_##x << 3) | MAC_ACCESS_FLAG_NEEDED)
>> +/* In the array below the meaning of the bits is: [f|f|f|f|f|n|p|a]
>> + * f - flag bits (up to 5 possible flags)
>> + * n - flag needed
>> + * p - partially implenented
>> + * a - access enabled always
>> + * n=p=a=0 - not implemented or unknown */
>
> Looks like n=p=0 implies a=0? If yes we can probably get rid of bit 'a'
> and save lots of lines below?
n=p=0 does not quite imply that a=0: a counter example would be a
register that is fully implemented, and does not require a special
flag to be accessed.
But that "a" bit is redundant indeed - the check if the register is
implemented is already performed by looking in
macreg_readops/macreg_writeops. I included it just so that the
information on which registers are implemented will be present in
mac_reg_chk, if the need for it will raise in the future.
But when thinking of it now, you are right - I will remove it, and
rename the new array to "mac_reg_access", to better represent its
purpose.
>
> [...]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v5 0/8] e1000: Various fixes and registers' implementation
2015-11-10 6:21 ` [Qemu-devel] [PATCH v5 0/8] e1000: Various fixes and registers' implementation Jason Wang
@ 2015-11-10 11:39 ` Leonid Bloch
2015-11-10 13:01 ` Jason Wang
0 siblings, 1 reply; 19+ messages in thread
From: Leonid Bloch @ 2015-11-10 11:39 UTC (permalink / raw)
To: Jason Wang; +Cc: Dmitry Fleytman, Leonid Bloch, qemu-devel, Shmulik Ladkani
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?
> 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(-)
>>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v5 0/8] e1000: Various fixes and registers' implementation
2015-11-10 11:39 ` Leonid Bloch
@ 2015-11-10 13:01 ` Jason Wang
2015-11-10 13:19 ` Leonid Bloch
0 siblings, 1 reply; 19+ messages in thread
From: Jason Wang @ 2015-11-10 13:01 UTC (permalink / raw)
To: Leonid Bloch; +Cc: Dmitry Fleytman, Leonid Bloch, qemu-devel, Shmulik Ladkani
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(-)
>>>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v5 0/8] e1000: Various fixes and registers' implementation
2015-11-10 13:01 ` Jason Wang
@ 2015-11-10 13:19 ` Leonid Bloch
2015-11-10 14:29 ` Leonid Bloch
2015-11-11 3:22 ` Jason Wang
0 siblings, 2 replies; 19+ messages in thread
From: Leonid Bloch @ 2015-11-10 13:19 UTC (permalink / raw)
To: Jason Wang; +Cc: Dmitry Fleytman, Leonid Bloch, qemu-devel, Shmulik Ladkani
On Tue, Nov 10, 2015 at 3:01 PM, Jason Wang <jasowang@redhat.com> wrote:
>
>
> 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.
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).
>> 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(-)
>>>>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v5 0/8] e1000: Various fixes and registers' implementation
2015-11-10 13:19 ` Leonid Bloch
@ 2015-11-10 14:29 ` Leonid Bloch
2015-11-11 3:22 ` Jason Wang
1 sibling, 0 replies; 19+ messages in thread
From: Leonid Bloch @ 2015-11-10 14:29 UTC (permalink / raw)
To: Jason Wang; +Cc: Dmitry Fleytman, Leonid Bloch, qemu-devel, Shmulik Ladkani
On Tue, Nov 10, 2015 at 3:19 PM, Leonid Bloch
<leonid.bloch@ravellosystems.com> wrote:
> On Tue, Nov 10, 2015 at 3:01 PM, Jason Wang <jasowang@redhat.com> wrote:
>>
>>
>> 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.
>
> 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(-)
>>>>>
>>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v5 0/8] e1000: Various fixes and registers' implementation
2015-11-10 13:19 ` Leonid Bloch
2015-11-10 14:29 ` Leonid Bloch
@ 2015-11-11 3:22 ` Jason Wang
2015-11-11 8:06 ` Leonid Bloch
1 sibling, 1 reply; 19+ messages in thread
From: Jason Wang @ 2015-11-11 3:22 UTC (permalink / raw)
To: Leonid Bloch; +Cc: Dmitry Fleytman, Leonid Bloch, qemu-devel, Shmulik Ladkani
On 11/10/2015 09:19 PM, Leonid Bloch wrote:
> On Tue, Nov 10, 2015 at 3:01 PM, Jason Wang <jasowang@redhat.com> wrote:
>>
>> 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.
> But won't there be compatibility issues between the patches if done
> like that?
Looks not, all new mac registers and mac array migration will be
functional or used only in last patch. We don't have any compatibility
issue since E1000_FLAG_MAC can only be set for the last patch since the
meaning of it should be "migrate the whole mac array and enable the
various mac registers". And if we just use the order like this series,
we may have compatibility issue during bisection. E.g migrate between w/
patch 8 and w/o patch 8.
> Why not to prepare the ground for compatibility, and only
> then introduce the new registers (as it is done now)?
We do prepare it from the first patch but just not enable it until we're
sure that everything is ready.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v5 0/8] e1000: Various fixes and registers' implementation
2015-11-11 3:22 ` Jason Wang
@ 2015-11-11 8:06 ` Leonid Bloch
2015-11-11 10:55 ` Jason Wang
0 siblings, 1 reply; 19+ messages in thread
From: Leonid Bloch @ 2015-11-11 8:06 UTC (permalink / raw)
To: Jason Wang; +Cc: Dmitry Fleytman, Leonid Bloch, qemu-devel, Shmulik Ladkani
On Wed, Nov 11, 2015 at 5:22 AM, Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 11/10/2015 09:19 PM, Leonid Bloch wrote:
>> On Tue, Nov 10, 2015 at 3:01 PM, Jason Wang <jasowang@redhat.com> wrote:
>>>
>>> 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.
>> But won't there be compatibility issues between the patches if done
>> like that?
>
> Looks not, all new mac registers and mac array migration will be
> functional or used only in last patch. We don't have any compatibility
> issue since E1000_FLAG_MAC can only be set for the last patch since the
> meaning of it should be "migrate the whole mac array and enable the
> various mac registers". And if we just use the order like this series,
> we may have compatibility issue during bisection. E.g migrate between w/
> patch 8 and w/o patch 8.
But the trivial MAC registers are introduced in patch 4 already (in
order to minimize code changes in the subsequent patches). Besides, if
migration of the full MAC registers' array will be enabled, even
before all the new registers are introduced, what will be the the
problem with migration? One should be able to migrate from w/ patch 8,
to anywhere after patch 2 with no problem, and to before that - with
compatibility flag set.
>
>> Why not to prepare the ground for compatibility, and only
>> then introduce the new registers (as it is done now)?
>
> We do prepare it from the first patch but just not enable it until we're
> sure that everything is ready.
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v5 0/8] e1000: Various fixes and registers' implementation
2015-11-11 8:06 ` Leonid Bloch
@ 2015-11-11 10:55 ` Jason Wang
0 siblings, 0 replies; 19+ messages in thread
From: Jason Wang @ 2015-11-11 10:55 UTC (permalink / raw)
To: Leonid Bloch; +Cc: Dmitry Fleytman, Leonid Bloch, qemu-devel, Shmulik Ladkani
On 11/11/2015 04:06 PM, Leonid Bloch wrote:
> On Wed, Nov 11, 2015 at 5:22 AM, Jason Wang <jasowang@redhat.com> wrote:
>> >
>> >
>> > On 11/10/2015 09:19 PM, Leonid Bloch wrote:
>>> >> On Tue, Nov 10, 2015 at 3:01 PM, Jason Wang <jasowang@redhat.com> wrote:
>>>> >>>
>>>> >>> 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.
>>> >> But won't there be compatibility issues between the patches if done
>>> >> like that?
>> >
>> > Looks not, all new mac registers and mac array migration will be
>> > functional or used only in last patch. We don't have any compatibility
>> > issue since E1000_FLAG_MAC can only be set for the last patch since the
>> > meaning of it should be "migrate the whole mac array and enable the
>> > various mac registers". And if we just use the order like this series,
>> > we may have compatibility issue during bisection. E.g migrate between w/
>> > patch 8 and w/o patch 8.
> But the trivial MAC registers are introduced in patch 4 already (in
> order to minimize code changes in the subsequent patches). Besides, if
> migration of the full MAC registers' array will be enabled, even
> before all the new registers are introduced, what will be the the
> problem with migration? One should be able to migrate from w/ patch 8,
> to anywhere after patch 2 with no problem, and to before that - with
> compatibility flag set.
Yes, migration can complete. But the value of the counters will suddenly
drop to zero after migration from patch 8 to patch 7. This is suboptimal
and can lead unexpected results.
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2015-11-11 10:59 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-09 14:59 [Qemu-devel] [PATCH v5 0/8] e1000: Various fixes and registers' implementation Leonid Bloch
2015-11-09 14:59 ` [Qemu-devel] [PATCH v5 1/8] e1000: Cosmetic and alignment fixes Leonid Bloch
2015-11-09 14:59 ` [Qemu-devel] [PATCH v5 2/8] e1000: Add support for migrating the entire MAC registers' array Leonid Bloch
2015-11-09 14:59 ` [Qemu-devel] [PATCH v5 3/8] e1000: Introduced an array to control the access to the MAC registers Leonid Bloch
2015-11-10 5:37 ` Jason Wang
2015-11-10 11:19 ` Leonid Bloch
2015-11-09 14:59 ` [Qemu-devel] [PATCH v5 4/8] e1000: Trivial implementation of various " Leonid Bloch
2015-11-09 14:59 ` [Qemu-devel] [PATCH v5 5/8] e1000: Fixing the received/transmitted packets' counters Leonid Bloch
2015-11-09 14:59 ` [Qemu-devel] [PATCH v5 6/8] e1000: Fixing the received/transmitted octets' counters Leonid Bloch
2015-11-09 14:59 ` [Qemu-devel] [PATCH v5 7/8] e1000: Fixing the packet address filtering procedure Leonid Bloch
2015-11-09 14:59 ` [Qemu-devel] [PATCH v5 8/8] e1000: Implementing various counters Leonid Bloch
2015-11-10 6:21 ` [Qemu-devel] [PATCH v5 0/8] e1000: Various fixes and registers' implementation Jason Wang
2015-11-10 11:39 ` Leonid Bloch
2015-11-10 13:01 ` Jason Wang
2015-11-10 13:19 ` Leonid Bloch
2015-11-10 14:29 ` Leonid Bloch
2015-11-11 3:22 ` Jason Wang
2015-11-11 8:06 ` Leonid Bloch
2015-11-11 10:55 ` Jason Wang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.