qemu-ppc
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] hw: ppc: sam460ex: Disable Ethernet devicetree nodes


From: BALATON Zoltan
Subject: Re: [PATCH] hw: ppc: sam460ex: Disable Ethernet devicetree nodes
Date: Tue, 17 Aug 2021 11:24:37 +0200 (CEST)

On Tue, 17 Aug 2021, David Gibson wrote:
On Mon, Aug 16, 2021 at 12:21:33PM +0200, BALATON Zoltan wrote:
On Mon, 16 Aug 2021, David Gibson wrote:
On Sun, Aug 15, 2021 at 07:59:15PM -0700, Guenter Roeck wrote:
IBM EMAC Ethernet controllers are not emulated by qemu. If they are
enabled in devicetree files, they are instantiated in Linux but
obviously won't work. Disable associated devicetree nodes to prevent
unpredictable behavior.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>

I'll wait for Zoltan's opinion on this, but this sort of thing is why
I was always pretty dubious about qemu *loading* a dtb file, rather
than generating a dt internally.

We are aiming to emulate the real SoC so we use the same dtb that belongs to
that SoC instead of generating something similar but not quite the same.

Well.. sure, but you don't *actually* emulate the real SoC, so you're
advertising a dtb that doesn't match the real hardware, which is a
bigger bug.

(QEMU also has a -dtb option but I'm not sure how many machines implement
it.) So loading a dtb is not bad in my opinion.

Well.... I'm not all that convinced that -dtb is a good idea either.
But to the extent that it is, I've assumed it's very much a "you must
know what you're doing" option (like -bios) where it's the user's
responsibility to make sure the dtb they're supplying matches the
emulated hardware.

Given that we don't fully
emulate every device in the SoC having devices described in the dtb that we
don't have might cause warnings or errors from OSes that try to accesss
these but that's all I've seen. I'm not sure what unpredictable behaviour
could result apart from some log messages about missing ethernet so this
should only be cosmetic to hide those errors. But other than that it likely
should not break anything so I'm OK with this patch. (I did not implement
ethernet ports becuase they are quite complex and we already have several
PCI ethernet devices that work already with guests so it's easier to use
those than spend time to implement another ethernet device.)

So, the thing I really dislike about this patch is that it's not
committing to either approach.  It's neither having a supplied dtb and
making it qemu's job to match that behaviour exactly, nor is qemu
supplying hardware and producing a dtb to describe that virtual
hardware.  It's doing a bit of both, which just seems like a recipe
for confusion to me.

We could also modify the pc-bios/canyonlands.dts to comment out the ethernet ports from it or add the disabled properties there, maybe also adding a comment that explains these are not emulated in QEMU but to me keeping the dts unmodified, matching real hardware and let the board code patch it according to what's emulated looks more obvious to clearly show what changes we have from the originial hardware which would be less clear if we loaded a modified dtb. Modifying the dtb simplifies the board code but hides the differences from real hardware. So since we already have to modify the loaded dtb anyway I'm OK with changing it at the same place as this patch proposes.

Regards,
BALATON Zoltan



reply via email to

[Prev in Thread] Current Thread [Next in Thread]