Re: [PATCH v3] arm/aspeed: Rework NIC attachment

["Cédric Le Goater" <clg@kaod.org>]

On 5/27/20 3:36 PM, Markus Armbruster wrote:
> Cédric Le Goater <clg@kaod.org> writes:
> 
>> The number of MACs supported by an Aspeed SoC is defined by "macs_num"
>> under the SoC model, that is two for the AST2400 and AST2500 and four
>> for the AST2600. The model initializes the maximum number of supported
>> MACs but the number of realized devices is capped by the number of
>> network device back-ends defined on the command line. This can leave
>> unrealized devices hanging around in the QOM composition tree.
>>
>> Modify the machine initialization to define which MACs are attached to
>> a network device back-end using a bit-field property "macs-mask" and
>> let the SoC realize all network devices.
>>
>> The default setting of "macs-mask" is "use MAC0" only, which works for
>> all our AST2400 and AST2500 machines. The AST2600 machines have
>> different configurations. The AST2600 EVB machine activates MAC1, MAC2
>> and MAC3 and the Tacoma BMC machine activates MAC2.
> 
> Let's be more clear on what this means, and "This is actually a device
> modelling fix for these two machines."  Okay?

Well, I guess so. It's a fix in the way we attach network backends to 
the MACs of the machines. 

On the tacoma-bmc, we had to use '-nic <foo> -nic <bar> -nic <good one>' 
to configure the MAC2 in use by the machine. Which was dubious.

Now, a single -nic is enough.

>> Inactive MACs will have no peer and QEMU may warn the user with :
>>
>>     qemu-system-arm: warning: nic ftgmac100.0 has no peer
>>     qemu-system-arm: warning: nic ftgmac100.1 has no peer
>>     qemu-system-arm: warning: nic ftgmac100.3 has no peer
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> 
> Here's the "info qom-tree" change for tacoma-bmc:
> 
>      /machine (tacoma-bmc-machine)
>        /peripheral (container)
>        /peripheral-anon (container)
>        /soc (ast2600-a1)
>          [...]
>          /ftgmac100[0] (ftgmac100)
>            /ftgmac100[0] (qemu:memory-region)
>          /ftgmac100[1] (ftgmac100)
>     +      /ftgmac100[0] (qemu:memory-region)
>          /ftgmac100[2] (ftgmac100)
>     +      /ftgmac100[0] (qemu:memory-region)
>          /ftgmac100[3] (ftgmac100)
>     +      /ftgmac100[0] (qemu:memory-region)

Yes. All are realized now.

>          [...]
>          /mii[0] (aspeed-mmi)
>            /aspeed-mmi[0] (qemu:memory-region)
>          /mii[1] (aspeed-mmi)
>     +      /aspeed-mmi[0] (qemu:memory-region)
>          /mii[2] (aspeed-mmi)
>     +      /aspeed-mmi[0] (qemu:memory-region)
>          /mii[3] (aspeed-mmi)
>     +      /aspeed-mmi[0] (qemu:memory-region)

Same for the MMI interfaces on AST2600.

> These changes are due to realizing MAC1, MAC2, MAC3.  Looks good.
> 
> Here's "info qtree":
> 
>        dev: ftgmac100, id ""
>          gpio-out "sysbus-irq" 1
>          aspeed = true
>     -    mac = "52:54:00:12:34:56"
>     -    netdev = "hub0port0"
>     +    mac = "52:54:00:12:34:57"
>     +    netdev = ""
>          mmio 000000001e660000/0000000000002000
>        dev: ftgmac100, id ""
>     -    aspeed = false
>     -    mac = "00:00:00:00:00:00"
>     +    gpio-out "sysbus-irq" 1
>     +    aspeed = true
>     +    mac = "52:54:00:12:34:58"
>          netdev = ""
>     +    mmio 000000001e680000/0000000000002000
>        dev: ftgmac100, id ""
>     -    aspeed = false
>     -    mac = "00:00:00:00:00:00"
>     -    netdev = ""
>     +    gpio-out "sysbus-irq" 1
>     +    aspeed = true
>     +    mac = "52:54:00:12:34:56"
>     +    netdev = "hub0port0"
>     +    mmio 000000001e670000/0000000000002000
>        dev: ftgmac100, id ""
>     -    aspeed = false
>     -    mac = "00:00:00:00:00:00"
>     +    gpio-out "sysbus-irq" 1
>     +    aspeed = true
>     +    mac = "52:54:00:12:34:59"
>          netdev = ""
>     +    mmio 000000001e690000/0000000000002000
>        [...]
>        dev: aspeed-mmi, id ""
>          mmio 000000001e650000/0000000000000008
>        dev: aspeed-mmi, id ""
>     +    mmio 000000001e650008/0000000000000008
>        dev: aspeed-mmi, id ""
>     +    mmio 000000001e650010/0000000000000008
>        dev: aspeed-mmi, id ""
>     +    mmio 000000001e650018/0000000000000008
> 
> Here we can see the network backend now gets connected to MAC2 instead
> of MAC0.

yes.

With only one nic on the command line, the backend was attached to the 
first (unused) MAC0 of the machine and now it is attached to the first 
active MAC2 of the machine.

> 
> This is without any networking-related options, i.e. we get just the
> single default network backend.
> 
>> ---
>>
>>  To be applied on top of patch :
>>
>>  "arm/aspeed: Compute the number of CPUs from the SoC definition" 
>>  http://patchwork.ozlabs.org/project/qemu-devel/patch/20200519091631.1006073-1-clg@kaod.org/
>>
>>  Markus, do you mind taking this patch in your QOM series also ?
> 
> On the contrary!
> 
> I'll work my "info qom-tree" and "info qtree" diffs into the commit
> message, if you don't mind.

Sure. 

Thanks,

C.