qemu-devel
[Top][All Lists]
Advanced

[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
> 
> 




reply via email to

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