All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] dp8393x: fixes for MacOS toolbox ROM
@ 2021-06-13 16:37 Mark Cave-Ayland
  2021-06-13 16:37 ` [PATCH 1/5] dp8393x: checkpatch fixes Mark Cave-Ayland
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: Mark Cave-Ayland @ 2021-06-13 16:37 UTC (permalink / raw)
  To: qemu-devel, hpoussin, aleksandar.rikalo, f4bug, aurelien,
	jiaxun.yang, jasowang

Here is the next set of patches from my attempts to boot MacOS under QEMU's
Q800 machine related to the Sonic network adapter.

Patches 1 and 2 sort out checkpatch and convert from DPRINTF macros to
trace-events.

Patch 3 fixes the PROM checksum and MAC address storage format as found by
stepping through the MacOS toolbox.

Patch 4 ensures that the CPU loads/stores are correctly converted to 16-bit
accesses for the network card and patch 5 fixes a bug when selecting the
index specified for CAM entries.

NOTE TO MIPS MAINTAINERS:

- The Sonic network adapter is used as part of the MIPS jazz machine, however
  I don't have a working kernel and system to test it with. Any pointers to
  test images would be appreciated.

- The changes to the PROM checksum in patch 3 were determined by stepping
  through the MacOS toolbox, and is different from the existing algorithm.
  Has the current PROM checksum algorithm been validated on a MIPS guest or
  was it just a guess? It might be that 2 different algorithms are needed for
  the Q800 vs. Jazz machine.

- My current guess is the jazzsonic driver is broken since the last set of
  dp8393x changes as the MIPS jazz machine does not set the "big_endian"
  property on the dp8393x device. I'd expect that the following diff would
  be needed, but I can't confirm this without a suitable test image.

diff --git a/hw/mips/jazz.c b/hw/mips/jazz.c
index 1e1cf8154e..1df67035aa 100644
--- a/hw/mips/jazz.c
+++ b/hw/mips/jazz.c
@@ -280,6 +280,7 @@ static void mips_jazz_init(MachineState *machine,
             dev = qdev_new("dp8393x");
             qdev_set_nic_properties(dev, nd);
             qdev_prop_set_uint8(dev, "it_shift", 2);
+            qdev_prop_set_bit(dev, "big_endian", true);
             object_property_set_link(OBJECT(dev), "dma_mr",
                                      OBJECT(rc4030_dma_mr), &error_abort);
             sysbus = SYS_BUS_DEVICE(dev);

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

[q800-macos-upstream patchset series: 3]

Mark Cave-Ayland (5):
  dp8393x: checkpatch fixes
  dp8393x: convert to trace-events
  dp8393x: fix PROM checksum and MAC address storage
  dp8393x: don't force 32-bit register access
  dp8393x: fix CAM descriptor entry index

 hw/net/dp8393x.c    | 332 ++++++++++++++++++++++++--------------------
 hw/net/trace-events |  17 +++
 2 files changed, 198 insertions(+), 151 deletions(-)

-- 
2.20.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 1/5] dp8393x: checkpatch fixes
  2021-06-13 16:37 [PATCH 0/5] dp8393x: fixes for MacOS toolbox ROM Mark Cave-Ayland
@ 2021-06-13 16:37 ` Mark Cave-Ayland
  2021-06-13 16:37 ` [PATCH 2/5] dp8393x: convert to trace-events Mark Cave-Ayland
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Mark Cave-Ayland @ 2021-06-13 16:37 UTC (permalink / raw)
  To: qemu-devel, hpoussin, aleksandar.rikalo, f4bug, aurelien,
	jiaxun.yang, jasowang

Also fix a simple comment typo of "constrainst" to "constraints".

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/net/dp8393x.c | 231 +++++++++++++++++++++++++----------------------
 1 file changed, 122 insertions(+), 109 deletions(-)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 533a8304d0..56af08f0fe 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -29,14 +29,14 @@
 #include <zlib.h>
 #include "qom/object.h"
 
-//#define DEBUG_SONIC
+/* #define DEBUG_SONIC */
 
 #define SONIC_PROM_SIZE 0x1000
 
 #ifdef DEBUG_SONIC
 #define DPRINTF(fmt, ...) \
 do { printf("sonic: " fmt , ##  __VA_ARGS__); } while (0)
-static const char* reg_names[] = {
+static const char *reg_names[] = {
     "CR", "DCR", "RCR", "TCR", "IMR", "ISR", "UTDA", "CTDA",
     "TPS", "TFC", "TSA0", "TSA1", "TFS", "URDA", "CRDA", "CRBA0",
     "CRBA1", "RBWC0", "RBWC1", "EOBC", "URRA", "RSA", "REA", "RRP",
@@ -185,7 +185,8 @@ struct dp8393xState {
     AddressSpace as;
 };
 
-/* Accessor functions for values which are formed by
+/*
+ * Accessor functions for values which are formed by
  * concatenating two 16 bit device registers. By putting these
  * in their own functions with a uint32_t return type we avoid the
  * pitfall of implicit sign extension where ((x << 16) | y) is a
@@ -350,8 +351,7 @@ static void dp8393x_do_read_rra(dp8393xState *s)
     }
 
     /* Warn the host if CRBA now has the last available resource */
-    if (s->regs[SONIC_RRP] == s->regs[SONIC_RWP])
-    {
+    if (s->regs[SONIC_RRP] == s->regs[SONIC_RWP]) {
         s->regs[SONIC_ISR] |= SONIC_ISR_RBE;
         dp8393x_update_irq(s);
     }
@@ -364,7 +364,8 @@ static void dp8393x_do_software_reset(dp8393xState *s)
 {
     timer_del(s->watchdog);
 
-    s->regs[SONIC_CR] &= ~(SONIC_CR_LCAM | SONIC_CR_RRRA | SONIC_CR_TXP | SONIC_CR_HTX);
+    s->regs[SONIC_CR] &= ~(SONIC_CR_LCAM | SONIC_CR_RRRA | SONIC_CR_TXP |
+                           SONIC_CR_HTX);
     s->regs[SONIC_CR] |= SONIC_CR_RST | SONIC_CR_RXDIS;
 }
 
@@ -490,8 +491,10 @@ static void dp8393x_do_transmit_packets(dp8393xState *s)
 
         /* Handle Ethernet checksum */
         if (!(s->regs[SONIC_TCR] & SONIC_TCR_CRCI)) {
-            /* Don't append FCS there, to look like slirp packets
-             * which don't have one */
+            /*
+             * Don't append FCS there, to look like slirp packets
+             * which don't have one
+             */
         } else {
             /* Remove existing FCS */
             tx_len -= 4;
@@ -558,26 +561,34 @@ static void dp8393x_do_command(dp8393xState *s, uint16_t command)
 
     s->regs[SONIC_CR] |= (command & SONIC_CR_MASK);
 
-    if (command & SONIC_CR_HTX)
+    if (command & SONIC_CR_HTX) {
         dp8393x_do_halt_transmission(s);
-    if (command & SONIC_CR_TXP)
+    }
+    if (command & SONIC_CR_TXP) {
         dp8393x_do_transmit_packets(s);
-    if (command & SONIC_CR_RXDIS)
+    }
+    if (command & SONIC_CR_RXDIS) {
         dp8393x_do_receiver_disable(s);
-    if (command & SONIC_CR_RXEN)
+    }
+    if (command & SONIC_CR_RXEN) {
         dp8393x_do_receiver_enable(s);
-    if (command & SONIC_CR_STP)
+    }
+    if (command & SONIC_CR_STP) {
         dp8393x_do_stop_timer(s);
-    if (command & SONIC_CR_ST)
+    }
+    if (command & SONIC_CR_ST) {
         dp8393x_do_start_timer(s);
-    if (command & SONIC_CR_RST)
+    }
+    if (command & SONIC_CR_RST) {
         dp8393x_do_software_reset(s);
+    }
     if (command & SONIC_CR_RRRA) {
         dp8393x_do_read_rra(s);
         s->regs[SONIC_CR] &= ~SONIC_CR_RRRA;
     }
-    if (command & SONIC_CR_LCAM)
+    if (command & SONIC_CR_LCAM) {
         dp8393x_do_load_cam(s);
+    }
 }
 
 static uint64_t dp8393x_read(void *opaque, hwaddr addr, unsigned int size)
@@ -587,24 +598,24 @@ static uint64_t dp8393x_read(void *opaque, hwaddr addr, unsigned int size)
     uint16_t val = 0;
 
     switch (reg) {
-        /* Update data before reading it */
-        case SONIC_WT0:
-        case SONIC_WT1:
-            dp8393x_update_wt_regs(s);
-            val = s->regs[reg];
-            break;
-        /* Accept read to some registers only when in reset mode */
-        case SONIC_CAP2:
-        case SONIC_CAP1:
-        case SONIC_CAP0:
-            if (s->regs[SONIC_CR] & SONIC_CR_RST) {
-                val = s->cam[s->regs[SONIC_CEP] & 0xf][2* (SONIC_CAP0 - reg) + 1] << 8;
-                val |= s->cam[s->regs[SONIC_CEP] & 0xf][2* (SONIC_CAP0 - reg)];
-            }
-            break;
-        /* All other registers have no special contrainst */
-        default:
-            val = s->regs[reg];
+    /* Update data before reading it */
+    case SONIC_WT0:
+    case SONIC_WT1:
+        dp8393x_update_wt_regs(s);
+        val = s->regs[reg];
+        break;
+    /* Accept read to some registers only when in reset mode */
+    case SONIC_CAP2:
+    case SONIC_CAP1:
+    case SONIC_CAP0:
+        if (s->regs[SONIC_CR] & SONIC_CR_RST) {
+            val = s->cam[s->regs[SONIC_CEP] & 0xf][2 * (SONIC_CAP0 - reg) + 1] << 8;
+            val |= s->cam[s->regs[SONIC_CEP] & 0xf][2 * (SONIC_CAP0 - reg)];
+        }
+        break;
+    /* All other registers have no special contraints */
+    default:
+        val = s->regs[reg];
     }
 
     DPRINTF("read 0x%04x from reg %s\n", val, reg_names[reg]);
@@ -622,75 +633,75 @@ static void dp8393x_write(void *opaque, hwaddr addr, uint64_t data,
     DPRINTF("write 0x%04x to reg %s\n", (uint16_t)val, reg_names[reg]);
 
     switch (reg) {
-        /* Command register */
-        case SONIC_CR:
-            dp8393x_do_command(s, val);
-            break;
-        /* Prevent write to read-only registers */
-        case SONIC_CAP2:
-        case SONIC_CAP1:
-        case SONIC_CAP0:
-        case SONIC_SR:
-        case SONIC_MDT:
-            DPRINTF("writing to reg %d invalid\n", reg);
-            break;
-        /* Accept write to some registers only when in reset mode */
-        case SONIC_DCR:
-            if (s->regs[SONIC_CR] & SONIC_CR_RST) {
-                s->regs[reg] = val & 0xbfff;
-            } else {
-                DPRINTF("writing to DCR invalid\n");
-            }
-            break;
-        case SONIC_DCR2:
-            if (s->regs[SONIC_CR] & SONIC_CR_RST) {
-                s->regs[reg] = val & 0xf017;
-            } else {
-                DPRINTF("writing to DCR2 invalid\n");
-            }
-            break;
-        /* 12 lower bytes are Read Only */
-        case SONIC_TCR:
-            s->regs[reg] = val & 0xf000;
-            break;
-        /* 9 lower bytes are Read Only */
-        case SONIC_RCR:
-            s->regs[reg] = val & 0xffe0;
-            break;
-        /* Ignore most significant bit */
-        case SONIC_IMR:
-            s->regs[reg] = val & 0x7fff;
-            dp8393x_update_irq(s);
-            break;
-        /* Clear bits by writing 1 to them */
-        case SONIC_ISR:
-            val &= s->regs[reg];
-            s->regs[reg] &= ~val;
-            if (val & SONIC_ISR_RBE) {
-                dp8393x_do_read_rra(s);
-            }
-            dp8393x_update_irq(s);
-            break;
-        /* The guest is required to store aligned pointers here */
-        case SONIC_RSA:
-        case SONIC_REA:
-        case SONIC_RRP:
-        case SONIC_RWP:
-            if (s->regs[SONIC_DCR] & SONIC_DCR_DW) {
-                s->regs[reg] = val & 0xfffc;
-            } else {
-                s->regs[reg] = val & 0xfffe;
-            }
-            break;
-        /* Invert written value for some registers */
-        case SONIC_CRCT:
-        case SONIC_FAET:
-        case SONIC_MPT:
-            s->regs[reg] = val ^ 0xffff;
-            break;
-        /* All other registers have no special contrainst */
-        default:
-            s->regs[reg] = val;
+    /* Command register */
+    case SONIC_CR:
+        dp8393x_do_command(s, val);
+        break;
+    /* Prevent write to read-only registers */
+    case SONIC_CAP2:
+    case SONIC_CAP1:
+    case SONIC_CAP0:
+    case SONIC_SR:
+    case SONIC_MDT:
+        DPRINTF("writing to reg %d invalid\n", reg);
+        break;
+    /* Accept write to some registers only when in reset mode */
+    case SONIC_DCR:
+        if (s->regs[SONIC_CR] & SONIC_CR_RST) {
+            s->regs[reg] = val & 0xbfff;
+        } else {
+            DPRINTF("writing to DCR invalid\n");
+        }
+        break;
+    case SONIC_DCR2:
+        if (s->regs[SONIC_CR] & SONIC_CR_RST) {
+            s->regs[reg] = val & 0xf017;
+        } else {
+            DPRINTF("writing to DCR2 invalid\n");
+        }
+        break;
+    /* 12 lower bytes are Read Only */
+    case SONIC_TCR:
+        s->regs[reg] = val & 0xf000;
+        break;
+    /* 9 lower bytes are Read Only */
+    case SONIC_RCR:
+        s->regs[reg] = val & 0xffe0;
+        break;
+    /* Ignore most significant bit */
+    case SONIC_IMR:
+        s->regs[reg] = val & 0x7fff;
+        dp8393x_update_irq(s);
+        break;
+    /* Clear bits by writing 1 to them */
+    case SONIC_ISR:
+        val &= s->regs[reg];
+        s->regs[reg] &= ~val;
+        if (val & SONIC_ISR_RBE) {
+            dp8393x_do_read_rra(s);
+        }
+        dp8393x_update_irq(s);
+        break;
+    /* The guest is required to store aligned pointers here */
+    case SONIC_RSA:
+    case SONIC_REA:
+    case SONIC_RRP:
+    case SONIC_RWP:
+        if (s->regs[SONIC_DCR] & SONIC_DCR_DW) {
+            s->regs[reg] = val & 0xfffc;
+        } else {
+            s->regs[reg] = val & 0xfffe;
+        }
+        break;
+    /* Invert written value for some registers */
+    case SONIC_CRCT:
+    case SONIC_FAET:
+    case SONIC_MPT:
+        s->regs[reg] = val ^ 0xffff;
+        break;
+    /* All other registers have no special contrainst */
+    default:
+        s->regs[reg] = val;
     }
 
     if (reg == SONIC_WT0 || reg == SONIC_WT1) {
@@ -747,17 +758,18 @@ static int dp8393x_receive_filter(dp8393xState *s, const uint8_t * buf,
     }
 
     /* Check broadcast */
-    if ((s->regs[SONIC_RCR] & SONIC_RCR_BRD) && !memcmp(buf, bcast, sizeof(bcast))) {
+    if ((s->regs[SONIC_RCR] & SONIC_RCR_BRD) &&
+         !memcmp(buf, bcast, sizeof(bcast))) {
         return SONIC_RCR_BC;
     }
 
     /* Check CAM */
     for (i = 0; i < 16; i++) {
         if (s->regs[SONIC_CE] & (1 << i)) {
-             /* Entry enabled */
-             if (!memcmp(buf, s->cam[i], sizeof(s->cam[i]))) {
-                 return 0;
-             }
+            /* Entry enabled */
+            if (!memcmp(buf, s->cam[i], sizeof(s->cam[i]))) {
+                return 0;
+            }
         }
     }
 
@@ -938,7 +950,8 @@ static void dp8393x_reset(DeviceState *dev)
     s->regs[SONIC_SR] = 0x0004; /* only revision recognized by Linux/mips */
     s->regs[SONIC_CR] = SONIC_CR_RST | SONIC_CR_STP | SONIC_CR_RXDIS;
     s->regs[SONIC_DCR] &= ~(SONIC_DCR_EXBUS | SONIC_DCR_LBR);
-    s->regs[SONIC_RCR] &= ~(SONIC_RCR_LB0 | SONIC_RCR_LB1 | SONIC_RCR_BRD | SONIC_RCR_RNT);
+    s->regs[SONIC_RCR] &= ~(SONIC_RCR_LB0 | SONIC_RCR_LB1 | SONIC_RCR_BRD |
+                            SONIC_RCR_RNT);
     s->regs[SONIC_TCR] |= SONIC_TCR_NCRS | SONIC_TCR_PTX;
     s->regs[SONIC_TCR] &= ~SONIC_TCR_BCM;
     s->regs[SONIC_IMR] = 0;
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/5] dp8393x: convert to trace-events
  2021-06-13 16:37 [PATCH 0/5] dp8393x: fixes for MacOS toolbox ROM Mark Cave-Ayland
  2021-06-13 16:37 ` [PATCH 1/5] dp8393x: checkpatch fixes Mark Cave-Ayland
@ 2021-06-13 16:37 ` Mark Cave-Ayland
  2021-06-13 16:37 ` [PATCH 3/5] dp8393x: fix PROM checksum and MAC address storage Mark Cave-Ayland
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Mark Cave-Ayland @ 2021-06-13 16:37 UTC (permalink / raw)
  To: qemu-devel, hpoussin, aleksandar.rikalo, f4bug, aurelien,
	jiaxun.yang, jasowang

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/net/dp8393x.c    | 55 +++++++++++++++++----------------------------
 hw/net/trace-events | 17 ++++++++++++++
 2 files changed, 37 insertions(+), 35 deletions(-)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 56af08f0fe..ea5b22f680 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -28,14 +28,10 @@
 #include "qemu/timer.h"
 #include <zlib.h>
 #include "qom/object.h"
-
-/* #define DEBUG_SONIC */
+#include "trace.h"
 
 #define SONIC_PROM_SIZE 0x1000
 
-#ifdef DEBUG_SONIC
-#define DPRINTF(fmt, ...) \
-do { printf("sonic: " fmt , ##  __VA_ARGS__); } while (0)
 static const char *reg_names[] = {
     "CR", "DCR", "RCR", "TCR", "IMR", "ISR", "UTDA", "CTDA",
     "TPS", "TFC", "TSA0", "TSA1", "TFS", "URDA", "CRDA", "CRBA0",
@@ -45,12 +41,6 @@ static const char *reg_names[] = {
     "SR", "WT0", "WT1", "RSC", "CRCT", "FAET", "MPT", "MDT",
     "0x30", "0x31", "0x32", "0x33", "0x34", "0x35", "0x36", "0x37",
     "0x38", "0x39", "0x3a", "0x3b", "0x3c", "0x3d", "0x3e", "DCR2" };
-#else
-#define DPRINTF(fmt, ...) do {} while (0)
-#endif
-
-#define SONIC_ERROR(fmt, ...) \
-do { printf("sonic ERROR: %s: " fmt, __func__ , ## __VA_ARGS__); } while (0)
 
 #define SONIC_CR     0x00
 #define SONIC_DCR    0x01
@@ -161,9 +151,7 @@ struct dp8393xState {
     bool big_endian;
     bool last_rba_is_full;
     qemu_irq irq;
-#ifdef DEBUG_SONIC
     int irq_level;
-#endif
     QEMUTimer *watchdog;
     int64_t wt_last_update;
     NICConf conf;
@@ -270,16 +258,14 @@ static void dp8393x_update_irq(dp8393xState *s)
 {
     int level = (s->regs[SONIC_IMR] & s->regs[SONIC_ISR]) ? 1 : 0;
 
-#ifdef DEBUG_SONIC
     if (level != s->irq_level) {
         s->irq_level = level;
         if (level) {
-            DPRINTF("raise irq, isr is 0x%04x\n", s->regs[SONIC_ISR]);
+            trace_dp8393x_raise_irq(s->regs[SONIC_ISR]);
         } else {
-            DPRINTF("lower irq\n");
+            trace_dp8393x_lower_irq();
         }
     }
-#endif
 
     qemu_set_irq(s->irq, level);
 }
@@ -302,9 +288,9 @@ static void dp8393x_do_load_cam(dp8393xState *s)
         s->cam[index][3] = dp8393x_get(s, width, 2) >> 8;
         s->cam[index][4] = dp8393x_get(s, width, 3) & 0xff;
         s->cam[index][5] = dp8393x_get(s, width, 3) >> 8;
-        DPRINTF("load cam[%d] with %02x%02x%02x%02x%02x%02x\n", index,
-            s->cam[index][0], s->cam[index][1], s->cam[index][2],
-            s->cam[index][3], s->cam[index][4], s->cam[index][5]);
+        trace_dp8393x_load_cam(index, s->cam[index][0], s->cam[index][1],
+                               s->cam[index][2], s->cam[index][3],
+                               s->cam[index][4], s->cam[index][5]);
         /* Move to next entry */
         s->regs[SONIC_CDC]--;
         s->regs[SONIC_CDP] += size;
@@ -315,7 +301,7 @@ static void dp8393x_do_load_cam(dp8393xState *s)
     address_space_read(&s->as, dp8393x_cdp(s),
                        MEMTXATTRS_UNSPECIFIED, s->data, size);
     s->regs[SONIC_CE] = dp8393x_get(s, width, 0);
-    DPRINTF("load cam done. cam enable mask 0x%04x\n", s->regs[SONIC_CE]);
+    trace_dp8393x_load_cam_done(s->regs[SONIC_CE]);
 
     /* Done */
     s->regs[SONIC_CR] &= ~SONIC_CR_LCAM;
@@ -338,9 +324,8 @@ static void dp8393x_do_read_rra(dp8393xState *s)
     s->regs[SONIC_CRBA1] = dp8393x_get(s, width, 1);
     s->regs[SONIC_RBWC0] = dp8393x_get(s, width, 2);
     s->regs[SONIC_RBWC1] = dp8393x_get(s, width, 3);
-    DPRINTF("CRBA0/1: 0x%04x/0x%04x, RBWC0/1: 0x%04x/0x%04x\n",
-        s->regs[SONIC_CRBA0], s->regs[SONIC_CRBA1],
-        s->regs[SONIC_RBWC0], s->regs[SONIC_RBWC1]);
+    trace_dp8393x_read_rra_regs(s->regs[SONIC_CRBA0], s->regs[SONIC_CRBA1],
+                                s->regs[SONIC_RBWC0], s->regs[SONIC_RBWC1]);
 
     /* Go to next entry */
     s->regs[SONIC_RRP] += size;
@@ -444,7 +429,7 @@ static void dp8393x_do_transmit_packets(dp8393xState *s)
         /* Read memory */
         size = sizeof(uint16_t) * 6 * width;
         s->regs[SONIC_TTDA] = s->regs[SONIC_CTDA];
-        DPRINTF("Transmit packet at %08x\n", dp8393x_ttda(s));
+        trace_dp8393x_transmit_packet(dp8393x_ttda(s));
         address_space_read(&s->as, dp8393x_ttda(s) + sizeof(uint16_t) * width,
                            MEMTXATTRS_UNSPECIFIED, s->data, size);
         tx_len = 0;
@@ -499,7 +484,7 @@ static void dp8393x_do_transmit_packets(dp8393xState *s)
             /* Remove existing FCS */
             tx_len -= 4;
             if (tx_len < 0) {
-                SONIC_ERROR("tx_len is %d\n", tx_len);
+                trace_dp8393x_transmit_txlen_error(tx_len);
                 break;
             }
         }
@@ -618,7 +603,7 @@ static uint64_t dp8393x_read(void *opaque, hwaddr addr, unsigned int size)
         val = s->regs[reg];
     }
 
-    DPRINTF("read 0x%04x from reg %s\n", val, reg_names[reg]);
+    trace_dp8393x_read(reg, reg_names[reg], val, size);
 
     return s->big_endian ? val << 16 : val;
 }
@@ -630,7 +615,7 @@ static void dp8393x_write(void *opaque, hwaddr addr, uint64_t data,
     int reg = addr >> s->it_shift;
     uint32_t val = s->big_endian ? data >> 16 : data;
 
-    DPRINTF("write 0x%04x to reg %s\n", (uint16_t)val, reg_names[reg]);
+    trace_dp8393x_write(reg, reg_names[reg], val, size);
 
     switch (reg) {
     /* Command register */
@@ -643,21 +628,21 @@ static void dp8393x_write(void *opaque, hwaddr addr, uint64_t data,
     case SONIC_CAP0:
     case SONIC_SR:
     case SONIC_MDT:
-        DPRINTF("writing to reg %d invalid\n", reg);
+        trace_dp8393x_write_invalid(reg);
         break;
     /* Accept write to some registers only when in reset mode */
     case SONIC_DCR:
         if (s->regs[SONIC_CR] & SONIC_CR_RST) {
             s->regs[reg] = val & 0xbfff;
         } else {
-            DPRINTF("writing to DCR invalid\n");
+            trace_dp8393x_write_invalid_dcr("DCR");
         }
         break;
     case SONIC_DCR2:
         if (s->regs[SONIC_CR] & SONIC_CR_RST) {
             s->regs[reg] = val & 0xf017;
         } else {
-            DPRINTF("writing to DCR2 invalid\n");
+            trace_dp8393x_write_invalid_dcr("DCR2");
         }
         break;
     /* 12 lower bytes are Read Only */
@@ -803,7 +788,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
     }
 
     if (padded_len > dp8393x_rbwc(s) * 2) {
-        DPRINTF("oversize packet, pkt_size is %d\n", pkt_size);
+        trace_dp8393x_receive_oversize(pkt_size);
         s->regs[SONIC_ISR] |= SONIC_ISR_RBAE;
         dp8393x_update_irq(s);
         s->regs[SONIC_RCR] |= SONIC_RCR_LPKT;
@@ -812,7 +797,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
 
     packet_type = dp8393x_receive_filter(s, buf, pkt_size);
     if (packet_type < 0) {
-        DPRINTF("packet not for netcard\n");
+        trace_dp8393x_receive_not_netcard();
         return -1;
     }
 
@@ -850,7 +835,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
     checksum = cpu_to_le32(crc32(0, buf, pkt_size));
 
     /* Put packet into RBA */
-    DPRINTF("Receive packet at %08x\n", dp8393x_crba(s));
+    trace_dp8393x_receive_packet(dp8393x_crba(s));
     address = dp8393x_crba(s);
     address_space_write(&s->as, address, MEMTXATTRS_UNSPECIFIED,
                         buf, pkt_size);
@@ -888,7 +873,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
     }
 
     /* Write status to memory */
-    DPRINTF("Write status at %08x\n", dp8393x_crda(s));
+    trace_dp8393x_receive_write_status(dp8393x_crda(s));
     dp8393x_put(s, width, 0, s->regs[SONIC_RCR]); /* status */
     dp8393x_put(s, width, 1, rx_len); /* byte count */
     dp8393x_put(s, width, 2, s->regs[SONIC_TRBA0]); /* pkt_ptr0 */
diff --git a/hw/net/trace-events b/hw/net/trace-events
index c28b91ee1a..643338f610 100644
--- a/hw/net/trace-events
+++ b/hw/net/trace-events
@@ -436,3 +436,20 @@ npcm7xx_emc_received_packet(uint32_t len) "Received %u byte packet"
 npcm7xx_emc_rx_done(uint32_t crxdsa) "RX done, CRXDSA=0x%x"
 npcm7xx_emc_reg_read(int emc_num, uint32_t result, const char *name, int regno) "emc%d: 0x%x = reg[%s/%d]"
 npcm7xx_emc_reg_write(int emc_num, const char *name, int regno, uint32_t value) "emc%d: reg[%s/%d] = 0x%x"
+
+# dp8398x.c
+dp8393x_raise_irq(int isr) "raise irq, isr is 0x%04x"
+dp8393x_lower_irq(void) "lower irq"
+dp8393x_load_cam(int idx, int cam0, int cam1, int cam2, int cam3, int cam4, int cam5) "load cam[%d] with 0x%02x0x%02x0x%02x0x%02x0x%02x0x%02x"
+dp8393x_load_cam_done(int cen) "load cam done. cam enable mask 0x%04x"
+dp8393x_read_rra_regs(int crba0, int crba1, int rbwc0, int rbwc1) "CRBA0/1: 0x%04x/0x%04x, RBWC0/1: 0x%04x/0x%04x"
+dp8393x_transmit_packet(int ttda) "Transmit packet at 0x%"PRIx32
+dp8393x_transmit_txlen_error(int len) "tx_len is %d"
+dp8393x_read(int reg, const char *name, int val, int size) "reg=0x%x [%s] val=0x%04x size=%d"
+dp8393x_write(int reg, const char *name, int val, int size) "reg=0x%x [%s] val=0x%04x size=%d"
+dp8393x_write_invalid(int reg) "writing to reg %d invalid"
+dp8393x_write_invalid_dcr(const char *name) "writing to %s invalid"
+dp8393x_receive_oversize(int size) "oversize packet, pkt_size is %d"
+dp8393x_receive_not_netcard(void) "packet not for netcard"
+dp8393x_receive_packet(int crba) "Receive packet at 0x%"PRIx32
+dp8393x_receive_write_status(int crba) "Write status at 0x%"PRIx32
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/5] dp8393x: fix PROM checksum and MAC address storage
  2021-06-13 16:37 [PATCH 0/5] dp8393x: fixes for MacOS toolbox ROM Mark Cave-Ayland
  2021-06-13 16:37 ` [PATCH 1/5] dp8393x: checkpatch fixes Mark Cave-Ayland
  2021-06-13 16:37 ` [PATCH 2/5] dp8393x: convert to trace-events Mark Cave-Ayland
@ 2021-06-13 16:37 ` Mark Cave-Ayland
  2021-06-13 16:37 ` [PATCH 4/5] dp8393x: don't force 32-bit register access Mark Cave-Ayland
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Mark Cave-Ayland @ 2021-06-13 16:37 UTC (permalink / raw)
  To: qemu-devel, hpoussin, aleksandar.rikalo, f4bug, aurelien,
	jiaxun.yang, jasowang

The checksum used by MacOS to validate the PROM content is an exclusive-OR
rather than a sum over the corresponding bytes. In addition the MAC address
must be stored in bit-reversed format as indicated in comments in Linux's
macsonic.c.

With the PROM contents fixed MacOS starts to probe the device registers
when AppleTalk is enabled in the Control Panel.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/net/dp8393x.c | 43 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 38 insertions(+), 5 deletions(-)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index ea5b22f680..7c745d7963 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -173,6 +173,42 @@ struct dp8393xState {
     AddressSpace as;
 };
 
+/* Table lookup for bitrev8 */
+static const uint8_t byte_rev_table[256] = {
+    0x00, 0x80, 0x40, 0xc0, 0x20, 0xa0, 0x60, 0xe0,
+    0x10, 0x90, 0x50, 0xd0, 0x30, 0xb0, 0x70, 0xf0,
+    0x08, 0x88, 0x48, 0xc8, 0x28, 0xa8, 0x68, 0xe8,
+    0x18, 0x98, 0x58, 0xd8, 0x38, 0xb8, 0x78, 0xf8,
+    0x04, 0x84, 0x44, 0xc4, 0x24, 0xa4, 0x64, 0xe4,
+    0x14, 0x94, 0x54, 0xd4, 0x34, 0xb4, 0x74, 0xf4,
+    0x0c, 0x8c, 0x4c, 0xcc, 0x2c, 0xac, 0x6c, 0xec,
+    0x1c, 0x9c, 0x5c, 0xdc, 0x3c, 0xbc, 0x7c, 0xfc,
+    0x02, 0x82, 0x42, 0xc2, 0x22, 0xa2, 0x62, 0xe2,
+    0x12, 0x92, 0x52, 0xd2, 0x32, 0xb2, 0x72, 0xf2,
+    0x0a, 0x8a, 0x4a, 0xca, 0x2a, 0xaa, 0x6a, 0xea,
+    0x1a, 0x9a, 0x5a, 0xda, 0x3a, 0xba, 0x7a, 0xfa,
+    0x06, 0x86, 0x46, 0xc6, 0x26, 0xa6, 0x66, 0xe6,
+    0x16, 0x96, 0x56, 0xd6, 0x36, 0xb6, 0x76, 0xf6,
+    0x0e, 0x8e, 0x4e, 0xce, 0x2e, 0xae, 0x6e, 0xee,
+    0x1e, 0x9e, 0x5e, 0xde, 0x3e, 0xbe, 0x7e, 0xfe,
+    0x01, 0x81, 0x41, 0xc1, 0x21, 0xa1, 0x61, 0xe1,
+    0x11, 0x91, 0x51, 0xd1, 0x31, 0xb1, 0x71, 0xf1,
+    0x09, 0x89, 0x49, 0xc9, 0x29, 0xa9, 0x69, 0xe9,
+    0x19, 0x99, 0x59, 0xd9, 0x39, 0xb9, 0x79, 0xf9,
+    0x05, 0x85, 0x45, 0xc5, 0x25, 0xa5, 0x65, 0xe5,
+    0x15, 0x95, 0x55, 0xd5, 0x35, 0xb5, 0x75, 0xf5,
+    0x0d, 0x8d, 0x4d, 0xcd, 0x2d, 0xad, 0x6d, 0xed,
+    0x1d, 0x9d, 0x5d, 0xdd, 0x3d, 0xbd, 0x7d, 0xfd,
+    0x03, 0x83, 0x43, 0xc3, 0x23, 0xa3, 0x63, 0xe3,
+    0x13, 0x93, 0x53, 0xd3, 0x33, 0xb3, 0x73, 0xf3,
+    0x0b, 0x8b, 0x4b, 0xcb, 0x2b, 0xab, 0x6b, 0xeb,
+    0x1b, 0x9b, 0x5b, 0xdb, 0x3b, 0xbb, 0x7b, 0xfb,
+    0x07, 0x87, 0x47, 0xc7, 0x27, 0xa7, 0x67, 0xe7,
+    0x17, 0x97, 0x57, 0xd7, 0x37, 0xb7, 0x77, 0xf7,
+    0x0f, 0x8f, 0x4f, 0xcf, 0x2f, 0xaf, 0x6f, 0xef,
+    0x1f, 0x9f, 0x5f, 0xdf, 0x3f, 0xbf, 0x7f, 0xff,
+};
+
 /*
  * Accessor functions for values which are formed by
  * concatenating two 16 bit device registers. By putting these
@@ -996,11 +1032,8 @@ static void dp8393x_realize(DeviceState *dev, Error **errp)
     prom = memory_region_get_ram_ptr(&s->prom);
     checksum = 0;
     for (i = 0; i < 6; i++) {
-        prom[i] = s->conf.macaddr.a[i];
-        checksum += prom[i];
-        if (checksum > 0xff) {
-            checksum = (checksum + 1) & 0xff;
-        }
+        prom[i] = byte_rev_table[s->conf.macaddr.a[i]];
+        checksum ^= prom[i];
     }
     prom[7] = 0xff - checksum;
 }
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/5] dp8393x: don't force 32-bit register access
  2021-06-13 16:37 [PATCH 0/5] dp8393x: fixes for MacOS toolbox ROM Mark Cave-Ayland
                   ` (2 preceding siblings ...)
  2021-06-13 16:37 ` [PATCH 3/5] dp8393x: fix PROM checksum and MAC address storage Mark Cave-Ayland
@ 2021-06-13 16:37 ` Mark Cave-Ayland
  2021-06-13 16:37 ` [PATCH 5/5] dp8393x: fix CAM descriptor entry index Mark Cave-Ayland
  2021-06-14  5:36 ` [PATCH 0/5] dp8393x: fixes for MacOS toolbox ROM Philippe Mathieu-Daudé
  5 siblings, 0 replies; 16+ messages in thread
From: Mark Cave-Ayland @ 2021-06-13 16:37 UTC (permalink / raw)
  To: qemu-devel, hpoussin, aleksandar.rikalo, f4bug, aurelien,
	jiaxun.yang, jasowang

Commit 3fe9a838ec "dp8393x: Always use 32-bit accesses" assumed that all accesses
to the registers were 32-bit but this is actually not the case. The access size is
determined by the CPU instruction used and not the number of physical address lines.

The big_endian workaround applied to the register read/writes was actually caused
by forcing the access size to 32-bit when the guest OS was using a 16-bit access.
Since the registers are 16-bit then we can simply set .impl.min_access to 2 and
then the memory API will automatically do the right thing for both 16-bit accesses
used by Linux and 32-bit accesses used by the MacOS toolbox ROM.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Fixes: 3fe9a838ec ("dp8393x: Always use 32-bit accesses")
---
 hw/net/dp8393x.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 7c745d7963..f9cb10a573 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -641,15 +641,14 @@ static uint64_t dp8393x_read(void *opaque, hwaddr addr, unsigned int size)
 
     trace_dp8393x_read(reg, reg_names[reg], val, size);
 
-    return s->big_endian ? val << 16 : val;
+    return val;
 }
 
-static void dp8393x_write(void *opaque, hwaddr addr, uint64_t data,
+static void dp8393x_write(void *opaque, hwaddr addr, uint64_t val,
                           unsigned int size)
 {
     dp8393xState *s = opaque;
     int reg = addr >> s->it_shift;
-    uint32_t val = s->big_endian ? data >> 16 : data;
 
     trace_dp8393x_write(reg, reg_names[reg], val, size);
 
@@ -733,7 +732,7 @@ static void dp8393x_write(void *opaque, hwaddr addr, uint64_t data,
 static const MemoryRegionOps dp8393x_ops = {
     .read = dp8393x_read,
     .write = dp8393x_write,
-    .impl.min_access_size = 4,
+    .impl.min_access_size = 2,
     .impl.max_access_size = 4,
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 5/5] dp8393x: fix CAM descriptor entry index
  2021-06-13 16:37 [PATCH 0/5] dp8393x: fixes for MacOS toolbox ROM Mark Cave-Ayland
                   ` (3 preceding siblings ...)
  2021-06-13 16:37 ` [PATCH 4/5] dp8393x: don't force 32-bit register access Mark Cave-Ayland
@ 2021-06-13 16:37 ` Mark Cave-Ayland
  2021-06-14  5:36 ` [PATCH 0/5] dp8393x: fixes for MacOS toolbox ROM Philippe Mathieu-Daudé
  5 siblings, 0 replies; 16+ messages in thread
From: Mark Cave-Ayland @ 2021-06-13 16:37 UTC (permalink / raw)
  To: qemu-devel, hpoussin, aleksandar.rikalo, f4bug, aurelien,
	jiaxun.yang, jasowang

Currently when a LOAD CAM command is executed the entries are loaded into the
CAM from memory in order which is incorrect. According to the datasheet the
first entry in the CAM descriptor is the entry index which means that each
descriptor may update any single entry in the CAM rather than the Nth entry.

Decode the CAM entry index and use it store the descriptor in the appropriate
slot in the CAM. This fixes the issue where the MacOS toolbox loads a single
CAM descriptor into the final slot in order to perform a loopback test which
must succeed before the Ethernet port is enabled.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/net/dp8393x.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index f9cb10a573..3ec9810b84 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -309,7 +309,7 @@ static void dp8393x_update_irq(dp8393xState *s)
 static void dp8393x_do_load_cam(dp8393xState *s)
 {
     int width, size;
-    uint16_t index = 0;
+    uint16_t index;
 
     width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1;
     size = sizeof(uint16_t) * 4 * width;
@@ -318,6 +318,7 @@ static void dp8393x_do_load_cam(dp8393xState *s)
         /* Fill current entry */
         address_space_read(&s->as, dp8393x_cdp(s),
                            MEMTXATTRS_UNSPECIFIED, s->data, size);
+        index = dp8393x_get(s, width, 0) & 0xf;
         s->cam[index][0] = dp8393x_get(s, width, 1) & 0xff;
         s->cam[index][1] = dp8393x_get(s, width, 1) >> 8;
         s->cam[index][2] = dp8393x_get(s, width, 2) & 0xff;
@@ -330,7 +331,6 @@ static void dp8393x_do_load_cam(dp8393xState *s)
         /* Move to next entry */
         s->regs[SONIC_CDC]--;
         s->regs[SONIC_CDP] += size;
-        index++;
     }
 
     /* Read CAM enable */
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/5] dp8393x: fixes for MacOS toolbox ROM
  2021-06-13 16:37 [PATCH 0/5] dp8393x: fixes for MacOS toolbox ROM Mark Cave-Ayland
                   ` (4 preceding siblings ...)
  2021-06-13 16:37 ` [PATCH 5/5] dp8393x: fix CAM descriptor entry index Mark Cave-Ayland
@ 2021-06-14  5:36 ` Philippe Mathieu-Daudé
  2021-06-14  7:51   ` Mark Cave-Ayland
  5 siblings, 1 reply; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-14  5:36 UTC (permalink / raw)
  To: Mark Cave-Ayland, qemu-devel, hpoussin, aleksandar.rikalo,
	aurelien, jiaxun.yang, jasowang, Finn Thain, Laurent Vivier

Cc'ing Finn & Laurent.

On 6/13/21 6:37 PM, Mark Cave-Ayland wrote:
> Here is the next set of patches from my attempts to boot MacOS under QEMU's
> Q800 machine related to the Sonic network adapter.
> 
> Patches 1 and 2 sort out checkpatch and convert from DPRINTF macros to
> trace-events.
> 
> Patch 3 fixes the PROM checksum and MAC address storage format as found by
> stepping through the MacOS toolbox.
> 
> Patch 4 ensures that the CPU loads/stores are correctly converted to 16-bit
> accesses for the network card and patch 5 fixes a bug when selecting the
> index specified for CAM entries.
> 
> NOTE TO MIPS MAINTAINERS:
> 
> - The Sonic network adapter is used as part of the MIPS jazz machine, however
>   I don't have a working kernel and system to test it with. Any pointers to
>   test images would be appreciated.
> 
> - The changes to the PROM checksum in patch 3 were determined by stepping
>   through the MacOS toolbox, and is different from the existing algorithm.
>   Has the current PROM checksum algorithm been validated on a MIPS guest or
>   was it just a guess? It might be that 2 different algorithms are needed for
>   the Q800 vs. Jazz machine.
> 
> - My current guess is the jazzsonic driver is broken since the last set of
>   dp8393x changes as the MIPS jazz machine does not set the "big_endian"
>   property on the dp8393x device. I'd expect that the following diff would
>   be needed, but I can't confirm this without a suitable test image.
> 
> diff --git a/hw/mips/jazz.c b/hw/mips/jazz.c
> index 1e1cf8154e..1df67035aa 100644
> --- a/hw/mips/jazz.c
> +++ b/hw/mips/jazz.c
> @@ -280,6 +280,7 @@ static void mips_jazz_init(MachineState *machine,
>              dev = qdev_new("dp8393x");
>              qdev_set_nic_properties(dev, nd);
>              qdev_prop_set_uint8(dev, "it_shift", 2);
> +            qdev_prop_set_bit(dev, "big_endian", true);
>              object_property_set_link(OBJECT(dev), "dma_mr",
>                                       OBJECT(rc4030_dma_mr), &error_abort);
>              sysbus = SYS_BUS_DEVICE(dev);
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> 
> [q800-macos-upstream patchset series: 3]
> 
> Mark Cave-Ayland (5):
>   dp8393x: checkpatch fixes
>   dp8393x: convert to trace-events
>   dp8393x: fix PROM checksum and MAC address storage
>   dp8393x: don't force 32-bit register access
>   dp8393x: fix CAM descriptor entry index
> 
>  hw/net/dp8393x.c    | 332 ++++++++++++++++++++++++--------------------
>  hw/net/trace-events |  17 +++
>  2 files changed, 198 insertions(+), 151 deletions(-)
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/5] dp8393x: fixes for MacOS toolbox ROM
  2021-06-14  5:36 ` [PATCH 0/5] dp8393x: fixes for MacOS toolbox ROM Philippe Mathieu-Daudé
@ 2021-06-14  7:51   ` Mark Cave-Ayland
  2021-06-16  3:09     ` Finn Thain
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Cave-Ayland @ 2021-06-14  7:51 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé,
	qemu-devel, hpoussin, aleksandar.rikalo, aurelien, jiaxun.yang,
	jasowang, Finn Thain, Laurent Vivier

On 14/06/2021 06:36, Philippe Mathieu-Daudé wrote:

> Cc'ing Finn & Laurent.
> 
> On 6/13/21 6:37 PM, Mark Cave-Ayland wrote:
>> Here is the next set of patches from my attempts to boot MacOS under QEMU's
>> Q800 machine related to the Sonic network adapter.
>>
>> Patches 1 and 2 sort out checkpatch and convert from DPRINTF macros to
>> trace-events.
>>
>> Patch 3 fixes the PROM checksum and MAC address storage format as found by
>> stepping through the MacOS toolbox.
>>
>> Patch 4 ensures that the CPU loads/stores are correctly converted to 16-bit
>> accesses for the network card and patch 5 fixes a bug when selecting the
>> index specified for CAM entries.
>>
>> NOTE TO MIPS MAINTAINERS:
>>
>> - The Sonic network adapter is used as part of the MIPS jazz machine, however
>>    I don't have a working kernel and system to test it with. Any pointers to
>>    test images would be appreciated.
>>
>> - The changes to the PROM checksum in patch 3 were determined by stepping
>>    through the MacOS toolbox, and is different from the existing algorithm.
>>    Has the current PROM checksum algorithm been validated on a MIPS guest or
>>    was it just a guess? It might be that 2 different algorithms are needed for
>>    the Q800 vs. Jazz machine.
>>
>> - My current guess is the jazzsonic driver is broken since the last set of
>>    dp8393x changes as the MIPS jazz machine does not set the "big_endian"
>>    property on the dp8393x device. I'd expect that the following diff would
>>    be needed, but I can't confirm this without a suitable test image.
>>
>> diff --git a/hw/mips/jazz.c b/hw/mips/jazz.c
>> index 1e1cf8154e..1df67035aa 100644
>> --- a/hw/mips/jazz.c
>> +++ b/hw/mips/jazz.c
>> @@ -280,6 +280,7 @@ static void mips_jazz_init(MachineState *machine,
>>               dev = qdev_new("dp8393x");
>>               qdev_set_nic_properties(dev, nd);
>>               qdev_prop_set_uint8(dev, "it_shift", 2);
>> +            qdev_prop_set_bit(dev, "big_endian", true);
>>               object_property_set_link(OBJECT(dev), "dma_mr",
>>                                        OBJECT(rc4030_dma_mr), &error_abort);
>>               sysbus = SYS_BUS_DEVICE(dev);
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>
>> [q800-macos-upstream patchset series: 3]
>>
>> Mark Cave-Ayland (5):
>>    dp8393x: checkpatch fixes
>>    dp8393x: convert to trace-events
>>    dp8393x: fix PROM checksum and MAC address storage
>>    dp8393x: don't force 32-bit register access
>>    dp8393x: fix CAM descriptor entry index
>>
>>   hw/net/dp8393x.c    | 332 ++++++++++++++++++++++++--------------------
>>   hw/net/trace-events |  17 +++
>>   2 files changed, 198 insertions(+), 151 deletions(-)

Just to add that I've done a large amount of testing on the q800 machine with 
Linux/MacOS so I'm happy that these patches do the right thing there.

The part I'm struggling with is testing against MIPS jazz since I don't have a Linux 
test image to hand, and there is no documentation in the original commit message as 
to where the existing PROM checksum algorithm came from.

Hervé, can you provide some more information on this? It looks like it was introduced 
in one of your commits:

commit 89ae0ff9b73ee74c9ba707a09a07ad77b9fdccb4
Author: Hervé Poussineau <hpoussin@reactos.org>
Date:   Wed Jun 3 22:45:46 2015 +0200

     net/dp8393x: add PROM to store MAC address

     Signed-off-by: Laurent Vivier <laurent@vivier.eu>
     Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
     Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
     Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>


ATB,

Mark.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/5] dp8393x: fixes for MacOS toolbox ROM
  2021-06-14  7:51   ` Mark Cave-Ayland
@ 2021-06-16  3:09     ` Finn Thain
  2021-06-16  6:10       ` Mark Cave-Ayland
  0 siblings, 1 reply; 16+ messages in thread
From: Finn Thain @ 2021-06-16  3:09 UTC (permalink / raw)
  To: Mark Cave-Ayland
  Cc: aleksandar.rikalo, jasowang, Philippe Mathieu-Daudé,
	qemu-devel, hpoussin, aurelien, Laurent Vivier

[-- Attachment #1: Type: text/plain, Size: 4648 bytes --]

On Mon, 14 Jun 2021, Mark Cave-Ayland wrote:

> On 14/06/2021 06:36, Philippe Mathieu-Daudé wrote:
> 
> > Cc'ing Finn & Laurent.
> > 
> > On 6/13/21 6:37 PM, Mark Cave-Ayland wrote:
> > > Here is the next set of patches from my attempts to boot MacOS under 
> > > QEMU's Q800 machine related to the Sonic network adapter.
> > > 
> > > Patches 1 and 2 sort out checkpatch and convert from DPRINTF macros 
> > > to trace-events.
> > > 
> > > Patch 3 fixes the PROM checksum and MAC address storage format as 
> > > found by stepping through the MacOS toolbox.
> > > 
> > > Patch 4 ensures that the CPU loads/stores are correctly converted to 
> > > 16-bit accesses for the network card and patch 5 fixes a bug when 
> > > selecting the index specified for CAM entries.
> > > 
> > > NOTE TO MIPS MAINTAINERS:
> > > 
> > > - The Sonic network adapter is used as part of the MIPS jazz machine, however
> > >    I don't have a working kernel and system to test it with. Any 
> > >    pointers to test images would be appreciated.
> > > 
> > > - The changes to the PROM checksum in patch 3 were determined by stepping
> > >    through the MacOS toolbox, and is different from the existing 
> > >    algorithm. Has the current PROM checksum algorithm been validated 
> > >    on a MIPS guest or was it just a guess? It might be that 2 
> > >    different algorithms are needed for the Q800 vs. Jazz machine.
> > > 
> > > - My current guess is the jazzsonic driver is broken since the last set of
> > >    dp8393x changes as the MIPS jazz machine does not set the "big_endian"
> > >    property on the dp8393x device. I'd expect that the following diff would
> > >    be needed, but I can't confirm this without a suitable test image.
> > > 
> > > diff --git a/hw/mips/jazz.c b/hw/mips/jazz.c
> > > index 1e1cf8154e..1df67035aa 100644
> > > --- a/hw/mips/jazz.c
> > > +++ b/hw/mips/jazz.c
> > > @@ -280,6 +280,7 @@ static void mips_jazz_init(MachineState *machine,
> > >               dev = qdev_new("dp8393x");
> > >               qdev_set_nic_properties(dev, nd);
> > >               qdev_prop_set_uint8(dev, "it_shift", 2);
> > > +             qdev_prop_set_bit(dev, "big_endian", true);
> > >               object_property_set_link(OBJECT(dev), "dma_mr",
> > >                                        OBJECT(rc4030_dma_mr), &error_abort);
> > >               sysbus = SYS_BUS_DEVICE(dev);
> > > 
> > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> > > 
> > > [q800-macos-upstream patchset series: 3]
> > > 
> > > Mark Cave-Ayland (5):
> > >    dp8393x: checkpatch fixes
> > >    dp8393x: convert to trace-events
> > >    dp8393x: fix PROM checksum and MAC address storage
> > >    dp8393x: don't force 32-bit register access
> > >    dp8393x: fix CAM descriptor entry index
> > > 
> > >   hw/net/dp8393x.c    | 332 ++++++++++++++++++++++++--------------------
> > >   hw/net/trace-events |  17 +++
> > >   2 files changed, 198 insertions(+), 151 deletions(-)
> 
> Just to add that I've done a large amount of testing on the q800 machine 
> with Linux/MacOS so I'm happy that these patches do the right thing 
> there.
> 
> The part I'm struggling with is testing against MIPS jazz since I don't 
> have a Linux test image to hand, and there is no documentation in the 
> original commit message as to where the existing PROM checksum algorithm 
> came from.
> 
> Hervé, can you provide some more information on this? It looks like it 
> was introduced in one of your commits:
> 
> commit 89ae0ff9b73ee74c9ba707a09a07ad77b9fdccb4
> Author: Hervé Poussineau <hpoussin@reactos.org>
> Date:   Wed Jun 3 22:45:46 2015 +0200
> 
>     net/dp8393x: add PROM to store MAC address
> 
>     Signed-off-by: Laurent Vivier <laurent@vivier.eu>
>     Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>     Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
>     Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
> 

With "qemu-system-mips -M magnum ..." I was able to boot both Linux and 
NetBSD. That was after commit 89ae0ff9b7 ("net/dp8393x: add PROM to store 
MAC address"). But that's not to say that the MAC address was decoded 
correctly.

Please see, 
https://lore.kernel.org/qemu-devel/alpine.LNX.2.21.1.1912241504560.11@nippy.intranet/

The Linux/mips (jazzsonic) testing that I did back in 2019 used a custom 
busybox initramfs. The NetBSD/mips testing used the official CD ISO image. 
I will look into reviving those test harnesses because I think patch 4/5 
and the proposed big-endian flag will need some regression testing.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/5] dp8393x: fixes for MacOS toolbox ROM
  2021-06-16  3:09     ` Finn Thain
@ 2021-06-16  6:10       ` Mark Cave-Ayland
  2021-06-18  6:22         ` Finn Thain
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Cave-Ayland @ 2021-06-16  6:10 UTC (permalink / raw)
  To: Finn Thain
  Cc: aleksandar.rikalo, jasowang, Philippe Mathieu-Daudé,
	qemu-devel, hpoussin, aurelien, Laurent Vivier

On 16/06/2021 04:09, Finn Thain wrote:

> On Mon, 14 Jun 2021, Mark Cave-Ayland wrote:
> 
>> On 14/06/2021 06:36, Philippe Mathieu-Daudé wrote:
>>
>>> Cc'ing Finn & Laurent.
>>>
>>> On 6/13/21 6:37 PM, Mark Cave-Ayland wrote:
>>>> Here is the next set of patches from my attempts to boot MacOS under
>>>> QEMU's Q800 machine related to the Sonic network adapter.
>>>>
>>>> Patches 1 and 2 sort out checkpatch and convert from DPRINTF macros
>>>> to trace-events.
>>>>
>>>> Patch 3 fixes the PROM checksum and MAC address storage format as
>>>> found by stepping through the MacOS toolbox.
>>>>
>>>> Patch 4 ensures that the CPU loads/stores are correctly converted to
>>>> 16-bit accesses for the network card and patch 5 fixes a bug when
>>>> selecting the index specified for CAM entries.
>>>>
>>>> NOTE TO MIPS MAINTAINERS:
>>>>
>>>> - The Sonic network adapter is used as part of the MIPS jazz machine, however
>>>>     I don't have a working kernel and system to test it with. Any
>>>>     pointers to test images would be appreciated.
>>>>
>>>> - The changes to the PROM checksum in patch 3 were determined by stepping
>>>>     through the MacOS toolbox, and is different from the existing
>>>>     algorithm. Has the current PROM checksum algorithm been validated
>>>>     on a MIPS guest or was it just a guess? It might be that 2
>>>>     different algorithms are needed for the Q800 vs. Jazz machine.
>>>>
>>>> - My current guess is the jazzsonic driver is broken since the last set of
>>>>     dp8393x changes as the MIPS jazz machine does not set the "big_endian"
>>>>     property on the dp8393x device. I'd expect that the following diff would
>>>>     be needed, but I can't confirm this without a suitable test image.
>>>>
>>>> diff --git a/hw/mips/jazz.c b/hw/mips/jazz.c
>>>> index 1e1cf8154e..1df67035aa 100644
>>>> --- a/hw/mips/jazz.c
>>>> +++ b/hw/mips/jazz.c
>>>> @@ -280,6 +280,7 @@ static void mips_jazz_init(MachineState *machine,
>>>>                dev = qdev_new("dp8393x");
>>>>                qdev_set_nic_properties(dev, nd);
>>>>                qdev_prop_set_uint8(dev, "it_shift", 2);
>>>> +             qdev_prop_set_bit(dev, "big_endian", true);
>>>>                object_property_set_link(OBJECT(dev), "dma_mr",
>>>>                                         OBJECT(rc4030_dma_mr), &error_abort);
>>>>                sysbus = SYS_BUS_DEVICE(dev);
>>>>
>>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>>
>>>> [q800-macos-upstream patchset series: 3]
>>>>
>>>> Mark Cave-Ayland (5):
>>>>     dp8393x: checkpatch fixes
>>>>     dp8393x: convert to trace-events
>>>>     dp8393x: fix PROM checksum and MAC address storage
>>>>     dp8393x: don't force 32-bit register access
>>>>     dp8393x: fix CAM descriptor entry index
>>>>
>>>>    hw/net/dp8393x.c    | 332 ++++++++++++++++++++++++--------------------
>>>>    hw/net/trace-events |  17 +++
>>>>    2 files changed, 198 insertions(+), 151 deletions(-)
>>
>> Just to add that I've done a large amount of testing on the q800 machine
>> with Linux/MacOS so I'm happy that these patches do the right thing
>> there.
>>
>> The part I'm struggling with is testing against MIPS jazz since I don't
>> have a Linux test image to hand, and there is no documentation in the
>> original commit message as to where the existing PROM checksum algorithm
>> came from.
>>
>> Hervé, can you provide some more information on this? It looks like it
>> was introduced in one of your commits:
>>
>> commit 89ae0ff9b73ee74c9ba707a09a07ad77b9fdccb4
>> Author: Hervé Poussineau <hpoussin@reactos.org>
>> Date:   Wed Jun 3 22:45:46 2015 +0200
>>
>>      net/dp8393x: add PROM to store MAC address
>>
>>      Signed-off-by: Laurent Vivier <laurent@vivier.eu>
>>      Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>>      Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
>>      Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
>>
> 
> With "qemu-system-mips -M magnum ..." I was able to boot both Linux and
> NetBSD. That was after commit 89ae0ff9b7 ("net/dp8393x: add PROM to store
> MAC address"). But that's not to say that the MAC address was decoded
> correctly.
> 
> Please see,
> https://lore.kernel.org/qemu-devel/alpine.LNX.2.21.1.1912241504560.11@nippy.intranet/
> 
> The Linux/mips (jazzsonic) testing that I did back in 2019 used a custom
> busybox initramfs. The NetBSD/mips testing used the official CD ISO image.
> I will look into reviving those test harnesses because I think patch 4/5
> and the proposed big-endian flag will need some regression testing.

Thanks for the reference - I've just discovered from the link above something I 
hadn't realised which is that -M magnum is present on both qemu-system-mips64 *AND* 
qemu-system-mips64el indicating that the endian needs to be set accordingly. 
Fortunately it should be possible to use a similar solution as to that used for the 
malta machine i.e.:


diff --git a/hw/mips/jazz.c b/hw/mips/jazz.c
index 1e1cf8154e..16b32d2b2c 100644
--- a/hw/mips/jazz.c
+++ b/hw/mips/jazz.c
@@ -124,7 +124,7 @@ static void mips_jazz_init(MachineState *machine,
  {
      MemoryRegion *address_space = get_system_memory();
      char *filename;
-    int bios_size, n;
+    int bios_size, n, big_endian;
      Clock *cpuclk;
      MIPSCPU *cpu;
      MIPSCPUClass *mcc;
@@ -155,6 +155,12 @@ static void mips_jazz_init(MachineState *machine,
          [JAZZ_PICA61] = {33333333, 4},
      };

+#ifdef TARGET_WORDS_BIGENDIAN
+    big_endian = 1;
+#else
+    big_endian = 0;
+#endif
+
      if (machine->ram_size > 256 * MiB) {
          error_report("RAM size more than 256Mb is not supported");
          exit(EXIT_FAILURE);
@@ -280,6 +286,7 @@ static void mips_jazz_init(MachineState *machine,
              dev = qdev_new("dp8393x");
              qdev_set_nic_properties(dev, nd);
              qdev_prop_set_uint8(dev, "it_shift", 2);
+            qdev_prop_set_bit(dev, "big_endian", big_endian);
              object_property_set_link(OBJECT(dev), "dma_mr",
                                       OBJECT(rc4030_dma_mr), &error_abort);
              sysbus = SYS_BUS_DEVICE(dev);


If you have bootable images available for -M magnum under qemu-system-mips64 and 
qemu-system-mips64el, is it possible to make them available to others for testing?


ATB,

Mark.


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/5] dp8393x: fixes for MacOS toolbox ROM
  2021-06-16  6:10       ` Mark Cave-Ayland
@ 2021-06-18  6:22         ` Finn Thain
  2021-06-24 14:42           ` Mark Cave-Ayland
  0 siblings, 1 reply; 16+ messages in thread
From: Finn Thain @ 2021-06-18  6:22 UTC (permalink / raw)
  To: Mark Cave-Ayland
  Cc: aleksandar.rikalo, jasowang, Philippe Mathieu-Daudé,
	qemu-devel, hpoussin, aurelien, Laurent Vivier

Hi Mark,

On Wed, 16 Jun 2021, Mark Cave-Ayland wrote:

> On 16/06/2021 04:09, Finn Thain wrote:
> 
> > With "qemu-system-mips -M magnum ..." I was able to boot both Linux 
> > and NetBSD. That was after commit 89ae0ff9b7 ("net/dp8393x: add PROM 
> > to store MAC address"). But that's not to say that the MAC address was 
> > decoded correctly.
> > 
> > Please see, 
> > https://lore.kernel.org/qemu-devel/alpine.LNX.2.21.1.1912241504560.11@nippy.intranet/
> > 
> > The Linux/mips (jazzsonic) testing that I did back in 2019 used a 
> > custom busybox initramfs. The NetBSD/mips testing used the official CD 
> > ISO image. I will look into reviving those test harnesses because I 
> > think patch 4/5 and the proposed big-endian flag will need some 
> > regression testing.
> 
> Thanks for the reference - I've just discovered from the link above 
> something I hadn't realised which is that -M magnum is present on both 
> qemu-system-mips64 *AND* qemu-system-mips64el indicating that the endian 
> needs to be set accordingly. Fortunately it should be possible to use a 
> similar solution as to that used for the malta machine i.e.:
> 
> 
> diff --git a/hw/mips/jazz.c b/hw/mips/jazz.c
> index 1e1cf8154e..16b32d2b2c 100644
> --- a/hw/mips/jazz.c
> +++ b/hw/mips/jazz.c
> @@ -124,7 +124,7 @@ static void mips_jazz_init(MachineState *machine,
>  {
>      MemoryRegion *address_space = get_system_memory();
>      char *filename;
> -    int bios_size, n;
> +    int bios_size, n, big_endian;
>      Clock *cpuclk;
>      MIPSCPU *cpu;
>      MIPSCPUClass *mcc;
> @@ -155,6 +155,12 @@ static void mips_jazz_init(MachineState *machine,
>          [JAZZ_PICA61] = {33333333, 4},
>      };
> 
> +#ifdef TARGET_WORDS_BIGENDIAN
> +    big_endian = 1;
> +#else
> +    big_endian = 0;
> +#endif
> +
>      if (machine->ram_size > 256 * MiB) {
>          error_report("RAM size more than 256Mb is not supported");
>          exit(EXIT_FAILURE);
> @@ -280,6 +286,7 @@ static void mips_jazz_init(MachineState *machine,
>              dev = qdev_new("dp8393x");
>              qdev_set_nic_properties(dev, nd);
>              qdev_prop_set_uint8(dev, "it_shift", 2);
> +            qdev_prop_set_bit(dev, "big_endian", big_endian);
>              object_property_set_link(OBJECT(dev), "dma_mr",
>                                       OBJECT(rc4030_dma_mr), &error_abort);
>              sysbus = SYS_BUS_DEVICE(dev);
> 

I was able to test this patch series successfully, using both 
"qemu-system-mipsel -M magnum" and "qemu-system-m68k -M q800". With the 
latter emulator I used a Linux/m68k guest and with the former I used both 
Linux/mips and NetBSD/arc guests.

Basic dp8393x functionality worked fine. I don't know how the m68k guest 
obtained its MAC address setting, but the macsonic driver banner message 
agrees with what's on the wire, at least.

The mips guest has ARC firmware which allowed the MAC address to be 
programmed manually:

 JAZZ Setup Program Version 0.17            Friday, 6-18-2021   5:06:02 AM
 Copyright (c) 1991, 1992  Microsoft Corporation

     The current Ethernet station address is: 900090001122
     Enter the new station address:  900090123456 
     The value written to NVRAM is: 9000901234560042
 Press any key to continue...

I'm guessing that the "0042" is the checksum?

BTW, the patch in the message quoted above does not seem to affect my 
results. (This host is little-endian...) I don't know how to test 
that one. For the others:
Tested-by: Finn Thain <fthain@linux-m68k.org>

> 
> If you have bootable images available for -M magnum under 
> qemu-system-mips64 and qemu-system-mips64el, is it possible to make them 
> available to others for testing?
> 

All of the ARC-compliant systems were little-endian according to the 
"Advanced RISC Computing Specification" published by MIPS Technology. 
There may have been some non-ARC machines but I've not explored that 
question.

Regarding bootable images: for NetBSD I just used the official installer, 
NetBSD-9.2-arc.iso. (The regressions I encountered in the past were fixed 
and hence NetBSD 5.1 isn't needed.)

For convenience, I used the NetBSD/arc bootloader to load either the 
NetBSD kernel or the Linux kernel. The ARC bios is required. For a copy of 
that please see https://gunkies.org/wiki/Installing_NetBSD_ARC_on_Qemu

For Linux/mips I updated my disk images, which are 1) an extfs image 
containing a minimal 32-bit mipsel busybox rootfs and 2) an ISO image 
containing a Linux kernel binary.

Unfortunately, the default mainline kernel build (jazz_defconfig) seems to 
be too big and crashes the bootloader (not a new problem though):

NetBSD/arc Bootstrap, Revision 1.1 (Wed May 12 13:15:55 UTC 2021)
devopen: scsi(0)cdrom(4)fdisk(0) type disk file vmlinux
6787344+134272 [775200+974808/
Jazz Monitor. Version 174
Press H for help, Q to quit.
AdEL exception occurred.
 at=ffffff7f v0=74736b5f v1=807f3e94 a0=e0002003 a1=00000090 a2=80eff05c
 a3=a07f0000 t0=00000002 t1=807f3ae8 t2=807f3e40 t3=80efe548 t4=00000001
 t5=00000001 t6=00000088 t7=00000080 s0=807f5060 s1=807f3df8 s2=807f3e94
 s3=8003f8e0 s4=807f3df8 s5=00000001 s6=ffffff3f s7=807f3d40 t8=61747274
 t9=61747274 k0=80041f50 k1=80000194 gp=80f0cc50 sp=80efe560 s8=8003f928
 ra=800213d4 psr=20000803 epc=800213e4 cause=00004010 errorepc=00000000
 badvaddr=74736bbf
>

This limitation prevents embedding the initramfs into 'vmlinux'.

A successful workaround is to shrink the kernel binary by disabling some 
Kconfig options.

$ git checkout v5.12
$ make ARCH=mips CROSS_COMPILE=mipsel-linux-gnu- clean jazz_defconfig
$ scripts/config -d IPV6 -d WIRELESS -d WLAN -d DEBUG_KERNEL -d EXPERT -d CC_OPTIMIZE_FOR_PERFORMANCE -e CC_OPTIMIZE_FOR_SIZE
$ make ARCH=mips CROSS_COMPILE=mipsel-linux-gnu-
$ mkisofs -o vmlinux.iso -J -iso-level 3 vmlinux
$ qemu-system-mips64el -M magnum -L . -drive if=scsi,unit=0,format=raw,file=rootfs.img -drive if=scsi,unit=2,media=cdrom,format=raw,file=vmlinux.iso -drive if=scsi,unit=4,media=cdrom,format=raw,file=NetBSD-9.2-arc.iso -global ds1225y.filename=nvram -global ds1225y.size=8200 -serial mon:stdio -serial null -net nic,model=dp83932,addr=00:00:00:01:02:03 -net bridge


 Actions:

     Start Windows NT
->   Run a program
     Run setup

 Use the arrow keys to select.I Version 2.6)
 Press Enter to choose


     Program to run:
scsi(0)cdrom(4)fdisk(0)boot scsi(0)cdrom(2)fdisk(0)vmlinux root=/dev/sda rw ignore_loglevel ip=192.168.66.123 console=ttyS0


NetBSD/arc Bootstrap, Revision 1.1 (Wed May 12 13:15:55 UTC 2021)
devopen: scsi(0)cdrom(2)fdisk(0) type disk file vmlinux
5326644+144284 [777648+947785]=0x6dd3a0
Linux version 5.12.0 (fthain@nippy) (mipsel-linux-gnu-gcc (btc) 6.4.0, GNU ld (btc) 2.28) #28 Fri Jun 18 13:50:42 AEST 2021
ARCH: Microsoft-Jazz
PROMLIB: ARC firmware Version 1 Revision 2
CPU0 revision is: 00000400 (R4000PC)
FPU revision is: 00000500
printk: debug: ignoring loglevel setting.
Primary instruction cache 8kB, VIPT, direct mapped, linesize 16 bytes.
Primary data cache 8kB, direct mapped, VIPT, cache aliases, linesize 16 bytes
Zone ranges:
  DMA      [mem 0x0000000000000000-0x0000000000ffffff]
  Normal   [mem 0x0000000001000000-0x0000000007ffffff]
Movable zone start for each node
Early memory node ranges
  node   0: [mem 0x0000000000000000-0x0000000007ffffff]
Initmem setup node 0 [mem 0x0000000000000000-0x0000000007ffffff]
On node 0 totalpages: 32768
  DMA zone: 32 pages used for memmap
  DMA zone: 0 pages reserved
  DMA zone: 4096 pages, LIFO batch:0
  Normal zone: 224 pages used for memmap
  Normal zone: 28672 pages, LIFO batch:7
pcpu-alloc: s0 r0 d32768 u32768 alloc=1*32768
pcpu-alloc: [0] 0
Built 1 zonelists, mobility grouping on.  Total pages: 32512
Kernel command line: scsi(0)cdrom(2)fdisk(0)vmlinux root=/dev/sda rw ignore_loglevel ip=192.168.66.123 console=ttyS0
Dentry cache hash table entries: 16384 (order: 4, 65536 bytes, linear)
Inode-cache hash table entries: 8192 (order: 3, 32768 bytes, linear)
mem auto-init: stack:off, heap alloc:off, heap free:off
Memory: 124092K/131072K available (3960K kernel code, 236K rwdata, 860K rodata, 184K init, 94K bss, 6980K reserved, 0K cma-reserved)
NR_IRQS: 256
random: get_random_bytes called from start_kernel+0x330/0x4dc with crng_init=0
Console: colour dummy device 80x25
sched_clock: 32 bits at 100 Hz, resolution 10000000ns, wraps every 21474836475000000ns
Calibrating delay loop... 984.67 BogoMIPS (lpj=4923392)
pid_max: default: 32768 minimum: 301
Mount-cache hash table entries: 1024 (order: 0, 4096 bytes, linear)
Mountpoint-cache hash table entries: 1024 (order: 0, 4096 bytes, linear)
devtmpfs: initialized
clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 19112604462750000 ns
futex hash table entries: 256 (order: -1, 3072 bytes, linear)
NET: Registered protocol family 16
VDMA: R4030 DMA pagetables initialized.
SCSI subsystem initialized
libata version 3.00 loaded.
NET: Registered protocol family 2
tcp_listen_portaddr_hash hash table entries: 512 (order: 0, 4096 bytes, linear)
TCP established hash table entries: 1024 (order: 0, 4096 bytes, linear)
TCP bind hash table entries: 1024 (order: 0, 4096 bytes, linear)
TCP: Hash tables configured (established 1024 bind 1024)
UDP hash table entries: 256 (order: 0, 4096 bytes, linear)
UDP-Lite hash table entries: 256 (order: 0, 4096 bytes, linear)
NET: Registered protocol family 1
workingset: timestamp_bits=30 max_order=15 bucket_order=0
Block layer SCSI generic (bsg) driver version 0.4 loaded (major 254)
io scheduler mq-deadline registered
io scheduler kyber registered
Console: switching to colour frame buffer device 100x37
Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
printk: console [ttyS0] disabled
serial8250.0: ttyS0 at MMIO 0xe0006000 (irq = 32, base_baud = 115200) is a 16550A
printk: console [ttyS0] enabled
serial8250.0: ttyS1 at MMIO 0xe0007000 (irq = 33, base_baud = 115200) is a 16550A
jazz_esp jazz_esp.0: esp0: regs[(ptrval):0] irq[29]
jazz_esp jazz_esp.0: esp0: is a FAS100A, 40 MHz (ccf=0), SCSI ID 7
random: fast init done
scsi host0: esp
PDC20230-C/20630 VLB ATA controller detected.
scsi host1: pata_legacy
ata1: PATA max PIO2 cmd 0x1f0 ctl 0x3f6 irq 14
scsi 0:0:0:0: Direct-Access     QEMU     QEMU HARDDISK    2.5+ PQ: 0 ANSI: 5
scsi target0:0:0: Beginning Domain Validation
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
scsi target0:0:0: Domain Validation skipping write tests
scsi target0:0:0: Ending Domain Validation
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
scsi 0:0:2:0: CD-ROM            QEMU     QEMU CD-ROM      2.5+ PQ: 0 ANSI: 5
scsi target0:0:2: Beginning Domain Validation
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
scsi target0:0:2: Domain Validation skipping write tests
scsi target0:0:2: Ending Domain Validation
VDMA: Channel 0: Memory error!
scsi 0:0:4:0: CD-ROM            QEMU     QEMU CD-ROM      2.5+ PQ: 0 ANSI: 5
scsi target0:0:4: Beginning Domain Validation
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
scsi target0:0:4: Domain Validation skipping write tests
scsi target0:0:4: Ending Domain Validation
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
sd 0:0:0:0: [sda] 40960 512-byte logical blocks: (21.0 MB/20.0 MiB)
VDMA: Channel 0: Memory error!
sd 0:0:0:0: [sda] Write Protect is off
sd 0:0:0:0: [sda] Mode Sense: 63 00 00 08
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
sd 0:0:0:0: [sda] Attached SCSI disk
scsi host1: pata_legacy
ata2: PATA max PIO4 cmd 0x170 ctl 0x376 irq 15
scsi host1: pata_legacy
ata3: PATA max PIO4 cmd 0x1e8 ctl 0x3ee irq 11
scsi host1: pata_legacy
ata4: PATA max PIO4 cmd 0x168 ctl 0x36e irq 10
scsi host1: pata_legacy
ata5: PATA max PIO4 cmd 0x1e0 ctl 0x3e6 irq 8
scsi host1: pata_legacy
ata6: PATA max PIO4 cmd 0x160 ctl 0x366 irq 12
SONIC ethernet @e0001000, MAC 90:00:90:12:34:56, IRQ 28
serio: i8042 KBD port at 0xe0005000,0xe0005001 irq 30
serio: i8042 AUX port at 0xe0005000,0xe0005001 irq 31
input: AT Raw Set 2 keyboard as /devices/platform/i8042/serio0/input/input0
input: ImExPS/2 Generic Explorer Mouse as /devices/platform/i8042/serio1/input/input2
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
EXT4-fs (sda): warning: mounting unchecked fs, running e2fsck is recommended
EXT4-fs (sda): mounted filesystem without journal. Opts: (null). Quota mode: disabled.
VFS: Mounted root (ext4 filesystem) on device 8:0.
Freeing prom memory: 376k freed
Freeing prom memory: 60k freed
Freeing prom memory: 4k freed
Freeing unused kernel memory: 184K
This architecture does not have kernel memory protection.
Run /sbin/init as init process
  with arguments:
    /sbin/init
    scsi(0)cdrom(2)fdisk(0)vmlinux
  with environment:
    HOME=/
    TERM=linux
    ip=192.168.66.123
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
process '/bin/busybox' started with executable stack
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
hwclock: can't open '/dev/misc/rtc': No such file or directory
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
VDMA: Channel 0: Memory error!
# ip link
1: lo: <LOOPBACK> mtu 65536 qdisc noop qlen 1000
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast qlen 1000
    link/ether 90:00:90:12:34:56 brd ff:ff:ff:ff:ff:ff
# ping 192.168.66.1
PING 192.168.66.1 (192.168.66.1): 56 data bytes
64 bytes from 192.168.66.1: seq=0 ttl=64 time=40.000 ms
64 bytes from 192.168.66.1: seq=1 ttl=64 time=0.000 ms
64 bytes from 192.168.66.1: seq=2 ttl=64 time=0.000 ms
^C
--- 192.168.66.1 ping statistics ---
3 packets transmitted, 3 packets received, 0% packet loss
round-trip min/avg/max = 0.000/13.333/40.000 ms
#


The process for booting NetBSD is the same except that "Program to run"
would be:
scsi(0)cdrom(4)fdisk(0)boot scsi(0)cdrom(4)fdisk(0)netbsd


NetBSD/arc Bootstrap, Revision 1.1 (Wed May 12 13:15:55 UTC 2021)
devopen: scsi(0)cdrom(4)fdisk(0) type disk file netbsd
5193200+83744=0x508968
[   1.0000000] Copyright (c) 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005,
[   1.0000000]     2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017,
[   1.0000000]     2018, 2019, 2020 The NetBSD Foundation, Inc.  All rights reserved.
[   1.0000000] Copyright (c) 1982, 1986, 1989, 1991, 1993
[   1.0000000]     The Regents of the University of California.  All rights reserved.

[   1.0000000] NetBSD 9.2 (RAMDISK) #0: Wed May 12 13:15:55 UTC 2021
[   1.0000000]  mkrepro@mkrepro.NetBSD.org:/usr/src/sys/arch/arc/compile/RAMDISK
[   1.0000000] MIPS Magnum
[   1.0000000] total memory = 128 MB
[   1.0000000] avail memory = 119 MB
[   1.0000000] mainbus0 (root)
[   1.0000000] cpu0 at mainbus0: MIPS R4000 CPU (0x400) Rev. 0.0 with MIPS R4010 FPC Rev. 0.0
[   1.0000000] cpu0: 48 TLB entries, 256MB max page size
[   1.0000000] cpu0: 8KB/16B direct-mapped L1 instruction cache
[   1.0000000] cpu0: 8KB/16B direct-mapped write-back L1 data cache
[   1.0000000] jazzio0 at mainbus0
[   1.0000000] timer0 at jazzio0 addr 0xe0000228
[   1.0000000] mcclock0 at jazzio0 addr 0xe0004000: mc146818 compatible time-of-day clock
[   1.0000000] LPT1 at jazzio0 addr 0xe0008000 intr 0 not configured
[   1.0000000] fdc0 at jazzio0 addr 0xe0003000 intr 1
[   1.0000000] fd0 at fdc0 drive 1: 1.44MB, 80 cyl, 2 head, 18 sec
[   1.0000000] MAGNUM at jazzio0 addr 0xe000c000 intr 2 not configured
[   1.0000000] VXL at jazzio0 addr 0xe0800000 intr 3 not configured
[   1.0000000] sn0 at jazzio0 addr 0xe0001000 intr 4: SONIC Ethernet
[   1.0000000] sn0: Ethernet address 90:00:90:12:34:56
[   1.0000000] asc0 at jazzio0 addr 0xe0002000 intr 5: NCR53C94, 25MHz, SCSI ID 7
[   1.0000000] scsibus0 at asc0: 8 targets, 8 luns per target
[   1.0000000] pckbc0 at jazzio0 addr 0xe0005000 intr 6
[   1.0000000] pckbd0 at pckbc0 (kbd slot)
[   1.0000000] wskbd0 at pckbd0 (mux ignored)
[   1.0000000] pms at jazzio0 addr 0xe0005000 intr 7 not configured
[   1.0000000] com0 at jazzio0 addr 0xe0006000 intr 8: ns16550a, working fifo
[   1.0000000] com0: txfifo disabled
[   1.0000000] com0: console
[   1.0000000] com1 at jazzio0 addr 0xe0007000 intr 9: ns16550a, working fifo
[   1.0000000] com1: txfifo disabled
[   1.0000000] jazzisabr0 at mainbus0
[   1.0000000] isa0 at jazzisabr0
[   1.0000000] isapnp0 at isa0 port 0x279
[   1.0000100] scsibus0: waiting 2 seconds for devices to settle...
[   2.3472719] sd0 at scsibus0 target 0 lun 0: <QEMU, QEMU HARDDISK, 2.5+> disk fixed
[   2.3472719] sd0: 20480 KB, 40 cyl, 16 head, 64 sec, 512 bytes/sect x 40960 sectors
[   2.3472719] cd0 at scsibus0 target 2 lun 0: <QEMU, QEMU CD-ROM, 2.5+> cdrom removable
[   2.3472719] cd1 at scsibus0 target 4 lun 0: <QEMU, QEMU CD-ROM, 2.5+> cdrom removable
[   2.3544734] boot device: <unknown>
[   2.3544734] root on md0a dumps on md0b
[   2.3611882] root file system type: ffs
[   2.3678883] kern.module.path=/stand/arc/9.2/modules
[   2.3678883] WARNING: preposterous TOD clock time
[   2.3678883] WARNING: using filesystem time
[   2.3712975] WARNING: CHECK AND RESET THE DATE!
erase ^H, werase ^W, kill ^U, intr ^C, status ^T
Terminal type? [vt100]
Erase is backspace.
(I)nstall, (S)hell or (H)alt ? s
# ifconfig sn0 192.168.66.123
# ping 192.168.66.1
PING 192.168.66.1 (192.168.66.1): 56 data bytes
64 bytes from 192.168.66.1: icmp_seq=0 ttl=64 time=7.208080 ms
64 bytes from 192.168.66.1: icmp_seq=1 ttl=64 time=1.696826 ms
^C
----192.168.66.1 PING Statistics----
2 packets transmitted, 2 packets received, 0.0% packet loss
round-trip min/avg/max/stddev = 1.696826/4.452453/7.208080/3.897045 ms
# ifconfig -a
sn0: flags=0x8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> mtu 1500
        ec_capabilities=1<VLAN_MTU>
        ec_enabled=0
        address: 90:00:90:12:34:56
        inet 192.168.66.123/24 broadcast 192.168.66.255 flags 0x0
lo0: flags=0x8048<LOOPBACK,RUNNING,MULTICAST> mtu 33160
#



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/5] dp8393x: fixes for MacOS toolbox ROM
  2021-06-18  6:22         ` Finn Thain
@ 2021-06-24 14:42           ` Mark Cave-Ayland
  2021-06-25  4:36             ` Finn Thain
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Cave-Ayland @ 2021-06-24 14:42 UTC (permalink / raw)
  To: Finn Thain
  Cc: aleksandar.rikalo, jasowang, qemu-devel,
	Philippe Mathieu-Daudé,
	hpoussin, aurelien, Laurent Vivier

On 18/06/2021 07:22, Finn Thain wrote:

> I was able to test this patch series successfully, using both
> "qemu-system-mipsel -M magnum" and "qemu-system-m68k -M q800". With the
> latter emulator I used a Linux/m68k guest and with the former I used both
> Linux/mips and NetBSD/arc guests.
> 
> Basic dp8393x functionality worked fine. I don't know how the m68k guest
> obtained its MAC address setting, but the macsonic driver banner message
> agrees with what's on the wire, at least.
> 
> The mips guest has ARC firmware which allowed the MAC address to be
> programmed manually:
> 
>   JAZZ Setup Program Version 0.17            Friday, 6-18-2021   5:06:02 AM
>   Copyright (c) 1991, 1992  Microsoft Corporation
> 
>       The current Ethernet station address is: 900090001122
>       Enter the new station address:  900090123456
>       The value written to NVRAM is: 9000901234560042
>   Press any key to continue...
> 
> I'm guessing that the "0042" is the checksum?
> 
> BTW, the patch in the message quoted above does not seem to affect my
> results. (This host is little-endian...) I don't know how to test
> that one. For the others:
> Tested-by: Finn Thain <fthain@linux-m68k.org>

Thanks for the link and the detailed testing information. I've been trying to 
understand why you had to set the MAC address in the ARC firmware so I had a bit of 
an experiment here.

The reason that you need to do this is because of the NVRAM configuration in your 
command line, in particular -global ds1225y.size=8200. What this does is extend the 
NVRAM over the top of the dp8393x-prom area where QEMU places the NIC MAC address and 
checksum on startup, so the NVRAM captures the MAC address reads/writes instead. The 
net effect of this is that the empty NVRAM initially reads all zeros and why an 
initial setup is required to set the MAC address.

This can be seen quite clearly in the "info mtree" output:

     0000000080009000-000000008000b007 (prio 0, i/o): nvram
     000000008000b000-000000008000bfff (prio 0, rom): dp8393x-prom

However if you completely drop -global ds1225y.size=8200 from your command line then 
the NVRAM doesn't overrun into the dp8393x-prom area, and the ARC firmware picks up 
the MAC address from QEMU correctly:

     0000000080009000-000000008000afff (prio 0, i/o): nvram
     000000008000b000-000000008000bfff (prio 0, rom): dp8393x-prom

I've also looked over the entire SONIC datasheet to see if the PROM format is 
documented, and according to that there is no non-volatile storage available on the 
chip itself. Testing shows that the checksum algorithm currently used for the dp8393x 
device generates the same result as that generated by the ARC firmware, which is 
known to be different than that used by the Q800 machine.

 From this I conclude that the PROM is provided by the board and not the chipset, and 
therefore each machine should construct its own PROM accordingly. I'll send a v2 
patchset shortly with these changes which shall also include the proposed endian patch.


ATB,

Mark.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/5] dp8393x: fixes for MacOS toolbox ROM
  2021-06-24 14:42           ` Mark Cave-Ayland
@ 2021-06-25  4:36             ` Finn Thain
  2021-06-25  6:17               ` Mark Cave-Ayland
  0 siblings, 1 reply; 16+ messages in thread
From: Finn Thain @ 2021-06-25  4:36 UTC (permalink / raw)
  To: Mark Cave-Ayland
  Cc: aleksandar.rikalo, jasowang, qemu-devel,
	Philippe Mathieu-Daudé,
	hpoussin, aurelien, Laurent Vivier

On Thu, 24 Jun 2021, Mark Cave-Ayland wrote:

> Thanks for the link and the detailed testing information. I've been 
> trying to understand why you had to set the MAC address in the ARC 
> firmware so I had a bit of an experiment here.
> 
> The reason that you need to do this is because of the NVRAM 
> configuration in your command line, in particular -global 
> ds1225y.size=8200. What this does is extend the NVRAM over the top of 
> the dp8393x-prom area where QEMU places the NIC MAC address and checksum 
> on startup, so the NVRAM captures the MAC address reads/writes instead. 
> The net effect of this is that the empty NVRAM initially reads all zeros 
> and why an initial setup is required to set the MAC address.
> 
> This can be seen quite clearly in the "info mtree" output:
> 
>     0000000080009000-000000008000b007 (prio 0, i/o): nvram
>     000000008000b000-000000008000bfff (prio 0, rom): dp8393x-prom
> 
> However if you completely drop -global ds1225y.size=8200 from your 
> command line then the NVRAM doesn't overrun into the dp8393x-prom area, 
> and the ARC firmware picks up the MAC address from QEMU correctly:
> 
>     0000000080009000-000000008000afff (prio 0, i/o): nvram
>     000000008000b000-000000008000bfff (prio 0, rom): dp8393x-prom
> 
> I've also looked over the entire SONIC datasheet to see if the PROM 
> format is documented, and according to that there is no non-volatile 
> storage available on the chip itself. 

Yes, that's my understanding also. The relevant National Semicondutor 
Application Notes seem to include a separate PROM. And if you closely 
examine the Linux macsonic.c driver, you'll see that the PowerBook 5x0 
models get a random MAC address because no-one (outside of Apple) knows 
where the real MAC address is stored.

> Testing shows that the checksum algorithm currently used for the dp8393x 
> device generates the same result as that generated by the ARC firmware, 
> which is known to be different than that used by the Q800 machine.
> 
> From this I conclude that the PROM is provided by the board and not the 
> chipset, and therefore each machine should construct its own PROM 
> accordingly. I'll send a v2 patchset shortly with these changes which 
> shall also include the proposed endian patch.
> 

If you potentially have both a ds1225y NVRAM and a dp8393x PROM (for the 
magnum machine) how do you avoid ending up with conflicting state? Would 
the two storage devices have to be mutually exclusive?


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/5] dp8393x: fixes for MacOS toolbox ROM
  2021-06-25  4:36             ` Finn Thain
@ 2021-06-25  6:17               ` Mark Cave-Ayland
  2021-06-25  9:32                 ` Finn Thain
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Cave-Ayland @ 2021-06-25  6:17 UTC (permalink / raw)
  To: Finn Thain
  Cc: aleksandar.rikalo, jasowang, Philippe Mathieu-Daudé,
	qemu-devel, hpoussin, aurelien, Laurent Vivier

On 25/06/2021 05:36, Finn Thain wrote:

> On Thu, 24 Jun 2021, Mark Cave-Ayland wrote:
> 
>> Thanks for the link and the detailed testing information. I've been
>> trying to understand why you had to set the MAC address in the ARC
>> firmware so I had a bit of an experiment here.
>>
>> The reason that you need to do this is because of the NVRAM
>> configuration in your command line, in particular -global
>> ds1225y.size=8200. What this does is extend the NVRAM over the top of
>> the dp8393x-prom area where QEMU places the NIC MAC address and checksum
>> on startup, so the NVRAM captures the MAC address reads/writes instead.
>> The net effect of this is that the empty NVRAM initially reads all zeros
>> and why an initial setup is required to set the MAC address.
>>
>> This can be seen quite clearly in the "info mtree" output:
>>
>>      0000000080009000-000000008000b007 (prio 0, i/o): nvram
>>      000000008000b000-000000008000bfff (prio 0, rom): dp8393x-prom
>>
>> However if you completely drop -global ds1225y.size=8200 from your
>> command line then the NVRAM doesn't overrun into the dp8393x-prom area,
>> and the ARC firmware picks up the MAC address from QEMU correctly:
>>
>>      0000000080009000-000000008000afff (prio 0, i/o): nvram
>>      000000008000b000-000000008000bfff (prio 0, rom): dp8393x-prom
>>
>> I've also looked over the entire SONIC datasheet to see if the PROM
>> format is documented, and according to that there is no non-volatile
>> storage available on the chip itself.
> 
> Yes, that's my understanding also. The relevant National Semicondutor
> Application Notes seem to include a separate PROM. And if you closely
> examine the Linux macsonic.c driver, you'll see that the PowerBook 5x0
> models get a random MAC address because no-one (outside of Apple) knows
> where the real MAC address is stored.

Agreed. This means that the revised patchset should now be doing the right thing here.

FWIW I felt that it had changed too much in its latest form to include your original 
Tested-by tag due to the extra PROM changes, so I'd be grateful if you could give it 
a quick test.

>> Testing shows that the checksum algorithm currently used for the dp8393x
>> device generates the same result as that generated by the ARC firmware,
>> which is known to be different than that used by the Q800 machine.
>>
>>  From this I conclude that the PROM is provided by the board and not the
>> chipset, and therefore each machine should construct its own PROM
>> accordingly. I'll send a v2 patchset shortly with these changes which
>> shall also include the proposed endian patch.
>>
> 
> If you potentially have both a ds1225y NVRAM and a dp8393x PROM (for the
> magnum machine) how do you avoid ending up with conflicting state? Would
> the two storage devices have to be mutually exclusive?

The ds1225y NVRAM is located between 0x80009000-0x8000afff and running the nvram file 
through hexdump shows only the first 0x1000 bytes contain any data, so any other 
changes made to NVRAM via the ARC firmware setup will be preserved.

The existing default behaviour (without -global ds1225y.size=8200) is that only the 
last few bytes at 0x8000b000 are mapped to the dp8393x PROM, and this area is marked 
read-only to ensure that the MAC address obtained by the guest OS always matches the 
one provided by the QEMU configuration.


ATB,

Mark.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/5] dp8393x: fixes for MacOS toolbox ROM
  2021-06-25  6:17               ` Mark Cave-Ayland
@ 2021-06-25  9:32                 ` Finn Thain
  2021-06-25 11:49                   ` Mark Cave-Ayland
  0 siblings, 1 reply; 16+ messages in thread
From: Finn Thain @ 2021-06-25  9:32 UTC (permalink / raw)
  To: Mark Cave-Ayland
  Cc: aleksandar.rikalo, jasowang, Philippe Mathieu-Daudé,
	qemu-devel, hpoussin, aurelien, Laurent Vivier

On Fri, 25 Jun 2021, Mark Cave-Ayland wrote:

> On 25/06/2021 05:36, Finn Thain wrote:
> 
> > On Thu, 24 Jun 2021, Mark Cave-Ayland wrote:
> > 
> > > Thanks for the link and the detailed testing information. I've been 
> > > trying to understand why you had to set the MAC address in the ARC 
> > > firmware so I had a bit of an experiment here.
> > > 
> > > The reason that you need to do this is because of the NVRAM 
> > > configuration in your command line, in particular -global 
> > > ds1225y.size=8200.

That configuration also shows up here, 
https://virtuallyfun.com/wordpress/2013/08/30/restoring-the-mips-magnum-in-qemu-1-6-0/
with the explanation, "you'll need the NVRam stuff to add extra space for 
the ethernet MAC address". So it seems that the 8200 figure was just a 
hack and does not reflect the size of the NVRAM in an actual Magnum.

> > > What this does is extend the NVRAM over the top of the dp8393x-prom 
> > > area where QEMU places the NIC MAC address and checksum on startup, 
> > > so the NVRAM captures the MAC address reads/writes instead. The net 
> > > effect of this is that the empty NVRAM initially reads all zeros and 
> > > why an initial setup is required to set the MAC address.
> > > 
> > > This can be seen quite clearly in the "info mtree" output:
> > > 
> > >      0000000080009000-000000008000b007 (prio 0, i/o): nvram 
> > >      000000008000b000-000000008000bfff (prio 0, rom): dp8393x-prom
> > > 
> > > However if you completely drop -global ds1225y.size=8200 from your 
> > > command line then the NVRAM doesn't overrun into the dp8393x-prom 
> > > area, and the ARC firmware picks up the MAC address from QEMU 
> > > correctly:
> > > 
> > >      0000000080009000-000000008000afff (prio 0, i/o): nvram 
> > >      000000008000b000-000000008000bfff (prio 0, rom): dp8393x-prom
> > > 
> > > I've also looked over the entire SONIC datasheet to see if the PROM 
> > > format is documented, and according to that there is no non-volatile 
> > > storage available on the chip itself.
> > 
> > Yes, that's my understanding also. The relevant National Semicondutor 
> > Application Notes seem to include a separate PROM. And if you closely 
> > examine the Linux macsonic.c driver, you'll see that the PowerBook 5x0 
> > models get a random MAC address because no-one (outside of Apple) 
> > knows where the real MAC address is stored.
> 
> Agreed. This means that the revised patchset should now be doing the 
> right thing here.
> 
> FWIW I felt that it had changed too much in its latest form to include 
> your original Tested-by tag due to the extra PROM changes, so I'd be 
> grateful if you could give it a quick test.
> 

Sure.

> > > Testing shows that the checksum algorithm currently used for the 
> > > dp8393x device generates the same result as that generated by the 
> > > ARC firmware, which is known to be different than that used by the 
> > > Q800 machine.
> > > 
> > >  From this I conclude that the PROM is provided by the board and not 
> > > the chipset, and therefore each machine should construct its own 
> > > PROM accordingly. I'll send a v2 patchset shortly with these changes 
> > > which shall also include the proposed endian patch.
> > > 
> > 
> > If you potentially have both a ds1225y NVRAM and a dp8393x PROM (for 
> > the magnum machine) how do you avoid ending up with conflicting state? 
> > Would the two storage devices have to be mutually exclusive?
> 
> The ds1225y NVRAM is located between 0x80009000-0x8000afff and running 
> the nvram file through hexdump shows only the first 0x1000 bytes contain 
> any data, so any other changes made to NVRAM via the ARC firmware setup 
> will be preserved.
> 

Perhaps '-global ds1225y.size=4096' could be used to test that assumption 
about ARC firmware behaviour. Anyway, the default for ds1225y.size seems 
to be 0x2000. And a glance at the DS1225Y datasheets agrees with that 
figure. (I'm going to assume that DS1225Y is the actual part found in 
Magnum machines even though MOS6522, for instance, was not used in 
Quadras.)

> The existing default behaviour (without -global ds1225y.size=8200) is 
> that only the last few bytes at 0x8000b000 are mapped to the dp8393x 
> PROM, and this area is marked read-only to ensure that the MAC address 
> obtained by the guest OS always matches the one provided by the QEMU 
> configuration.
> 

Well, I asked about conflicting state having assumed that the NVRAM in a 
real Magnum was used to store the MAC address. But that's probably not the 
case. There's probably some other chip involved and your PROM device seems 
like a good way to model that. (Unfortunately I don't have access to a 
Magnum machine so you should take what I say about that machine with a 
grain of salt.)


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/5] dp8393x: fixes for MacOS toolbox ROM
  2021-06-25  9:32                 ` Finn Thain
@ 2021-06-25 11:49                   ` Mark Cave-Ayland
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Cave-Ayland @ 2021-06-25 11:49 UTC (permalink / raw)
  To: Finn Thain
  Cc: aleksandar.rikalo, jasowang, qemu-devel,
	Philippe Mathieu-Daudé,
	hpoussin, aurelien, Laurent Vivier

On 25/06/2021 10:32, Finn Thain wrote:

>>>> Thanks for the link and the detailed testing information. I've been
>>>> trying to understand why you had to set the MAC address in the ARC
>>>> firmware so I had a bit of an experiment here.
>>>>
>>>> The reason that you need to do this is because of the NVRAM
>>>> configuration in your command line, in particular -global
>>>> ds1225y.size=8200.
> 
> That configuration also shows up here,
> https://virtuallyfun.com/wordpress/2013/08/30/restoring-the-mips-magnum-in-qemu-1-6-0/
> with the explanation, "you'll need the NVRam stuff to add extra space for
> the ethernet MAC address". So it seems that the 8200 figure was just a
> hack and does not reflect the size of the NVRAM in an actual Magnum.

Ah that makes sense! QEMU 1.6.0 was released in 2013 and Hervé's patches to add the 
PROM MAC address support were added in 2015 so this information is outdated.

>>>> What this does is extend the NVRAM over the top of the dp8393x-prom
>>>> area where QEMU places the NIC MAC address and checksum on startup,
>>>> so the NVRAM captures the MAC address reads/writes instead. The net
>>>> effect of this is that the empty NVRAM initially reads all zeros and
>>>> why an initial setup is required to set the MAC address.
>>>>
>>>> This can be seen quite clearly in the "info mtree" output:
>>>>
>>>>       0000000080009000-000000008000b007 (prio 0, i/o): nvram
>>>>       000000008000b000-000000008000bfff (prio 0, rom): dp8393x-prom
>>>>
>>>> However if you completely drop -global ds1225y.size=8200 from your
>>>> command line then the NVRAM doesn't overrun into the dp8393x-prom
>>>> area, and the ARC firmware picks up the MAC address from QEMU
>>>> correctly:
>>>>
>>>>       0000000080009000-000000008000afff (prio 0, i/o): nvram
>>>>       000000008000b000-000000008000bfff (prio 0, rom): dp8393x-prom
>>>>
>>>> I've also looked over the entire SONIC datasheet to see if the PROM
>>>> format is documented, and according to that there is no non-volatile
>>>> storage available on the chip itself.
>>>
>>> Yes, that's my understanding also. The relevant National Semicondutor
>>> Application Notes seem to include a separate PROM. And if you closely
>>> examine the Linux macsonic.c driver, you'll see that the PowerBook 5x0
>>> models get a random MAC address because no-one (outside of Apple)
>>> knows where the real MAC address is stored.
>>
>> Agreed. This means that the revised patchset should now be doing the
>> right thing here.
>>
>> FWIW I felt that it had changed too much in its latest form to include
>> your original Tested-by tag due to the extra PROM changes, so I'd be
>> grateful if you could give it a quick test.
>>
> 
> Sure.
> 
>>>> Testing shows that the checksum algorithm currently used for the
>>>> dp8393x device generates the same result as that generated by the
>>>> ARC firmware, which is known to be different than that used by the
>>>> Q800 machine.
>>>>
>>>>   From this I conclude that the PROM is provided by the board and not
>>>> the chipset, and therefore each machine should construct its own
>>>> PROM accordingly. I'll send a v2 patchset shortly with these changes
>>>> which shall also include the proposed endian patch.
>>>>
>>>
>>> If you potentially have both a ds1225y NVRAM and a dp8393x PROM (for
>>> the magnum machine) how do you avoid ending up with conflicting state?
>>> Would the two storage devices have to be mutually exclusive?
>>
>> The ds1225y NVRAM is located between 0x80009000-0x8000afff and running
>> the nvram file through hexdump shows only the first 0x1000 bytes contain
>> any data, so any other changes made to NVRAM via the ARC firmware setup
>> will be preserved.
>>
> 
> Perhaps '-global ds1225y.size=4096' could be used to test that assumption
> about ARC firmware behaviour. Anyway, the default for ds1225y.size seems
> to be 0x2000. And a glance at the DS1225Y datasheets agrees with that
> figure. (I'm going to assume that DS1225Y is the actual part found in
> Magnum machines even though MOS6522, for instance, was not used in
> Quadras.)
> 
>> The existing default behaviour (without -global ds1225y.size=8200) is
>> that only the last few bytes at 0x8000b000 are mapped to the dp8393x
>> PROM, and this area is marked read-only to ensure that the MAC address
>> obtained by the guest OS always matches the one provided by the QEMU
>> configuration.
>>
> 
> Well, I asked about conflicting state having assumed that the NVRAM in a
> real Magnum was used to store the MAC address. But that's probably not the
> case. There's probably some other chip involved and your PROM device seems
> like a good way to model that. (Unfortunately I don't have access to a
> Magnum machine so you should take what I say about that machine with a
> grain of salt.)

Certainly on real Magnum hardware the area of memory containing the MAC address is 
backed by NVRAM and QEMU can access it, as validated by increasing the NVRAM size.

However the current default behaviour seems correct to me, since NIC MAC addresses 
are specified on the QEMU command line and so if you make this area NVRAM you can end 
up with a mismatch between what QEMU thinks the MAC address is internally vs. the 
value in the PROM...


ATB,

Mark.


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2021-06-25 11:50 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-13 16:37 [PATCH 0/5] dp8393x: fixes for MacOS toolbox ROM Mark Cave-Ayland
2021-06-13 16:37 ` [PATCH 1/5] dp8393x: checkpatch fixes Mark Cave-Ayland
2021-06-13 16:37 ` [PATCH 2/5] dp8393x: convert to trace-events Mark Cave-Ayland
2021-06-13 16:37 ` [PATCH 3/5] dp8393x: fix PROM checksum and MAC address storage Mark Cave-Ayland
2021-06-13 16:37 ` [PATCH 4/5] dp8393x: don't force 32-bit register access Mark Cave-Ayland
2021-06-13 16:37 ` [PATCH 5/5] dp8393x: fix CAM descriptor entry index Mark Cave-Ayland
2021-06-14  5:36 ` [PATCH 0/5] dp8393x: fixes for MacOS toolbox ROM Philippe Mathieu-Daudé
2021-06-14  7:51   ` Mark Cave-Ayland
2021-06-16  3:09     ` Finn Thain
2021-06-16  6:10       ` Mark Cave-Ayland
2021-06-18  6:22         ` Finn Thain
2021-06-24 14:42           ` Mark Cave-Ayland
2021-06-25  4:36             ` Finn Thain
2021-06-25  6:17               ` Mark Cave-Ayland
2021-06-25  9:32                 ` Finn Thain
2021-06-25 11:49                   ` Mark Cave-Ayland

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.