[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.
__________
[Qemu-devel] [PATCH v3 4/6] e1000: Fixing the received/transmitted octets' counters, Leonid Bloch, 2015/10/28
[Qemu-devel] [PATCH v3 6/6] e1000: Implementing various counters, Leonid Bloch, 2015/10/28
[Qemu-devel] [PATCH v3 5/6] e1000: Fixing the packet address filtering procedure, Leonid Bloch, 2015/10/28