diff mbox series

[PULL,33/36] hw/net: GMAC Tx Implementation

Message ID 20240202153637.3710444-34-peter.maydell@linaro.org
State New
Headers show
Series [PULL,01/36] target/arm: fix exception syndrome for AArch32 bkpt insn | expand

Commit Message

Peter Maydell Feb. 2, 2024, 3:36 p.m. UTC
From: Nabih Estefan Diaz <nabihestefan@google.com>

- Implementation of Transmit function for packets
- Implementation for reading and writing from and to descriptors in
  memory for Tx

Added relevant trace-events

NOTE: This function implements the steps detailed in the datasheet for
transmitting messages from the GMAC.

Change-Id: Icf14f9fcc6cc7808a41acd872bca67c9832087e6
Signed-off-by: Nabih Estefan <nabihestefan@google.com>
Reviewed-by: Tyrone Ting <kfting@nuvoton.com>
Message-id: 20240131002800.989285-6-nabihestefan@google.com
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/net/npcm_gmac.c  | 203 ++++++++++++++++++++++++++++++++++++++++++++
 hw/net/trace-events |   2 +
 2 files changed, 205 insertions(+)

Comments

Peter Maydell March 8, 2024, 1:54 p.m. UTC | #1
On Fri, 2 Feb 2024 at 15:36, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> From: Nabih Estefan Diaz <nabihestefan@google.com>
>
> - Implementation of Transmit function for packets
> - Implementation for reading and writing from and to descriptors in
>   memory for Tx
>
> Added relevant trace-events
>
> NOTE: This function implements the steps detailed in the datasheet for
> transmitting messages from the GMAC.
>
> Change-Id: Icf14f9fcc6cc7808a41acd872bca67c9832087e6
> Signed-off-by: Nabih Estefan <nabihestefan@google.com>
> Reviewed-by: Tyrone Ting <kfting@nuvoton.com>
> Message-id: 20240131002800.989285-6-nabihestefan@google.com
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---


Hi; Coverity points out an issue in this code (CID 1534027):




> +static void gmac_try_send_next_packet(NPCMGMACState *gmac)
> +{
> +    /*
> +     * Comments about steps refer to steps for
> +     * transmitting in page 384 of datasheet
> +     */
> +    uint16_t tx_buffer_size = 2048;
> +    g_autofree uint8_t *tx_send_buffer = g_malloc(tx_buffer_size);
> +    uint32_t desc_addr;
> +    struct NPCMGMACTxDesc tx_desc;
> +    uint32_t tx_buf_addr, tx_buf_len;
> +    uint16_t length = 0;
> +    uint8_t *buf = tx_send_buffer;
> +    uint32_t prev_buf_size = 0;
> +    int csum = 0;
> +
> +    /* steps 1&2 */
> +    if (!gmac->regs[R_NPCM_DMA_HOST_TX_DESC]) {
> +        gmac->regs[R_NPCM_DMA_HOST_TX_DESC] =
> +            NPCM_DMA_HOST_TX_DESC_MASK(gmac->regs[R_NPCM_DMA_TX_BASE_ADDR]);
> +    }
> +    desc_addr = gmac->regs[R_NPCM_DMA_HOST_TX_DESC];
> +
> +    while (true) {

In this loop...

> +        gmac_dma_set_state(gmac, NPCM_DMA_STATUS_TX_PROCESS_STATE_SHIFT,
> +            NPCM_DMA_STATUS_TX_RUNNING_FETCHING_STATE);
> +        if (gmac_read_tx_desc(desc_addr, &tx_desc)) {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                          "TX Descriptor @ 0x%x can't be read\n",
> +                          desc_addr);
> +            return;
> +        }
> +        /* step 3 */
> +
> +        trace_npcm_gmac_packet_desc_read(DEVICE(gmac)->canonical_path,
> +            desc_addr);
> +        trace_npcm_gmac_debug_desc_data(DEVICE(gmac)->canonical_path, &tx_desc,
> +            tx_desc.tdes0, tx_desc.tdes1, tx_desc.tdes2, tx_desc.tdes3);
> +
> +        /* 1 = DMA Owned, 0 = Software Owned */
> +        if (!(tx_desc.tdes0 & TX_DESC_TDES0_OWN)) {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                          "TX Descriptor @ 0x%x is owned by software\n",
> +                          desc_addr);
> +            gmac->regs[R_NPCM_DMA_STATUS] |= NPCM_DMA_STATUS_TU;
> +            gmac_dma_set_state(gmac, NPCM_DMA_STATUS_TX_PROCESS_STATE_SHIFT,
> +                NPCM_DMA_STATUS_TX_SUSPENDED_STATE);
> +            gmac_update_irq(gmac);
> +            return;
> +        }
> +
> +        gmac_dma_set_state(gmac, NPCM_DMA_STATUS_TX_PROCESS_STATE_SHIFT,
> +            NPCM_DMA_STATUS_TX_RUNNING_READ_STATE);
> +        /* Give the descriptor back regardless of what happens. */
> +        tx_desc.tdes0 &= ~TX_DESC_TDES0_OWN;
> +
> +        if (tx_desc.tdes1 & TX_DESC_TDES1_FIRST_SEG_MASK) {
> +            csum = gmac_tx_get_csum(tx_desc.tdes1);
> +        }
> +
> +        /* step 4 */
> +        tx_buf_addr = tx_desc.tdes2;
> +        gmac->regs[R_NPCM_DMA_CUR_TX_BUF_ADDR] = tx_buf_addr;
> +        tx_buf_len = TX_DESC_TDES1_BFFR1_SZ_MASK(tx_desc.tdes1);
> +        buf = &tx_send_buffer[prev_buf_size];

...we never use 'buf' before we initialize it down here in step 4...

> +
> +        if ((prev_buf_size + tx_buf_len) > sizeof(buf)) {
> +            tx_buffer_size = prev_buf_size + tx_buf_len;
> +            tx_send_buffer = g_realloc(tx_send_buffer, tx_buffer_size);
> +            buf = &tx_send_buffer[prev_buf_size];
> +        }
> +
> +        /* step 5 */
> +        if (dma_memory_read(&address_space_memory, tx_buf_addr, buf,
> +                            tx_buf_len, MEMTXATTRS_UNSPECIFIED)) {
> +            qemu_log_mask(LOG_GUEST_ERROR, "%s: Failed to read packet @ 0x%x\n",
> +                        __func__, tx_buf_addr);
> +            return;
> +        }
> +        length += tx_buf_len;
> +        prev_buf_size += tx_buf_len;
> +
> +        /* If not chained we'll have a second buffer. */
> +        if (!(tx_desc.tdes1 & TX_DESC_TDES1_SEC_ADDR_CHND_MASK)) {
> +            tx_buf_addr = tx_desc.tdes3;
> +            gmac->regs[R_NPCM_DMA_CUR_TX_BUF_ADDR] = tx_buf_addr;
> +            tx_buf_len = TX_DESC_TDES1_BFFR2_SZ_MASK(tx_desc.tdes1);
> +            buf = &tx_send_buffer[prev_buf_size];
> +
> +            if ((prev_buf_size + tx_buf_len) > sizeof(buf)) {
> +                tx_buffer_size = prev_buf_size + tx_buf_len;
> +                tx_send_buffer = g_realloc(tx_send_buffer, tx_buffer_size);
> +                buf = &tx_send_buffer[prev_buf_size];
> +            }
> +
> +            if (dma_memory_read(&address_space_memory, tx_buf_addr, buf,
> +                                tx_buf_len, MEMTXATTRS_UNSPECIFIED)) {
> +                qemu_log_mask(LOG_GUEST_ERROR,
> +                              "%s: Failed to read packet @ 0x%x\n",
> +                              __func__, tx_buf_addr);
> +                return;
> +            }
> +            length += tx_buf_len;
> +            prev_buf_size += tx_buf_len;
> +        }
> +        if (tx_desc.tdes1 & TX_DESC_TDES1_LAST_SEG_MASK) {
> +            net_checksum_calculate(tx_send_buffer, length, csum);
> +            qemu_send_packet(qemu_get_queue(gmac->nic), tx_send_buffer, length);
> +            trace_npcm_gmac_packet_sent(DEVICE(gmac)->canonical_path, length);
> +            buf = tx_send_buffer;
> +            length = 0;

...and in this bit of code, we set 'buf' to something else...

> +        }
> +
> +        /* step 6 */
> +        gmac_dma_set_state(gmac, NPCM_DMA_STATUS_TX_PROCESS_STATE_SHIFT,
> +            NPCM_DMA_STATUS_TX_RUNNING_CLOSING_STATE);
> +        gmac_write_tx_desc(desc_addr, &tx_desc);
> +        if (tx_desc.tdes1 & TX_DESC_TDES1_TX_END_RING_MASK) {
> +            desc_addr = gmac->regs[R_NPCM_DMA_TX_BASE_ADDR];
> +        } else if (tx_desc.tdes1 & TX_DESC_TDES1_SEC_ADDR_CHND_MASK) {
> +            desc_addr = tx_desc.tdes3;
> +        } else {
> +            desc_addr += sizeof(tx_desc);
> +        }
> +        gmac->regs[R_NPCM_DMA_HOST_TX_DESC] = desc_addr;
> +
> +        /* step 7 */
> +        if (tx_desc.tdes1 & TX_DESC_TDES1_INTERR_COMP_MASK) {
> +            gmac->regs[R_NPCM_DMA_STATUS] |= NPCM_DMA_STATUS_TI;
> +            gmac_update_irq(gmac);
> +        }

...but we never use 'buf' in the rest of the loop before we go back
around to the top again, so that value that we set will never be
used before it is overwritten with something else in the next
iteration's step 4.

What was the intention here?

> +    }
> +}

thanks
-- PMM
diff mbox series

Patch

diff --git a/hw/net/npcm_gmac.c b/hw/net/npcm_gmac.c
index a3c626e1b83..1b71e2526e3 100644
--- a/hw/net/npcm_gmac.c
+++ b/hw/net/npcm_gmac.c
@@ -238,6 +238,37 @@  static int gmac_write_rx_desc(dma_addr_t addr, struct NPCMGMACRxDesc *desc)
     return 0;
 }
 
+static int gmac_read_tx_desc(dma_addr_t addr, struct NPCMGMACTxDesc *desc)
+{
+    if (dma_memory_read(&address_space_memory, addr, desc,
+                        sizeof(*desc), MEMTXATTRS_UNSPECIFIED)) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: Failed to read descriptor @ 0x%"
+                      HWADDR_PRIx "\n", __func__, addr);
+        return -1;
+    }
+    desc->tdes0 = le32_to_cpu(desc->tdes0);
+    desc->tdes1 = le32_to_cpu(desc->tdes1);
+    desc->tdes2 = le32_to_cpu(desc->tdes2);
+    desc->tdes3 = le32_to_cpu(desc->tdes3);
+    return 0;
+}
+
+static int gmac_write_tx_desc(dma_addr_t addr, struct NPCMGMACTxDesc *desc)
+{
+    struct NPCMGMACTxDesc le_desc;
+    le_desc.tdes0 = cpu_to_le32(desc->tdes0);
+    le_desc.tdes1 = cpu_to_le32(desc->tdes1);
+    le_desc.tdes2 = cpu_to_le32(desc->tdes2);
+    le_desc.tdes3 = cpu_to_le32(desc->tdes3);
+    if (dma_memory_write(&address_space_memory, addr, &le_desc,
+                        sizeof(le_desc), MEMTXATTRS_UNSPECIFIED)) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: Failed to write descriptor @ 0x%"
+                      HWADDR_PRIx "\n", __func__, addr);
+        return -1;
+    }
+    return 0;
+}
+
 static int gmac_rx_transfer_frame_to_buffer(uint32_t rx_buf_len,
                                             uint32_t *left_frame,
                                             uint32_t rx_buf_addr,
@@ -459,6 +490,155 @@  static ssize_t gmac_receive(NetClientState *nc, const uint8_t *buf, size_t len)
     return len;
 }
 
+static int gmac_tx_get_csum(uint32_t tdes1)
+{
+    uint32_t mask = TX_DESC_TDES1_CHKSM_INS_CTRL_MASK(tdes1);
+    int csum = 0;
+
+    if (likely(mask > 0)) {
+        csum |= CSUM_IP;
+    }
+    if (likely(mask > 1)) {
+        csum |= CSUM_TCP | CSUM_UDP;
+    }
+
+    return csum;
+}
+
+static void gmac_try_send_next_packet(NPCMGMACState *gmac)
+{
+    /*
+     * Comments about steps refer to steps for
+     * transmitting in page 384 of datasheet
+     */
+    uint16_t tx_buffer_size = 2048;
+    g_autofree uint8_t *tx_send_buffer = g_malloc(tx_buffer_size);
+    uint32_t desc_addr;
+    struct NPCMGMACTxDesc tx_desc;
+    uint32_t tx_buf_addr, tx_buf_len;
+    uint16_t length = 0;
+    uint8_t *buf = tx_send_buffer;
+    uint32_t prev_buf_size = 0;
+    int csum = 0;
+
+    /* steps 1&2 */
+    if (!gmac->regs[R_NPCM_DMA_HOST_TX_DESC]) {
+        gmac->regs[R_NPCM_DMA_HOST_TX_DESC] =
+            NPCM_DMA_HOST_TX_DESC_MASK(gmac->regs[R_NPCM_DMA_TX_BASE_ADDR]);
+    }
+    desc_addr = gmac->regs[R_NPCM_DMA_HOST_TX_DESC];
+
+    while (true) {
+        gmac_dma_set_state(gmac, NPCM_DMA_STATUS_TX_PROCESS_STATE_SHIFT,
+            NPCM_DMA_STATUS_TX_RUNNING_FETCHING_STATE);
+        if (gmac_read_tx_desc(desc_addr, &tx_desc)) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "TX Descriptor @ 0x%x can't be read\n",
+                          desc_addr);
+            return;
+        }
+        /* step 3 */
+
+        trace_npcm_gmac_packet_desc_read(DEVICE(gmac)->canonical_path,
+            desc_addr);
+        trace_npcm_gmac_debug_desc_data(DEVICE(gmac)->canonical_path, &tx_desc,
+            tx_desc.tdes0, tx_desc.tdes1, tx_desc.tdes2, tx_desc.tdes3);
+
+        /* 1 = DMA Owned, 0 = Software Owned */
+        if (!(tx_desc.tdes0 & TX_DESC_TDES0_OWN)) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "TX Descriptor @ 0x%x is owned by software\n",
+                          desc_addr);
+            gmac->regs[R_NPCM_DMA_STATUS] |= NPCM_DMA_STATUS_TU;
+            gmac_dma_set_state(gmac, NPCM_DMA_STATUS_TX_PROCESS_STATE_SHIFT,
+                NPCM_DMA_STATUS_TX_SUSPENDED_STATE);
+            gmac_update_irq(gmac);
+            return;
+        }
+
+        gmac_dma_set_state(gmac, NPCM_DMA_STATUS_TX_PROCESS_STATE_SHIFT,
+            NPCM_DMA_STATUS_TX_RUNNING_READ_STATE);
+        /* Give the descriptor back regardless of what happens. */
+        tx_desc.tdes0 &= ~TX_DESC_TDES0_OWN;
+
+        if (tx_desc.tdes1 & TX_DESC_TDES1_FIRST_SEG_MASK) {
+            csum = gmac_tx_get_csum(tx_desc.tdes1);
+        }
+
+        /* step 4 */
+        tx_buf_addr = tx_desc.tdes2;
+        gmac->regs[R_NPCM_DMA_CUR_TX_BUF_ADDR] = tx_buf_addr;
+        tx_buf_len = TX_DESC_TDES1_BFFR1_SZ_MASK(tx_desc.tdes1);
+        buf = &tx_send_buffer[prev_buf_size];
+
+        if ((prev_buf_size + tx_buf_len) > sizeof(buf)) {
+            tx_buffer_size = prev_buf_size + tx_buf_len;
+            tx_send_buffer = g_realloc(tx_send_buffer, tx_buffer_size);
+            buf = &tx_send_buffer[prev_buf_size];
+        }
+
+        /* step 5 */
+        if (dma_memory_read(&address_space_memory, tx_buf_addr, buf,
+                            tx_buf_len, MEMTXATTRS_UNSPECIFIED)) {
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: Failed to read packet @ 0x%x\n",
+                        __func__, tx_buf_addr);
+            return;
+        }
+        length += tx_buf_len;
+        prev_buf_size += tx_buf_len;
+
+        /* If not chained we'll have a second buffer. */
+        if (!(tx_desc.tdes1 & TX_DESC_TDES1_SEC_ADDR_CHND_MASK)) {
+            tx_buf_addr = tx_desc.tdes3;
+            gmac->regs[R_NPCM_DMA_CUR_TX_BUF_ADDR] = tx_buf_addr;
+            tx_buf_len = TX_DESC_TDES1_BFFR2_SZ_MASK(tx_desc.tdes1);
+            buf = &tx_send_buffer[prev_buf_size];
+
+            if ((prev_buf_size + tx_buf_len) > sizeof(buf)) {
+                tx_buffer_size = prev_buf_size + tx_buf_len;
+                tx_send_buffer = g_realloc(tx_send_buffer, tx_buffer_size);
+                buf = &tx_send_buffer[prev_buf_size];
+            }
+
+            if (dma_memory_read(&address_space_memory, tx_buf_addr, buf,
+                                tx_buf_len, MEMTXATTRS_UNSPECIFIED)) {
+                qemu_log_mask(LOG_GUEST_ERROR,
+                              "%s: Failed to read packet @ 0x%x\n",
+                              __func__, tx_buf_addr);
+                return;
+            }
+            length += tx_buf_len;
+            prev_buf_size += tx_buf_len;
+        }
+        if (tx_desc.tdes1 & TX_DESC_TDES1_LAST_SEG_MASK) {
+            net_checksum_calculate(tx_send_buffer, length, csum);
+            qemu_send_packet(qemu_get_queue(gmac->nic), tx_send_buffer, length);
+            trace_npcm_gmac_packet_sent(DEVICE(gmac)->canonical_path, length);
+            buf = tx_send_buffer;
+            length = 0;
+        }
+
+        /* step 6 */
+        gmac_dma_set_state(gmac, NPCM_DMA_STATUS_TX_PROCESS_STATE_SHIFT,
+            NPCM_DMA_STATUS_TX_RUNNING_CLOSING_STATE);
+        gmac_write_tx_desc(desc_addr, &tx_desc);
+        if (tx_desc.tdes1 & TX_DESC_TDES1_TX_END_RING_MASK) {
+            desc_addr = gmac->regs[R_NPCM_DMA_TX_BASE_ADDR];
+        } else if (tx_desc.tdes1 & TX_DESC_TDES1_SEC_ADDR_CHND_MASK) {
+            desc_addr = tx_desc.tdes3;
+        } else {
+            desc_addr += sizeof(tx_desc);
+        }
+        gmac->regs[R_NPCM_DMA_HOST_TX_DESC] = desc_addr;
+
+        /* step 7 */
+        if (tx_desc.tdes1 & TX_DESC_TDES1_INTERR_COMP_MASK) {
+            gmac->regs[R_NPCM_DMA_STATUS] |= NPCM_DMA_STATUS_TI;
+            gmac_update_irq(gmac);
+        }
+    }
+}
+
 static void gmac_cleanup(NetClientState *nc)
 {
     /* Nothing to do yet. */
@@ -613,6 +793,29 @@  static void npcm_gmac_write(void *opaque, hwaddr offset,
             NPCM_DMA_STATUS_RX_RUNNING_WAITING_STATE);
         break;
 
+    case A_NPCM_DMA_XMT_POLL_DEMAND:
+        /* We dont actually care about the value */
+        gmac_try_send_next_packet(gmac);
+        break;
+
+    case A_NPCM_DMA_CONTROL:
+        gmac->regs[offset / sizeof(uint32_t)] = v;
+        if (v & NPCM_DMA_CONTROL_START_STOP_TX) {
+            gmac_try_send_next_packet(gmac);
+        } else {
+            gmac_dma_set_state(gmac, NPCM_DMA_STATUS_TX_PROCESS_STATE_SHIFT,
+                NPCM_DMA_STATUS_TX_STOPPED_STATE);
+        }
+        if (v & NPCM_DMA_CONTROL_START_STOP_RX) {
+            gmac_dma_set_state(gmac, NPCM_DMA_STATUS_RX_PROCESS_STATE_SHIFT,
+                NPCM_DMA_STATUS_RX_RUNNING_WAITING_STATE);
+            qemu_flush_queued_packets(qemu_get_queue(gmac->nic));
+        } else {
+            gmac_dma_set_state(gmac, NPCM_DMA_STATUS_RX_PROCESS_STATE_SHIFT,
+                NPCM_DMA_STATUS_RX_STOPPED_STATE);
+        }
+        break;
+
     case A_NPCM_DMA_STATUS:
         /* Check that RO bits are not written to */
         if (NPCM_DMA_STATUS_RO_MASK(v)) {
diff --git a/hw/net/trace-events b/hw/net/trace-events
index f91b1a4a3de..78efa2ec2cc 100644
--- a/hw/net/trace-events
+++ b/hw/net/trace-events
@@ -478,7 +478,9 @@  npcm_gmac_packet_desc_read(const char* name, uint32_t desc_addr) "%s: attempting
 npcm_gmac_packet_receive(const char* name, uint32_t len) "%s: RX packet length: 0x%04" PRIX32
 npcm_gmac_packet_receiving_buffer(const char* name, uint32_t buf_len, uint32_t rx_buf_addr) "%s: Receiving into Buffer size: 0x%04" PRIX32 " at address 0x%04" PRIX32
 npcm_gmac_packet_received(const char* name, uint32_t len) "%s: Reception finished, packet left: 0x%04" PRIX32
+npcm_gmac_packet_sent(const char* name, uint16_t len) "%s: TX packet sent!, length: 0x%04" PRIX16
 npcm_gmac_debug_desc_data(const char* name, void* addr, uint32_t des0, uint32_t des1, uint32_t des2, uint32_t des3)"%s: Address: %p Descriptor 0: 0x%04" PRIX32 " Descriptor 1: 0x%04" PRIX32 "Descriptor 2: 0x%04" PRIX32 " Descriptor 3: 0x%04" PRIX32
+npcm_gmac_packet_tx_desc_data(const char* name, uint32_t tdes0, uint32_t tdes1) "%s: Tdes0: 0x%04" PRIX32 " Tdes1: 0x%04" PRIX32
 
 # npcm_pcs.c
 npcm_pcs_reg_read(const char *name, uint16_t indirect_access_baes, uint64_t offset, uint16_t value) "%s: IND: 0x%02" PRIx16 " offset: 0x%04" PRIx64 " value: 0x%04" PRIx16