Re: [Qemu-devel] [PATCH 2/6] e1000: Trivial implementation of various MAC registers

[Jason Wang]

On 10/18/2015 03:53 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.
>
> The registers implemented here are:
>
> Transmit:
> RW: AIT
>
> Management:
> RW: WUC     WUS     IPAV*   IP6AT*  IP4AT*  FFLT*   WUPM*   FFMT*   FFVT*

My version of DSM (Revision) said WUS is read only.

>
> Diagnostic:
> RW: RDFH    RDFT    RDFHS   RDFTS   RDFPC   PBM*

For those diagnostic register, isn't it better to warn the incomplete
emulation instead of trying to give all zero values silently? I suspect
this can make diagnostic software think the device is malfunction?

>
> Statistic:
> RW: FCRUC   TDFH    TDFT    TDFHS   TDFTS   TDFPC

TDFH    TDFT    TDFHS   TDFTS   TDFPC should be Diagnostic?

> 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 <address@hidden>
> Signed-off-by: Dmitry Fleytman <address@hidden>
> ---
>  hw/net/e1000.c      | 52 +++++++++++++++++++++++++++++++++++++++++++++++++---
>  hw/net/e1000_regs.h |  6 ++++++
>  2 files changed, 55 insertions(+), 3 deletions(-)
>
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index 6d57663..6f754ac 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
> @@ -1114,6 +1124,18 @@ mac_readreg(E1000State *s, int index)
>  }
>  
>  static uint32_t
> +mac_low11_read(E1000State *s, int index)
> +{
> +    return s->mac_reg[index] & 0x7ff;
> +}
> +
> +static uint32_t
> +mac_low13_read(E1000State *s, int index)
> +{
> +    return s->mac_reg[index] & 0x1fff;
> +}
> +
> +static uint32_t
>  mac_icr_read(E1000State *s, int index)
>  {
>      uint32_t ret = s->mac_reg[ICR];
> @@ -1215,16 +1237,31 @@ 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(AIT),      getreg(SCC),

For AIT should we use low16_read() ?

And low4_read() for FFMT?

> +    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,   [TPR] = 
> mac_read_clr4,
>      [TPT] = mac_read_clr4,
>      [ICR] = mac_icr_read,     [EECD] = get_eecd,        [EERD] = 
> flash_eerd_read,
> +    [RDFH] = mac_low13_read,  [RDFT] = mac_low13_read,
> +    [RDFHS] = mac_low13_read, [RDFTS] = mac_low13_read, [RDFPC] = 
> mac_low13_read,
> +    [TDFH] = mac_low11_read,  [TDFT] = mac_low11_read,
> +    [TDFHS] = mac_low13_read, [TDFTS] = mac_low13_read, [TDFPC] = 
> mac_low13_read,
>      [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_readreg, [FFVT ... FFVT+254] = &mac_readreg,
> +    [PBM ... PBM+16383] = &mac_readreg,
>  };
>  enum { NREADOPS = ARRAY_SIZE(macreg_readops) };
>  
> @@ -1232,7 +1269,11 @@ enum { NREADOPS = ARRAY_SIZE(macreg_readops) };
>  static void (*macreg_writeops[])(E1000State *, int, uint32_t) = {
>      putreg(PBA),      putreg(EERD),     putreg(SWSM),     putreg(WUFC),
>      putreg(TDBAL),    putreg(TDBAH),    putreg(TXDCTL),   putreg(RDBAH),
> -    putreg(RDBAL),    putreg(LEDCTL),   putreg(VET),
> +    putreg(RDBAL),    putreg(LEDCTL),   putreg(VET),      putreg(FCRUC),
> +    putreg(TDFH),     putreg(TDFT),     putreg(TDFHS),    putreg(TDFTS),
> +    putreg(TDFPC),    putreg(RDFH),     putreg(RDFT),     putreg(RDFHS),
> +    putreg(RDFTS),    putreg(RDFPC),    putreg(IPAV),     putreg(WUC),
> +    putreg(WUS),      putreg(AIT),
>  
>      [TDLEN] = set_dlen, [RDLEN] = set_dlen,      [TCTL] = set_tctl,
>      [TDT] = set_tctl,   [MDIC] = set_mdic,       [ICS] = set_ics,
> @@ -1241,9 +1282,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,
>  };
>  
>  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 */