From: Jason Wang <jasowang@redhat.com>
To: Leonid Bloch <leonid.bloch@ravellosystems.com>, qemu-devel@nongnu.org
Cc: Dmitry Fleytman <dmitry@daynix.com>,
Leonid Bloch <leonid@daynix.com>,
Shmulik Ladkani <shmulik.ladkani@ravellosystems.com>
Subject: Re: [Qemu-devel] [PATCH v3 2/6] e1000: Trivial implementation of various MAC registers
Date: Thu, 29 Oct 2015 11:04:23 +0800 [thread overview]
Message-ID: <56318CB7.1000507@redhat.com> (raw)
In-Reply-To: <1446046288-25650-3-git-send-email-leonid.bloch@ravellosystems.com>
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 */
next prev parent reply other threads:[~2015-10-29 3:04 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-28 15:31 [Qemu-devel] [PATCH v3 0/6] e1000: Various fixes and registers' implementation Leonid Bloch
2015-10-28 15:31 ` [Qemu-devel] [PATCH v3 1/6] e1000: Cosmetic and alignment fixes Leonid Bloch
2015-10-28 15:31 ` [Qemu-devel] [PATCH v3 2/6] e1000: Trivial implementation of various MAC registers Leonid Bloch
2015-10-29 3:04 ` Jason Wang [this message]
2015-10-29 9:33 ` Leonid Bloch
2015-10-29 10:01 ` Jason Wang
2015-10-29 10:27 ` Leonid Bloch
2015-10-30 3:22 ` Jason Wang
2015-10-28 15:31 ` [Qemu-devel] [PATCH v3 3/6] e1000: Fixing the received/transmitted packets' counters Leonid Bloch
2015-10-28 15:31 ` [Qemu-devel] [PATCH v3 4/6] e1000: Fixing the received/transmitted octets' counters Leonid Bloch
2015-10-28 15:31 ` [Qemu-devel] [PATCH v3 5/6] e1000: Fixing the packet address filtering procedure Leonid Bloch
2015-10-28 15:31 ` [Qemu-devel] [PATCH v3 6/6] e1000: Implementing various counters Leonid Bloch
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56318CB7.1000507@redhat.com \
--to=jasowang@redhat.com \
--cc=dmitry@daynix.com \
--cc=leonid.bloch@ravellosystems.com \
--cc=leonid@daynix.com \
--cc=qemu-devel@nongnu.org \
--cc=shmulik.ladkani@ravellosystems.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).