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 > > > > > > [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 > Date: Wed Jun 3 22:45:46 2015 +0200 > > net/dp8393x: add PROM to store MAC address > > Signed-off-by: Laurent Vivier > Signed-off-by: Hervé Poussineau > Reviewed-by: Aurelien Jarno > Signed-off-by: Leon Alrae > 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.