diff mbox

[v3,2/6] e1000: Trivial implementation of various MAC registers

Message ID 1446046288-25650-3-git-send-email-leonid.bloch@ravellosystems.com
State New
Headers show

Commit Message

Leonid Bloch Oct. 28, 2015, 3:31 p.m. UTC
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      | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 hw/net/e1000_regs.h |   6 ++
 2 files changed, 157 insertions(+), 3 deletions(-)

Comments

Jason Wang Oct. 29, 2015, 3:04 a.m. UTC | #1
On 10/28/2015 11:31 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      | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>  hw/net/e1000_regs.h |   6 ++
>  2 files changed, 157 insertions(+), 3 deletions(-)
>
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index 232edf1..fa65e79 100644
> --- a/hw/net/e1000.c
> +++ b/hw/net/e1000.c
> @@ -168,7 +168,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
> @@ -1116,6 +1126,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];
> @@ -1159,6 +1211,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;
> @@ -1217,25 +1277,49 @@ 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,
> @@ -1244,9 +1328,14 @@ static void (*macreg_writeops[])(E1000State *, int, uint32_t) = {
>      [EECD] = set_eecd,  [RCTL] = set_rx_control, [CTRL] = set_ctrl,
>      [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) };
> @@ -1390,6 +1479,64 @@ static const VMStateDescription vmstate_e1000_mit_state = {
>      }
>  };
>  
> +static bool e1000_extra_trivial_regs_needed(void *opaque)
> +{
> +    return true;
> +}

This reminds me  that we need care the migration compatibility to older
version here. Probably we need another property for e1000, and only
migrate and implement the new mac registers for version >= 2.5. An
example is mit implementation. (see
e9845f0985f088dd01790f4821026df0afba5795). And need to do the same for
patch 6.

> +
> +static const VMStateDescription vmstate_e1000_extra_trivial_regs = {
> +    .name = "e1000/extra_trivial_regs",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = e1000_extra_trivial_regs_needed,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32(mac_reg[AIT], E1000State),
> +        VMSTATE_UINT32(mac_reg[SCC], E1000State),
> +        VMSTATE_UINT32(mac_reg[ECOL], E1000State),
> +        VMSTATE_UINT32(mac_reg[MCC], E1000State),
> +        VMSTATE_UINT32(mac_reg[LATECOL], E1000State),
> +        VMSTATE_UINT32(mac_reg[COLC], E1000State),
> +        VMSTATE_UINT32(mac_reg[DC], E1000State),
> +        VMSTATE_UINT32(mac_reg[TNCRS], E1000State),
> +        VMSTATE_UINT32(mac_reg[SEC], E1000State),
> +        VMSTATE_UINT32(mac_reg[CEXTERR], E1000State),
> +        VMSTATE_UINT32(mac_reg[RLEC], E1000State),
> +        VMSTATE_UINT32(mac_reg[XONRXC], E1000State),
> +        VMSTATE_UINT32(mac_reg[XONTXC], E1000State),
> +        VMSTATE_UINT32(mac_reg[XOFFRXC], E1000State),
> +        VMSTATE_UINT32(mac_reg[XOFFTXC], E1000State),
> +        VMSTATE_UINT32(mac_reg[FCRUC], E1000State),
> +        VMSTATE_UINT32(mac_reg[RNBC], E1000State),
> +        VMSTATE_UINT32(mac_reg[RFC], E1000State),
> +        VMSTATE_UINT32(mac_reg[RJC], E1000State),
> +        VMSTATE_UINT32(mac_reg[MGTPRC], E1000State),
> +        VMSTATE_UINT32(mac_reg[MGTPDC], E1000State),
> +        VMSTATE_UINT32(mac_reg[MGTPTC], E1000State),
> +        VMSTATE_UINT32(mac_reg[TSCTFC], E1000State),
> +        VMSTATE_UINT32(mac_reg[WUC], E1000State),
> +        VMSTATE_UINT32(mac_reg[WUS], E1000State),
> +        VMSTATE_UINT32(mac_reg[IPAV], E1000State),
> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, IP4AT, 7),
> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, IP6AT, 4),
> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, WUPM, 32),
> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, FFLT, 7),
> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, FFMT, 255),
> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, FFVT, 255),
> +        VMSTATE_UINT32(mac_reg[RDFH], E1000State),
> +        VMSTATE_UINT32(mac_reg[RDFT], E1000State),
> +        VMSTATE_UINT32(mac_reg[RDFHS], E1000State),
> +        VMSTATE_UINT32(mac_reg[RDFTS], E1000State),
> +        VMSTATE_UINT32(mac_reg[RDFPC], E1000State),
> +        VMSTATE_UINT32(mac_reg[TDFH], E1000State),
> +        VMSTATE_UINT32(mac_reg[TDFT], E1000State),
> +        VMSTATE_UINT32(mac_reg[TDFHS], E1000State),
> +        VMSTATE_UINT32(mac_reg[TDFTS], E1000State),
> +        VMSTATE_UINT32(mac_reg[TDFPC], E1000State),
> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, PBM, 16384),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};

I was considering a better approach here. Since we may want to add more
mac register implementation in the future, so we probably don't want to
add another subsections like this. How about just migrate mac_reg array
instead of specific registers here?

Thanks

> +
>  static const VMStateDescription vmstate_e1000 = {
>      .name = "e1000",
>      .version_id = 2,
> @@ -1469,6 +1616,7 @@ static const VMStateDescription vmstate_e1000 = {
>      },
>      .subsections = (const VMStateDescription*[]) {
>          &vmstate_e1000_mit_state,
> +        &vmstate_e1000_extra_trivial_regs,
>          NULL
>      }
>  };
> 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 */
Leonid Bloch Oct. 29, 2015, 9:33 a.m. UTC | #2
On Thu, Oct 29, 2015 at 5:04 AM, Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 10/28/2015 11:31 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      | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  hw/net/e1000_regs.h |   6 ++
>>  2 files changed, 157 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
>> index 232edf1..fa65e79 100644
>> --- a/hw/net/e1000.c
>> +++ b/hw/net/e1000.c
>> @@ -168,7 +168,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
>> @@ -1116,6 +1126,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];
>> @@ -1159,6 +1211,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;
>> @@ -1217,25 +1277,49 @@ 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,
>> @@ -1244,9 +1328,14 @@ static void (*macreg_writeops[])(E1000State *, int, uint32_t) = {
>>      [EECD] = set_eecd,  [RCTL] = set_rx_control, [CTRL] = set_ctrl,
>>      [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) };
>> @@ -1390,6 +1479,64 @@ static const VMStateDescription vmstate_e1000_mit_state = {
>>      }
>>  };
>>
>> +static bool e1000_extra_trivial_regs_needed(void *opaque)
>> +{
>> +    return true;
>> +}
>
> This reminds me  that we need care the migration compatibility to older
> version here. Probably we need another property for e1000, and only
> migrate and implement the new mac registers for version >= 2.5. An
> example is mit implementation. (see
> e9845f0985f088dd01790f4821026df0afba5795). And need to do the same for
> patch 6.
>
>> +
>> +static const VMStateDescription vmstate_e1000_extra_trivial_regs = {
>> +    .name = "e1000/extra_trivial_regs",
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .needed = e1000_extra_trivial_regs_needed,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_UINT32(mac_reg[AIT], E1000State),
>> +        VMSTATE_UINT32(mac_reg[SCC], E1000State),
>> +        VMSTATE_UINT32(mac_reg[ECOL], E1000State),
>> +        VMSTATE_UINT32(mac_reg[MCC], E1000State),
>> +        VMSTATE_UINT32(mac_reg[LATECOL], E1000State),
>> +        VMSTATE_UINT32(mac_reg[COLC], E1000State),
>> +        VMSTATE_UINT32(mac_reg[DC], E1000State),
>> +        VMSTATE_UINT32(mac_reg[TNCRS], E1000State),
>> +        VMSTATE_UINT32(mac_reg[SEC], E1000State),
>> +        VMSTATE_UINT32(mac_reg[CEXTERR], E1000State),
>> +        VMSTATE_UINT32(mac_reg[RLEC], E1000State),
>> +        VMSTATE_UINT32(mac_reg[XONRXC], E1000State),
>> +        VMSTATE_UINT32(mac_reg[XONTXC], E1000State),
>> +        VMSTATE_UINT32(mac_reg[XOFFRXC], E1000State),
>> +        VMSTATE_UINT32(mac_reg[XOFFTXC], E1000State),
>> +        VMSTATE_UINT32(mac_reg[FCRUC], E1000State),
>> +        VMSTATE_UINT32(mac_reg[RNBC], E1000State),
>> +        VMSTATE_UINT32(mac_reg[RFC], E1000State),
>> +        VMSTATE_UINT32(mac_reg[RJC], E1000State),
>> +        VMSTATE_UINT32(mac_reg[MGTPRC], E1000State),
>> +        VMSTATE_UINT32(mac_reg[MGTPDC], E1000State),
>> +        VMSTATE_UINT32(mac_reg[MGTPTC], E1000State),
>> +        VMSTATE_UINT32(mac_reg[TSCTFC], E1000State),
>> +        VMSTATE_UINT32(mac_reg[WUC], E1000State),
>> +        VMSTATE_UINT32(mac_reg[WUS], E1000State),
>> +        VMSTATE_UINT32(mac_reg[IPAV], E1000State),
>> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, IP4AT, 7),
>> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, IP6AT, 4),
>> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, WUPM, 32),
>> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, FFLT, 7),
>> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, FFMT, 255),
>> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, FFVT, 255),
>> +        VMSTATE_UINT32(mac_reg[RDFH], E1000State),
>> +        VMSTATE_UINT32(mac_reg[RDFT], E1000State),
>> +        VMSTATE_UINT32(mac_reg[RDFHS], E1000State),
>> +        VMSTATE_UINT32(mac_reg[RDFTS], E1000State),
>> +        VMSTATE_UINT32(mac_reg[RDFPC], E1000State),
>> +        VMSTATE_UINT32(mac_reg[TDFH], E1000State),
>> +        VMSTATE_UINT32(mac_reg[TDFT], E1000State),
>> +        VMSTATE_UINT32(mac_reg[TDFHS], E1000State),
>> +        VMSTATE_UINT32(mac_reg[TDFTS], E1000State),
>> +        VMSTATE_UINT32(mac_reg[TDFPC], E1000State),
>> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, PBM, 16384),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>
> I was considering a better approach here. Since we may want to add more
> mac register implementation in the future, so we probably don't want to
> add another subsections like this. How about just migrate mac_reg array
> instead of specific registers here?
>
> Thanks
>
>> +
>>  static const VMStateDescription vmstate_e1000 = {
>>      .name = "e1000",
>>      .version_id = 2,
>> @@ -1469,6 +1616,7 @@ static const VMStateDescription vmstate_e1000 = {
>>      },
>>      .subsections = (const VMStateDescription*[]) {
>>          &vmstate_e1000_mit_state,
>> +        &vmstate_e1000_extra_trivial_regs,
>>          NULL
>>      }
>>  };
>> 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 */
>

So you mean adding another boolean parameter, "full_mac_migration",
for instance, and if it is set - migrate the full mac_reg array,
otherwise - migrate just the registers that were previously
implemented?

Leonid.
Jason Wang Oct. 29, 2015, 10:01 a.m. UTC | #3
On 10/29/2015 05:33 PM, Leonid Bloch wrote:
> On Thu, Oct 29, 2015 at 5:04 AM, Jason Wang <jasowang@redhat.com> wrote:
>>
>> On 10/28/2015 11:31 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      | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>  hw/net/e1000_regs.h |   6 ++
>>>  2 files changed, 157 insertions(+), 3 deletions(-)

[...]

>>>
>>> +static bool e1000_extra_trivial_regs_needed(void *opaque)
>>> +{
>>> +    return true;
>>> +}
>> This reminds me  that we need care the migration compatibility to older
>> version here. Probably we need another property for e1000, and only
>> migrate and implement the new mac registers for version >= 2.5. An
>> example is mit implementation. (see
>> e9845f0985f088dd01790f4821026df0afba5795). And need to do the same for
>> patch 6.
>>
>>> +
>>> +static const VMStateDescription vmstate_e1000_extra_trivial_regs = {
>>> +    .name = "e1000/extra_trivial_regs",
>>> +    .version_id = 1,
>>> +    .minimum_version_id = 1,
>>> +    .needed = e1000_extra_trivial_regs_needed,
>>> +    .fields = (VMStateField[]) {
>>> +        VMSTATE_UINT32(mac_reg[AIT], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[SCC], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[ECOL], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[MCC], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[LATECOL], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[COLC], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[DC], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[TNCRS], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[SEC], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[CEXTERR], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[RLEC], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[XONRXC], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[XONTXC], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[XOFFRXC], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[XOFFTXC], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[FCRUC], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[RNBC], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[RFC], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[RJC], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[MGTPRC], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[MGTPDC], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[MGTPTC], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[TSCTFC], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[WUC], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[WUS], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[IPAV], E1000State),
>>> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, IP4AT, 7),
>>> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, IP6AT, 4),
>>> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, WUPM, 32),
>>> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, FFLT, 7),
>>> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, FFMT, 255),
>>> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, FFVT, 255),
>>> +        VMSTATE_UINT32(mac_reg[RDFH], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[RDFT], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[RDFHS], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[RDFTS], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[RDFPC], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[TDFH], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[TDFT], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[TDFHS], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[TDFTS], E1000State),
>>> +        VMSTATE_UINT32(mac_reg[TDFPC], E1000State),
>>> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, PBM, 16384),
>>> +        VMSTATE_END_OF_LIST()
>>> +    }
>>> +};
>> I was considering a better approach here. Since we may want to add more
>> mac register implementation in the future, so we probably don't want to
>> add another subsections like this. How about just migrate mac_reg array
>> instead of specific registers here?
>>
>> Thanks
>>
>>> +
>>>  static const VMStateDescription vmstate_e1000 = {
>>>      .name = "e1000",
>>>      .version_id = 2,
>>> @@ -1469,6 +1616,7 @@ static const VMStateDescription vmstate_e1000 = {
>>>      },
>>>      .subsections = (const VMStateDescription*[]) {
>>>          &vmstate_e1000_mit_state,
>>> +        &vmstate_e1000_extra_trivial_regs,
>>>          NULL
>>>      }
>>>  };
>>> 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 */
> So you mean adding another boolean parameter, "full_mac_migration",
> for instance, and if it is set - migrate the full mac_reg array,
> otherwise - migrate just the registers that were previously
> implemented?
>
> Leonid.
> __________
>

Something like this, but this parameter may also be used to decide
whether or not we should emulate the new mac registers, so maybe a
better name is needed and we it will imply to migrate full mac_reg
array. (Since patch 6 adds even more mac registers, maybe it's better to
squash that one into this?)
Leonid Bloch Oct. 29, 2015, 10:27 a.m. UTC | #4
On Thu, Oct 29, 2015 at 12:01 PM, Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 10/29/2015 05:33 PM, Leonid Bloch wrote:
>> On Thu, Oct 29, 2015 at 5:04 AM, Jason Wang <jasowang@redhat.com> wrote:
>>>
>>> On 10/28/2015 11:31 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      | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>>  hw/net/e1000_regs.h |   6 ++
>>>>  2 files changed, 157 insertions(+), 3 deletions(-)
>
> [...]
>
>>>>
>>>> +static bool e1000_extra_trivial_regs_needed(void *opaque)
>>>> +{
>>>> +    return true;
>>>> +}
>>> This reminds me  that we need care the migration compatibility to older
>>> version here. Probably we need another property for e1000, and only
>>> migrate and implement the new mac registers for version >= 2.5. An
>>> example is mit implementation. (see
>>> e9845f0985f088dd01790f4821026df0afba5795). And need to do the same for
>>> patch 6.
>>>
>>>> +
>>>> +static const VMStateDescription vmstate_e1000_extra_trivial_regs = {
>>>> +    .name = "e1000/extra_trivial_regs",
>>>> +    .version_id = 1,
>>>> +    .minimum_version_id = 1,
>>>> +    .needed = e1000_extra_trivial_regs_needed,
>>>> +    .fields = (VMStateField[]) {
>>>> +        VMSTATE_UINT32(mac_reg[AIT], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[SCC], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[ECOL], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[MCC], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[LATECOL], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[COLC], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[DC], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[TNCRS], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[SEC], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[CEXTERR], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[RLEC], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[XONRXC], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[XONTXC], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[XOFFRXC], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[XOFFTXC], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[FCRUC], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[RNBC], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[RFC], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[RJC], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[MGTPRC], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[MGTPDC], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[MGTPTC], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[TSCTFC], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[WUC], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[WUS], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[IPAV], E1000State),
>>>> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, IP4AT, 7),
>>>> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, IP6AT, 4),
>>>> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, WUPM, 32),
>>>> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, FFLT, 7),
>>>> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, FFMT, 255),
>>>> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, FFVT, 255),
>>>> +        VMSTATE_UINT32(mac_reg[RDFH], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[RDFT], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[RDFHS], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[RDFTS], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[RDFPC], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[TDFH], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[TDFT], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[TDFHS], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[TDFTS], E1000State),
>>>> +        VMSTATE_UINT32(mac_reg[TDFPC], E1000State),
>>>> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, PBM, 16384),
>>>> +        VMSTATE_END_OF_LIST()
>>>> +    }
>>>> +};
>>> I was considering a better approach here. Since we may want to add more
>>> mac register implementation in the future, so we probably don't want to
>>> add another subsections like this. How about just migrate mac_reg array
>>> instead of specific registers here?
>>>
>>> Thanks
>>>
>>>> +
>>>>  static const VMStateDescription vmstate_e1000 = {
>>>>      .name = "e1000",
>>>>      .version_id = 2,
>>>> @@ -1469,6 +1616,7 @@ static const VMStateDescription vmstate_e1000 = {
>>>>      },
>>>>      .subsections = (const VMStateDescription*[]) {
>>>>          &vmstate_e1000_mit_state,
>>>> +        &vmstate_e1000_extra_trivial_regs,
>>>>          NULL
>>>>      }
>>>>  };
>>>> 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 */
>> So you mean adding another boolean parameter, "full_mac_migration",
>> for instance, and if it is set - migrate the full mac_reg array,
>> otherwise - migrate just the registers that were previously
>> implemented?
>>
>> Leonid.
>> __________
>>
>
> Something like this, but this parameter may also be used to decide
> whether or not we should emulate the new mac registers, so maybe a
> better name is needed and we it will imply to migrate full mac_reg
> array. (Since patch 6 adds even more mac registers, maybe it's better to
> squash that one into this?)
>
In such case, I'd add a patch #2, introducing this parameter, say
"extra_mac_registers", that at first will take care of the migration
of the entire mac_reg array, and in the consecutive patches, where new
registers are introduced, it will also control their implementation.
But: can you think of a use case where someone will start a Qemu VM
knowing ahead of time that migration to an older version may occur?
This seems highly unlikely to me, but more code will be needed for
this unlikely scenario, which will make it less understandable and
harder to maintain, IMHO.

Leonid.
Jason Wang Oct. 30, 2015, 3:22 a.m. UTC | #5
On 10/29/2015 06:27 PM, Leonid Bloch wrote:
> On Thu, Oct 29, 2015 at 12:01 PM, Jason Wang <jasowang@redhat.com> wrote:
>>
>> On 10/29/2015 05:33 PM, Leonid Bloch wrote:
>>> On Thu, Oct 29, 2015 at 5:04 AM, Jason Wang <jasowang@redhat.com> wrote:
>>>> On 10/28/2015 11:31 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      | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>>>  hw/net/e1000_regs.h |   6 ++
>>>>>  2 files changed, 157 insertions(+), 3 deletions(-)
>> [...]
>>
>>>>> +static bool e1000_extra_trivial_regs_needed(void *opaque)
>>>>> +{
>>>>> +    return true;
>>>>> +}
>>>> This reminds me  that we need care the migration compatibility to older
>>>> version here. Probably we need another property for e1000, and only
>>>> migrate and implement the new mac registers for version >= 2.5. An
>>>> example is mit implementation. (see
>>>> e9845f0985f088dd01790f4821026df0afba5795). And need to do the same for
>>>> patch 6.
>>>>
>>>>> +
>>>>> +static const VMStateDescription vmstate_e1000_extra_trivial_regs = {
>>>>> +    .name = "e1000/extra_trivial_regs",
>>>>> +    .version_id = 1,
>>>>> +    .minimum_version_id = 1,
>>>>> +    .needed = e1000_extra_trivial_regs_needed,
>>>>> +    .fields = (VMStateField[]) {
>>>>> +        VMSTATE_UINT32(mac_reg[AIT], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[SCC], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[ECOL], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[MCC], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[LATECOL], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[COLC], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[DC], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[TNCRS], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[SEC], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[CEXTERR], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[RLEC], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[XONRXC], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[XONTXC], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[XOFFRXC], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[XOFFTXC], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[FCRUC], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[RNBC], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[RFC], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[RJC], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[MGTPRC], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[MGTPDC], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[MGTPTC], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[TSCTFC], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[WUC], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[WUS], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[IPAV], E1000State),
>>>>> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, IP4AT, 7),
>>>>> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, IP6AT, 4),
>>>>> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, WUPM, 32),
>>>>> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, FFLT, 7),
>>>>> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, FFMT, 255),
>>>>> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, FFVT, 255),
>>>>> +        VMSTATE_UINT32(mac_reg[RDFH], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[RDFT], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[RDFHS], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[RDFTS], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[RDFPC], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[TDFH], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[TDFT], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[TDFHS], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[TDFTS], E1000State),
>>>>> +        VMSTATE_UINT32(mac_reg[TDFPC], E1000State),
>>>>> +        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, PBM, 16384),
>>>>> +        VMSTATE_END_OF_LIST()
>>>>> +    }
>>>>> +};
>>>> I was considering a better approach here. Since we may want to add more
>>>> mac register implementation in the future, so we probably don't want to
>>>> add another subsections like this. How about just migrate mac_reg array
>>>> instead of specific registers here?
>>>>
>>>> Thanks
>>>>
>>>>> +
>>>>>  static const VMStateDescription vmstate_e1000 = {
>>>>>      .name = "e1000",
>>>>>      .version_id = 2,
>>>>> @@ -1469,6 +1616,7 @@ static const VMStateDescription vmstate_e1000 = {
>>>>>      },
>>>>>      .subsections = (const VMStateDescription*[]) {
>>>>>          &vmstate_e1000_mit_state,
>>>>> +        &vmstate_e1000_extra_trivial_regs,
>>>>>          NULL
>>>>>      }
>>>>>  };
>>>>> 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 */
>>> So you mean adding another boolean parameter, "full_mac_migration",
>>> for instance, and if it is set - migrate the full mac_reg array,
>>> otherwise - migrate just the registers that were previously
>>> implemented?
>>>
>>> Leonid.
>>> __________
>>>
>> Something like this, but this parameter may also be used to decide
>> whether or not we should emulate the new mac registers, so maybe a
>> better name is needed and we it will imply to migrate full mac_reg
>> array. (Since patch 6 adds even more mac registers, maybe it's better to
>> squash that one into this?)
>>
> In such case, I'd add a patch #2, introducing this parameter, say
> "extra_mac_registers", that at first will take care of the migration
> of the entire mac_reg array, and in the consecutive patches, where new
> registers are introduced, it will also control their implementation.
> But: can you think of a use case where someone will start a Qemu VM
> knowing ahead of time that migration to an older version may occur?

Yes. It's not rare in production environment. Consider some old machines
are running qemu 2.4, but some new ones are running qemu 2.5. If we want
to do migration between those machine for some reason (e.g load balance
or other), the only thing that can preserve guest ABI is using machine
type 2.4. So in this case, it's possible that we may migrate a VM from
2.5 (with machine type 2.4) to 2.4. We've made lots of effort for keep
compatibility in the past.

> This seems highly unlikely to me, but more code will be needed for
> this unlikely scenario, which will make it less understandable and
> harder to maintain, IMHO.
>
> Leonid.
> _

I agree the code may become more complex than just implementing the
function itself. But keeping migration works across different versions
are important for users or customers. For the case of e1000, it would be
better that all mac regs was migrated since first today. But for some
reason, it doesn't, and we couldn't change exist codes. So we need
handle this.
diff mbox

Patch

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 232edf1..fa65e79 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -168,7 +168,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
@@ -1116,6 +1126,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];
@@ -1159,6 +1211,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;
@@ -1217,25 +1277,49 @@  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,
@@ -1244,9 +1328,14 @@  static void (*macreg_writeops[])(E1000State *, int, uint32_t) = {
     [EECD] = set_eecd,  [RCTL] = set_rx_control, [CTRL] = set_ctrl,
     [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) };
@@ -1390,6 +1479,64 @@  static const VMStateDescription vmstate_e1000_mit_state = {
     }
 };
 
+static bool e1000_extra_trivial_regs_needed(void *opaque)
+{
+    return true;
+}
+
+static const VMStateDescription vmstate_e1000_extra_trivial_regs = {
+    .name = "e1000/extra_trivial_regs",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = e1000_extra_trivial_regs_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(mac_reg[AIT], E1000State),
+        VMSTATE_UINT32(mac_reg[SCC], E1000State),
+        VMSTATE_UINT32(mac_reg[ECOL], E1000State),
+        VMSTATE_UINT32(mac_reg[MCC], E1000State),
+        VMSTATE_UINT32(mac_reg[LATECOL], E1000State),
+        VMSTATE_UINT32(mac_reg[COLC], E1000State),
+        VMSTATE_UINT32(mac_reg[DC], E1000State),
+        VMSTATE_UINT32(mac_reg[TNCRS], E1000State),
+        VMSTATE_UINT32(mac_reg[SEC], E1000State),
+        VMSTATE_UINT32(mac_reg[CEXTERR], E1000State),
+        VMSTATE_UINT32(mac_reg[RLEC], E1000State),
+        VMSTATE_UINT32(mac_reg[XONRXC], E1000State),
+        VMSTATE_UINT32(mac_reg[XONTXC], E1000State),
+        VMSTATE_UINT32(mac_reg[XOFFRXC], E1000State),
+        VMSTATE_UINT32(mac_reg[XOFFTXC], E1000State),
+        VMSTATE_UINT32(mac_reg[FCRUC], E1000State),
+        VMSTATE_UINT32(mac_reg[RNBC], E1000State),
+        VMSTATE_UINT32(mac_reg[RFC], E1000State),
+        VMSTATE_UINT32(mac_reg[RJC], E1000State),
+        VMSTATE_UINT32(mac_reg[MGTPRC], E1000State),
+        VMSTATE_UINT32(mac_reg[MGTPDC], E1000State),
+        VMSTATE_UINT32(mac_reg[MGTPTC], E1000State),
+        VMSTATE_UINT32(mac_reg[TSCTFC], E1000State),
+        VMSTATE_UINT32(mac_reg[WUC], E1000State),
+        VMSTATE_UINT32(mac_reg[WUS], E1000State),
+        VMSTATE_UINT32(mac_reg[IPAV], E1000State),
+        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, IP4AT, 7),
+        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, IP6AT, 4),
+        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, WUPM, 32),
+        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, FFLT, 7),
+        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, FFMT, 255),
+        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, FFVT, 255),
+        VMSTATE_UINT32(mac_reg[RDFH], E1000State),
+        VMSTATE_UINT32(mac_reg[RDFT], E1000State),
+        VMSTATE_UINT32(mac_reg[RDFHS], E1000State),
+        VMSTATE_UINT32(mac_reg[RDFTS], E1000State),
+        VMSTATE_UINT32(mac_reg[RDFPC], E1000State),
+        VMSTATE_UINT32(mac_reg[TDFH], E1000State),
+        VMSTATE_UINT32(mac_reg[TDFT], E1000State),
+        VMSTATE_UINT32(mac_reg[TDFHS], E1000State),
+        VMSTATE_UINT32(mac_reg[TDFTS], E1000State),
+        VMSTATE_UINT32(mac_reg[TDFPC], E1000State),
+        VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, PBM, 16384),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription vmstate_e1000 = {
     .name = "e1000",
     .version_id = 2,
@@ -1469,6 +1616,7 @@  static const VMStateDescription vmstate_e1000 = {
     },
     .subsections = (const VMStateDescription*[]) {
         &vmstate_e1000_mit_state,
+        &vmstate_e1000_extra_trivial_regs,
         NULL
     }
 };
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 */