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

[Philippe Mathieu-Daudé]

On 5/19/20 7:45 AM, Markus Armbruster wrote:
> "Andrew Jeffery" <address@hidden> writes:
>
> > On Mon, 18 May 2020, at 21:49, Cédric Le Goater 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 ?

If the SoC has N ftgmac100 peripherals, you need to mmio-map the N 
instances, else your guest will get MEMTX_DECODE_ERROR trying to access 
it, regardless command line NIC plugged.

> > That's my feeling too, though I'm not sure what to make of the unrealised 
> > devices
> > in the QOM tree. Does it matter? It hasn't bothered me.
>
> Depending on what the initialization code does, unrealized devices can
> be anything from a little wasted memory to open bear trap.  I don't
> really expect the latter extreme in the code, as I expect bear traps to
> quickly catch the developer that set them.
>
> I guess the unrealized devices cleaned up in this patch did no actual
> harm.
>
> Still, it's an unhealthy state, and that's why I clean it up.  "[PATCH
> 24/24] qdev: Assert onboard devices all get realized properly" should
> ensure we stay clean.