All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
To: Finn Thain <fthain@fastmail.com.au>
Cc: aleksandar.rikalo@syrmia.com, jasowang@redhat.com,
	qemu-devel@nongnu.org, "Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	hpoussin@reactos.org, aurelien@aurel32.net,
	"Laurent Vivier" <laurent@vivier.eu>
Subject: Re: [PATCH 0/5] dp8393x: fixes for MacOS toolbox ROM
Date: Fri, 25 Jun 2021 12:49:21 +0100	[thread overview]
Message-ID: <84a33676-fa68-9d21-7cf6-984b415d79a3@ilande.co.uk> (raw)
In-Reply-To: <4bd8b183-6cb-ec29-e8e1-acbe74d2dd@nippy.intranet>

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.


      reply	other threads:[~2021-06-25 11:50 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=84a33676-fa68-9d21-7cf6-984b415d79a3@ilande.co.uk \
    --to=mark.cave-ayland@ilande.co.uk \
    --cc=aleksandar.rikalo@syrmia.com \
    --cc=aurelien@aurel32.net \
    --cc=f4bug@amsat.org \
    --cc=fthain@fastmail.com.au \
    --cc=hpoussin@reactos.org \
    --cc=jasowang@redhat.com \
    --cc=laurent@vivier.eu \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.