Re: [PATCH 04/24] aspeed: Don't create unwanted "ftgmac100", "aspeed-mmi" devices

From: Markus Armbruster <armbru@redhat.com>
To: Joel Stanley <joel@jms.id.au>
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	berrange@redhat.com, "Eduardo Habkost" <ehabkost@redhat.com>,
	"Andrew Jeffery" <andrew@aj.id.au>,
	"QEMU Developers" <qemu-devel@nongnu.org>,
	qemu-arm <qemu-arm@nongnu.org>, "Cédric Le Goater" <clg@kaod.org>,
	"Paolo Bonzini" <pbonzini@redhat.com>
Subject: Re: [PATCH 04/24] aspeed: Don't create unwanted "ftgmac100", "aspeed-mmi" devices
Date: Tue, 19 May 2020 07:35:42 +0200	[thread overview]
Message-ID: <87zha45vwx.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <CACPK8XcxVrA3E-c=jkszXd2Z6PMLO=tWDF60DTu-E3cyUdXD2A@mail.gmail.com> (Joel Stanley's message of "Tue, 19 May 2020 00:38:51 +0000")

Joel Stanley <joel@jms.id.au> writes:

> On Mon, 18 May 2020 at 12:19, Cédric Le Goater <clg@kaod.org> wrote:
>>
>> On 5/18/20 7:03 AM, Markus Armbruster wrote:
>> > These devices are optional, and controlled by @nb_nics.
>> > aspeed_soc_ast2600_init() and aspeed_soc_init() create the maximum
>> > supported number.  aspeed_soc_ast2600_realize() and
>> > aspeed_soc_realize() realize only the wanted number.  Works, although
>> > it can leave unrealized devices hanging around in the QOM composition
>> > tree.  Affects machines ast2500-evb, ast2600-evb, palmetto-bmc,
>> > romulus-bmc, swift-bmc, tacoma-bmc, and witherspoon-bmc.
>> >
>> > Make the init functions create only the wanted ones.  Visible in "info
>> > qom-tree"; here's the change for ast2600-evb:
>> >
>> >      /machine (ast2600-evb-machine)
>> >        [...]
>> >        /soc (ast2600-a1)
>> >          [...]
>> >          /ftgmac100[0] (ftgmac100)
>> >            /ftgmac100[0] (qemu:memory-region)
>> >     -    /ftgmac100[1] (ftgmac100)
>> >     -    /ftgmac100[2] (ftgmac100)
>> >     -    /ftgmac100[3] (ftgmac100)
>> >          /gpio (aspeed.gpio-ast2600)
>> >          [...]
>> >          /mii[0] (aspeed-mmi)
>> >            /aspeed-mmi[0] (qemu:memory-region)
>> >     -    /mii[1] (aspeed-mmi)
>> >     -    /mii[2] (aspeed-mmi)
>> >     -    /mii[3] (aspeed-mmi)
>> >          /rtc (aspeed.rtc)
>> >
>> > I'm not sure creating @nb_nics devices makes sense.  How many does the
>> > physical chip provide?
>>
>> The AST2400, AST2500 SoC have 2 macs and the AST2600 has 4. Each machine
>> define the one it uses, generally MAC0 but the tacoma board uses MAC3.
>>
>> Shouldn't the model reflect the real address space independently from
>> the NIC backends defined on the command line ?
>
> Agreed, the MAC hardware is present in all instances of the AST2600,
> so they should be present in qemu. Only some boards wire up a network
> device to the other side.

I guess an unwired NIC behaves as if no cable was plugged into the
external connector ("no carrier").

We can model that.

> It would be advantageous for us to be able to specify which device is
> being connected to on the command line. Currently we do this by
> connecting all devices up to the one we care about which is an ugly
> workaround.

We use -nic to configure onboard NICs.

The configuration gets deposited in nd_table[] for board code to pick
up.

Boards use nd_table[0] for their first NIC, nd_table[1] for the second,
and so forth.  How they order their NICs is part of their stable user
interface.

To leave a NIC unplugged, use -nice none.  Example: -nic none -nic user
leaves the first NIC unplugged, and plugs the second one using a user
network backend.

Say the board contains a SoC that provides four NICs, but the board
wires up only the last one.  Then board code should use nd_table[0] for
that last one.

I don't remember whether network device frontends can work without a
backend, or need a null backend.  If the latter, then board code needs
to supply such null backends.

>> How should we proceed in such cases ?

Model the physical hardware as faithfully as we can.

Follow-up patches welcome!