[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH] eepro100: Restructure code (new function tx_com
From: |
Michael S. Tsirkin |
Subject: |
[Qemu-devel] Re: [PATCH] eepro100: Restructure code (new function tx_command) |
Date: |
Mon, 30 Nov 2009 11:30:15 +0200 |
User-agent: |
Mutt/1.5.19 (2009-01-05) |
On Thu, Nov 19, 2009 at 09:54:46PM +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 <address@hidden>
> ---
> hw/eepro100.c | 192
> +++++++++++++++++++++++++++++----------------------------
> 1 files changed, 98 insertions(+), 94 deletions(-)
>
> diff --git a/hw/eepro100.c b/hw/eepro100.c
> index 4210d8a..7093af8 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;
> +
That's pretty evil, as it makes routines non-reentrant.
How bad is it to pass 2 additional arguments around?
If not, maybe define struct tx_command and put necessary stuff there,
then pass pointer to that?
> /* 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->vc, 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->vc, 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.5.6.5
>
>