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

[BALATON Zoltan]

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. (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. 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.)

Regards,
BALATON Zoltan

> > ---
> >  hw/ppc/sam460ex.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
> > index 0737234d66..feb356e625 100644
> > --- a/hw/ppc/sam460ex.c
> > +++ b/hw/ppc/sam460ex.c
> > @@ -194,6 +194,14 @@ static int sam460ex_load_device_tree(hwaddr addr,
> >          _FDT(fdt_nop_node(fdt, offset));
> >      }
> >
> > +    /* Ethernet interfaces are not emulated */
> > +    offset = fdt_node_offset_by_compatible(fdt, -1, "ibm,emac-460ex");
> > +    while (offset >= 0) {
> > +        _FDT(fdt_setprop_string(fdt, offset, "status", "disabled"));
> > +        offset = fdt_node_offset_by_compatible(fdt, offset, "ibm,emac-460ex");
> > +    }
> > +
> > +
> >      /* set serial port clocks */
> >      offset = fdt_node_offset_by_compatible(fdt, -1, "ns16550");
> >      while (offset >= 0) {