qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Leonid Bloch
Subject: Re: [Qemu-devel] [PATCH v3 2/6] e1000: Trivial implementation of various MAC registers
Date: Thu, 29 Oct 2015 11:33:22 +0200

On Thu, Oct 29, 2015 at 5:04 AM, Jason Wang <address@hidden> 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 <address@hidden>
>> Signed-off-by: Dmitry Fleytman <address@hidden>
>> ---
>>  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.
__________



reply via email to

[Prev in Thread] Current Thread [Next in Thread]