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

[David Gibson]

On Mon, Aug 16, 2021 at 01:58:08PM +0200, BALATON Zoltan wrote:
> 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.

I'm sympathetic to that, but in that case I think you shold drop the
pretense of using this external dtb and matching it.  Either generate
the dtb in qemu, or snapshot the dtb, modify it to be the
qemu-emulated version of the hardware and load that file explicitly.
The second being basically just an easy way of generating a dtb when
it's near-static.


> 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


-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature