Re: [Qemu-devel] aspeed qemu question

[Wim Vervoorn <wvervoorn@eltan.com>]

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

Hello Cedríc,

Thanks for your response. I created and attached the patch. You are right the code snipped turns out unreadable.

In the patch I enable the MAC's depending on the value in hwstrap1 just as in the real hardware. In the Palmetto both nics will be enabled (just as in the real palmetto). 

I added a 2nd AST2400 BMC the Eltan Piestewa Peak. Here I enabled the 2nd NIC only.

What I see is that when I use Palmetto I get an assert in net/net.c line 256, this is in the "qemu_net_client_setup()". I assume I have to prepare something regarding the host side of the network implementation but I this point I have no clue what and I have a hard time figuring out how the networking architecture really works. 

If you could point me in the right direction I would really appreciate it.

Best Regards,
Wim Vervoorn

Eltan B.V.
Ambachtstraat 23
5481 SM Schijndel
The Netherlands

T : +31-(0)73-594 46 64
E : wvervoorn@eltan.com
W : http://www.eltan.com


-----Original Message-----
From: Cédric Le Goater [mailto:clg@kaod.org] 
Sent: Thursday, May 16, 2019 5:06 PM
To: Wim Vervoorn <wvervoorn@eltan.com>
Subject: Re: aspeed qemu question

Hello Wim,

On 5/16/19 10:46 AM, Wim Vervoorn wrote:
> Hello Cédric,
> 
>  
> 
> I noticed you are responsible for most of the Aspeed additions to Qemu perhaps you can help me.

Sure,

>  
> 
> I am trying to add support for the 2^nd NIC on the Aspeed SoC.

ok. This should be possible. 

> This seemed not that hard. Basically I added a 2^nd structure for the device and initialized that. I included the code below.

hmm. This is difficult for me to read. Do you know how to send patches ?  

It would be good to send a patch to the QEMU devel and the QEMU ARM mailing list so that we can discuss openly.

I can help you doing that. 

 
Cheers,

C.

> The code is building fine no problem about that.
> 
>  
> 
> The issue is that I get an assert net.c line 256, this only happens if I realize both NICs if I realize only one all works as expected. My question is, am I overlooking something here? Do I need to take more actions to be able to support the 2^nd NIC? If you could point me in the right direction or know where I can find more information on the qemu architecture I would really appreciate it.
> 
>  
> 
>  
> 
>  
> 
> staticvoidqemu_net_client_setup(NetClientState *nc,
> 
>                                   NetClientInfo *info,
> 
>                                   NetClientState *peer,
> 
>                                   constchar*model,
> 
>                                   constchar*name,
> 
>                                   NetClientDestructor *destructor)
> 
> {
> 
>     nc->info= info;
> 
>     nc->model= g_strdup(model);
> 
>     if(name) {
> 
>         nc->name= g_strdup(name);
> 
>     } else{
> 
>         nc->name= assign_name(nc, model);
> 
>     }
> 
>  
> 
>     if(peer) {
> 
> à>     assert(!peer->peer);
> 
>         nc->peer= peer;
> 
>         peer->peer= nc;
> 
>     }
> 
>  
> 
>  
> 
>     object_initialize(&s->ftgmac100[0], sizeof(s->ftgmac100), 
> TYPE_FTGMAC100);
> 
>     object_property_add_child(obj, "ftgmac100[0]", 
> OBJECT(&s->ftgmac100[0]), NULL);
> 
>     qdev_set_parent_bus(DEVICE(&s->ftgmac100[0]), 
> sysbus_get_default());
> 
>  
> 
>     object_initialize(&s->ftgmac100[1], sizeof(s->ftgmac100), 
> TYPE_FTGMAC100);
> 
>     object_property_add_child(obj, "ftgmac100[1]", 
> OBJECT(&s->ftgmac100[1]), NULL);
> 
>     qdev_set_parent_bus(DEVICE(&s->ftgmac100[1]), 
> sysbus_get_default());
> 
>  
> 
>  
> 
>  
> 
> /* Net LAN 0*/
> 
>     #if(0)
> 
>     qemu_log_mask(LOG_GUEST_ERROR, "%s: LAN0\n", __func__);
> 
>     qdev_set_nic_properties(DEVICE(&s->ftgmac100[0]), &nd_table[0]);
> 
>     object_property_set_bool(OBJECT(&s->ftgmac100[0]), true, "aspeed", 
> &err);
> 
>     object_property_set_bool(OBJECT(&s->ftgmac100[0]), true, 
> "realized",
> 
>                             &local_err);
> 
>     error_propagate(&err, local_err);
> 
>     if (err) {
> 
>         error_propagate(errp, err);
> 
>         return;
> 
>     }
> 
>     sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100[0]), 0, 
> ASPEED_SOC_ETH1_BASE);
> 
>     sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100[0]), 0,
> 
>                        qdev_get_gpio_in(DEVICE(&s->vic), 2));
> 
>     #endif
> 
>  
> 
>     /* Net LAN 1*/
> 
>     #if(1)
> 
>     qemu_log_mask(LOG_GUEST_ERROR, "%s: LAN1\n", __func__);
> 
>     qdev_set_nic_properties(DEVICE(&s->ftgmac100[1]), &nd_table[1]);
> 
>     object_property_set_bool(OBJECT(&s->ftgmac100[1]), true, "aspeed", 
> &err);
> 
>     object_property_set_bool(OBJECT(&s->ftgmac100[1]), true, 
> "realized",
> 
>                              &local_err);
> 
>     error_propagate(&err, local_err);
> 
>     if(err) {
> 
>         error_propagate(errp, err);
> 
>         return;
> 
>     }
> 
>     sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100[1]), 0, 
> ASPEED_SOC_ETH2_BASE);
> 
>     sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100[1]), 0,
> 
>                        qdev_get_gpio_in(DEVICE(&s->vic), 3));
> 
>     #endif
> 
>  
> 
>  
> 
> Best Regards,
> 
> Wim Vervoorn
> 
>  
> 
> Eltan B.V.
> 
> Ambachtstraat 23
> 
> 5481 SM Schijndel
> 
> The Netherlands
> 
>  
> 
> T : +31-(0)73-594 46 64
> 
> E : wvervoorn@eltan.com
> 
> W : http://www.eltan.com <http://www.eltan.com/>
> 
>  
> 
>  
> 
> "THIS MESSAGE CONTAINS CONFIDENTIAL INFORMATION. UNLESS YOU ARE THE 
> INTENDED RECIPIENT OF THIS MESSAGE, ANY USE OF THIS MESSAGE IS STRICTLY PROHIBITED. IF YOU HAVE RECEIVED THIS MESSAGE IN ERROR, PLEASE IMMEDIATELY NOTIFY THE SENDER BY TELEPHONE +31-(0)73-5944664 OR REPLY EMAIL, AND IMMEDIATELY DELETE THIS MESSAGE AND ALL COPIES."
> 
>  
> 




[-- Attachment #2: 0002-ASPEED-BMC-Add-support-for-2nd-NIC.patch --]
[-- Type: application/octet-stream, Size: 7278 bytes --]

From bbf9392b84f38531b89e4ea39e06210b99ec7a0c Mon Sep 17 00:00:00 2001
Message-Id: <bbf9392b84f38531b89e4ea39e06210b99ec7a0c.1558078463.git.wvervoorn@eltan.com>
From: Wim Vervoorn <wvervoorn@eltan.com>
Date: Fri, 17 May 2019 09:26:16 +0200
Subject: [PATCH] ASPEED BMC: Add support for 2nd NIC

Add support for 2nd NIC.

This solution is using the hwstrap1 value to enable disable the
controllers.

The Palmetto leaves both NICs enabled while the Piestewa Peak only
enables one.

The Palmetto asserts in net/net.c line 256 when qemu starts so something
must be missing to support the 2nd nic.
---
 hw/arm/aspeed.c             | 26 ++++++++++++++++++++++
 hw/arm/aspeed_soc.c         | 54 ++++++++++++++++++++++++++++++++-------------
 include/hw/arm/aspeed_soc.h |  2 +-
 3 files changed, 66 insertions(+), 16 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
old mode 100644
new mode 100755
index 0203642..f957a5b
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -43,6 +43,23 @@ struct AspeedBoardState {
         SCU_AST2400_HW_STRAP_SET_CLK_SOURCE(AST2400_CLK_48M_IN) |       \
         SCU_HW_STRAP_VGA_CLASS_CODE |                                   \
         SCU_HW_STRAP_LPC_RESET_PIN |                                    \
+        SCU_HW_STRAP_MAC0_RGMII |                                       \
+        SCU_HW_STRAP_MAC1_RGMII |                                       \
+        SCU_HW_STRAP_SPI_MODE(SCU_HW_STRAP_SPI_M_S_EN) |                \
+        SCU_AST2400_HW_STRAP_SET_CPU_AHB_RATIO(AST2400_CPU_AHB_RATIO_2_1) | \
+        SCU_HW_STRAP_SPI_WIDTH |                                        \
+        SCU_HW_STRAP_VGA_SIZE_SET(VGA_16M_DRAM) |                       \
+        SCU_AST2400_HW_STRAP_BOOT_MODE(AST2400_SPI_BOOT))
+
+/* Piestewa Peak hardware value:  */
+#define ELTANPWP_BMC_HW_STRAP1 (                                        \
+        SCU_AST2400_HW_STRAP_DRAM_SIZE(DRAM_SIZE_256MB) |               \
+        SCU_AST2400_HW_STRAP_DRAM_CONFIG(2 /* DDR3 with CL=6, CWL=5 */) | \
+        SCU_AST2400_HW_STRAP_ACPI_DIS |                                 \
+        SCU_AST2400_HW_STRAP_SET_CLK_SOURCE(AST2400_CLK_48M_IN) |       \
+        SCU_HW_STRAP_VGA_CLASS_CODE |                                   \
+        SCU_HW_STRAP_LPC_RESET_PIN |                                    \
+        SCU_HW_STRAP_MAC1_RGMII |                                       \
         SCU_HW_STRAP_SPI_MODE(SCU_HW_STRAP_SPI_M_S_EN) |                \
         SCU_AST2400_HW_STRAP_SET_CPU_AHB_RATIO(AST2400_CPU_AHB_RATIO_2_1) | \
         SCU_HW_STRAP_SPI_WIDTH |                                        \
@@ -423,6 +440,15 @@ static const AspeedBoardConfig aspeed_boards[] = {
         .num_cs    = 1,
         .i2c_init  = palmetto_bmc_i2c_init,
     }, {
+        .name      = MACHINE_TYPE_NAME("eltanpwp-bmc"),
+        .desc      = "Eltan Piestewa Peak BMC (ARM926EJ-S)",
+        .soc_name  = "ast2400-a1",
+        .hw_strap1 = ELTANPWP_BMC_HW_STRAP1,
+        .fmc_model = "n25q256a",
+        .spi_model = "mx25l25635e",
+        .num_cs    = 1,
+        .i2c_init  = palmetto_bmc_i2c_init,
+    }, {
         .name      = MACHINE_TYPE_NAME("ast2500-evb"),
         .desc      = "Aspeed AST2500 EVB (ARM1176)",
         .soc_name  = "ast2500-a1",
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
old mode 100644
new mode 100755
index 0f6e5be..8ed7902
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -184,9 +184,13 @@ static void aspeed_soc_init(Object *obj)
                                        OBJECT(&s->scu), &error_abort);
     }
 
-    object_initialize(&s->ftgmac100, sizeof(s->ftgmac100), TYPE_FTGMAC100);
-    object_property_add_child(obj, "ftgmac100", OBJECT(&s->ftgmac100), NULL);
-    qdev_set_parent_bus(DEVICE(&s->ftgmac100), sysbus_get_default());
+    object_initialize(&s->ftgmac100[0], sizeof(s->ftgmac100), TYPE_FTGMAC100);
+    object_property_add_child(obj, "ftgmac100[0]", OBJECT(&s->ftgmac100[0]), NULL);
+    qdev_set_parent_bus(DEVICE(&s->ftgmac100[0]), sysbus_get_default());
+
+    object_initialize(&s->ftgmac100[1], sizeof(s->ftgmac100), TYPE_FTGMAC100);
+    object_property_add_child(obj, "ftgmac100[1]", OBJECT(&s->ftgmac100[1]), NULL);
+    qdev_set_parent_bus(DEVICE(&s->ftgmac100[1]), sysbus_get_default());
 
     object_initialize(&s->ibt, sizeof(s->ibt), TYPE_ASPEED_IBT);
     object_property_add_child(obj, "bt", OBJECT(&s->ibt), NULL);
@@ -384,19 +388,39 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
                         ASPEED_SOC_WDT_BASE + i * 0x20);
     }
 
-    /* Net */
-    qdev_set_nic_properties(DEVICE(&s->ftgmac100), &nd_table[0]);
-    object_property_set_bool(OBJECT(&s->ftgmac100), true, "aspeed", &err);
-    object_property_set_bool(OBJECT(&s->ftgmac100), true, "realized",
-                             &local_err);
-    error_propagate(&err, local_err);
-    if (err) {
-        error_propagate(errp, err);
-        return;
+    /* Net LAN 0*/
+        qdev_set_nic_properties(DEVICE(&s->ftgmac100[0]), &nd_table[0]);
+        object_property_set_bool(OBJECT(&s->ftgmac100[0]), true, "aspeed", &err);
+    if (s->scu.hw_strap1 & SCU_HW_STRAP_MAC0_RGMII) {
+        qemu_log("%s: LAN0 enabled\n", __func__);
+        object_property_set_bool(OBJECT(&s->ftgmac100[0]), true, "realized",
+                                &local_err);
+        error_propagate(&err, local_err);
+        if (err) {
+            error_propagate(errp, err);
+            return;
+        }
+        sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100[0]), 0, ASPEED_SOC_ETH1_BASE);
+        sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100[0]), 0,
+                        qdev_get_gpio_in(DEVICE(&s->vic), 2));
+    }
+
+    /* Net LAN 1*/
+        qdev_set_nic_properties(DEVICE(&s->ftgmac100[1]), &nd_table[1]);
+        object_property_set_bool(OBJECT(&s->ftgmac100[1]), true, "aspeed", &err);
+    if (s->scu.hw_strap1 & SCU_HW_STRAP_MAC1_RGMII) {
+        qemu_log("%s: LAN1 enabled\n", __func__);
+        object_property_set_bool(OBJECT(&s->ftgmac100[1]), true, "realized",
+                                &local_err);
+        error_propagate(&err, local_err);
+        if (err) {
+            error_propagate(errp, err);
+            return;
+        }
+        sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100[1]), 0, ASPEED_SOC_ETH2_BASE);
+        sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100[1]), 0,
+                        qdev_get_gpio_in(DEVICE(&s->vic), 3));
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100), 0, ASPEED_SOC_ETH1_BASE);
-    sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100), 0,
-                       qdev_get_gpio_in(DEVICE(&s->vic), 2));
 
     /* iBT */
     object_property_set_bool(OBJECT(&s->ibt), true, "realized", &err);
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
old mode 100644
new mode 100755
index 623223d..0799d61
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -47,7 +47,7 @@ typedef struct AspeedSoCState {
     AspeedSMCState spi[ASPEED_SPIS_NUM];
     AspeedSDMCState sdmc;
     AspeedWDTState wdt[ASPEED_WDTS_NUM];
-    FTGMAC100State ftgmac100;
+    FTGMAC100State ftgmac100[2];
     AspeedIBTState ibt;
     AspeedGPIOState gpio;
     AspeedPWMState pwm;
-- 
2.7.4