RE: [PATCH v5 07/11] include/hw/net: Implemented Classes and Masks for GMAC Descriptors

[address@hidden]

-----Original Message-----
From: Nabih Estefan <nabihestefan@google.com>
Sent: Saturday, October 28, 2023 1:55 AM
To: peter.maydell@linaro.org
Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org; CS20 KFTing 
<KFTING@nuvoton.com>; wuhaotsh@google.com; jasonwang@redhat.com; IS20 Avi 
Fishman <Avi.Fishman@nuvoton.com>; nabihestefan@google.com; CS20 KWLiu 
<KWLIU@nuvoton.com>; IS20 Tomer Maimon <tomer.maimon@nuvoton.com>; IN20 Hila 
Miranda-Kuzi <Hila.Miranda-Kuzi@nuvoton.com>
Subject: [PATCH v5 07/11] include/hw/net: Implemented Classes and Masks for 
GMAC Descriptors


From: Nabih Estefan Diaz <nabihestefan@google.com>

 - Implemeted classes for GMAC Receive and Transmit Descriptors
 - Implemented Masks for said descriptors

Change-Id: Id671a69137927e169afc9e610dde8dcd7811f6be
Signed-off-by: Nabih Estefan <nabihestefan@google.com>
---
 hw/net/npcm_gmac.c           | 183 +++++++++++++++++++++++++++--------
 hw/net/trace-events          |   9 ++
 include/hw/net/npcm_gmac.h   |   2 -
 tests/qtest/npcm_gmac-test.c |   2 +-
 4 files changed, 150 insertions(+), 46 deletions(-)

diff --git a/hw/net/npcm_gmac.c b/hw/net/npcm_gmac.c index 
5ce632858d..6f8109e0ee 100644
--- a/hw/net/npcm_gmac.c
+++ b/hw/net/npcm_gmac.c
@@ -32,7 +32,7 @@
 REG32(NPCM_DMA_BUS_MODE, 0x1000)
 REG32(NPCM_DMA_XMT_POLL_DEMAND, 0x1004)  REG32(NPCM_DMA_RCV_POLL_DEMAND, 
0x1008) -REG32(NPCM_DMA_RCV_BASE_ADDR, 0x100c)
+REG32(NPCM_DMA_RX_BASE_ADDR, 0x100c)
 REG32(NPCM_DMA_TX_BASE_ADDR, 0x1010)
 REG32(NPCM_DMA_STATUS, 0x1014)
 REG32(NPCM_DMA_CONTROL, 0x1018)
@@ -91,7 +91,8 @@ REG32(NPCM_GMAC_PTP_TTSR, 0x71c)
 #define NPCM_DMA_BUS_MODE_SWR               BIT(0)

 static const uint32_t npcm_gmac_cold_reset_values[NPCM_GMAC_NR_REGS] = {
-    [R_NPCM_GMAC_VERSION]         = 0x00001037,
+    /* Reduce version to 3.2 so that the kernel can enable interrupt. */
+    [R_NPCM_GMAC_VERSION]         = 0x00001032,
     [R_NPCM_GMAC_TIMER_CTRL]      = 0x03e80000,
     [R_NPCM_GMAC_MAC0_ADDR_HI]    = 0x8000ffff,
     [R_NPCM_GMAC_MAC0_ADDR_LO]    = 0xffffffff,
@@ -125,12 +126,12 @@ static const uint16_t phy_reg_init[] = {
     [MII_EXTSTAT]   = 0x3000, /* 1000BASTE_T full-duplex capable */
 };

-static void npcm_gmac_soft_reset(NPCMGMACState *s)
+static void npcm_gmac_soft_reset(NPCMGMACState *gmac)
 {
-    memcpy(s->regs, npcm_gmac_cold_reset_values,
+    memcpy(gmac->regs, npcm_gmac_cold_reset_values,
            NPCM_GMAC_NR_REGS * sizeof(uint32_t));
     /* Clear reset bits */
-    s->regs[R_NPCM_DMA_BUS_MODE] &= ~NPCM_DMA_BUS_MODE_SWR;
+    gmac->regs[R_NPCM_DMA_BUS_MODE] &= ~NPCM_DMA_BUS_MODE_SWR;
 }

 static void gmac_phy_set_link(NPCMGMACState *s, bool active) @@ -148,11 
+149,53 @@ static bool gmac_can_receive(NetClientState *nc)
     return true;
 }

-static ssize_t gmac_receive(NetClientState *nc, const uint8_t *buf, size_t 
len1)
+/*
+ * Function that updates the GMAC IRQ
+ * It find the logical OR of the enabled bits for NIS (if enabled)
+ * It find the logical OR of the enabled bits for AIS (if enabled)  */
+static void gmac_update_irq(NPCMGMACState *gmac)
 {
-    return 0;
+    /*
+     * Check if the normal interrupts summery is enabled
+     * if so, add the bits for the summary that are enabled
+     */
+    if (gmac->regs[R_NPCM_DMA_INTR_ENA] & gmac->regs[R_NPCM_DMA_STATUS] &
+        (NPCM_DMA_INTR_ENAB_NIE_BITS))
+    {
+        gmac->regs[R_NPCM_DMA_STATUS] |=  NPCM_DMA_STATUS_NIS;
+    }
+    /*
+     * Check if the abnormal interrupts summery is enabled
+     * if so, add the bits for the summary that are enabled
+     */
+    if (gmac->regs[R_NPCM_DMA_INTR_ENA] & gmac->regs[R_NPCM_DMA_STATUS] &
+        (NPCM_DMA_INTR_ENAB_AIE_BITS))
+    {
+        gmac->regs[R_NPCM_DMA_STATUS] |=  NPCM_DMA_STATUS_AIS;
+    }
+
+    /* Get the logical OR of both normal and abnormal interrupts */
+    int level = !!((gmac->regs[R_NPCM_DMA_STATUS] &
+                    gmac->regs[R_NPCM_DMA_INTR_ENA] &
+                    NPCM_DMA_STATUS_NIS) |
+                   (gmac->regs[R_NPCM_DMA_STATUS] &
+                   gmac->regs[R_NPCM_DMA_INTR_ENA] &
+                   NPCM_DMA_STATUS_AIS));
+
+    /* Set the IRQ */
+    trace_npcm_gmac_update_irq(DEVICE(gmac)->canonical_path,
+                               gmac->regs[R_NPCM_DMA_STATUS],
+                               gmac->regs[R_NPCM_DMA_INTR_ENA],
+                               level);
+    qemu_set_irq(gmac->irq, level);
 }

+static ssize_t gmac_receive(NetClientState *nc, const uint8_t *buf,
+size_t len) {
+    /* Placeholder */
+    return 0;
+}
 static void gmac_cleanup(NetClientState *nc)  {
     /* Nothing to do yet. */
@@ -166,7 +209,7 @@ static void gmac_set_link(NetClientState *nc)
     gmac_phy_set_link(s, !nc->link_down);  }

-static void npcm_gmac_mdio_access(NPCMGMACState *s, uint16_t v)
+static void npcm_gmac_mdio_access(NPCMGMACState *gmac, uint16_t v)
 {
     bool busy = v & NPCM_GMAC_MII_ADDR_BUSY;
     uint8_t is_write;
@@ -183,33 +226,38 @@ static void npcm_gmac_mdio_access(NPCMGMACState *s, 
uint16_t v)


         if (v & NPCM_GMAC_MII_ADDR_WRITE) {
-            data = s->regs[R_NPCM_GMAC_MII_DATA];
+            data = gmac->regs[R_NPCM_GMAC_MII_DATA];
             /* Clear reset bit for BMCR register */
             switch (gr) {
             case MII_BMCR:
                 data &= ~MII_BMCR_RESET;
-                /* Complete auto-negotiation immediately and set as complete */
-                if (data & MII_BMCR_AUTOEN) {
+                /* Autonegotiation is a W1C bit*/
+                if (data & MII_BMCR_ANRESTART) {
                     /* Tells autonegotiation to not restart again */
                     data &= ~MII_BMCR_ANRESTART;
+                }
+                if ((data & MII_BMCR_AUTOEN) &&
+                    !(gmac->phy_regs[pa][MII_BMSR] & MII_BMSR_AN_COMP))
+ {
                     /* sets autonegotiation as complete */
-                    s->phy_regs[pa][MII_BMSR] |= MII_BMSR_AN_COMP;
+                    gmac->phy_regs[pa][MII_BMSR] |= MII_BMSR_AN_COMP;
+                    /* Resolve AN automatically->need to set this */
+                    gmac->phy_regs[0][MII_ANLPAR] = 0x0000;
                 }
             }
-            s->phy_regs[pa][gr] = data;
+            gmac->phy_regs[pa][gr] = data;
         } else {
-            data = s->phy_regs[pa][gr];
-            s->regs[R_NPCM_GMAC_MII_DATA] = data;
+            data = gmac->phy_regs[pa][gr];
+            gmac->regs[R_NPCM_GMAC_MII_DATA] = data;
         }
-        trace_npcm_gmac_mdio_access(DEVICE(s)->canonical_path, is_write, pa,
-                                    gr, data);
+        trace_npcm_gmac_mdio_access(DEVICE(gmac)->canonical_path, is_write, pa,
+                                        gr, data);
     }
-    s->regs[R_NPCM_GMAC_MII_ADDR] = v & ~NPCM_GMAC_MII_ADDR_BUSY;
+    gmac->regs[R_NPCM_GMAC_MII_ADDR] = v & ~NPCM_GMAC_MII_ADDR_BUSY;
 }

 static uint64_t npcm_gmac_read(void *opaque, hwaddr offset, unsigned size)  {
-    NPCMGMACState *s = opaque;
+    NPCMGMACState *gmac = opaque;
     uint32_t v = 0;

     switch (offset) {
@@ -218,22 +266,25 @@ static uint64_t npcm_gmac_read(void *opaque, hwaddr 
offset, unsigned size)
     case A_NPCM_DMA_RCV_POLL_DEMAND:
         qemu_log_mask(LOG_GUEST_ERROR,
                       "%s: Read of write-only reg: offset: 0x%04" HWADDR_PRIx
-                      "\n", DEVICE(s)->canonical_path, offset);
+                      "\n", DEVICE(gmac)->canonical_path, offset);
         break;

     default:
-        v = s->regs[offset / sizeof(uint32_t)];
+        v = gmac->regs[offset / sizeof(uint32_t)];
     }
-    trace_npcm_gmac_reg_read(DEVICE(s)->canonical_path, offset, v);
+
+    trace_npcm_gmac_reg_read(DEVICE(gmac)->canonical_path, offset, v);
     return v;
 }

 static void npcm_gmac_write(void *opaque, hwaddr offset,
                               uint64_t v, unsigned size)  {
-    NPCMGMACState *s = opaque;
+    NPCMGMACState *gmac = opaque;
+    uint32_t prev;
+
+    trace_npcm_gmac_reg_write(DEVICE(gmac)->canonical_path, offset, v);

-    trace_npcm_gmac_reg_write(DEVICE(s)->canonical_path, offset, v);
     switch (offset) {
     /* Read only registers */
     case A_NPCM_GMAC_VERSION:
@@ -250,25 +301,44 @@ static void npcm_gmac_write(void *opaque, hwaddr offset,
         qemu_log_mask(LOG_GUEST_ERROR,
                       "%s: Write of read-only reg: offset: 0x%04" HWADDR_PRIx
                       ", value: 0x%04" PRIx64 "\n",
-                      DEVICE(s)->canonical_path, offset, v);
+                      DEVICE(gmac)->canonical_path, offset, v);
+        break;
+
+    case A_NPCM_GMAC_MAC_CONFIG:
+        prev = gmac->regs[offset / sizeof(uint32_t)];
+        gmac->regs[offset / sizeof(uint32_t)] = v;
+
+        /* If transmit is being enabled for first time, update desc addr */
+        if (~(prev & NPCM_GMAC_MAC_CONFIG_TX_EN) &
+             (v & NPCM_GMAC_MAC_CONFIG_TX_EN)) {
+            gmac->regs[R_NPCM_DMA_HOST_TX_DESC] =
+                gmac->regs[R_NPCM_DMA_TX_BASE_ADDR];
+        }
+
+        /* If receive is being enabled for first time, update desc addr */
+        if (~(prev & NPCM_GMAC_MAC_CONFIG_RX_EN) &
+             (v & NPCM_GMAC_MAC_CONFIG_RX_EN)) {
+            gmac->regs[R_NPCM_DMA_HOST_RX_DESC] =
+                gmac->regs[R_NPCM_DMA_RX_BASE_ADDR];
+        }
         break;

     case A_NPCM_GMAC_MII_ADDR:
-        npcm_gmac_mdio_access(s, v);
+        npcm_gmac_mdio_access(gmac, v);
         break;

     case A_NPCM_GMAC_MAC0_ADDR_HI:
-        s->regs[offset / sizeof(uint32_t)] = v;
-        s->conf.macaddr.a[0] = v >> 8;
-        s->conf.macaddr.a[1] = v >> 0;
+        gmac->regs[offset / sizeof(uint32_t)] = v;
+        gmac->conf.macaddr.a[0] = v >> 8;
+        gmac->conf.macaddr.a[1] = v >> 0;
         break;

     case A_NPCM_GMAC_MAC0_ADDR_LO:
-        s->regs[offset / sizeof(uint32_t)] = v;
-        s->conf.macaddr.a[2] = v >> 24;
-        s->conf.macaddr.a[3] = v >> 16;
-        s->conf.macaddr.a[4] = v >> 8;
-        s->conf.macaddr.a[5] = v >> 0;
+        gmac->regs[offset / sizeof(uint32_t)] = v;
+        gmac->conf.macaddr.a[2] = v >> 24;
+        gmac->conf.macaddr.a[3] = v >> 16;
+        gmac->conf.macaddr.a[4] = v >> 8;
+        gmac->conf.macaddr.a[5] = v >> 0;
         break;

     case A_NPCM_GMAC_MAC1_ADDR_HI:
@@ -277,33 +347,60 @@ static void npcm_gmac_write(void *opaque, hwaddr offset,
     case A_NPCM_GMAC_MAC2_ADDR_LO:
     case A_NPCM_GMAC_MAC3_ADDR_HI:
     case A_NPCM_GMAC_MAC3_ADDR_LO:
-        s->regs[offset / sizeof(uint32_t)] = v;
+        gmac->regs[offset / sizeof(uint32_t)] = v;
         qemu_log_mask(LOG_UNIMP,
                       "%s: Only MAC Address 0 is supported. This request "
-                      "is ignored.\n", DEVICE(s)->canonical_path);
+                      "is ignored.\n", DEVICE(gmac)->canonical_path);
         break;

     case A_NPCM_DMA_BUS_MODE:
-        s->regs[offset / sizeof(uint32_t)] = v;
+        gmac->regs[offset / sizeof(uint32_t)] = v;
         if (v & NPCM_DMA_BUS_MODE_SWR) {
-            npcm_gmac_soft_reset(s);
+            npcm_gmac_soft_reset(gmac);
+        }
+        break;
+
+    case A_NPCM_DMA_RCV_POLL_DEMAND:
+        /* We dont actually care about the value */
+        break;
+
+    case A_NPCM_DMA_STATUS:
+        /* Check that RO bits are not written to */
+        if (NPCM_DMA_STATUS_RO_MASK(v)) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "%s: Write of read-only bits of reg: offset: 0x%04"
+                           HWADDR_PRIx ", value: 0x%04" PRIx64 "\n",
+                           DEVICE(gmac)->canonical_path, offset, v);
+        } else {
+            /* for W1c bits, implement W1C */
+            gmac->regs[offset / sizeof(uint32_t)] &=
+                ~NPCM_DMA_STATUS_W1C_MASK(v);
+            if (v & NPCM_DMA_STATUS_NIS_BITS) {
+                gmac->regs[offset / sizeof(uint32_t)] &= ~NPCM_DMA_STATUS_NIS;
+            }
+            if (v & NPCM_DMA_STATUS_AIS_BITS) {
+                gmac->regs[offset / sizeof(uint32_t)] &= ~NPCM_DMA_STATUS_AIS;
+            }
         }
         break;

     default:
-        s->regs[offset / sizeof(uint32_t)] = v;
+        gmac->regs[offset / sizeof(uint32_t)] = v;
         break;
     }
+
+    gmac_update_irq(gmac);
 }

 static void npcm_gmac_reset(DeviceState *dev)  {
-    NPCMGMACState *s = NPCM_GMAC(dev);
+    NPCMGMACState *gmac = NPCM_GMAC(dev);

-    npcm_gmac_soft_reset(s);
-    memcpy(s->phy_regs[0], phy_reg_init, sizeof(phy_reg_init));
+    npcm_gmac_soft_reset(gmac);
+    memcpy(gmac->phy_regs[0], phy_reg_init, sizeof(phy_reg_init));

-    trace_npcm_gmac_reset(DEVICE(s)->canonical_path, s->phy_regs[0][MII_BMSR]);
+    trace_npcm_gmac_reset(DEVICE(gmac)->canonical_path,
+                          gmac->phy_regs[0][MII_BMSR]);
 }

 static NetClientInfo net_npcm_gmac_info = { diff --git a/hw/net/trace-events 
b/hw/net/trace-events index 1dbb5d2d64..2843f1eaf8 100644
--- a/hw/net/trace-events
+++ b/hw/net/trace-events
@@ -472,6 +472,15 @@ npcm_gmac_reg_write(const char *name, uint64_t offset, 
uint32_t value) "%s: offs  npcm_gmac_mdio_access(const char *name, uint8_t 
is_write, uint8_t pa, uint8_t gr, uint16_t val) "%s: is_write: %" PRIu8 " pa: 
%" PRIu8 " gr: %" PRIu8 " val: 0x%04" PRIx16  npcm_gmac_reset(const char *name, 
uint16_t value) "%s: phy_regs[0][1]: 0x%04" PRIx16  npcm_gmac_set_link(bool 
active) "Set link: active=%u"
+npcm_gmac_update_irq(const char *name, uint32_t status, uint32_t intr_en, int 
level) "%s: Status Reg: 0x%04" PRIX32 " Interrupt Enable Reg: 0x%04" PRIX32 " 
IRQ Set: %d"
+npcm_gmac_packet_desc_read(const char* name, uint32_t desc_addr) "%s:
+attempting to read descriptor @0x%04" PRIX32
+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_transmit(const char* name, uint16_t len) "%s: TX
+transmission start, packed length 0x%04" PRIX16
+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 diff --git a/include/hw/net/npcm_gmac.h 
b/include/hw/net/npcm_gmac.h index a92a654278..e5729e83ea 100644
--- a/include/hw/net/npcm_gmac.h
+++ b/include/hw/net/npcm_gmac.h
@@ -37,8 +37,6 @@ struct NPCMGMACRxDesc {
 /* RDES2 and RDES3 are buffer address pointers */
 /* Owner: 0 = software, 1 = gmac */
 #define RX_DESC_RDES0_OWNER_MASK BIT(31)
-/* Owner*/
-#define RX_DESC_RDES0_OWNER_SHIFT 31
 /* Destination Address Filter Fail */
 #define RX_DESC_RDES0_DEST_ADDR_FILT_FAIL_MASK BIT(30)
 /* Frame length*/
diff --git a/tests/qtest/npcm_gmac-test.c b/tests/qtest/npcm_gmac-test.c index 
77a83c4c58..130a1599a8 100644
--- a/tests/qtest/npcm_gmac-test.c
+++ b/tests/qtest/npcm_gmac-test.c
@@ -154,7 +154,7 @@ static void test_init(gconstpointer test_data)
     CHECK_REG32(NPCM_GMAC_MII_DATA, 0);
     CHECK_REG32(NPCM_GMAC_FLOW_CTRL, 0);
     CHECK_REG32(NPCM_GMAC_VLAN_FLAG, 0);
-    CHECK_REG32(NPCM_GMAC_VERSION, 0x00001037);
+    CHECK_REG32(NPCM_GMAC_VERSION, 0x00001032);
     CHECK_REG32(NPCM_GMAC_WAKEUP_FILTER, 0);
     CHECK_REG32(NPCM_GMAC_PMT, 0);
     CHECK_REG32(NPCM_GMAC_LPI_CTRL, 0);
--
2.42.0.820.g83a721a137-goog


Reviewed-by: Tyrone Ting <kfting@nuvoton.com>
________________________________
________________________________
 The privileged confidential information contained in this email is intended 
for use only by the addressees as indicated by the original sender of this 
email. If you are not the addressee indicated in this email or are not 
responsible for delivery of the email to such a person, please kindly reply to 
the sender indicating this fact and delete all copies of it from your computer 
and network server immediately. Your cooperation is highly appreciated. It is 
advised that any unauthorized use of confidential information of Nuvoton is 
strictly prohibited; and any information in this email irrelevant to the 
official business of Nuvoton shall be deemed as neither given nor endorsed by 
Nuvoton.