Re: [PATCH 0/5] dp8393x: fixes for MacOS toolbox ROM

[Mark Cave-Ayland]

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.