diff mbox series

[PULL,10/12] hw/net/cadence_gem: Fix the mask/compare/disable-mask logic

Message ID 20250210154942.3634878-11-peter.maydell@linaro.org
State Accepted
Commit 76723b8ed7b8274a4e30cdf0a9f3e7c47ebe251a
Headers show
Series [PULL,01/12] target/arm: deprecate the pxa2xx CPUs and iwMMXt emulation | expand

Commit Message

Peter Maydell Feb. 10, 2025, 3:49 p.m. UTC 
From: Andrew Yuan <andrew.yuan@jaguarmicro.com>

Our current handling of the mask/compare logic in the Cadence
GEM ethernet device is wrong:
 (1) we load the same byte twice from rx_buf when
     creating the compare value
 (2) we ignore the DISABLE_MASK flag

The "Cadence IP for Gigabit Ethernet MAC Part Number: IP7014 IP Rev:
R1p12 - Doc Rev: 1.3 User Guide" states that if the DISABLE_MASK bit
in type2_compare_x_word_1 is set, the mask_value field in
type2_compare_x_word_0 is used as an additional 2 byte Compare Value.

Correct these bugs:
 * in the !disable_mask codepath, use lduw_le_p() so we
   correctly load a 16-bit value for comparison
 * in the disable_mask codepath, we load a full 4-byte value
   from rx_buf for the comparison, set the compare value to
   the whole of the cr0 register (i.e. the concatenation of
   the mask and compare fields), and set mask to 0xffffffff
   to force a 32-bit comparison

Signed-off-by: Andrew Yuan <andrew.yuan@jaguarmicro.com>
Message-id: 20241219061658.805-1-andrew.yuan@jaguarmicro.com
Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
[PMM: Expand commit message and comment]
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/net/cadence_gem.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)
diff mbox series

Patch

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index f744054a6d8..80fbbacc1e7 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -909,8 +909,8 @@  static int get_queue_from_screen(CadenceGEMState *s, uint8_t *rxbuf_ptr,
 
         /* Compare A, B, C */
         for (j = 0; j < 3; j++) {
-            uint32_t cr0, cr1, mask, compare;
-            uint16_t rx_cmp;
+            uint32_t cr0, cr1, mask, compare, disable_mask;
+            uint32_t rx_cmp;
             int offset;
             int cr_idx = extract32(reg, R_SCREENING_TYPE2_REG0_COMPARE_A_SHIFT + j * 6,
                                    R_SCREENING_TYPE2_REG0_COMPARE_A_LENGTH);
@@ -946,9 +946,25 @@  static int get_queue_from_screen(CadenceGEMState *s, uint8_t *rxbuf_ptr,
                 break;
             }
 
-            rx_cmp = rxbuf_ptr[offset] << 8 | rxbuf_ptr[offset];
-            mask = FIELD_EX32(cr0, TYPE2_COMPARE_0_WORD_0, MASK_VALUE);
-            compare = FIELD_EX32(cr0, TYPE2_COMPARE_0_WORD_0, COMPARE_VALUE);
+            disable_mask =
+                FIELD_EX32(cr1, TYPE2_COMPARE_0_WORD_1, DISABLE_MASK);
+            if (disable_mask) {
+                /*
+                 * If disable_mask is set, mask_value is used as an
+                 * additional 2 byte Compare Value; that is equivalent
+                 * to using the whole cr0 register as the comparison value.
+                 * Load 32 bits of data from rx_buf, and set mask to
+                 * all-ones so we compare all 32 bits.
+                 */
+                rx_cmp = ldl_le_p(rxbuf_ptr + offset);
+                mask = 0xFFFFFFFF;
+                compare = cr0;
+            } else {
+                rx_cmp = lduw_le_p(rxbuf_ptr + offset);
+                mask = FIELD_EX32(cr0, TYPE2_COMPARE_0_WORD_0, MASK_VALUE);
+                compare =
+                    FIELD_EX32(cr0, TYPE2_COMPARE_0_WORD_0, COMPARE_VALUE);
+            }
 
             if ((rx_cmp & mask) == (compare & mask)) {
                 matched = true;