Re: [PATCH] hw/net: npcm7xx_emc: set MAC in register space

[Peter Maydell <peter.maydell@linaro.org>]

On Thu, 29 Sept 2022 at 16:28, Patrick Venture <venture@google.com> wrote:
> On Mon, Sep 26, 2022 at 8:45 AM Patrick Venture <venture@google.com> wrote:
>>> I think what Jason is suggesting is that that should depend on what
>>> the real hardware does. On a physical board, if the guest sets the
>>> MAC address, and then you power-cycle the hardware, does the MAC
>>> that it set still persist after powercycle ? Does the guest writing
>>> to these MAC registers correspond to writing to an EEPROM ?
>>
>>
>> Thanks, Peter - we've reached out to the vendor off-list to seek out some details, I'll update this with a v2 when I get an answer.

> "No, The EMC driver reset the MAC address registers during boot cycle/reset."

OK, I guess that's clear enough. In a real full-software-stack
setup is the MAC address setup usually done by a BIOS/firmware,
or is it done by Linux ?

> So in that case, we should disregard the value the user sets in
> reset and use the value provided through Qemu.  Or, should we just
> not allow Qemu to set the MAC address at all?

I think that the behaviour for QEMU's model should be that
on reset we should reset the MAC address registers to the
user-provided value. That is, if the guest writes to the
MAC address registers then that does have an effect, but
only until the next reset.

That gives you reasonably plausible behaviour for both:
(1) you're emulating some guest that always sets up its
own MAC address when it starts up (eg if it's done by
some BIOS-level code that you're running in the guest)
(2) you're booting a guest kernel directly that expects
that the firmware/BIOS/whatever has already set up
a MAC address -- then the MAC address provided by QEMU/the
user fills that role

More concretely:
 * on reset, set the emc->regs[] fields from emc->conf.macaddr
 * when using the MAC address, always use emc->regs[], never
   emc->conf.macaddr
 * to handle the guest writes to the MAC registers, set
   emc->regs[], but not emc->conf.macaddr

Assuming that doesn't break your existing booting workloads,
of course :-)

thanks
-- PMM