From: "Michael S. Tsirkin" <mst@redhat.com>
To: Stefan Weil <weil@mail.berlios.de>
Cc: QEMU Developers <qemu-devel@nongnu.org>
Subject: [Qemu-devel] Re: [PATCH 1/3] eepro100: Restructure code (new function tx_command)
Date: Thu, 14 Jan 2010 13:19:05 +0200 [thread overview]
Message-ID: <20100114111905.GA5115@redhat.com> (raw)
In-Reply-To: <1261324344-10642-1-git-send-email-weil@mail.berlios.de>
On Sun, Dec 20, 2009 at 04:52:22PM +0100, Stefan Weil wrote:
> Handling of transmit commands is rather complex,
> so about 80 lines of code were moved from function
> action_command to the new function tx_command.
>
> The two new values "tx" and "cb_address" in the
> eepro100 status structure made this possible without
> passing too many parameters.
>
> In addition, the moved code was cleaned a little bit:
> old comments marked with //~ were removed, C++ style
> comments were replaced by C style comments, C++ like
> variable declarations after code were reordered.
>
> Simplified mode is still broken. Nor did I fix
> endianess issues. Both problems will be fixed in
> additional patches (which need this one).
>
> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
Thanks, applied.
I merged this with the following commit: no reason
to change the same line twice.
> ---
> hw/eepro100.c | 192 +++++++++++++++++++++++++++++----------------------------
> 1 files changed, 98 insertions(+), 94 deletions(-)
>
> diff --git a/hw/eepro100.c b/hw/eepro100.c
> index 2a9e3b5..5635f61 100644
> --- a/hw/eepro100.c
> +++ b/hw/eepro100.c
> @@ -213,6 +213,10 @@ typedef struct {
> uint32_t ru_offset; /* RU address offset */
> uint32_t statsaddr; /* pointer to eepro100_stats_t */
>
> + /* Temporary data. */
> + eepro100_tx_t tx;
> + uint32_t cb_address;
> +
> /* Statistical counters. Also used for wake-up packet (i82559). */
> eepro100_stats_t statistics;
>
> @@ -748,17 +752,100 @@ static void dump_statistics(EEPRO100State * s)
> //~ missing("CU dump statistical counters");
> }
>
> +static void tx_command(EEPRO100State *s)
> +{
> + uint32_t tbd_array = le32_to_cpu(s->tx.tx_desc_addr);
> + uint16_t tcb_bytes = (le16_to_cpu(s->tx.tcb_bytes) & 0x3fff);
> + /* Sends larger than MAX_ETH_FRAME_SIZE are allowed, up to 2600 bytes. */
> + uint8_t buf[2600];
> + uint16_t size = 0;
> + uint32_t tbd_address = s->cb_address + 0x10;
> + TRACE(RXTX, logout
> + ("transmit, TBD array address 0x%08x, TCB byte count 0x%04x, TBD count %u\n",
> + tbd_array, tcb_bytes, s->tx.tbd_count));
> +
> + if (tcb_bytes > 2600) {
> + logout("TCB byte count too large, using 2600\n");
> + tcb_bytes = 2600;
> + }
> + if (!((tcb_bytes > 0) || (tbd_array != 0xffffffff))) {
> + logout
> + ("illegal values of TBD array address and TCB byte count!\n");
> + }
> + assert(tcb_bytes <= sizeof(buf));
> + while (size < tcb_bytes) {
> + uint32_t tx_buffer_address = ldl_phys(tbd_address);
> + uint16_t tx_buffer_size = lduw_phys(tbd_address + 4);
> + //~ uint16_t tx_buffer_el = lduw_phys(tbd_address + 6);
> + tbd_address += 8;
> + TRACE(RXTX, logout
> + ("TBD (simplified mode): buffer address 0x%08x, size 0x%04x\n",
> + tx_buffer_address, tx_buffer_size));
> + tx_buffer_size = MIN(tx_buffer_size, sizeof(buf) - size);
> + cpu_physical_memory_read(tx_buffer_address, &buf[size],
> + tx_buffer_size);
> + size += tx_buffer_size;
> + }
> + if (tbd_array == 0xffffffff) {
> + /* Simplified mode. Was already handled by code above. */
> + } else {
> + /* Flexible mode. */
> + uint8_t tbd_count = 0;
> + if (s->has_extended_tcb_support && !(s->configuration[6] & BIT(4))) {
> + /* Extended Flexible TCB. */
> + for (; tbd_count < 2; tbd_count++) {
> + uint32_t tx_buffer_address = ldl_phys(tbd_address);
> + uint16_t tx_buffer_size = lduw_phys(tbd_address + 4);
> + uint16_t tx_buffer_el = lduw_phys(tbd_address + 6);
> + tbd_address += 8;
> + TRACE(RXTX, logout
> + ("TBD (extended flexible mode): buffer address 0x%08x, size 0x%04x\n",
> + tx_buffer_address, tx_buffer_size));
> + tx_buffer_size = MIN(tx_buffer_size, sizeof(buf) - size);
> + cpu_physical_memory_read(tx_buffer_address, &buf[size],
> + tx_buffer_size);
> + size += tx_buffer_size;
> + if (tx_buffer_el & 1) {
> + break;
> + }
> + }
> + }
> + tbd_address = tbd_array;
> + for (; tbd_count < s->tx.tbd_count; tbd_count++) {
> + uint32_t tx_buffer_address = ldl_phys(tbd_address);
> + uint16_t tx_buffer_size = lduw_phys(tbd_address + 4);
> + uint16_t tx_buffer_el = lduw_phys(tbd_address + 6);
> + tbd_address += 8;
> + TRACE(RXTX, logout
> + ("TBD (flexible mode): buffer address 0x%08x, size 0x%04x\n",
> + tx_buffer_address, tx_buffer_size));
> + tx_buffer_size = MIN(tx_buffer_size, sizeof(buf) - size);
> + cpu_physical_memory_read(tx_buffer_address, &buf[size],
> + tx_buffer_size);
> + size += tx_buffer_size;
> + if (tx_buffer_el & 1) {
> + break;
> + }
> + }
> + }
> + TRACE(RXTX, logout("%p sending frame, len=%d,%s\n", s, size, nic_dump(buf, size)));
> + qemu_send_packet(&s->nic->nc, buf, size);
> + s->statistics.tx_good_frames++;
> + /* Transmit with bad status would raise an CX/TNO interrupt.
> + * (82557 only). Emulation never has bad status. */
> + //~ eepro100_cx_interrupt(s);
> +}
> +
> static void action_command(EEPRO100State *s)
> {
> for (;;) {
> - uint32_t cb_address = s->cu_base + s->cu_offset;
> - eepro100_tx_t tx;
> - cpu_physical_memory_read(cb_address, (uint8_t *) & tx, sizeof(tx));
> - uint16_t status = le16_to_cpu(tx.status);
> - uint16_t command = le16_to_cpu(tx.command);
> + s->cb_address = s->cu_base + s->cu_offset;
> + cpu_physical_memory_read(s->cb_address, (uint8_t *)&s->tx, sizeof(s->tx));
> + uint16_t status = le16_to_cpu(s->tx.status);
> + uint16_t command = le16_to_cpu(s->tx.command);
> logout
> ("val=0x%02x (cu start), status=0x%04x, command=0x%04x, link=0x%08x\n",
> - val, status, command, tx.link);
> + val, status, command, s->tx.link);
> bool bit_el = ((command & 0x8000) != 0);
> bool bit_s = ((command & 0x4000) != 0);
> bool bit_i = ((command & 0x2000) != 0);
> @@ -766,17 +853,17 @@ static void action_command(EEPRO100State *s)
> bool success = true;
> //~ bool bit_sf = ((command & 0x0008) != 0);
> uint16_t cmd = command & 0x0007;
> - s->cu_offset = le32_to_cpu(tx.link);
> + s->cu_offset = le32_to_cpu(s->tx.link);
> switch (cmd) {
> case CmdNOp:
> /* Do nothing. */
> break;
> case CmdIASetup:
> - cpu_physical_memory_read(cb_address + 8, &s->conf.macaddr.a[0], 6);
> + cpu_physical_memory_read(s->cb_address + 8, &s->conf.macaddr.a[0], 6);
> TRACE(OTHER, logout("macaddr: %s\n", nic_dump(&s->macaddr[0], 6)));
> break;
> case CmdConfigure:
> - cpu_physical_memory_read(cb_address + 8, &s->configuration[0],
> + cpu_physical_memory_read(s->cb_address + 8, &s->configuration[0],
> sizeof(s->configuration));
> TRACE(OTHER, logout("configuration: %s\n", nic_dump(&s->configuration[0], 16)));
> break;
> @@ -784,95 +871,12 @@ static void action_command(EEPRO100State *s)
> //~ missing("multicast list");
> break;
> case CmdTx:
> - (void)0;
> - uint32_t tbd_array = le32_to_cpu(tx.tx_desc_addr);
> - uint16_t tcb_bytes = (le16_to_cpu(tx.tcb_bytes) & 0x3fff);
> - TRACE(RXTX, logout
> - ("transmit, TBD array address 0x%08x, TCB byte count 0x%04x, TBD count %u\n",
> - tbd_array, tcb_bytes, tx.tbd_count));
> -
> if (bit_nc) {
> missing("CmdTx: NC = 0");
> success = false;
> break;
> }
> - //~ assert(!bit_sf);
> - if (tcb_bytes > 2600) {
> - logout("TCB byte count too large, using 2600\n");
> - tcb_bytes = 2600;
> - }
> - /* Next assertion fails for local configuration. */
> - //~ assert((tcb_bytes > 0) || (tbd_array != 0xffffffff));
> - if (!((tcb_bytes > 0) || (tbd_array != 0xffffffff))) {
> - logout
> - ("illegal values of TBD array address and TCB byte count!\n");
> - }
> - // sends larger than MAX_ETH_FRAME_SIZE are allowed, up to 2600 bytes
> - uint8_t buf[2600];
> - uint16_t size = 0;
> - uint32_t tbd_address = cb_address + 0x10;
> - assert(tcb_bytes <= sizeof(buf));
> - while (size < tcb_bytes) {
> - uint32_t tx_buffer_address = ldl_phys(tbd_address);
> - uint16_t tx_buffer_size = lduw_phys(tbd_address + 4);
> - //~ uint16_t tx_buffer_el = lduw_phys(tbd_address + 6);
> - tbd_address += 8;
> - TRACE(RXTX, logout
> - ("TBD (simplified mode): buffer address 0x%08x, size 0x%04x\n",
> - tx_buffer_address, tx_buffer_size));
> - tx_buffer_size = MIN(tx_buffer_size, sizeof(buf) - size);
> - cpu_physical_memory_read(tx_buffer_address, &buf[size],
> - tx_buffer_size);
> - size += tx_buffer_size;
> - }
> - if (tbd_array == 0xffffffff) {
> - /* Simplified mode. Was already handled by code above. */
> - } else {
> - /* Flexible mode. */
> - uint8_t tbd_count = 0;
> - if (s->has_extended_tcb_support && !(s->configuration[6] & BIT(4))) {
> - /* Extended Flexible TCB. */
> - for (; tbd_count < 2; tbd_count++) {
> - uint32_t tx_buffer_address = ldl_phys(tbd_address);
> - uint16_t tx_buffer_size = lduw_phys(tbd_address + 4);
> - uint16_t tx_buffer_el = lduw_phys(tbd_address + 6);
> - tbd_address += 8;
> - TRACE(RXTX, logout
> - ("TBD (extended flexible mode): buffer address 0x%08x, size 0x%04x\n",
> - tx_buffer_address, tx_buffer_size));
> - tx_buffer_size = MIN(tx_buffer_size, sizeof(buf) - size);
> - cpu_physical_memory_read(tx_buffer_address, &buf[size],
> - tx_buffer_size);
> - size += tx_buffer_size;
> - if (tx_buffer_el & 1) {
> - break;
> - }
> - }
> - }
> - tbd_address = tbd_array;
> - for (; tbd_count < tx.tbd_count; tbd_count++) {
> - uint32_t tx_buffer_address = ldl_phys(tbd_address);
> - uint16_t tx_buffer_size = lduw_phys(tbd_address + 4);
> - uint16_t tx_buffer_el = lduw_phys(tbd_address + 6);
> - tbd_address += 8;
> - TRACE(RXTX, logout
> - ("TBD (flexible mode): buffer address 0x%08x, size 0x%04x\n",
> - tx_buffer_address, tx_buffer_size));
> - tx_buffer_size = MIN(tx_buffer_size, sizeof(buf) - size);
> - cpu_physical_memory_read(tx_buffer_address, &buf[size],
> - tx_buffer_size);
> - size += tx_buffer_size;
> - if (tx_buffer_el & 1) {
> - break;
> - }
> - }
> - }
> - TRACE(RXTX, logout("%p sending frame, len=%d,%s\n", s, size, nic_dump(buf, size)));
> - qemu_send_packet(&s->nic->nc, buf, size);
> - s->statistics.tx_good_frames++;
> - /* Transmit with bad status would raise an CX/TNO interrupt.
> - * (82557 only). Emulation never has bad status. */
> - //~ eepro100_cx_interrupt(s);
> + tx_command(s);
> break;
> case CmdTDR:
> TRACE(OTHER, logout("load microcode\n"));
> @@ -885,7 +889,7 @@ static void action_command(EEPRO100State *s)
> break;
> }
> /* Write new status. */
> - stw_phys(cb_address, status | 0x8000 | (success ? 0x2000 : 0));
> + stw_phys(s->cb_address, status | 0x8000 | (success ? 0x2000 : 0));
> if (bit_i) {
> /* CU completed action. */
> eepro100_cx_interrupt(s);
> --
> 1.6.5
>
>
next prev parent reply other threads:[~2010-01-14 11:22 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-12-19 15:08 [Qemu-devel] eepro100.c patches Stefan Weil
2009-12-20 14:51 ` Anthony Liguori
2009-12-20 15:46 ` Stefan Weil
2009-12-20 15:52 ` [Qemu-devel] [PATCH 1/3] eepro100: Restructure code (new function tx_command) Stefan Weil
2009-12-20 15:52 ` [Qemu-devel] [PATCH 2/3] eepro100: Better documentation for temporary data Stefan Weil
2009-12-20 15:52 ` [Qemu-devel] [PATCH 3/3] eepro100: Fix multicast support Stefan Weil
2009-12-20 16:23 ` [Qemu-devel] [PATCH 2/3] eepro100: Better documentation for temporary data Andreas Färber
2009-12-20 18:05 ` Stefan Weil
2010-01-14 11:19 ` Michael S. Tsirkin [this message]
2009-12-20 16:19 ` [Qemu-devel] eepro100.c patches Anthony Liguori
2010-01-05 21:28 ` Stefan Weil
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=20100114111905.GA5115@redhat.com \
--to=mst@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=weil@mail.berlios.de \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.