All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: BALATON Zoltan <balaton@eik.bme.hu>
Cc: qemu-ppc@nongnu.org, Greg Kurz <groug@kaod.org>,
	Guenter Roeck <linux@roeck-us.net>,
	qemu-devel@nongnu.org
Subject: Re: [PATCH] hw: ppc: sam460ex: Disable Ethernet devicetree nodes
Date: Tue, 17 Aug 2021 13:06:45 +1000	[thread overview]
Message-ID: <YRsnxY+0PplppxeU@yekko> (raw)
In-Reply-To: <68327a9-a317-9be8-92a6-ed23a91f4d7e@eik.bme.hu>

[-- Attachment #1: Type: text/plain, Size: 2698 bytes --]

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.

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2021-08-17  3:17 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-16  2:59 [PATCH] hw: ppc: sam460ex: Disable Ethernet devicetree nodes Guenter Roeck
2021-08-16  5:41 ` David Gibson
2021-08-16 10:11   ` Philippe Mathieu-Daudé
2021-08-16 15:13     ` Guenter Roeck
2021-08-16 10:21   ` BALATON Zoltan
2021-08-17  3:06     ` David Gibson [this message]
2021-08-17  9:24       ` BALATON Zoltan
2021-08-17  9:42         ` Peter Maydell
2021-08-16 10:26   ` Peter Maydell
2021-08-16 10:58     ` Philippe Mathieu-Daudé
2021-08-16 11:58       ` BALATON Zoltan
2021-08-17  3:09         ` David Gibson
2021-08-16 13:59     ` Guenter Roeck
2021-08-16 14:03       ` Peter Maydell
2021-08-16 14:11         ` Guenter Roeck
2021-08-16 14:19           ` Peter Maydell
2021-08-16 14:57             ` Guenter Roeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YRsnxY+0PplppxeU@yekko \
    --to=david@gibson.dropbear.id.au \
    --cc=balaton@eik.bme.hu \
    --cc=groug@kaod.org \
    --cc=linux@roeck-us.net \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.