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

[BALATON Zoltan]

On Mon, 16 Aug 2021, Philippe Mathieu-Daudé wrote:
> On 8/16/21 12:26 PM, Peter Maydell wrote:
> > On Mon, 16 Aug 2021 at 06:46, David Gibson <david@gibson.dropbear.id.au> 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.
> >
> > My take is that if you have to modify the dtb file to get QEMU to
> > work, then that's a bug in QEMU that should be fixed. It's no
> > worse than for the machines we have that don't use dtb and where
> > the kernel just knows what the hardware is. (In my experience
> > Arm kernels can be a bit finicky about wanting the right dtb
> > and not some earlier or later one, so I think at least for Arm
> > trying to generate a dt for our non-virt boards would have been
> > pretty painful and liable to "new kernels don't boot any more" bugs.)
> >
> > Is it sufficient to create stub "unimplemented-device" ethernet
> > controllers here, or does the guest want more behaviour than that?
>
> For raspi4 "unimplemented-device" is not enough, so what would
> be the ideal resolution for "the guest wants more behaviour"
> when we don't have time / interest / specs for the missing
> pieces?

I guess ideal solution is to implement the missing device emulation, if we 
don't have resources for that effort then that's less than ideal but in 
that case patching the dtb to disable the device does not look too bad to 
me. I think that's an acceptable way to save the effort of implementing 
the device that's not srtictly needed. For sam460ex I think Linux has 
booted so far but displays an error about missing ethernet ports but this 
did not prevent it from booting. So unless recent kernels fail now, this 
patch is only suppresses those errors (and avoid going to code paths in 
guest kernel that probably never run on real hadware). I think there are 
arguments for and against it (the errors look ugly so we could prevent it 
but on the other hand then we have something different than on real 
hardware however missing the device means it's really different) so I'm 
not quite decided about either approach but I tend to try to keep it as 
similar to real hardware as possible (so using unmodified dtb) as long as 
it does not cause real problems. But if patching the dtb makes it nicer by 
avoiding a big and maybe confusing error message on boot and it does not 
break anything else then that's OK too.

Regards,
BALATON Zoltan