Re: [PATCH 0/5] dp8393x: fixes for MacOS toolbox ROM - Philippe Mathieu-Daudé

From: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
To: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>,
	qemu-devel@nongnu.org, hpoussin@reactos.org,
	aleksandar.rikalo@syrmia.com, aurelien@aurel32.net,
	jiaxun.yang@flygoat.com, jasowang@redhat.com,
	Finn Thain <fthain@telegraphics.com.au>,
	Laurent Vivier <laurent@vivier.eu>
Subject: Re: [PATCH 0/5] dp8393x: fixes for MacOS toolbox ROM
Date: Mon, 14 Jun 2021 07:36:24 +0200	[thread overview]
Message-ID: <20a706c7-9b44-13cc-b294-1ee0f3cff6bb@amsat.org> (raw)
In-Reply-To: <20210613163738.2141-1-mark.cave-ayland@ilande.co.uk>

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(-)
> 