* [Qemu-devel] [PATCH v4 1/7] e1000: Cosmetic and alignment fixes
2015-11-03 11:14 [Qemu-devel] [PATCH v4 0/7] e1000: Various fixes and registers' implementation Leonid Bloch
@ 2015-11-03 11:14 ` Leonid Bloch
2015-11-03 11:14 ` [Qemu-devel] [PATCH v4 2/7] e1000: Add support for migrating the entire MAC registers' array Leonid Bloch
` (5 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Leonid Bloch @ 2015-11-03 11:14 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 | 149 +++++++++++++++++++++++++++-------------------------
hw/net/e1000_regs.h | 2 +-
2 files changed, 79 insertions(+), 72 deletions(-)
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 09c9e9d..7036842 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,12 +226,12 @@ 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,
};
@@ -510,17 +510,17 @@ 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,11 +621,11 @@ 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
+ } else /* IPv6 */
stw_be_p(tp->data+css+4, tp->size - css);
css = tp->tucss;
len = tp->size - css;
@@ -634,8 +634,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 +657,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 +681,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 +696,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 +720,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 +1208,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] 17+ messages in thread* [Qemu-devel] [PATCH v4 2/7] e1000: Add support for migrating the entire MAC registers' array
2015-11-03 11:14 [Qemu-devel] [PATCH v4 0/7] e1000: Various fixes and registers' implementation Leonid Bloch
2015-11-03 11:14 ` [Qemu-devel] [PATCH v4 1/7] e1000: Cosmetic and alignment fixes Leonid Bloch
@ 2015-11-03 11:14 ` Leonid Bloch
2015-11-04 2:35 ` Jason Wang
2015-11-03 11:14 ` [Qemu-devel] [PATCH v4 3/7] e1000: Trivial implementation of various MAC registers Leonid Bloch
` (4 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Leonid Bloch @ 2015-11-03 11:14 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, over the
individual registers. But the advantages are more compact code, and no
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 "full_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 "full_mac_registers"
will be set to "off", e.g.:
qemu-system-x86_64 -device e1000,full_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 | 105 +++++++++++++++++++++++++++++++++++++++------------------
1 file changed, 72 insertions(+), 33 deletions(-)
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 7036842..1190bbe 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;
@@ -1377,6 +1379,18 @@ 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 bool e1000_compat_mac_needed(void *opaque)
+{
+ return !e1000_full_mac_needed(opaque);
+}
+
static const VMStateDescription vmstate_e1000_mit_state = {
.name = "e1000/mit_state",
.version_id = 1,
@@ -1392,41 +1406,23 @@ static const VMStateDescription vmstate_e1000_mit_state = {
}
};
-static const VMStateDescription vmstate_e1000 = {
- .name = "e1000",
- .version_id = 2,
+static const VMStateDescription vmstate_e1000_full_mac_state = {
+ .name = "e1000/full_mac_state",
+ .version_id = 1,
.minimum_version_id = 1,
- .pre_save = e1000_pre_save,
- .post_load = e1000_post_load,
+ .needed = e1000_full_mac_needed,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT32_ARRAY(mac_reg, E1000State, 0x8000),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static const VMStateDescription vmstate_e1000_compat_mac_state = {
+ .name = "e1000/compat_mac_state",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .needed = e1000_compat_mac_needed,
.fields = (VMStateField[]) {
- VMSTATE_PCI_DEVICE(parent_obj, E1000State),
- VMSTATE_UNUSED_TEST(is_version_1, 4), /* was instance id */
- VMSTATE_UNUSED(4), /* Was mmio_base. */
- VMSTATE_UINT32(rxbuf_size, E1000State),
- VMSTATE_UINT32(rxbuf_min_shift, E1000State),
- VMSTATE_UINT32(eecd_state.val_in, E1000State),
- VMSTATE_UINT16(eecd_state.bitnum_in, E1000State),
- VMSTATE_UINT16(eecd_state.bitnum_out, E1000State),
- VMSTATE_UINT16(eecd_state.reading, E1000State),
- VMSTATE_UINT32(eecd_state.old_eecd, E1000State),
- VMSTATE_UINT8(tx.ipcss, E1000State),
- VMSTATE_UINT8(tx.ipcso, E1000State),
- VMSTATE_UINT16(tx.ipcse, E1000State),
- VMSTATE_UINT8(tx.tucss, E1000State),
- VMSTATE_UINT8(tx.tucso, E1000State),
- VMSTATE_UINT16(tx.tucse, E1000State),
- VMSTATE_UINT32(tx.paylen, E1000State),
- VMSTATE_UINT8(tx.hdr_len, E1000State),
- VMSTATE_UINT16(tx.mss, E1000State),
- VMSTATE_UINT16(tx.size, E1000State),
- VMSTATE_UINT16(tx.tso_frames, E1000State),
- VMSTATE_UINT8(tx.sum_needed, E1000State),
- VMSTATE_INT8(tx.ip, E1000State),
- VMSTATE_INT8(tx.tcp, E1000State),
- VMSTATE_BUFFER(tx.header, E1000State),
- VMSTATE_BUFFER(tx.data, E1000State),
- VMSTATE_UINT16_ARRAY(eeprom_data, E1000State, 64),
- VMSTATE_UINT16_ARRAY(phy_reg, E1000State, 0x20),
VMSTATE_UINT32(mac_reg[CTRL], E1000State),
VMSTATE_UINT32(mac_reg[EECD], E1000State),
VMSTATE_UINT32(mac_reg[EERD], E1000State),
@@ -1468,9 +1464,50 @@ static const VMStateDescription vmstate_e1000 = {
VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, MTA, 128),
VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, VFTA, 128),
VMSTATE_END_OF_LIST()
+ }
+};
+
+static const VMStateDescription vmstate_e1000 = {
+ .name = "e1000",
+ .version_id = 2,
+ .minimum_version_id = 1,
+ .pre_save = e1000_pre_save,
+ .post_load = e1000_post_load,
+ .fields = (VMStateField[]) {
+ VMSTATE_PCI_DEVICE(parent_obj, E1000State),
+ VMSTATE_UNUSED_TEST(is_version_1, 4), /* was instance id */
+ VMSTATE_UNUSED(4), /* Was mmio_base. */
+ VMSTATE_UINT32(rxbuf_size, E1000State),
+ VMSTATE_UINT32(rxbuf_min_shift, E1000State),
+ VMSTATE_UINT32(eecd_state.val_in, E1000State),
+ VMSTATE_UINT16(eecd_state.bitnum_in, E1000State),
+ VMSTATE_UINT16(eecd_state.bitnum_out, E1000State),
+ VMSTATE_UINT16(eecd_state.reading, E1000State),
+ VMSTATE_UINT32(eecd_state.old_eecd, E1000State),
+ VMSTATE_UINT8(tx.ipcss, E1000State),
+ VMSTATE_UINT8(tx.ipcso, E1000State),
+ VMSTATE_UINT16(tx.ipcse, E1000State),
+ VMSTATE_UINT8(tx.tucss, E1000State),
+ VMSTATE_UINT8(tx.tucso, E1000State),
+ VMSTATE_UINT16(tx.tucse, E1000State),
+ VMSTATE_UINT32(tx.paylen, E1000State),
+ VMSTATE_UINT8(tx.hdr_len, E1000State),
+ VMSTATE_UINT16(tx.mss, E1000State),
+ VMSTATE_UINT16(tx.size, E1000State),
+ VMSTATE_UINT16(tx.tso_frames, E1000State),
+ VMSTATE_UINT8(tx.sum_needed, E1000State),
+ VMSTATE_INT8(tx.ip, E1000State),
+ VMSTATE_INT8(tx.tcp, E1000State),
+ VMSTATE_BUFFER(tx.header, E1000State),
+ VMSTATE_BUFFER(tx.data, E1000State),
+ VMSTATE_UINT16_ARRAY(eeprom_data, E1000State, 64),
+ VMSTATE_UINT16_ARRAY(phy_reg, E1000State, 0x20),
+ VMSTATE_END_OF_LIST()
},
.subsections = (const VMStateDescription*[]) {
&vmstate_e1000_mit_state,
+ &vmstate_e1000_full_mac_state,
+ &vmstate_e1000_compat_mac_state,
NULL
}
};
@@ -1603,6 +1640,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("full_mac_registers", E1000State,
+ compat_flags, E1000_FLAG_MAC_BIT, true),
DEFINE_PROP_END_OF_LIST(),
};
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH v4 2/7] e1000: Add support for migrating the entire MAC registers' array
2015-11-03 11:14 ` [Qemu-devel] [PATCH v4 2/7] e1000: Add support for migrating the entire MAC registers' array Leonid Bloch
@ 2015-11-04 2:35 ` Jason Wang
2015-11-04 14:48 ` Leonid Bloch
0 siblings, 1 reply; 17+ messages in thread
From: Jason Wang @ 2015-11-04 2:35 UTC (permalink / raw)
To: Leonid Bloch, qemu-devel; +Cc: Dmitry Fleytman, Leonid Bloch, Shmulik Ladkani
On 11/03/2015 07:14 PM, Leonid Bloch wrote:
> 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, over the
> individual registers. But the advantages are more compact code, and no
> 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 "full_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 "full_mac_registers"
> will be set to "off", e.g.:
>
> qemu-system-x86_64 -device e1000,full_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 | 105 +++++++++++++++++++++++++++++++++++++++------------------
> 1 file changed, 72 insertions(+), 33 deletions(-)
>
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index 7036842..1190bbe 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;
>
> @@ -1377,6 +1379,18 @@ 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 bool e1000_compat_mac_needed(void *opaque)
> +{
> + return !e1000_full_mac_needed(opaque);
> +}
> +
> static const VMStateDescription vmstate_e1000_mit_state = {
> .name = "e1000/mit_state",
> .version_id = 1,
> @@ -1392,41 +1406,23 @@ static const VMStateDescription vmstate_e1000_mit_state = {
> }
> };
>
> -static const VMStateDescription vmstate_e1000 = {
> - .name = "e1000",
> - .version_id = 2,
> +static const VMStateDescription vmstate_e1000_full_mac_state = {
> + .name = "e1000/full_mac_state",
> + .version_id = 1,
> .minimum_version_id = 1,
> - .pre_save = e1000_pre_save,
> - .post_load = e1000_post_load,
> + .needed = e1000_full_mac_needed,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT32_ARRAY(mac_reg, E1000State, 0x8000),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static const VMStateDescription vmstate_e1000_compat_mac_state = {
> + .name = "e1000/compat_mac_state",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .needed = e1000_compat_mac_needed,
> .fields = (VMStateField[]) {
> - VMSTATE_PCI_DEVICE(parent_obj, E1000State),
> - VMSTATE_UNUSED_TEST(is_version_1, 4), /* was instance id */
> - VMSTATE_UNUSED(4), /* Was mmio_base. */
> - VMSTATE_UINT32(rxbuf_size, E1000State),
> - VMSTATE_UINT32(rxbuf_min_shift, E1000State),
> - VMSTATE_UINT32(eecd_state.val_in, E1000State),
> - VMSTATE_UINT16(eecd_state.bitnum_in, E1000State),
> - VMSTATE_UINT16(eecd_state.bitnum_out, E1000State),
> - VMSTATE_UINT16(eecd_state.reading, E1000State),
> - VMSTATE_UINT32(eecd_state.old_eecd, E1000State),
> - VMSTATE_UINT8(tx.ipcss, E1000State),
> - VMSTATE_UINT8(tx.ipcso, E1000State),
> - VMSTATE_UINT16(tx.ipcse, E1000State),
> - VMSTATE_UINT8(tx.tucss, E1000State),
> - VMSTATE_UINT8(tx.tucso, E1000State),
> - VMSTATE_UINT16(tx.tucse, E1000State),
> - VMSTATE_UINT32(tx.paylen, E1000State),
> - VMSTATE_UINT8(tx.hdr_len, E1000State),
> - VMSTATE_UINT16(tx.mss, E1000State),
> - VMSTATE_UINT16(tx.size, E1000State),
> - VMSTATE_UINT16(tx.tso_frames, E1000State),
> - VMSTATE_UINT8(tx.sum_needed, E1000State),
> - VMSTATE_INT8(tx.ip, E1000State),
> - VMSTATE_INT8(tx.tcp, E1000State),
> - VMSTATE_BUFFER(tx.header, E1000State),
> - VMSTATE_BUFFER(tx.data, E1000State),
> - VMSTATE_UINT16_ARRAY(eeprom_data, E1000State, 64),
> - VMSTATE_UINT16_ARRAY(phy_reg, E1000State, 0x20),
> VMSTATE_UINT32(mac_reg[CTRL], E1000State),
> VMSTATE_UINT32(mac_reg[EECD], E1000State),
> VMSTATE_UINT32(mac_reg[EERD], E1000State),
> @@ -1468,9 +1464,50 @@ static const VMStateDescription vmstate_e1000 = {
> VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, MTA, 128),
> VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, VFTA, 128),
> VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static const VMStateDescription vmstate_e1000 = {
> + .name = "e1000",
> + .version_id = 2,
> + .minimum_version_id = 1,
> + .pre_save = e1000_pre_save,
> + .post_load = e1000_post_load,
> + .fields = (VMStateField[]) {
> + VMSTATE_PCI_DEVICE(parent_obj, E1000State),
> + VMSTATE_UNUSED_TEST(is_version_1, 4), /* was instance id */
> + VMSTATE_UNUSED(4), /* Was mmio_base. */
> + VMSTATE_UINT32(rxbuf_size, E1000State),
> + VMSTATE_UINT32(rxbuf_min_shift, E1000State),
> + VMSTATE_UINT32(eecd_state.val_in, E1000State),
> + VMSTATE_UINT16(eecd_state.bitnum_in, E1000State),
> + VMSTATE_UINT16(eecd_state.bitnum_out, E1000State),
> + VMSTATE_UINT16(eecd_state.reading, E1000State),
> + VMSTATE_UINT32(eecd_state.old_eecd, E1000State),
> + VMSTATE_UINT8(tx.ipcss, E1000State),
> + VMSTATE_UINT8(tx.ipcso, E1000State),
> + VMSTATE_UINT16(tx.ipcse, E1000State),
> + VMSTATE_UINT8(tx.tucss, E1000State),
> + VMSTATE_UINT8(tx.tucso, E1000State),
> + VMSTATE_UINT16(tx.tucse, E1000State),
> + VMSTATE_UINT32(tx.paylen, E1000State),
> + VMSTATE_UINT8(tx.hdr_len, E1000State),
> + VMSTATE_UINT16(tx.mss, E1000State),
> + VMSTATE_UINT16(tx.size, E1000State),
> + VMSTATE_UINT16(tx.tso_frames, E1000State),
> + VMSTATE_UINT8(tx.sum_needed, E1000State),
> + VMSTATE_INT8(tx.ip, E1000State),
> + VMSTATE_INT8(tx.tcp, E1000State),
> + VMSTATE_BUFFER(tx.header, E1000State),
> + VMSTATE_BUFFER(tx.data, E1000State),
> + VMSTATE_UINT16_ARRAY(eeprom_data, E1000State, 64),
> + VMSTATE_UINT16_ARRAY(phy_reg, E1000State, 0x20),
> + VMSTATE_END_OF_LIST()
> },
> .subsections = (const VMStateDescription*[]) {
> &vmstate_e1000_mit_state,
> + &vmstate_e1000_full_mac_state,
> + &vmstate_e1000_compat_mac_state,
> NULL
> }
I'm afraid this will break migration to older qemu. Consider the case
when full_mac_registers=off, we save compat mac registers as a
subsection, this breaks the assumption on load who expect all compact
registers just after phy_reg. (You can just test this with a simple
'savevm' with full_mac_register_off and the revert this patch and do a
'loadvm'), Looks like the safe way is to just add a
vmstate_e1000_full_mac_state and keep other fields of vmstate_e1000
unmodified.
> };
> @@ -1603,6 +1640,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("full_mac_registers", E1000State,
> + compat_flags, E1000_FLAG_MAC_BIT, true),
> DEFINE_PROP_END_OF_LIST(),
> };
>
Need also turn this off for pre 2.5 machines in HW_COMPAT_2_4.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH v4 2/7] e1000: Add support for migrating the entire MAC registers' array
2015-11-04 2:35 ` Jason Wang
@ 2015-11-04 14:48 ` Leonid Bloch
2015-11-05 2:35 ` Jason Wang
0 siblings, 1 reply; 17+ messages in thread
From: Leonid Bloch @ 2015-11-04 14:48 UTC (permalink / raw)
To: Jason Wang; +Cc: Dmitry Fleytman, Leonid Bloch, qemu-devel, Shmulik Ladkani
On Wed, Nov 4, 2015 at 4:35 AM, Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 11/03/2015 07:14 PM, Leonid Bloch wrote:
>> 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, over the
>> individual registers. But the advantages are more compact code, and no
>> 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 "full_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 "full_mac_registers"
>> will be set to "off", e.g.:
>>
>> qemu-system-x86_64 -device e1000,full_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 | 105 +++++++++++++++++++++++++++++++++++++++------------------
>> 1 file changed, 72 insertions(+), 33 deletions(-)
>>
>> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
>> index 7036842..1190bbe 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;
>>
>> @@ -1377,6 +1379,18 @@ 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 bool e1000_compat_mac_needed(void *opaque)
>> +{
>> + return !e1000_full_mac_needed(opaque);
>> +}
>> +
>> static const VMStateDescription vmstate_e1000_mit_state = {
>> .name = "e1000/mit_state",
>> .version_id = 1,
>> @@ -1392,41 +1406,23 @@ static const VMStateDescription vmstate_e1000_mit_state = {
>> }
>> };
>>
>> -static const VMStateDescription vmstate_e1000 = {
>> - .name = "e1000",
>> - .version_id = 2,
>> +static const VMStateDescription vmstate_e1000_full_mac_state = {
>> + .name = "e1000/full_mac_state",
>> + .version_id = 1,
>> .minimum_version_id = 1,
>> - .pre_save = e1000_pre_save,
>> - .post_load = e1000_post_load,
>> + .needed = e1000_full_mac_needed,
>> + .fields = (VMStateField[]) {
>> + VMSTATE_UINT32_ARRAY(mac_reg, E1000State, 0x8000),
>> + VMSTATE_END_OF_LIST()
>> + }
>> +};
>> +
>> +static const VMStateDescription vmstate_e1000_compat_mac_state = {
>> + .name = "e1000/compat_mac_state",
>> + .version_id = 1,
>> + .minimum_version_id = 1,
>> + .needed = e1000_compat_mac_needed,
>> .fields = (VMStateField[]) {
>> - VMSTATE_PCI_DEVICE(parent_obj, E1000State),
>> - VMSTATE_UNUSED_TEST(is_version_1, 4), /* was instance id */
>> - VMSTATE_UNUSED(4), /* Was mmio_base. */
>> - VMSTATE_UINT32(rxbuf_size, E1000State),
>> - VMSTATE_UINT32(rxbuf_min_shift, E1000State),
>> - VMSTATE_UINT32(eecd_state.val_in, E1000State),
>> - VMSTATE_UINT16(eecd_state.bitnum_in, E1000State),
>> - VMSTATE_UINT16(eecd_state.bitnum_out, E1000State),
>> - VMSTATE_UINT16(eecd_state.reading, E1000State),
>> - VMSTATE_UINT32(eecd_state.old_eecd, E1000State),
>> - VMSTATE_UINT8(tx.ipcss, E1000State),
>> - VMSTATE_UINT8(tx.ipcso, E1000State),
>> - VMSTATE_UINT16(tx.ipcse, E1000State),
>> - VMSTATE_UINT8(tx.tucss, E1000State),
>> - VMSTATE_UINT8(tx.tucso, E1000State),
>> - VMSTATE_UINT16(tx.tucse, E1000State),
>> - VMSTATE_UINT32(tx.paylen, E1000State),
>> - VMSTATE_UINT8(tx.hdr_len, E1000State),
>> - VMSTATE_UINT16(tx.mss, E1000State),
>> - VMSTATE_UINT16(tx.size, E1000State),
>> - VMSTATE_UINT16(tx.tso_frames, E1000State),
>> - VMSTATE_UINT8(tx.sum_needed, E1000State),
>> - VMSTATE_INT8(tx.ip, E1000State),
>> - VMSTATE_INT8(tx.tcp, E1000State),
>> - VMSTATE_BUFFER(tx.header, E1000State),
>> - VMSTATE_BUFFER(tx.data, E1000State),
>> - VMSTATE_UINT16_ARRAY(eeprom_data, E1000State, 64),
>> - VMSTATE_UINT16_ARRAY(phy_reg, E1000State, 0x20),
>> VMSTATE_UINT32(mac_reg[CTRL], E1000State),
>> VMSTATE_UINT32(mac_reg[EECD], E1000State),
>> VMSTATE_UINT32(mac_reg[EERD], E1000State),
>> @@ -1468,9 +1464,50 @@ static const VMStateDescription vmstate_e1000 = {
>> VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, MTA, 128),
>> VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, VFTA, 128),
>> VMSTATE_END_OF_LIST()
>> + }
>> +};
>> +
>> +static const VMStateDescription vmstate_e1000 = {
>> + .name = "e1000",
>> + .version_id = 2,
>> + .minimum_version_id = 1,
>> + .pre_save = e1000_pre_save,
>> + .post_load = e1000_post_load,
>> + .fields = (VMStateField[]) {
>> + VMSTATE_PCI_DEVICE(parent_obj, E1000State),
>> + VMSTATE_UNUSED_TEST(is_version_1, 4), /* was instance id */
>> + VMSTATE_UNUSED(4), /* Was mmio_base. */
>> + VMSTATE_UINT32(rxbuf_size, E1000State),
>> + VMSTATE_UINT32(rxbuf_min_shift, E1000State),
>> + VMSTATE_UINT32(eecd_state.val_in, E1000State),
>> + VMSTATE_UINT16(eecd_state.bitnum_in, E1000State),
>> + VMSTATE_UINT16(eecd_state.bitnum_out, E1000State),
>> + VMSTATE_UINT16(eecd_state.reading, E1000State),
>> + VMSTATE_UINT32(eecd_state.old_eecd, E1000State),
>> + VMSTATE_UINT8(tx.ipcss, E1000State),
>> + VMSTATE_UINT8(tx.ipcso, E1000State),
>> + VMSTATE_UINT16(tx.ipcse, E1000State),
>> + VMSTATE_UINT8(tx.tucss, E1000State),
>> + VMSTATE_UINT8(tx.tucso, E1000State),
>> + VMSTATE_UINT16(tx.tucse, E1000State),
>> + VMSTATE_UINT32(tx.paylen, E1000State),
>> + VMSTATE_UINT8(tx.hdr_len, E1000State),
>> + VMSTATE_UINT16(tx.mss, E1000State),
>> + VMSTATE_UINT16(tx.size, E1000State),
>> + VMSTATE_UINT16(tx.tso_frames, E1000State),
>> + VMSTATE_UINT8(tx.sum_needed, E1000State),
>> + VMSTATE_INT8(tx.ip, E1000State),
>> + VMSTATE_INT8(tx.tcp, E1000State),
>> + VMSTATE_BUFFER(tx.header, E1000State),
>> + VMSTATE_BUFFER(tx.data, E1000State),
>> + VMSTATE_UINT16_ARRAY(eeprom_data, E1000State, 64),
>> + VMSTATE_UINT16_ARRAY(phy_reg, E1000State, 0x20),
>> + VMSTATE_END_OF_LIST()
>> },
>> .subsections = (const VMStateDescription*[]) {
>> &vmstate_e1000_mit_state,
>> + &vmstate_e1000_full_mac_state,
>> + &vmstate_e1000_compat_mac_state,
>> NULL
>> }
>
> I'm afraid this will break migration to older qemu. Consider the case
> when full_mac_registers=off, we save compat mac registers as a
> subsection, this breaks the assumption on load who expect all compact
> registers just after phy_reg. (You can just test this with a simple
> 'savevm' with full_mac_register_off and the revert this patch and do a
> 'loadvm'), Looks like the safe way is to just add a
> vmstate_e1000_full_mac_state and keep other fields of vmstate_e1000
> unmodified.
You are right! Thanks.
>
>> };
>> @@ -1603,6 +1640,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("full_mac_registers", E1000State,
>> + compat_flags, E1000_FLAG_MAC_BIT, true),
>> DEFINE_PROP_END_OF_LIST(),
>> };
>>
>
> Need also turn this off for pre 2.5 machines in HW_COMPAT_2_4.
Thanks for reminding!
BTW, HW_COMPAT is simply the new version of PC_COMPAT, or am I wrong?
Leonid.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH v4 2/7] e1000: Add support for migrating the entire MAC registers' array
2015-11-04 14:48 ` Leonid Bloch
@ 2015-11-05 2:35 ` Jason Wang
0 siblings, 0 replies; 17+ messages in thread
From: Jason Wang @ 2015-11-05 2:35 UTC (permalink / raw)
To: Leonid Bloch; +Cc: Dmitry Fleytman, Leonid Bloch, qemu-devel, Shmulik Ladkani
On 11/04/2015 10:48 PM, Leonid Bloch wrote:
>>> >> },
>>> >> .subsections = (const VMStateDescription*[]) {
>>> >> &vmstate_e1000_mit_state,
>>> >> + &vmstate_e1000_full_mac_state,
>>> >> + &vmstate_e1000_compat_mac_state,
>>> >> NULL
>>> >> }
>> >
>> > I'm afraid this will break migration to older qemu. Consider the case
>> > when full_mac_registers=off, we save compat mac registers as a
>> > subsection, this breaks the assumption on load who expect all compact
>> > registers just after phy_reg. (You can just test this with a simple
>> > 'savevm' with full_mac_register_off and the revert this patch and do a
>> > 'loadvm'), Looks like the safe way is to just add a
>> > vmstate_e1000_full_mac_state and keep other fields of vmstate_e1000
>> > unmodified.
> You are right! Thanks.
>> >
>>> >> };
>>> >> @@ -1603,6 +1640,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("full_mac_registers", E1000State,
>>> >> + compat_flags, E1000_FLAG_MAC_BIT, true),
>>> >> DEFINE_PROP_END_OF_LIST(),
>>> >> };
>>> >>
>> >
>> > Need also turn this off for pre 2.5 machines in HW_COMPAT_2_4.
> Thanks for reminding!
> BTW, HW_COMPAT is simply the new version of PC_COMPAT, or am I wrong?
>
> Leonid.
>
HW_COMPAT contains compatibility options that could be used by all
targets (e.g both ppc and x86). PC_COMPAT contains pc specific options.
So usually PC_COMPAT is a superset of HW_COMPAT.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH v4 3/7] e1000: Trivial implementation of various MAC registers
2015-11-03 11:14 [Qemu-devel] [PATCH v4 0/7] e1000: Various fixes and registers' implementation Leonid Bloch
2015-11-03 11:14 ` [Qemu-devel] [PATCH v4 1/7] e1000: Cosmetic and alignment fixes Leonid Bloch
2015-11-03 11:14 ` [Qemu-devel] [PATCH v4 2/7] e1000: Add support for migrating the entire MAC registers' array Leonid Bloch
@ 2015-11-03 11:14 ` Leonid Bloch
2015-11-04 2:44 ` Jason Wang
2015-11-03 11:14 ` [Qemu-devel] [PATCH v4 4/7] e1000: Fixing the received/transmitted packets' counters Leonid Bloch
` (3 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Leonid Bloch @ 2015-11-03 11:14 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 | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++--
hw/net/e1000_regs.h | 6 ++++
2 files changed, 98 insertions(+), 3 deletions(-)
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 1190bbe..7db6614 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -170,7 +170,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
@@ -1118,6 +1128,48 @@ mac_readreg(E1000State *s, int index)
}
static uint32_t
+mac_readreg_prt(E1000State *s, int index)
+{
+ DBGOUT(GENERAL, "Reading register at offset: 0x%08x. "
+ "It is not fully implemented.\n", index<<2);
+ return s->mac_reg[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_low11_read_prt(E1000State *s, int index)
+{
+ DBGOUT(GENERAL, "Reading register at offset: 0x%08x. "
+ "It is not fully implemented.\n", index<<2);
+ return s->mac_reg[index] & 0x7ff;
+}
+
+static uint32_t
+mac_low13_read_prt(E1000State *s, int index)
+{
+ DBGOUT(GENERAL, "Reading register at offset: 0x%08x. "
+ "It is not fully implemented.\n", index<<2);
+ 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];
@@ -1161,6 +1213,14 @@ mac_writereg(E1000State *s, int index, uint32_t val)
}
static void
+mac_writereg_prt(E1000State *s, int index, uint32_t val)
+{
+ DBGOUT(GENERAL, "Writing to register at offset: 0x%08x. "
+ "It is not fully implemented.\n", index<<2);
+ s->mac_reg[index] = val;
+}
+
+static void
set_rdt(E1000State *s, int index, uint32_t val)
{
s->mac_reg[index] = val & 0xffff;
@@ -1219,26 +1279,50 @@ 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_prt, [RDFT] = mac_low13_read_prt,
+ [RDFHS] = mac_low13_read_prt, [RDFTS] = mac_low13_read_prt,
+ [RDFPC] = mac_low13_read_prt,
+ [TDFH] = mac_low11_read_prt, [TDFT] = mac_low11_read_prt,
+ [TDFHS] = mac_low13_read_prt, [TDFTS] = mac_low13_read_prt,
+ [TDFPC] = mac_low13_read_prt,
+ [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_prt,
};
enum { NREADOPS = ARRAY_SIZE(macreg_readops) };
#define putreg(x) [x] = mac_writereg
+#define putPreg(x) [x] = mac_writereg_prt
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),
+ putPreg(TDFH), putPreg(TDFT), putPreg(TDFHS), putPreg(TDFTS),
+ putPreg(TDFPC), putPreg(RDFH), putPreg(RDFT), putPreg(RDFHS),
+ putPreg(RDFTS), putPreg(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,
@@ -1248,9 +1332,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_prt,
};
enum { NWRITEOPS = ARRAY_SIZE(macreg_writeops) };
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] 17+ messages in thread* Re: [Qemu-devel] [PATCH v4 3/7] e1000: Trivial implementation of various MAC registers
2015-11-03 11:14 ` [Qemu-devel] [PATCH v4 3/7] e1000: Trivial implementation of various MAC registers Leonid Bloch
@ 2015-11-04 2:44 ` Jason Wang
2015-11-04 15:21 ` Leonid Bloch
0 siblings, 1 reply; 17+ messages in thread
From: Jason Wang @ 2015-11-04 2:44 UTC (permalink / raw)
To: Leonid Bloch, qemu-devel; +Cc: Dmitry Fleytman, Leonid Bloch, Shmulik Ladkani
On 11/03/2015 07:14 PM, Leonid Bloch wrote:
> 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 | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++--
> hw/net/e1000_regs.h | 6 ++++
> 2 files changed, 98 insertions(+), 3 deletions(-)
>
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index 1190bbe..7db6614 100644
> --- a/hw/net/e1000.c
> +++ b/hw/net/e1000.c
> @@ -170,7 +170,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
> @@ -1118,6 +1128,48 @@ mac_readreg(E1000State *s, int index)
> }
>
> static uint32_t
> +mac_readreg_prt(E1000State *s, int index)
> +{
> + DBGOUT(GENERAL, "Reading register at offset: 0x%08x. "
> + "It is not fully implemented.\n", index<<2);
> + return s->mac_reg[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_low11_read_prt(E1000State *s, int index)
> +{
> + DBGOUT(GENERAL, "Reading register at offset: 0x%08x. "
> + "It is not fully implemented.\n", index<<2);
> + return s->mac_reg[index] & 0x7ff;
> +}
> +
> +static uint32_t
> +mac_low13_read_prt(E1000State *s, int index)
> +{
> + DBGOUT(GENERAL, "Reading register at offset: 0x%08x. "
> + "It is not fully implemented.\n", index<<2);
> + 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];
> @@ -1161,6 +1213,14 @@ mac_writereg(E1000State *s, int index, uint32_t val)
> }
>
> static void
> +mac_writereg_prt(E1000State *s, int index, uint32_t val)
> +{
> + DBGOUT(GENERAL, "Writing to register at offset: 0x%08x. "
> + "It is not fully implemented.\n", index<<2);
> + s->mac_reg[index] = val;
> +}
> +
> +static void
> set_rdt(E1000State *s, int index, uint32_t val)
> {
> s->mac_reg[index] = val & 0xffff;
> @@ -1219,26 +1279,50 @@ 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_prt, [RDFT] = mac_low13_read_prt,
> + [RDFHS] = mac_low13_read_prt, [RDFTS] = mac_low13_read_prt,
> + [RDFPC] = mac_low13_read_prt,
> + [TDFH] = mac_low11_read_prt, [TDFT] = mac_low11_read_prt,
> + [TDFHS] = mac_low13_read_prt, [TDFTS] = mac_low13_read_prt,
> + [TDFPC] = mac_low13_read_prt,
> + [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_prt,
> };
Need to limit the function of those MAC registers to 2.5 and above
(probably another flag and turn it off by default for pre 2.5) only.
Otherwise, the values of those registers will be zero after migrating to
old qemu.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH v4 3/7] e1000: Trivial implementation of various MAC registers
2015-11-04 2:44 ` Jason Wang
@ 2015-11-04 15:21 ` Leonid Bloch
2015-11-05 2:57 ` Jason Wang
0 siblings, 1 reply; 17+ messages in thread
From: Leonid Bloch @ 2015-11-04 15:21 UTC (permalink / raw)
To: Jason Wang; +Cc: Dmitry Fleytman, Leonid Bloch, qemu-devel, Shmulik Ladkani
On Wed, Nov 4, 2015 at 4:44 AM, Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 11/03/2015 07:14 PM, Leonid Bloch wrote:
>> 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 | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++--
>> hw/net/e1000_regs.h | 6 ++++
>> 2 files changed, 98 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
>> index 1190bbe..7db6614 100644
>> --- a/hw/net/e1000.c
>> +++ b/hw/net/e1000.c
>> @@ -170,7 +170,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
>> @@ -1118,6 +1128,48 @@ mac_readreg(E1000State *s, int index)
>> }
>>
>> static uint32_t
>> +mac_readreg_prt(E1000State *s, int index)
>> +{
>> + DBGOUT(GENERAL, "Reading register at offset: 0x%08x. "
>> + "It is not fully implemented.\n", index<<2);
>> + return s->mac_reg[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_low11_read_prt(E1000State *s, int index)
>> +{
>> + DBGOUT(GENERAL, "Reading register at offset: 0x%08x. "
>> + "It is not fully implemented.\n", index<<2);
>> + return s->mac_reg[index] & 0x7ff;
>> +}
>> +
>> +static uint32_t
>> +mac_low13_read_prt(E1000State *s, int index)
>> +{
>> + DBGOUT(GENERAL, "Reading register at offset: 0x%08x. "
>> + "It is not fully implemented.\n", index<<2);
>> + 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];
>> @@ -1161,6 +1213,14 @@ mac_writereg(E1000State *s, int index, uint32_t val)
>> }
>>
>> static void
>> +mac_writereg_prt(E1000State *s, int index, uint32_t val)
>> +{
>> + DBGOUT(GENERAL, "Writing to register at offset: 0x%08x. "
>> + "It is not fully implemented.\n", index<<2);
>> + s->mac_reg[index] = val;
>> +}
>> +
>> +static void
>> set_rdt(E1000State *s, int index, uint32_t val)
>> {
>> s->mac_reg[index] = val & 0xffff;
>> @@ -1219,26 +1279,50 @@ 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_prt, [RDFT] = mac_low13_read_prt,
>> + [RDFHS] = mac_low13_read_prt, [RDFTS] = mac_low13_read_prt,
>> + [RDFPC] = mac_low13_read_prt,
>> + [TDFH] = mac_low11_read_prt, [TDFT] = mac_low11_read_prt,
>> + [TDFHS] = mac_low13_read_prt, [TDFTS] = mac_low13_read_prt,
>> + [TDFPC] = mac_low13_read_prt,
>> + [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_prt,
>> };
>
> Need to limit the function of those MAC registers to 2.5 and above
> (probably another flag and turn it off by default for pre 2.5) only.
> Otherwise, the values of those registers will be zero after migrating to
> old qemu.
The registers in this patch are just for reading/writing, there is no
functionality for them in the emulated device itself. In case of
migration to an older version, there will be no access to them anyhow
- an attempt to access them will probably raise an error on an older
version (in fact, this is the reason why they are implemented now) so
zero value will not be a concern.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH v4 3/7] e1000: Trivial implementation of various MAC registers
2015-11-04 15:21 ` Leonid Bloch
@ 2015-11-05 2:57 ` Jason Wang
0 siblings, 0 replies; 17+ messages in thread
From: Jason Wang @ 2015-11-05 2:57 UTC (permalink / raw)
To: Leonid Bloch; +Cc: Dmitry Fleytman, Leonid Bloch, qemu-devel, Shmulik Ladkani
On 11/04/2015 11:21 PM, Leonid Bloch wrote:
> On Wed, Nov 4, 2015 at 4:44 AM, Jason Wang <jasowang@redhat.com> wrote:
>>
>> On 11/03/2015 07:14 PM, Leonid Bloch wrote:
>>> 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 | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++--
>>> hw/net/e1000_regs.h | 6 ++++
>>> 2 files changed, 98 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
>>> index 1190bbe..7db6614 100644
>>> --- a/hw/net/e1000.c
>>> +++ b/hw/net/e1000.c
>>> @@ -170,7 +170,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
>>> @@ -1118,6 +1128,48 @@ mac_readreg(E1000State *s, int index)
>>> }
>>>
>>> static uint32_t
>>> +mac_readreg_prt(E1000State *s, int index)
>>> +{
>>> + DBGOUT(GENERAL, "Reading register at offset: 0x%08x. "
>>> + "It is not fully implemented.\n", index<<2);
>>> + return s->mac_reg[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_low11_read_prt(E1000State *s, int index)
>>> +{
>>> + DBGOUT(GENERAL, "Reading register at offset: 0x%08x. "
>>> + "It is not fully implemented.\n", index<<2);
>>> + return s->mac_reg[index] & 0x7ff;
>>> +}
>>> +
>>> +static uint32_t
>>> +mac_low13_read_prt(E1000State *s, int index)
>>> +{
>>> + DBGOUT(GENERAL, "Reading register at offset: 0x%08x. "
>>> + "It is not fully implemented.\n", index<<2);
>>> + 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];
>>> @@ -1161,6 +1213,14 @@ mac_writereg(E1000State *s, int index, uint32_t val)
>>> }
>>>
>>> static void
>>> +mac_writereg_prt(E1000State *s, int index, uint32_t val)
>>> +{
>>> + DBGOUT(GENERAL, "Writing to register at offset: 0x%08x. "
>>> + "It is not fully implemented.\n", index<<2);
>>> + s->mac_reg[index] = val;
>>> +}
>>> +
>>> +static void
>>> set_rdt(E1000State *s, int index, uint32_t val)
>>> {
>>> s->mac_reg[index] = val & 0xffff;
>>> @@ -1219,26 +1279,50 @@ 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_prt, [RDFT] = mac_low13_read_prt,
>>> + [RDFHS] = mac_low13_read_prt, [RDFTS] = mac_low13_read_prt,
>>> + [RDFPC] = mac_low13_read_prt,
>>> + [TDFH] = mac_low11_read_prt, [TDFT] = mac_low11_read_prt,
>>> + [TDFHS] = mac_low13_read_prt, [TDFTS] = mac_low13_read_prt,
>>> + [TDFPC] = mac_low13_read_prt,
>>> + [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_prt,
>>> };
>> Need to limit the function of those MAC registers to 2.5 and above
>> (probably another flag and turn it off by default for pre 2.5) only.
>> Otherwise, the values of those registers will be zero after migrating to
>> old qemu.
> The registers in this patch are just for reading/writing, there is no
> functionality for them in the emulated device itself.
Yes, but this causes guest noticeable changes, e.g reading can have non
zero result after writing to them. For migration, we need to make sure
device behave same before and after migration. Otherwise, this may lead
unexpected result in guest which may be very hard to debug.
> In case of
> migration to an older version, there will be no access to them anyhow
> - an attempt to access them will probably raise an error on an older
> version (in fact, this is the reason why they are implemented now) so
> zero value will not be a concern.
>
For older version (at least 2.4), writing will be ignored and reading
will result zero. (no errors but just some warnings).
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH v4 4/7] e1000: Fixing the received/transmitted packets' counters
2015-11-03 11:14 [Qemu-devel] [PATCH v4 0/7] e1000: Various fixes and registers' implementation Leonid Bloch
` (2 preceding siblings ...)
2015-11-03 11:14 ` [Qemu-devel] [PATCH v4 3/7] e1000: Trivial implementation of various MAC registers Leonid Bloch
@ 2015-11-03 11:14 ` Leonid Bloch
2015-11-03 11:14 ` [Qemu-devel] [PATCH v4 5/7] e1000: Fixing the received/transmitted octets' counters Leonid Bloch
` (2 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Leonid Bloch @ 2015-11-03 11:14 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 7db6614..871f1b5 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -577,6 +577,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)
{
@@ -673,8 +681,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]++;
@@ -1087,8 +1095,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] 17+ messages in thread* [Qemu-devel] [PATCH v4 5/7] e1000: Fixing the received/transmitted octets' counters
2015-11-03 11:14 [Qemu-devel] [PATCH v4 0/7] e1000: Various fixes and registers' implementation Leonid Bloch
` (3 preceding siblings ...)
2015-11-03 11:14 ` [Qemu-devel] [PATCH v4 4/7] e1000: Fixing the received/transmitted packets' counters Leonid Bloch
@ 2015-11-03 11:14 ` Leonid Bloch
2015-11-03 11:14 ` [Qemu-devel] [PATCH v4 6/7] e1000: Fixing the packet address filtering procedure Leonid Bloch
2015-11-03 11:14 ` [Qemu-devel] [PATCH v4 7/7] e1000: Implementing various counters Leonid Bloch
6 siblings, 0 replies; 17+ messages in thread
From: Leonid Bloch @ 2015-11-03 11:14 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 871f1b5..d176a8a 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -585,6 +585,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)
{
@@ -634,7 +648,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) {
@@ -682,10 +696,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
@@ -1100,11 +1112,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] 17+ messages in thread* [Qemu-devel] [PATCH v4 6/7] e1000: Fixing the packet address filtering procedure
2015-11-03 11:14 [Qemu-devel] [PATCH v4 0/7] e1000: Various fixes and registers' implementation Leonid Bloch
` (4 preceding siblings ...)
2015-11-03 11:14 ` [Qemu-devel] [PATCH v4 5/7] e1000: Fixing the received/transmitted octets' counters Leonid Bloch
@ 2015-11-03 11:14 ` Leonid Bloch
2015-11-03 11:14 ` [Qemu-devel] [PATCH v4 7/7] e1000: Implementing various counters Leonid Bloch
6 siblings, 0 replies; 17+ messages in thread
From: Leonid Bloch @ 2015-11-03 11:14 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 d176a8a..af97e8a 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -868,6 +868,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));
@@ -877,14 +878,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] 17+ messages in thread* [Qemu-devel] [PATCH v4 7/7] e1000: Implementing various counters
2015-11-03 11:14 [Qemu-devel] [PATCH v4 0/7] e1000: Various fixes and registers' implementation Leonid Bloch
` (5 preceding siblings ...)
2015-11-03 11:14 ` [Qemu-devel] [PATCH v4 6/7] e1000: Fixing the packet address filtering procedure Leonid Bloch
@ 2015-11-03 11:14 ` Leonid Bloch
2015-11-04 2:46 ` Jason Wang
6 siblings, 1 reply; 17+ messages in thread
From: Leonid Bloch @ 2015-11-03 11:14 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 | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 73 insertions(+), 5 deletions(-)
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index af97e8a..fbda0d1 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -37,6 +37,8 @@
#include "e1000_regs.h"
+static const uint8_t bcast[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
+
#define E1000_DEBUG
#ifdef E1000_DEBUG
@@ -180,7 +182,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
@@ -585,6 +593,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)
{
@@ -599,6 +617,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)
{
@@ -636,12 +672,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
@@ -667,8 +708,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) {
@@ -698,6 +742,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
@@ -865,7 +911,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);
@@ -883,10 +928,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;
}
@@ -908,8 +955,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],
@@ -998,6 +1047,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;
@@ -1011,6 +1062,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;
@@ -1026,6 +1078,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;
}
@@ -1111,6 +1164,7 @@ e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt)
}
} while (desc_offset < total_size);
+ increase_size_stats(s, PRCregs, total_size);
inc_reg_if_not_full(s, TPR);
s->mac_reg[GPRC] = s->mac_reg[TPR];
/* TOR - Total Octets Received:
@@ -1119,6 +1173,8 @@ e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt)
* Always include FCS length (4) in size.
*/
grow_8reg_if_not_full(s, TORL, size+4);
+ s->mac_reg[GORCL] = s->mac_reg[TORL];
+ s->mac_reg[GORCH] = s->mac_reg[TORH];
n = E1000_ICS_RXT0;
if ((rdt = s->mac_reg[RDT]) < s->mac_reg[RDH])
@@ -1307,11 +1363,23 @@ static uint32_t (*macreg_readops[])(E1000State *, int) = {
getreg(TNCRS), getreg(SEC), getreg(CEXTERR), getreg(RLEC),
getreg(XONRXC), getreg(XONTXC), getreg(XOFFRXC), getreg(XOFFTXC),
getreg(RFC), getreg(RJC), getreg(RNBC), getreg(TSCTFC),
- getreg(MGTPRC), getreg(MGTPDC), getreg(MGTPTC),
+ getreg(MGTPRC), getreg(MGTPDC), getreg(MGTPTC), getreg(GORCL),
+ getreg(GOTCL),
[TOTH] = mac_read_clr8, [TORH] = mac_read_clr8,
+ [GOTCH] = mac_read_clr8, [GORCH] = mac_read_clr8,
+ [PRC64] = mac_read_clr4, [PRC127] = mac_read_clr4,
+ [PRC255] = mac_read_clr4, [PRC511] = mac_read_clr4,
+ [PRC1023] = mac_read_clr4, [PRC1522] = mac_read_clr4,
+ [PTC64] = mac_read_clr4, [PTC127] = mac_read_clr4,
+ [PTC255] = mac_read_clr4, [PTC511] = mac_read_clr4,
+ [PTC1023] = mac_read_clr4, [PTC1522] = mac_read_clr4,
[GPRC] = mac_read_clr4, [GPTC] = mac_read_clr4,
[TPT] = mac_read_clr4, [TPR] = mac_read_clr4,
+ [RUC] = mac_read_clr4, [ROC] = mac_read_clr4,
+ [BPRC] = mac_read_clr4, [MPRC] = mac_read_clr4,
+ [TSCTC] = mac_read_clr4, [BPTC] = mac_read_clr4,
+ [MPTC] = mac_read_clr4,
[ICR] = mac_icr_read, [EECD] = get_eecd,
[EERD] = flash_eerd_read,
[RDFH] = mac_low13_read_prt, [RDFT] = mac_low13_read_prt,
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH v4 7/7] e1000: Implementing various counters
2015-11-03 11:14 ` [Qemu-devel] [PATCH v4 7/7] e1000: Implementing various counters Leonid Bloch
@ 2015-11-04 2:46 ` Jason Wang
2015-11-04 15:44 ` Leonid Bloch
0 siblings, 1 reply; 17+ messages in thread
From: Jason Wang @ 2015-11-04 2:46 UTC (permalink / raw)
To: Leonid Bloch, qemu-devel; +Cc: Dmitry Fleytman, Leonid Bloch, Shmulik Ladkani
On 11/03/2015 07:14 PM, Leonid Bloch wrote:
> This implements the following Statistic registers (various counters)
> according to Intel's specs:
>
> TSCTC GOTCL GOTCH GORCL GORCH MPRC BPRC RUC ROC
> BPTC MPTC PTC... PRC...
>
> Signed-off-by: Leonid Bloch <leonid.bloch@ravellosystems.com>
> Signed-off-by: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
> ---
> hw/net/e1000.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 73 insertions(+), 5 deletions(-)
>
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index af97e8a..fbda0d1 100644
> --- a/hw/net/e1000.c
> +++ b/hw/net/e1000.c
> @@ -37,6 +37,8 @@
>
> #include "e1000_regs.h"
>
[...]
>
> @@ -1111,6 +1164,7 @@ e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt)
> }
> } while (desc_offset < total_size);
>
> + increase_size_stats(s, PRCregs, total_size);
> inc_reg_if_not_full(s, TPR);
> s->mac_reg[GPRC] = s->mac_reg[TPR];
> /* TOR - Total Octets Received:
> @@ -1119,6 +1173,8 @@ e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt)
> * Always include FCS length (4) in size.
> */
> grow_8reg_if_not_full(s, TORL, size+4);
> + s->mac_reg[GORCL] = s->mac_reg[TORL];
> + s->mac_reg[GORCH] = s->mac_reg[TORH];
>
> n = E1000_ICS_RXT0;
> if ((rdt = s->mac_reg[RDT]) < s->mac_reg[RDH])
> @@ -1307,11 +1363,23 @@ static uint32_t (*macreg_readops[])(E1000State *, int) = {
> getreg(TNCRS), getreg(SEC), getreg(CEXTERR), getreg(RLEC),
> getreg(XONRXC), getreg(XONTXC), getreg(XOFFRXC), getreg(XOFFTXC),
> getreg(RFC), getreg(RJC), getreg(RNBC), getreg(TSCTFC),
> - getreg(MGTPRC), getreg(MGTPDC), getreg(MGTPTC),
> + getreg(MGTPRC), getreg(MGTPDC), getreg(MGTPTC), getreg(GORCL),
> + getreg(GOTCL),
>
> [TOTH] = mac_read_clr8, [TORH] = mac_read_clr8,
> + [GOTCH] = mac_read_clr8, [GORCH] = mac_read_clr8,
> + [PRC64] = mac_read_clr4, [PRC127] = mac_read_clr4,
> + [PRC255] = mac_read_clr4, [PRC511] = mac_read_clr4,
> + [PRC1023] = mac_read_clr4, [PRC1522] = mac_read_clr4,
> + [PTC64] = mac_read_clr4, [PTC127] = mac_read_clr4,
> + [PTC255] = mac_read_clr4, [PTC511] = mac_read_clr4,
> + [PTC1023] = mac_read_clr4, [PTC1522] = mac_read_clr4,
> [GPRC] = mac_read_clr4, [GPTC] = mac_read_clr4,
> [TPT] = mac_read_clr4, [TPR] = mac_read_clr4,
> + [RUC] = mac_read_clr4, [ROC] = mac_read_clr4,
> + [BPRC] = mac_read_clr4, [MPRC] = mac_read_clr4,
> + [TSCTC] = mac_read_clr4, [BPTC] = mac_read_clr4,
> + [MPTC] = mac_read_clr4,
> [ICR] = mac_icr_read, [EECD] = get_eecd,
> [EERD] = flash_eerd_read,
> [RDFH] = mac_low13_read_prt, [RDFT] = mac_low13_read_prt,
Same issue with patch 3. Need limit the function of those registers
works only for 2.5 and post 2.5.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH v4 7/7] e1000: Implementing various counters
2015-11-04 2:46 ` Jason Wang
@ 2015-11-04 15:44 ` Leonid Bloch
2015-11-05 3:16 ` Jason Wang
0 siblings, 1 reply; 17+ messages in thread
From: Leonid Bloch @ 2015-11-04 15:44 UTC (permalink / raw)
To: Jason Wang; +Cc: Dmitry Fleytman, Leonid Bloch, qemu-devel, Shmulik Ladkani
On Wed, Nov 4, 2015 at 4:46 AM, Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 11/03/2015 07:14 PM, Leonid Bloch wrote:
>> This implements the following Statistic registers (various counters)
>> according to Intel's specs:
>>
>> TSCTC GOTCL GOTCH GORCL GORCH MPRC BPRC RUC ROC
>> BPTC MPTC PTC... PRC...
>>
>> Signed-off-by: Leonid Bloch <leonid.bloch@ravellosystems.com>
>> Signed-off-by: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
>> ---
>> hw/net/e1000.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++----
>> 1 file changed, 73 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
>> index af97e8a..fbda0d1 100644
>> --- a/hw/net/e1000.c
>> +++ b/hw/net/e1000.c
>> @@ -37,6 +37,8 @@
>>
>> #include "e1000_regs.h"
>>
>
> [...]
>
>>
>> @@ -1111,6 +1164,7 @@ e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt)
>> }
>> } while (desc_offset < total_size);
>>
>> + increase_size_stats(s, PRCregs, total_size);
>> inc_reg_if_not_full(s, TPR);
>> s->mac_reg[GPRC] = s->mac_reg[TPR];
>> /* TOR - Total Octets Received:
>> @@ -1119,6 +1173,8 @@ e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt)
>> * Always include FCS length (4) in size.
>> */
>> grow_8reg_if_not_full(s, TORL, size+4);
>> + s->mac_reg[GORCL] = s->mac_reg[TORL];
>> + s->mac_reg[GORCH] = s->mac_reg[TORH];
>>
>> n = E1000_ICS_RXT0;
>> if ((rdt = s->mac_reg[RDT]) < s->mac_reg[RDH])
>> @@ -1307,11 +1363,23 @@ static uint32_t (*macreg_readops[])(E1000State *, int) = {
>> getreg(TNCRS), getreg(SEC), getreg(CEXTERR), getreg(RLEC),
>> getreg(XONRXC), getreg(XONTXC), getreg(XOFFRXC), getreg(XOFFTXC),
>> getreg(RFC), getreg(RJC), getreg(RNBC), getreg(TSCTFC),
>> - getreg(MGTPRC), getreg(MGTPDC), getreg(MGTPTC),
>> + getreg(MGTPRC), getreg(MGTPDC), getreg(MGTPTC), getreg(GORCL),
>> + getreg(GOTCL),
>>
>> [TOTH] = mac_read_clr8, [TORH] = mac_read_clr8,
>> + [GOTCH] = mac_read_clr8, [GORCH] = mac_read_clr8,
>> + [PRC64] = mac_read_clr4, [PRC127] = mac_read_clr4,
>> + [PRC255] = mac_read_clr4, [PRC511] = mac_read_clr4,
>> + [PRC1023] = mac_read_clr4, [PRC1522] = mac_read_clr4,
>> + [PTC64] = mac_read_clr4, [PTC127] = mac_read_clr4,
>> + [PTC255] = mac_read_clr4, [PTC511] = mac_read_clr4,
>> + [PTC1023] = mac_read_clr4, [PTC1522] = mac_read_clr4,
>> [GPRC] = mac_read_clr4, [GPTC] = mac_read_clr4,
>> [TPT] = mac_read_clr4, [TPR] = mac_read_clr4,
>> + [RUC] = mac_read_clr4, [ROC] = mac_read_clr4,
>> + [BPRC] = mac_read_clr4, [MPRC] = mac_read_clr4,
>> + [TSCTC] = mac_read_clr4, [BPTC] = mac_read_clr4,
>> + [MPTC] = mac_read_clr4,
>> [ICR] = mac_icr_read, [EECD] = get_eecd,
>> [EERD] = flash_eerd_read,
>> [RDFH] = mac_low13_read_prt, [RDFT] = mac_low13_read_prt,
>
> Same issue with patch 3. Need limit the function of those registers
> works only for 2.5 and post 2.5.
Contrary to the registers in patch 3, these registers do have a
functionality in the device - they are counters. But they have a
simple logic and they are distributed throughout the code.
Contrary to that, for example, the registers that were implemented in
the patch that added interrupt mitigation support (e9845f098), are
concentrated in a single location, and a single "if" statement checks
for a flag: if it is set, some non-trivial logic begins (which is
skipped if these registers are not needed anyway).
In the current case, some 10 "if"s are needed to enable a simple logic
in many places. That may influence performance, and will certainly
make the code more bulky.
On the other hand, if these registers will function always, absolutely
no harm will be done if migrating to an older version: these registers
will simply be inaccessible, as they were so far.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH v4 7/7] e1000: Implementing various counters
2015-11-04 15:44 ` Leonid Bloch
@ 2015-11-05 3:16 ` Jason Wang
0 siblings, 0 replies; 17+ messages in thread
From: Jason Wang @ 2015-11-05 3:16 UTC (permalink / raw)
To: Leonid Bloch; +Cc: Dmitry Fleytman, Leonid Bloch, qemu-devel, Shmulik Ladkani
On 11/04/2015 11:44 PM, Leonid Bloch wrote:
> On Wed, Nov 4, 2015 at 4:46 AM, Jason Wang <jasowang@redhat.com> wrote:
>>
>> On 11/03/2015 07:14 PM, Leonid Bloch wrote:
>>> This implements the following Statistic registers (various counters)
>>> according to Intel's specs:
>>>
>>> TSCTC GOTCL GOTCH GORCL GORCH MPRC BPRC RUC ROC
>>> BPTC MPTC PTC... PRC...
>>>
>>> Signed-off-by: Leonid Bloch <leonid.bloch@ravellosystems.com>
>>> Signed-off-by: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
>>> ---
>>> hw/net/e1000.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++----
>>> 1 file changed, 73 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
>>> index af97e8a..fbda0d1 100644
>>> --- a/hw/net/e1000.c
>>> +++ b/hw/net/e1000.c
>>> @@ -37,6 +37,8 @@
>>>
>>> #include "e1000_regs.h"
>>>
>> [...]
>>
>>> @@ -1111,6 +1164,7 @@ e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt)
>>> }
>>> } while (desc_offset < total_size);
>>>
>>> + increase_size_stats(s, PRCregs, total_size);
>>> inc_reg_if_not_full(s, TPR);
>>> s->mac_reg[GPRC] = s->mac_reg[TPR];
>>> /* TOR - Total Octets Received:
>>> @@ -1119,6 +1173,8 @@ e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt)
>>> * Always include FCS length (4) in size.
>>> */
>>> grow_8reg_if_not_full(s, TORL, size+4);
>>> + s->mac_reg[GORCL] = s->mac_reg[TORL];
>>> + s->mac_reg[GORCH] = s->mac_reg[TORH];
>>>
>>> n = E1000_ICS_RXT0;
>>> if ((rdt = s->mac_reg[RDT]) < s->mac_reg[RDH])
>>> @@ -1307,11 +1363,23 @@ static uint32_t (*macreg_readops[])(E1000State *, int) = {
>>> getreg(TNCRS), getreg(SEC), getreg(CEXTERR), getreg(RLEC),
>>> getreg(XONRXC), getreg(XONTXC), getreg(XOFFRXC), getreg(XOFFTXC),
>>> getreg(RFC), getreg(RJC), getreg(RNBC), getreg(TSCTFC),
>>> - getreg(MGTPRC), getreg(MGTPDC), getreg(MGTPTC),
>>> + getreg(MGTPRC), getreg(MGTPDC), getreg(MGTPTC), getreg(GORCL),
>>> + getreg(GOTCL),
>>>
>>> [TOTH] = mac_read_clr8, [TORH] = mac_read_clr8,
>>> + [GOTCH] = mac_read_clr8, [GORCH] = mac_read_clr8,
>>> + [PRC64] = mac_read_clr4, [PRC127] = mac_read_clr4,
>>> + [PRC255] = mac_read_clr4, [PRC511] = mac_read_clr4,
>>> + [PRC1023] = mac_read_clr4, [PRC1522] = mac_read_clr4,
>>> + [PTC64] = mac_read_clr4, [PTC127] = mac_read_clr4,
>>> + [PTC255] = mac_read_clr4, [PTC511] = mac_read_clr4,
>>> + [PTC1023] = mac_read_clr4, [PTC1522] = mac_read_clr4,
>>> [GPRC] = mac_read_clr4, [GPTC] = mac_read_clr4,
>>> [TPT] = mac_read_clr4, [TPR] = mac_read_clr4,
>>> + [RUC] = mac_read_clr4, [ROC] = mac_read_clr4,
>>> + [BPRC] = mac_read_clr4, [MPRC] = mac_read_clr4,
>>> + [TSCTC] = mac_read_clr4, [BPTC] = mac_read_clr4,
>>> + [MPTC] = mac_read_clr4,
>>> [ICR] = mac_icr_read, [EECD] = get_eecd,
>>> [EERD] = flash_eerd_read,
>>> [RDFH] = mac_low13_read_prt, [RDFT] = mac_low13_read_prt,
>> Same issue with patch 3. Need limit the function of those registers
>> works only for 2.5 and post 2.5.
> Contrary to the registers in patch 3, these registers do have a
> functionality in the device - they are counters. But they have a
> simple logic and they are distributed throughout the code.
> Contrary to that, for example, the registers that were implemented in
> the patch that added interrupt mitigation support (e9845f098), are
> concentrated in a single location, and a single "if" statement checks
> for a flag: if it is set, some non-trivial logic begins (which is
> skipped if these registers are not needed anyway).
> In the current case, some 10 "if"s are needed to enable a simple logic
> in many places. That may influence performance, and will certainly
> make the code more bulky.
I agree, but this is the price of compatibility. (And which can make our
user happy). For performance, I doubt a single condition will cause
noticeable difference.
> On the other hand, if these registers will function always, absolutely
> no harm will be done if migrating to an older version: these registers
> will simply be inaccessible, as they were so far.
Same as patch 3, reading to those registers will have a zero value.
For the issue of bulky, how about something like this?
- introduce another array mac_regcap[] (which is like phy_regcap).
- store the compat flags required for the function of the registers in
this array, zero means no requirement
- check the enabled compat flag again this in e1000_mmio_write() and
e1000_mmio_read()
And this method would be even useful for future extension for e1000.
^ permalink raw reply [flat|nested] 17+ messages in thread