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

2020-05-19 Thread Cédric Le Goater
On 5/19/20 1:42 PM, Philippe Mathieu-Daudé wrote:
> On 5/19/20 7:45 AM, Markus Armbruster wrote:
>> "Andrew Jeffery"  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.
 
yes. This is what I do with the patch attached below but I have another 
problem.

Get a witherspoon-tacoma flash image :


https://openpower.xyz/job/openbmc-build/distro=ubuntu,label=builder,target=witherspoon-tacoma/lastSuccessfulBuild/artifact/deploy/images/witherspoon-tacoma/flash-witherspoon-tacoma

Run :

qemu-system-arm -M tacoma-bmc -nic user -drive 
file=./flash-witherspoon-tacoma,format=raw,if=mtd -nographic -nodefaults 
-serial mon:stdio
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


U-Boot 2019.04 (May 06 2020 - 04:20:01 +)

SOC: AST2600-A1 
LPC Mode: SIO:Enable : SuperIO-2e
Eth: MAC0: RMII/NCSI, MAC1: RMII/NCSI, MAC2: RMII/NCSI, MAC3: RMII/NCSI
Model: Tacoma
DRAM:  already initialized, 1008 MiB
...

How do I deal with the "no peer" warnings ? 

Thanks,

C.


>From a3c2772eca8a541158345e6f219ce524f1bc017b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= 
Date: Tue, 19 May 2020 14:39:55 +0200
Subject: [PATCH] arm/aspeed: Rework NIC attachment
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Cédric Le Goater 
---
 include/hw/arm/aspeed.h |  1 +
 include/hw/arm/aspeed_soc.h |  1 +
 hw/arm/aspeed.c |  5 +
 hw/arm/aspeed_ast2600.c |  9 +++--
 hw/arm/aspeed_soc.c | 10 --
 5 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h
index 18521484b90e..7e71152b3554 100644
--- a/include/hw/arm/aspeed.h
+++ b/include/hw/arm/aspeed.h
@@ -39,6 +39,7 @@ typedef struct AspeedMachineClass {
 const char *fmc_model;
 const char *spi_model;
 uint32_t num_cs;
+uint32_t nic_mask;
 void (*i2c_init)(AspeedBoardState *bmc);
 } AspeedMachineClass;
 
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 914115f3ef77..32e9a232a049 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -55,6 +55,7 @@ typedef struct AspeedSoCState {
 AspeedSDMCState sdmc;
 AspeedWDTState wdt[ASPEED_WDTS_NUM];
 FTGMAC100State ftgmac100[ASPEED_MACS_NUM];
+uint32_t nic_mask;
 AspeedMiiState mii[ASPEED_MACS_NUM];
 AspeedGPIOState gpio;
 AspeedGPIOState gpio_1_8v;
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 6f8f4b88f8ab..338b5db20cf9 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -283,6 +283,8 @@ static void aspeed_machine_init(MachineState *machine)
 &error_abort);
 object_property_set_int(OBJECT(&bmc->soc), amc->num_cs, "num-cs",
 &error_abort);
+object_property_set_int(OBJECT(&bmc->soc), amc->nic_mask, "nic-mask",
+&error_abort);
 object

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

2020-05-19 Thread Philippe Mathieu-Daudé

On 5/19/20 7:45 AM, Markus Armbruster wrote:

"Andrew Jeffery"  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.







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

2020-05-18 Thread Markus Armbruster
"Andrew Jeffery"  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 ?  
>
> 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.




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

2020-05-18 Thread Markus Armbruster
Joel Stanley  writes:

> On Mon, 18 May 2020 at 12:19, 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 ?
>
> 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!




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

2020-05-18 Thread Joel Stanley
On Mon, 18 May 2020 at 12:19, 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 ?

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.

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.

> How should we proceed in such cases ?
>
> Thanks,
>
> C.
>
> >
> > Cc: "Cédric Le Goater" 
> > Cc: Peter Maydell 
> > Cc: Andrew Jeffery 
> > Cc: Joel Stanley 
> > Cc: qemu-...@nongnu.org
> > Signed-off-by: Markus Armbruster 
> > ---
> >  hw/arm/aspeed_ast2600.c | 2 +-
> >  hw/arm/aspeed_soc.c | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> > index 71a0acfe26..0a6a77dd54 100644
> > --- a/hw/arm/aspeed_ast2600.c
> > +++ b/hw/arm/aspeed_ast2600.c
> > @@ -188,7 +188,7 @@ static void aspeed_soc_ast2600_init(Object *obj)
> >sizeof(s->wdt[i]), typename);
> >  }
> >
> > -for (i = 0; i < sc->macs_num; i++) {
> > +for (i = 0; i < nb_nics && i < sc->macs_num; i++) {
> >  sysbus_init_child_obj(obj, "ftgmac100[*]", 
> > OBJECT(&s->ftgmac100[i]),
> >sizeof(s->ftgmac100[i]), TYPE_FTGMAC100);
> >
> > diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> > index cf6b6dd116..7ca860392a 100644
> > --- a/hw/arm/aspeed_soc.c
> > +++ b/hw/arm/aspeed_soc.c
> > @@ -203,7 +203,7 @@ static void aspeed_soc_init(Object *obj)
> >sizeof(s->wdt[i]), typename);
> >  }
> >
> > -for (i = 0; i < sc->macs_num; i++) {
> > +for (i = 0; i < nb_nics && i < sc->macs_num; i++) {
> >  sysbus_init_child_obj(obj, "ftgmac100[*]", 
> > OBJECT(&s->ftgmac100[i]),
> >sizeof(s->ftgmac100[i]), TYPE_FTGMAC100);
> >  }
> >
>



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

2020-05-18 Thread Andrew Jeffery



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 ?  

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.

Andrew



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

2020-05-18 Thread Cédric Le Goater
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 ?  

How should we proceed in such cases ? 

Thanks,

C. 

> 
> Cc: "Cédric Le Goater" 
> Cc: Peter Maydell 
> Cc: Andrew Jeffery 
> Cc: Joel Stanley 
> Cc: qemu-...@nongnu.org
> Signed-off-by: Markus Armbruster 
> ---
>  hw/arm/aspeed_ast2600.c | 2 +-
>  hw/arm/aspeed_soc.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> index 71a0acfe26..0a6a77dd54 100644
> --- a/hw/arm/aspeed_ast2600.c
> +++ b/hw/arm/aspeed_ast2600.c
> @@ -188,7 +188,7 @@ static void aspeed_soc_ast2600_init(Object *obj)
>sizeof(s->wdt[i]), typename);
>  }
>  
> -for (i = 0; i < sc->macs_num; i++) {
> +for (i = 0; i < nb_nics && i < sc->macs_num; i++) {
>  sysbus_init_child_obj(obj, "ftgmac100[*]", OBJECT(&s->ftgmac100[i]),
>sizeof(s->ftgmac100[i]), TYPE_FTGMAC100);
>  
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index cf6b6dd116..7ca860392a 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -203,7 +203,7 @@ static void aspeed_soc_init(Object *obj)
>sizeof(s->wdt[i]), typename);
>  }
>  
> -for (i = 0; i < sc->macs_num; i++) {
> +for (i = 0; i < nb_nics && i < sc->macs_num; i++) {
>  sysbus_init_child_obj(obj, "ftgmac100[*]", OBJECT(&s->ftgmac100[i]),
>sizeof(s->ftgmac100[i]), TYPE_FTGMAC100);
>  }
> 




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

2020-05-17 Thread Markus Armbruster
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?

Cc: "Cédric Le Goater" 
Cc: Peter Maydell 
Cc: Andrew Jeffery 
Cc: Joel Stanley 
Cc: qemu-...@nongnu.org
Signed-off-by: Markus Armbruster 
---
 hw/arm/aspeed_ast2600.c | 2 +-
 hw/arm/aspeed_soc.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 71a0acfe26..0a6a77dd54 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -188,7 +188,7 @@ static void aspeed_soc_ast2600_init(Object *obj)
   sizeof(s->wdt[i]), typename);
 }
 
-for (i = 0; i < sc->macs_num; i++) {
+for (i = 0; i < nb_nics && i < sc->macs_num; i++) {
 sysbus_init_child_obj(obj, "ftgmac100[*]", OBJECT(&s->ftgmac100[i]),
   sizeof(s->ftgmac100[i]), TYPE_FTGMAC100);
 
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index cf6b6dd116..7ca860392a 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -203,7 +203,7 @@ static void aspeed_soc_init(Object *obj)
   sizeof(s->wdt[i]), typename);
 }
 
-for (i = 0; i < sc->macs_num; i++) {
+for (i = 0; i < nb_nics && i < sc->macs_num; i++) {
 sysbus_init_child_obj(obj, "ftgmac100[*]", OBJECT(&s->ftgmac100[i]),
   sizeof(s->ftgmac100[i]), TYPE_FTGMAC100);
 }
-- 
2.21.1