[PATCH v2 00/21] aspeed: cleanups and some extensions

[PATCH v2 00/21] aspeed: cleanups and some extensions

[Cédric Le Goater]

Hello,

This series includes various fixes improving the support of Aspeed
machines. Extra attention was given to the robustness of the ftgmac100
model. A small kernel module tester was created for this purpose :

   https://github.com/legoater/ftgmac100-test/

Changes in v2 :

 - definitions for some new flash models in m25p80 by Igor
 - All Joel's comments should have been addressed
 - A better fix of the integer overflow in ftgmac100_do_tx suggested
   by Peter.


This needs a couple more reviewed-by before I can send a PR.

Thanks,

C.

Cédric Le Goater (16):
  m25p80: Return the JEDEC ID twice for mx25l25635e
  m25p80: Add support for mx25l25635f
  m25p80: Add support for n25q512ax3
  aspeed/scu: Fix valid access size on AST2400
  aspeed/smc: Fix MemoryRegionOps definition
  aspeed/smc: Fix max_slaves of the legacy SMC device
  aspeed/sdhci: Fix reset sequence
  ftgmac100: Fix registers that can be read
  ftgmac100: Fix interrupt status "Packet transmitted on ethernet"
  ftgmac100: Fix interrupt status "Packet moved to RX FIFO"
  ftgmac100: Change interrupt status when a DMA error occurs
  ftgmac100: Check for invalid len and address before doing a DMA
    transfer
  ftgmac100: Fix integer overflow in ftgmac100_do_tx()
  ftgmac100: Improve software reset
  aspeed/sdmc: Simplify calculation of RAM bits
  aspeed/smc: Open AHB window of the second chip of the AST2600 FMC
    controller

Igor Kononenko (2):
  arm: aspeed: add strap define `25HZ` of AST2500
  hw: add a number of SPI-flash's of m25p80 family

Joel Stanley (2):
  aspeed/sdmc: Perform memory training
  aspeed/sdmc: Allow writes to unprotected registers

erik-smit (1):
  hw/arm/aspeed: Add board model for Supermicro X11 BMC

 include/hw/misc/aspeed_scu.h  |   1 +
 include/hw/misc/aspeed_sdmc.h |  13 +++-
 hw/arm/aspeed.c               |  35 ++++++++++
 hw/block/m25p80.c             |   6 +-
 hw/misc/aspeed_scu.c          |   9 +--
 hw/misc/aspeed_sdmc.c         | 125 +++++++++++++++++++---------------
 hw/net/ftgmac100.c            |  95 ++++++++++++++++++--------
 hw/sd/aspeed_sdhci.c          |  14 +++-
 hw/ssi/aspeed_smc.c           |   6 +-
 9 files changed, 209 insertions(+), 95 deletions(-)

-- 
2.25.4

[PATCH v2 01/21] m25p80: Return the JEDEC ID twice for mx25l25635e

[Cédric Le Goater]

The mx25l25635e returns the JEDEC ID twice when issuing a RDID command :

  [    2.512027] aspeed-smc 1e630000.spi: reading JEDEC ID C2:20:19:C2:20:19

This can break some firmware testing for this condition on the
supermicrox11-bmc machine.

Reported-by: erik-smit <erik.lucas.smit@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/block/m25p80.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 82270884416e..605ff55c6756 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -217,7 +217,7 @@ static const FlashPartInfo known_devices[] = {
     { INFO("mx25l6405d",  0xc22017,      0,  64 << 10, 128, 0) },
     { INFO("mx25l12805d", 0xc22018,      0,  64 << 10, 256, 0) },
     { INFO("mx25l12855e", 0xc22618,      0,  64 << 10, 256, 0) },
-    { INFO("mx25l25635e", 0xc22019,      0,  64 << 10, 512, 0) },
+    { INFO6("mx25l25635e", 0xc22019,     0xc22019,  64 << 10, 512, 0) },
     { INFO("mx25l25655e", 0xc22619,      0,  64 << 10, 512, 0) },
     { INFO("mx66u51235f", 0xc2253a,      0,  64 << 10, 1024, ER_4K | ER_32K) },
     { INFO("mx66u1g45g",  0xc2253b,      0,  64 << 10, 2048, ER_4K | ER_32K) },
-- 
2.25.4

[PATCH v2 02/21] m25p80: Add support for mx25l25635f

[Cédric Le Goater]

The mx25l25635f is an extenstion of the mx25l25635e. It includes QPI
support, 4-Byte Address Command Set and faster transfers. See this
document for more details :

https://www.macronix.com/Lists/ApplicationNote/Attachments/1892/AN0200V1_MGRT_MX25L25635E_25735E%20to%20MX25L25635F_25735F.pdf

Both devices have the same 3bytes JEDEC ID: 0xc22019. They can be
distinguished with the QPIID command which is only available on
mx25l25635f and their Serial Flash Discoverable Parameters. However,
some FW use the JEDEC ID. For instance, on a SuperMicro P9 Boston, the
BMC FW reports :

  BMC flash ID: 0xc21920c2
  jedec_id: 0xc21920c2
  flash type: MX25L25635F
  ReadClk=0x32, WriteClk=0x85, EraseClk=0x85
  [smcfw_spi] cpuclk: 198000000 MHz, RefCLK: 24000000 MHz, AXI-AHB ratio: 2:1
  platform_flash: MX25L25635F (32768 Kbytes)

Define the mx25l25635f with an extended JEDEC ID.

Reviewed-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/block/m25p80.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 605ff55c6756..1696ab1f7821 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -218,6 +218,7 @@ static const FlashPartInfo known_devices[] = {
     { INFO("mx25l12805d", 0xc22018,      0,  64 << 10, 256, 0) },
     { INFO("mx25l12855e", 0xc22618,      0,  64 << 10, 256, 0) },
     { INFO6("mx25l25635e", 0xc22019,     0xc22019,  64 << 10, 512, 0) },
+    { INFO("mx25l25635f", 0xc22019,      0xc200,  64 << 10, 512, 0) },
     { INFO("mx25l25655e", 0xc22619,      0,  64 << 10, 512, 0) },
     { INFO("mx66u51235f", 0xc2253a,      0,  64 << 10, 1024, ER_4K | ER_32K) },
     { INFO("mx66u1g45g",  0xc2253b,      0,  64 << 10, 2048, ER_4K | ER_32K) },
-- 
2.25.4

[PATCH v2 03/21] m25p80: Add support for n25q512ax3

[Cédric Le Goater]

Datasheet available here :

https://www.micron.com/-/media/client/global/Documents/Products/Data%20Sheet/NOR%20Flash/Serial%20NOR/N25Q/n25q_512mb_1ce_3v_65nm.pdf

Reviewed-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/block/m25p80.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 1696ab1f7821..8a3fd959e218 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -238,6 +238,7 @@ static const FlashPartInfo known_devices[] = {
     { INFO("n25q128",     0x20ba18,      0,  64 << 10, 256, 0) },
     { INFO("n25q256a",    0x20ba19,      0,  64 << 10, 512, ER_4K) },
     { INFO("n25q512a",    0x20ba20,      0,  64 << 10, 1024, ER_4K) },
+    { INFO("n25q512ax3",  0x20ba20,  0x1000,  64 << 10, 1024, ER_4K) },
     { INFO_STACKED("n25q00",    0x20ba21, 0x1000, 64 << 10, 2048, ER_4K, 4) },
     { INFO_STACKED("n25q00a",   0x20bb21, 0x1000, 64 << 10, 2048, ER_4K, 4) },
     { INFO_STACKED("mt25ql01g", 0x20ba21, 0x1040, 64 << 10, 2048, ER_4K, 2) },
-- 
2.25.4

[PATCH v2 04/21] aspeed/scu: Fix valid access size on AST2400

[Cédric Le Goater]

The read access size of the SCU registers can be 1/2/4 bytes and write
is 4 bytes and all Aspeed models would need a .valid.accepts() handler.

For the moment, set the min access size to 1 byte to cover both read
and write operations on the AST2400 but keep the min access size of
the other SoCs to 4 bytes as this is an unusual access size.

This fixes support for some old firmware doing 2 bytes reads on the
AST2400 SoC.

Reported-by: erik-smit <erik.lucas.smit@gmail.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/misc/aspeed_scu.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
index ec4fef900e27..764222404bef 100644
--- a/hw/misc/aspeed_scu.c
+++ b/hw/misc/aspeed_scu.c
@@ -328,9 +328,10 @@ static const MemoryRegionOps aspeed_ast2400_scu_ops = {
     .read = aspeed_scu_read,
     .write = aspeed_ast2400_scu_write,
     .endianness = DEVICE_LITTLE_ENDIAN,
-    .valid.min_access_size = 4,
-    .valid.max_access_size = 4,
-    .valid.unaligned = false,
+    .valid = {
+        .min_access_size = 1,
+        .max_access_size = 4,
+    },
 };
 
 static const MemoryRegionOps aspeed_ast2500_scu_ops = {
-- 
2.25.4

[PATCH v2 05/21] hw/arm/aspeed: Add board model for Supermicro X11 BMC

[Cédric Le Goater]

From: erik-smit <erik.lucas.smit@gmail.com>

The BMC Firmware can be downloaded from :

  https://www.supermicro.com/en/products/motherboard/X11SSL-F

Signed-off-by: erik-smit <erik.lucas.smit@gmail.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
[ clg: Modified commit log ]
Message-Id: <20200715173418.186-1-erik.lucas.smit@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/arm/aspeed.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index fcb1a7cd8729..d17a4885a03c 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -57,6 +57,20 @@ struct AspeedMachineState {
         SCU_HW_STRAP_VGA_SIZE_SET(VGA_16M_DRAM) |                       \
         SCU_AST2400_HW_STRAP_BOOT_MODE(AST2400_SPI_BOOT))
 
+/* TODO: Find the actual hardware value */
+#define SUPERMICROX11_BMC_HW_STRAP1 (                                   \
+        SCU_AST2400_HW_STRAP_DRAM_SIZE(DRAM_SIZE_128MB) |               \
+        SCU_AST2400_HW_STRAP_DRAM_CONFIG(2) |                           \
+        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_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))
+
 /* AST2500 evb hardware value: 0xF100C2E6 */
 #define AST2500_EVB_HW_STRAP1 ((                                        \
         AST2500_HW_STRAP1_DEFAULTS |                                    \
@@ -603,6 +617,23 @@ static void aspeed_machine_palmetto_class_init(ObjectClass *oc, void *data)
         aspeed_soc_num_cpus(amc->soc_name);
 };
 
+static void aspeed_machine_supermicrox11_bmc_class_init(ObjectClass *oc,
+                                                        void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+    AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
+
+    mc->desc       = "Supermicro X11 BMC (ARM926EJ-S)";
+    amc->soc_name  = "ast2400-a1";
+    amc->hw_strap1 = SUPERMICROX11_BMC_HW_STRAP1;
+    amc->fmc_model = "mx25l25635e";
+    amc->spi_model = "mx25l25635e";
+    amc->num_cs    = 1;
+    amc->macs_mask = ASPEED_MAC0_ON | ASPEED_MAC1_ON;
+    amc->i2c_init  = palmetto_bmc_i2c_init;
+    mc->default_ram_size = 256 * MiB;
+}
+
 static void aspeed_machine_ast2500_evb_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -731,6 +762,10 @@ static const TypeInfo aspeed_machine_types[] = {
         .name          = MACHINE_TYPE_NAME("palmetto-bmc"),
         .parent        = TYPE_ASPEED_MACHINE,
         .class_init    = aspeed_machine_palmetto_class_init,
+    }, {
+        .name          = MACHINE_TYPE_NAME("supermicrox11-bmc"),
+        .parent        = TYPE_ASPEED_MACHINE,
+        .class_init    = aspeed_machine_supermicrox11_bmc_class_init,
     }, {
         .name          = MACHINE_TYPE_NAME("ast2500-evb"),
         .parent        = TYPE_ASPEED_MACHINE,
-- 
2.25.4

[PATCH v2 06/21] aspeed/smc: Fix MemoryRegionOps definition

[Cédric Le Goater]

Unaligned access support is a leftover from the initial commit. There
is no such need on this device register mapping. Remove it.

Cc: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ssi/aspeed_smc.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index 4fab1f5f855e..0646e0dca72e 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -1299,10 +1299,8 @@ static const MemoryRegionOps aspeed_smc_ops = {
     .read = aspeed_smc_read,
     .write = aspeed_smc_write,
     .endianness = DEVICE_LITTLE_ENDIAN,
-    .valid.unaligned = true,
 };
 
-
 /*
  * Initialize the custom address spaces for DMAs
  */
-- 
2.25.4

[PATCH v2 07/21] aspeed/smc: Fix max_slaves of the legacy SMC device

[Cédric Le Goater]

The legacy controller only has one slave.

Reviewed-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ssi/aspeed_smc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index 0646e0dca72e..8c79a5552f93 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -259,7 +259,7 @@ static const AspeedSMCController controllers[] = {
         .r_timings         = R_TIMINGS,
         .nregs_timings     = 1,
         .conf_enable_w0    = CONF_ENABLE_W0,
-        .max_slaves        = 5,
+        .max_slaves        = 1,
         .segments          = aspeed_segments_legacy,
         .flash_window_base = ASPEED_SOC_SMC_FLASH_BASE,
         .flash_window_size = 0x6000000,
-- 
2.25.4

[PATCH v2 08/21] aspeed/sdhci: Fix reset sequence

[Cédric Le Goater]

BIT(0) of the ASPEED_SDHCI_INFO register is set by SW and polled until
the bit is cleared by HW.

Use the number of supported slots to define the default value of this
register (The AST2600 eMMC Controller only has one). Fix the reset
sequence by clearing automatically the RESET bit.

Cc: Eddie James <eajames@linux.ibm.com>
Fixes: 2bea128c3d0b ("hw/sd/aspeed_sdhci: New device")
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/aspeed_sdhci.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/hw/sd/aspeed_sdhci.c b/hw/sd/aspeed_sdhci.c
index 22cafce0fbdc..4f24b7d2f942 100644
--- a/hw/sd/aspeed_sdhci.c
+++ b/hw/sd/aspeed_sdhci.c
@@ -16,7 +16,9 @@
 #include "hw/qdev-properties.h"
 
 #define ASPEED_SDHCI_INFO            0x00
-#define  ASPEED_SDHCI_INFO_RESET     0x00030000
+#define  ASPEED_SDHCI_INFO_SLOT1     (1 << 17)
+#define  ASPEED_SDHCI_INFO_SLOT0     (1 << 16)
+#define  ASPEED_SDHCI_INFO_RESET     (1 << 0)
 #define ASPEED_SDHCI_DEBOUNCE        0x04
 #define  ASPEED_SDHCI_DEBOUNCE_RESET 0x00000005
 #define ASPEED_SDHCI_BUS             0x08
@@ -67,6 +69,10 @@ static void aspeed_sdhci_write(void *opaque, hwaddr addr, uint64_t val,
     AspeedSDHCIState *sdhci = opaque;
 
     switch (addr) {
+    case ASPEED_SDHCI_INFO:
+        /* The RESET bit automatically clears. */
+        sdhci->regs[TO_REG(addr)] = (uint32_t)val & ~ASPEED_SDHCI_INFO_RESET;
+        break;
     case ASPEED_SDHCI_SDIO_140:
         sdhci->slots[0].capareg = (uint64_t)(uint32_t)val;
         break;
@@ -155,7 +161,11 @@ static void aspeed_sdhci_reset(DeviceState *dev)
     AspeedSDHCIState *sdhci = ASPEED_SDHCI(dev);
 
     memset(sdhci->regs, 0, ASPEED_SDHCI_REG_SIZE);
-    sdhci->regs[TO_REG(ASPEED_SDHCI_INFO)] = ASPEED_SDHCI_INFO_RESET;
+
+    sdhci->regs[TO_REG(ASPEED_SDHCI_INFO)] = ASPEED_SDHCI_INFO_SLOT0;
+    if (sdhci->num_slots == 2) {
+        sdhci->regs[TO_REG(ASPEED_SDHCI_INFO)] |= ASPEED_SDHCI_INFO_SLOT1;
+    }
     sdhci->regs[TO_REG(ASPEED_SDHCI_DEBOUNCE)] = ASPEED_SDHCI_DEBOUNCE_RESET;
 }
 
-- 
2.25.4

[PATCH v2 09/21] ftgmac100: Fix registers that can be read

[Cédric Le Goater]

Receive Ring Base Address Register (RXR_BADR) and the Normal Priority
Transmit Receive Ring Base Address Register (NPTXR_BADR) can also be
read.

Cc: Frederic Konrad <konrad.frederic@yahoo.fr>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/net/ftgmac100.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
index 5f4b26fc5f3c..0348fcf45676 100644
--- a/hw/net/ftgmac100.c
+++ b/hw/net/ftgmac100.c
@@ -669,6 +669,10 @@ static uint64_t ftgmac100_read(void *opaque, hwaddr addr, unsigned size)
         return s->math[0];
     case FTGMAC100_MATH1:
         return s->math[1];
+    case FTGMAC100_RXR_BADR:
+        return s->rx_ring;
+    case FTGMAC100_NPTXR_BADR:
+        return s->tx_ring;
     case FTGMAC100_ITC:
         return s->itc;
     case FTGMAC100_DBLAC:
-- 
2.25.4

[PATCH v2 10/21] ftgmac100: Fix interrupt status "Packet transmitted on ethernet"

[Cédric Le Goater]

The second field of the TX descriptor has a set of flags to choose
when the transmit interrupt is raised : after the packet has been sent
on the ethernet or after it has been moved into the TX FIFO. But we
don't model that today.

Simply raise the "Packet transmitted on ethernet" interrupt status bit
as soon as the packet is sent by QEMU.

Cc: Frederic Konrad <konrad.frederic@yahoo.fr>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/net/ftgmac100.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
index 0348fcf45676..aa3c05ef9882 100644
--- a/hw/net/ftgmac100.c
+++ b/hw/net/ftgmac100.c
@@ -547,9 +547,7 @@ static void ftgmac100_do_tx(FTGMAC100State *s, uint32_t tx_ring,
             qemu_send_packet(qemu_get_queue(s->nic), s->frame, frame_size);
             ptr = s->frame;
             frame_size = 0;
-            if (flags & FTGMAC100_TXDES1_TXIC) {
-                s->isr |= FTGMAC100_INT_XPKT_ETH;
-            }
+            s->isr |= FTGMAC100_INT_XPKT_ETH;
         }
 
         if (flags & FTGMAC100_TXDES1_TX2FIC) {
-- 
2.25.4

[PATCH v2 11/21] ftgmac100: Fix interrupt status "Packet moved to RX FIFO"

[Cédric Le Goater]

As we don't model the RX or TX FIFO, raise the "Packet moved to RX
FIFO" interrupt status bit as soon as we are handling a RX packet.

Cc: Frederic Konrad <konrad.frederic@yahoo.fr>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/net/ftgmac100.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
index aa3c05ef9882..5c0fe2d8cb75 100644
--- a/hw/net/ftgmac100.c
+++ b/hw/net/ftgmac100.c
@@ -950,6 +950,7 @@ static ssize_t ftgmac100_receive(NetClientState *nc, const uint8_t *buf,
         break;
     }
 
+    s->isr |= FTGMAC100_INT_RPKT_FIFO;
     addr = s->rx_descriptor;
     while (size > 0) {
         if (!ftgmac100_can_receive(nc)) {
@@ -1001,8 +1002,6 @@ static ssize_t ftgmac100_receive(NetClientState *nc, const uint8_t *buf,
             /* Last buffer in frame.  */
             bd.des0 |= flags | FTGMAC100_RXDES0_LRS;
             s->isr |= FTGMAC100_INT_RPKT_BUF;
-        } else {
-            s->isr |= FTGMAC100_INT_RPKT_FIFO;
         }
         ftgmac100_write_bd(&bd, addr);
         if (bd.des0 & s->rxdes0_edorr) {
-- 
2.25.4

[PATCH v2 12/21] ftgmac100: Change interrupt status when a DMA error occurs

[Cédric Le Goater]

The model uses today the "Normal priority transmit buffer unavailable"
interrupt status which it is not appropriate. According to the Aspeed
specs, no interrupts are raised in that case. An "AHB error" status
seems like a better modeling choice for all implementations since it
is covered by the Linux kernel.

Cc: Frederic Konrad <konrad.frederic@yahoo.fr>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/net/ftgmac100.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
index 5c0fe2d8cb75..014980d30aca 100644
--- a/hw/net/ftgmac100.c
+++ b/hw/net/ftgmac100.c
@@ -517,7 +517,7 @@ static void ftgmac100_do_tx(FTGMAC100State *s, uint32_t tx_ring,
         if (dma_memory_read(&address_space_memory, bd.des3, ptr, len)) {
             qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to read packet @ 0x%x\n",
                           __func__, bd.des3);
-            s->isr |= FTGMAC100_INT_NO_NPTXBUF;
+            s->isr |= FTGMAC100_INT_AHB_ERR;
             break;
         }
 
-- 
2.25.4

[PATCH v2 13/21] ftgmac100: Check for invalid len and address before doing a DMA transfer

[Cédric Le Goater]

According to the Aspeed specs, no interrupts are raised in that case
but a "Tx-packets lost" status seems like a good modeling choice for
all implementations. It is covered by the Linux kernel.

Cc: Frederic Konrad <konrad.frederic@yahoo.fr>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/net/ftgmac100.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
index 014980d30aca..280aa3d3a1e2 100644
--- a/hw/net/ftgmac100.c
+++ b/hw/net/ftgmac100.c
@@ -507,6 +507,15 @@ static void ftgmac100_do_tx(FTGMAC100State *s, uint32_t tx_ring,
         }
 
         len = FTGMAC100_TXDES0_TXBUF_SIZE(bd.des0);
+        if (!len) {
+            /*
+             * 0 is an invalid size, however the HW does not raise any
+             * interrupt. Flag an error because the guest is buggy.
+             */
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid segment size\n",
+                          __func__);
+        }
+
         if (frame_size + len > sizeof(s->frame)) {
             qemu_log_mask(LOG_GUEST_ERROR, "%s: frame too big : %d bytes\n",
                           __func__, len);
-- 
2.25.4

[PATCH v2 14/21] ftgmac100: Fix integer overflow in ftgmac100_do_tx()

[Cédric Le Goater]

When inserting the VLAN tag in packets, memmove() can generate an
integer overflow for packets whose length is less than 12 bytes.

Move the VLAN insertion when the last segment of the frame is reached
and check length against the size of the ethernet header (14 bytes) to
avoid the crash. Return FTGMAC100_INT_XPKT_LOST status if the frame is
too small. This seems like a good modeling choice even if Aspeed does
not specify anything in that case.

Cc: Frederic Konrad <konrad.frederic@yahoo.fr>
Cc: Mauro Matteo Cascella <mcascell@redhat.com>
Reported-by: Ziming Zhang <ezrakiez@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/net/ftgmac100.c | 55 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 39 insertions(+), 16 deletions(-)

diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
index 280aa3d3a1e2..7c9fa720df03 100644
--- a/hw/net/ftgmac100.c
+++ b/hw/net/ftgmac100.c
@@ -481,6 +481,37 @@ static int ftgmac100_write_bd(FTGMAC100Desc *bd, dma_addr_t addr)
     return 0;
 }
 
+static int ftgmac100_insert_vlan(FTGMAC100State *s, int frame_size,
+                                  uint8_t vlan_tci)
+{
+    uint8_t *vlan_hdr = s->frame + (ETH_ALEN * 2);
+    uint8_t *payload = vlan_hdr + sizeof(struct vlan_header);
+
+    if (frame_size < sizeof(struct eth_header)) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: frame too small for VLAN insertion : %d bytes\n",
+                      __func__, frame_size);
+        s->isr |= FTGMAC100_INT_XPKT_LOST;
+        goto out;
+    }
+
+    if (frame_size + sizeof(struct vlan_header) > sizeof(s->frame)) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: frame too big : %d bytes\n",
+                      __func__, frame_size);
+        s->isr |= FTGMAC100_INT_XPKT_LOST;
+        frame_size -= sizeof(struct vlan_header);
+    }
+
+    memmove(payload, vlan_hdr, frame_size - (ETH_ALEN * 2));
+    stw_be_p(vlan_hdr, ETH_P_VLAN);
+    stw_be_p(vlan_hdr + 2, vlan_tci);
+    frame_size += sizeof(struct vlan_header);
+
+out:
+    return frame_size;
+}
+
 static void ftgmac100_do_tx(FTGMAC100State *s, uint32_t tx_ring,
                             uint32_t tx_descriptor)
 {
@@ -530,25 +561,17 @@ static void ftgmac100_do_tx(FTGMAC100State *s, uint32_t tx_ring,
             break;
         }
 
-        /* Check for VLAN */
-        if (bd.des0 & FTGMAC100_TXDES0_FTS &&
-            bd.des1 & FTGMAC100_TXDES1_INS_VLANTAG &&
-            be16_to_cpu(PKT_GET_ETH_HDR(ptr)->h_proto) != ETH_P_VLAN) {
-            if (frame_size + len + 4 > sizeof(s->frame)) {
-                qemu_log_mask(LOG_GUEST_ERROR, "%s: frame too big : %d bytes\n",
-                              __func__, len);
-                s->isr |= FTGMAC100_INT_XPKT_LOST;
-                len =  sizeof(s->frame) - frame_size - 4;
-            }
-            memmove(ptr + 16, ptr + 12, len - 12);
-            stw_be_p(ptr + 12, ETH_P_VLAN);
-            stw_be_p(ptr + 14, bd.des1);
-            len += 4;
-        }
-
         ptr += len;
         frame_size += len;
         if (bd.des0 & FTGMAC100_TXDES0_LTS) {
+
+            /* Check for VLAN */
+            if (flags & FTGMAC100_TXDES1_INS_VLANTAG &&
+                be16_to_cpu(PKT_GET_ETH_HDR(s->frame)->h_proto) != ETH_P_VLAN) {
+                frame_size = ftgmac100_insert_vlan(s, frame_size,
+                                            FTGMAC100_TXDES1_VLANTAG_CI(flags));
+            }
+
             if (flags & FTGMAC100_TXDES1_IP_CHKSUM) {
                 net_checksum_calculate(s->frame, frame_size);
             }
-- 
2.25.4

[PATCH v2 15/21] ftgmac100: Improve software reset

[Cédric Le Goater]

The software reset of the MAC needs a finer granularity. Some settings
in MACCR are kept.

Cc: Frederic Konrad <konrad.frederic@yahoo.fr>
Fixes: bd44300d1afc ("net: add FTGMAC100 support")
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/net/ftgmac100.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
index 7c9fa720df03..782ff192cedc 100644
--- a/hw/net/ftgmac100.c
+++ b/hw/net/ftgmac100.c
@@ -649,10 +649,8 @@ static uint32_t ftgmac100_rxpoll(FTGMAC100State *s)
     return cnt / div[speed];
 }
 
-static void ftgmac100_reset(DeviceState *d)
+static void ftgmac100_do_reset(FTGMAC100State *s, bool sw_reset)
 {
-    FTGMAC100State *s = FTGMAC100(d);
-
     /* Reset the FTGMAC100 */
     s->isr = 0;
     s->ier = 0;
@@ -671,7 +669,12 @@ static void ftgmac100_reset(DeviceState *d)
     s->fear1 = 0;
     s->tpafcr = 0xf1;
 
-    s->maccr = 0;
+    if (sw_reset) {
+        s->maccr &= FTGMAC100_MACCR_GIGA_MODE | FTGMAC100_MACCR_FAST_MODE;
+    } else {
+        s->maccr = 0;
+    }
+
     s->phycr = 0;
     s->phydata = 0;
     s->fcr = 0x400;
@@ -680,6 +683,11 @@ static void ftgmac100_reset(DeviceState *d)
     phy_reset(s);
 }
 
+static void ftgmac100_reset(DeviceState *d)
+{
+    ftgmac100_do_reset(FTGMAC100(d), false);
+}
+
 static uint64_t ftgmac100_read(void *opaque, hwaddr addr, unsigned size)
 {
     FTGMAC100State *s = FTGMAC100(opaque);
@@ -824,7 +832,7 @@ static void ftgmac100_write(void *opaque, hwaddr addr,
     case FTGMAC100_MACCR: /* MAC Device control */
         s->maccr = value;
         if (value & FTGMAC100_MACCR_SW_RST) {
-            ftgmac100_reset(DEVICE(s));
+            ftgmac100_do_reset(s, true);
         }
 
         if (ftgmac100_can_receive(qemu_get_queue(s->nic))) {
-- 
2.25.4

[PATCH v2 16/21] aspeed/sdmc: Perform memory training

[Cédric Le Goater]

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

This allows qemu to run the "normal" power on reset boot path through
u-boot, where the DDR is trained.

An enhancement would be to have the SCU bit stick across qemu reboots,
but be unset on initial boot.

Proper modelling would be to discard all writes to the phy setting regs
at offset 0x100 - 0x400 and to model the phy status regs at offset
0x400.

The status regs model would only need to account for offets 0x00,
0x50, 0x68 and 0x7c.

Signed-off-by: Joel Stanley <joel@jms.id.au>
[ clg: checkpatch fixes ]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/misc/aspeed_sdmc.h | 13 ++++++++++++-
 hw/misc/aspeed_scu.c          |  2 +-
 hw/misc/aspeed_sdmc.c         | 19 +++++++++++++++++--
 3 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/include/hw/misc/aspeed_sdmc.h b/include/hw/misc/aspeed_sdmc.h
index cea1e67fe365..c6226957dd3d 100644
--- a/include/hw/misc/aspeed_sdmc.h
+++ b/include/hw/misc/aspeed_sdmc.h
@@ -17,7 +17,18 @@
 #define TYPE_ASPEED_2500_SDMC TYPE_ASPEED_SDMC "-ast2500"
 #define TYPE_ASPEED_2600_SDMC TYPE_ASPEED_SDMC "-ast2600"
 
-#define ASPEED_SDMC_NR_REGS (0x174 >> 2)
+/*
+ * SDMC has 174 documented registers. In addition the u-boot device tree
+ * describes the following regions:
+ *  - PHY status regs at offset 0x400, length 0x200
+ *  - PHY setting regs at offset 0x100, length 0x300
+ *
+ * There are two sets of MRS (Mode Registers) configuration in ast2600 memory
+ * system: one is in the SDRAM MC (memory controller) which is used in run
+ * time, and the other is in the DDR-PHY IP which is used during DDR-PHY
+ * training.
+ */
+#define ASPEED_SDMC_NR_REGS (0x500 >> 2)
 
 typedef struct AspeedSDMCState {
     /*< private >*/
diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
index 764222404bef..dc6dd87c22f4 100644
--- a/hw/misc/aspeed_scu.c
+++ b/hw/misc/aspeed_scu.c
@@ -656,7 +656,7 @@ static const uint32_t ast2600_a1_resets[ASPEED_AST2600_SCU_NR_REGS] = {
     [AST2600_SYS_RST_CTRL2]     = 0xFFFFFFFC,
     [AST2600_CLK_STOP_CTRL]     = 0xFFFF7F8A,
     [AST2600_CLK_STOP_CTRL2]    = 0xFFF0FFF0,
-    [AST2600_SDRAM_HANDSHAKE]   = 0x00000040,  /* SoC completed DRAM init */
+    [AST2600_SDRAM_HANDSHAKE]   = 0x00000000,
     [AST2600_HPLL_PARAM]        = 0x1000405F,
     [AST2600_CHIP_ID0]          = 0x1234ABCD,
     [AST2600_CHIP_ID1]          = 0x88884444,
diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c
index 855848b7d23a..ff2809a09965 100644
--- a/hw/misc/aspeed_sdmc.c
+++ b/hw/misc/aspeed_sdmc.c
@@ -113,7 +113,7 @@ static uint64_t aspeed_sdmc_read(void *opaque, hwaddr addr, unsigned size)
     if (addr >= ARRAY_SIZE(s->regs)) {
         qemu_log_mask(LOG_GUEST_ERROR,
                       "%s: Out-of-bounds read at offset 0x%" HWADDR_PRIx "\n",
-                      __func__, addr);
+                      __func__, addr * 4);
         return 0;
     }
 
@@ -206,6 +206,19 @@ static void aspeed_sdmc_reset(DeviceState *dev)
 
     /* Set ram size bit and defaults values */
     s->regs[R_CONF] = asc->compute_conf(s, 0);
+
+    /*
+     * PHY status:
+     *  - set phy status ok (set bit 1)
+     *  - initial PVT calibration ok (clear bit 3)
+     *  - runtime calibration ok (clear bit 5)
+     */
+    s->regs[0x100] = BIT(1);
+
+    /* PHY eye window: set all as passing */
+    s->regs[0x100 | (0x68 / 4)] = 0xff;
+    s->regs[0x100 | (0x7c / 4)] = 0xff;
+    s->regs[0x100 | (0x50 / 4)] = 0xfffffff;
 }
 
 static void aspeed_sdmc_get_ram_size(Object *obj, Visitor *v, const char *name,
@@ -443,7 +456,9 @@ static void aspeed_2600_sdmc_write(AspeedSDMCState *s, uint32_t reg,
     }
 
     if (reg != R_PROT && s->regs[R_PROT] == PROT_SOFTLOCKED) {
-        qemu_log_mask(LOG_GUEST_ERROR, "%s: SDMC is locked!\n", __func__);
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: SDMC is locked! (write to MCR%02x blocked)\n",
+                      __func__, reg * 4);
         return;
     }
 
-- 
2.25.4

[PATCH v2 17/21] aspeed/sdmc: Allow writes to unprotected registers

[Cédric Le Goater]

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

A subset of registers are not protected by the lock behaviour, so allow
unconditionally writing to those.

Signed-off-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/misc/aspeed_sdmc.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c
index ff2809a09965..81c73450ab5d 100644
--- a/hw/misc/aspeed_sdmc.c
+++ b/hw/misc/aspeed_sdmc.c
@@ -33,15 +33,28 @@
 /* Configuration Register */
 #define R_CONF            (0x04 / 4)
 
+/* Interrupt control/status */
+#define R_ISR             (0x50 / 4)
+
 /* Control/Status Register #1 (ast2500) */
 #define R_STATUS1         (0x60 / 4)
 #define   PHY_BUSY_STATE      BIT(0)
 #define   PHY_PLL_LOCK_STATUS BIT(4)
 
+/* Reserved */
+#define R_MCR6C           (0x6c / 4)
+
 #define R_ECC_TEST_CTRL   (0x70 / 4)
 #define   ECC_TEST_FINISHED   BIT(12)
 #define   ECC_TEST_FAIL       BIT(13)
 
+#define R_TEST_START_LEN  (0x74 / 4)
+#define R_TEST_FAIL_DQ    (0x78 / 4)
+#define R_TEST_INIT_VAL   (0x7c / 4)
+#define R_DRAM_SW         (0x88 / 4)
+#define R_DRAM_TIME       (0x8c / 4)
+#define R_ECC_ERR_INJECT  (0xb4 / 4)
+
 /*
  * Configuration register Ox4 (for Aspeed AST2400 SOC)
  *
@@ -449,6 +462,20 @@ static uint32_t aspeed_2600_sdmc_compute_conf(AspeedSDMCState *s, uint32_t data)
 static void aspeed_2600_sdmc_write(AspeedSDMCState *s, uint32_t reg,
                                    uint32_t data)
 {
+    /* Unprotected registers */
+    switch (reg) {
+    case R_ISR:
+    case R_MCR6C:
+    case R_TEST_START_LEN:
+    case R_TEST_FAIL_DQ:
+    case R_TEST_INIT_VAL:
+    case R_DRAM_SW:
+    case R_DRAM_TIME:
+    case R_ECC_ERR_INJECT:
+        s->regs[reg] = data;
+        return;
+    }
+
     if (s->regs[R_PROT] == PROT_HARDLOCKED) {
         qemu_log_mask(LOG_GUEST_ERROR, "%s: SDMC is locked until system reset!\n",
                 __func__);
-- 
2.25.4

[PATCH v2 18/21] aspeed/sdmc: Simplify calculation of RAM bits

[Cédric Le Goater]

Changes in commit 533eb415df2e ("arm/aspeed: actually check RAM size")
introduced a 'valid_ram_sizes' array which can be used to compute the
associated bit field value encoding the RAM size. The field is simply
the index of the array.

Reviewed-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/misc/aspeed_sdmc.c | 79 ++++++++++++++-----------------------------
 1 file changed, 25 insertions(+), 54 deletions(-)

diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c
index 81c73450ab5d..08f856cbda7e 100644
--- a/hw/misc/aspeed_sdmc.c
+++ b/hw/misc/aspeed_sdmc.c
@@ -159,57 +159,6 @@ static const MemoryRegionOps aspeed_sdmc_ops = {
     .valid.max_access_size = 4,
 };
 
-static int ast2400_rambits(AspeedSDMCState *s)
-{
-    switch (s->ram_size >> 20) {
-    case 64:
-        return ASPEED_SDMC_DRAM_64MB;
-    case 128:
-        return ASPEED_SDMC_DRAM_128MB;
-    case 256:
-        return ASPEED_SDMC_DRAM_256MB;
-    case 512:
-        return ASPEED_SDMC_DRAM_512MB;
-    default:
-        g_assert_not_reached();
-        break;
-    }
-}
-
-static int ast2500_rambits(AspeedSDMCState *s)
-{
-    switch (s->ram_size >> 20) {
-    case 128:
-        return ASPEED_SDMC_AST2500_128MB;
-    case 256:
-        return ASPEED_SDMC_AST2500_256MB;
-    case 512:
-        return ASPEED_SDMC_AST2500_512MB;
-    case 1024:
-        return ASPEED_SDMC_AST2500_1024MB;
-    default:
-        g_assert_not_reached();
-        break;
-    }
-}
-
-static int ast2600_rambits(AspeedSDMCState *s)
-{
-    switch (s->ram_size >> 20) {
-    case 256:
-        return ASPEED_SDMC_AST2600_256MB;
-    case 512:
-        return ASPEED_SDMC_AST2600_512MB;
-    case 1024:
-        return ASPEED_SDMC_AST2600_1024MB;
-    case 2048:
-        return ASPEED_SDMC_AST2600_2048MB;
-    default:
-        g_assert_not_reached();
-        break;
-    }
-}
-
 static void aspeed_sdmc_reset(DeviceState *dev)
 {
     AspeedSDMCState *s = ASPEED_SDMC(dev);
@@ -324,10 +273,32 @@ static const TypeInfo aspeed_sdmc_info = {
     .abstract   = true,
 };
 
+static int aspeed_sdmc_get_ram_bits(AspeedSDMCState *s)
+{
+    AspeedSDMCClass *asc = ASPEED_SDMC_GET_CLASS(s);
+    int i;
+
+    /*
+     * The bitfield value encoding the RAM size is the index of the
+     * possible RAM size array
+     */
+    for (i = 0; asc->valid_ram_sizes[i]; i++) {
+        if (s->ram_size == asc->valid_ram_sizes[i]) {
+            return i;
+        }
+    }
+
+    /*
+     * Invalid RAM sizes should have been excluded when setting the
+     * SoC RAM size.
+     */
+    g_assert_not_reached();
+}
+
 static uint32_t aspeed_2400_sdmc_compute_conf(AspeedSDMCState *s, uint32_t data)
 {
     uint32_t fixed_conf = ASPEED_SDMC_VGA_COMPAT |
-        ASPEED_SDMC_DRAM_SIZE(ast2400_rambits(s));
+        ASPEED_SDMC_DRAM_SIZE(aspeed_sdmc_get_ram_bits(s));
 
     /* Make sure readonly bits are kept */
     data &= ~ASPEED_SDMC_READONLY_MASK;
@@ -385,7 +356,7 @@ static uint32_t aspeed_2500_sdmc_compute_conf(AspeedSDMCState *s, uint32_t data)
     uint32_t fixed_conf = ASPEED_SDMC_HW_VERSION(1) |
         ASPEED_SDMC_VGA_APERTURE(ASPEED_SDMC_VGA_64MB) |
         ASPEED_SDMC_CACHE_INITIAL_DONE |
-        ASPEED_SDMC_DRAM_SIZE(ast2500_rambits(s));
+        ASPEED_SDMC_DRAM_SIZE(aspeed_sdmc_get_ram_bits(s));
 
     /* Make sure readonly bits are kept */
     data &= ~ASPEED_SDMC_AST2500_READONLY_MASK;
@@ -451,7 +422,7 @@ static uint32_t aspeed_2600_sdmc_compute_conf(AspeedSDMCState *s, uint32_t data)
 {
     uint32_t fixed_conf = ASPEED_SDMC_HW_VERSION(3) |
         ASPEED_SDMC_VGA_APERTURE(ASPEED_SDMC_VGA_64MB) |
-        ASPEED_SDMC_DRAM_SIZE(ast2600_rambits(s));
+        ASPEED_SDMC_DRAM_SIZE(aspeed_sdmc_get_ram_bits(s));
 
     /* Make sure readonly bits are kept (use ast2500 mask) */
     data &= ~ASPEED_SDMC_AST2500_READONLY_MASK;
-- 
2.25.4

[PATCH v2 19/21] aspeed/smc: Open AHB window of the second chip of the AST2600 FMC controller

[Cédric Le Goater]

This change works around the HW default values to be able to test the
Tacoma board with -kernel command line option. This was required when
we had both flash chips enabled in the device tree, otherwise Linux
would fail to probe the entire controller leaving it with no rootfs.

Reviewed-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ssi/aspeed_smc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index 8c79a5552f93..795784e5f364 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -230,7 +230,7 @@ static void aspeed_smc_reg_to_segment(const AspeedSMCState *s, uint32_t reg,
 
 static const AspeedSegments aspeed_segments_ast2600_fmc[] = {
     { 0x0, 128 * MiB }, /* start address is readonly */
-    { 0x0, 0 }, /* disabled */
+    { 128 * MiB, 128 * MiB }, /* default is disabled but needed for -kernel */
     { 0x0, 0 }, /* disabled */
 };
 
-- 
2.25.4

[PATCH v2 20/21] arm: aspeed: add strap define `25HZ` of AST2500

[Cédric Le Goater]

From: Igor Kononenko <i.kononenko@yadro.com>

Provide a definition for the "25Hz reference clock input mode" strap

Signed-off-by: Igor Kononenko <i.kononenko@yadro.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Message-Id: <20200811203502.20382-1-i.kononenko@yadro.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/misc/aspeed_scu.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/hw/misc/aspeed_scu.h b/include/hw/misc/aspeed_scu.h
index a6739bb846b6..9cd530afa23a 100644
--- a/include/hw/misc/aspeed_scu.h
+++ b/include/hw/misc/aspeed_scu.h
@@ -286,6 +286,7 @@ uint32_t aspeed_scu_get_apb_freq(AspeedSCUState *s);
 #define SCU_AST2500_HW_STRAP_ESPI_FLASH_ENABLE     (0x1 << 26)
 #define SCU_AST2500_HW_STRAP_ESPI_ENABLE           (0x1 << 25)
 #define SCU_AST2500_HW_STRAP_DDR4_ENABLE           (0x1 << 24)
+#define SCU_AST2500_HW_STRAP_25HZ_CLOCK_MODE       (0x1 << 23)
 
 #define SCU_AST2500_HW_STRAP_ACPI_ENABLE           (0x1 << 19)
 #define SCU_AST2500_HW_STRAP_USBCKI_FREQ           (0x1 << 18)
-- 
2.25.4

[PATCH v2 21/21] hw: add a number of SPI-flash's of m25p80 family

[Cédric Le Goater]

From: Igor Kononenko <i.kononenko@yadro.com>

Support a following SPI flashes:
* mx66l51235f
* mt25ql512ab

Signed-off-by: Igor Kononenko <i.kononenko@yadro.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Message-Id: <20200811203724.20699-1-i.kononenko@yadro.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/block/m25p80.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 8a3fd959e218..d812e9fb6cfb 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -220,6 +220,7 @@ static const FlashPartInfo known_devices[] = {
     { INFO6("mx25l25635e", 0xc22019,     0xc22019,  64 << 10, 512, 0) },
     { INFO("mx25l25635f", 0xc22019,      0xc200,  64 << 10, 512, 0) },
     { INFO("mx25l25655e", 0xc22619,      0,  64 << 10, 512, 0) },
+    { INFO("mx66l51235f", 0xc2201a,      0,  64 << 10, 1024, ER_4K | ER_32K) },
     { INFO("mx66u51235f", 0xc2253a,      0,  64 << 10, 1024, ER_4K | ER_32K) },
     { INFO("mx66u1g45g",  0xc2253b,      0,  64 << 10, 2048, ER_4K | ER_32K) },
     { INFO("mx66l1g45g",  0xc2201b,      0,  64 << 10, 2048, ER_4K | ER_32K) },
@@ -239,6 +240,7 @@ static const FlashPartInfo known_devices[] = {
     { INFO("n25q256a",    0x20ba19,      0,  64 << 10, 512, ER_4K) },
     { INFO("n25q512a",    0x20ba20,      0,  64 << 10, 1024, ER_4K) },
     { INFO("n25q512ax3",  0x20ba20,  0x1000,  64 << 10, 1024, ER_4K) },
+    { INFO("mt25ql512ab", 0x20ba20, 0x1044, 64 << 10, 1024, ER_4K | ER_32K) },
     { INFO_STACKED("n25q00",    0x20ba21, 0x1000, 64 << 10, 2048, ER_4K, 4) },
     { INFO_STACKED("n25q00a",   0x20bb21, 0x1000, 64 << 10, 2048, ER_4K, 4) },
     { INFO_STACKED("mt25ql01g", 0x20ba21, 0x1040, 64 << 10, 2048, ER_4K, 2) },
-- 
2.25.4

Re: [PATCH v2 08/21] aspeed/sdhci: Fix reset sequence

[Joel Stanley]

On Wed, 19 Aug 2020 at 10:10, Cédric Le Goater <clg@kaod.org> wrote:
>
> BIT(0) of the ASPEED_SDHCI_INFO register is set by SW and polled until
> the bit is cleared by HW.
>
> Use the number of supported slots to define the default value of this
> register (The AST2600 eMMC Controller only has one). Fix the reset
> sequence by clearing automatically the RESET bit.
>
> Cc: Eddie James <eajames@linux.ibm.com>
> Fixes: 2bea128c3d0b ("hw/sd/aspeed_sdhci: New device")
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Joel Stanley <joel@jms.id.au>

> ---
>  hw/sd/aspeed_sdhci.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/hw/sd/aspeed_sdhci.c b/hw/sd/aspeed_sdhci.c
> index 22cafce0fbdc..4f24b7d2f942 100644
> --- a/hw/sd/aspeed_sdhci.c
> +++ b/hw/sd/aspeed_sdhci.c
> @@ -16,7 +16,9 @@
>  #include "hw/qdev-properties.h"
>
>  #define ASPEED_SDHCI_INFO            0x00
> -#define  ASPEED_SDHCI_INFO_RESET     0x00030000
> +#define  ASPEED_SDHCI_INFO_SLOT1     (1 << 17)
> +#define  ASPEED_SDHCI_INFO_SLOT0     (1 << 16)
> +#define  ASPEED_SDHCI_INFO_RESET     (1 << 0)
>  #define ASPEED_SDHCI_DEBOUNCE        0x04
>  #define  ASPEED_SDHCI_DEBOUNCE_RESET 0x00000005
>  #define ASPEED_SDHCI_BUS             0x08
> @@ -67,6 +69,10 @@ static void aspeed_sdhci_write(void *opaque, hwaddr addr, uint64_t val,
>      AspeedSDHCIState *sdhci = opaque;
>
>      switch (addr) {
> +    case ASPEED_SDHCI_INFO:
> +        /* The RESET bit automatically clears. */
> +        sdhci->regs[TO_REG(addr)] = (uint32_t)val & ~ASPEED_SDHCI_INFO_RESET;
> +        break;
>      case ASPEED_SDHCI_SDIO_140:
>          sdhci->slots[0].capareg = (uint64_t)(uint32_t)val;
>          break;
> @@ -155,7 +161,11 @@ static void aspeed_sdhci_reset(DeviceState *dev)
>      AspeedSDHCIState *sdhci = ASPEED_SDHCI(dev);
>
>      memset(sdhci->regs, 0, ASPEED_SDHCI_REG_SIZE);
> -    sdhci->regs[TO_REG(ASPEED_SDHCI_INFO)] = ASPEED_SDHCI_INFO_RESET;
> +
> +    sdhci->regs[TO_REG(ASPEED_SDHCI_INFO)] = ASPEED_SDHCI_INFO_SLOT0;
> +    if (sdhci->num_slots == 2) {
> +        sdhci->regs[TO_REG(ASPEED_SDHCI_INFO)] |= ASPEED_SDHCI_INFO_SLOT1;
> +    }
>      sdhci->regs[TO_REG(ASPEED_SDHCI_DEBOUNCE)] = ASPEED_SDHCI_DEBOUNCE_RESET;
>  }
>
> --
> 2.25.4
>

Re: [PATCH v2 00/21] aspeed: cleanups and some extensions

[Joel Stanley]

On Wed, 19 Aug 2020 at 10:10, Cédric Le Goater <clg@kaod.org> wrote:
>
> Hello,
>
> This series includes various fixes improving the support of Aspeed
> machines. Extra attention was given to the robustness of the ftgmac100
> model. A small kernel module tester was created for this purpose :
>
>    https://github.com/legoater/ftgmac100-test/

I gave this a test and it successfully broke the machine for me
without your fixes.

I tried to test this series but the new build system stopped me from
being able to complete a build. It failed with:

Found ninjatool-1.8 at qemu/upstream/build/ninjatool
./ninjatool -t ninja2make --omit clean dist uninstall < build.ninja >
Makefile.ninja
/bin/sh: build.ninja: No such file or directory

:(

>
> Changes in v2 :
>
>  - definitions for some new flash models in m25p80 by Igor
>  - All Joel's comments should have been addressed
>  - A better fix of the integer overflow in ftgmac100_do_tx suggested
>    by Peter.
>
>
> This needs a couple more reviewed-by before I can send a PR.

I have read through all the patches and I have no objections.

Cheers,

Joel

>
> Thanks,
>
> C.
>
> Cédric Le Goater (16):
>   m25p80: Return the JEDEC ID twice for mx25l25635e
>   m25p80: Add support for mx25l25635f
>   m25p80: Add support for n25q512ax3
>   aspeed/scu: Fix valid access size on AST2400
>   aspeed/smc: Fix MemoryRegionOps definition
>   aspeed/smc: Fix max_slaves of the legacy SMC device
>   aspeed/sdhci: Fix reset sequence
>   ftgmac100: Fix registers that can be read
>   ftgmac100: Fix interrupt status "Packet transmitted on ethernet"
>   ftgmac100: Fix interrupt status "Packet moved to RX FIFO"
>   ftgmac100: Change interrupt status when a DMA error occurs
>   ftgmac100: Check for invalid len and address before doing a DMA
>     transfer
>   ftgmac100: Fix integer overflow in ftgmac100_do_tx()
>   ftgmac100: Improve software reset
>   aspeed/sdmc: Simplify calculation of RAM bits
>   aspeed/smc: Open AHB window of the second chip of the AST2600 FMC
>     controller
>
> Igor Kononenko (2):
>   arm: aspeed: add strap define `25HZ` of AST2500
>   hw: add a number of SPI-flash's of m25p80 family
>
> Joel Stanley (2):
>   aspeed/sdmc: Perform memory training
>   aspeed/sdmc: Allow writes to unprotected registers
>
> erik-smit (1):
>   hw/arm/aspeed: Add board model for Supermicro X11 BMC
>
>  include/hw/misc/aspeed_scu.h  |   1 +
>  include/hw/misc/aspeed_sdmc.h |  13 +++-
>  hw/arm/aspeed.c               |  35 ++++++++++
>  hw/block/m25p80.c             |   6 +-
>  hw/misc/aspeed_scu.c          |   9 +--
>  hw/misc/aspeed_sdmc.c         | 125 +++++++++++++++++++---------------
>  hw/net/ftgmac100.c            |  95 ++++++++++++++++++--------
>  hw/sd/aspeed_sdhci.c          |  14 +++-
>  hw/ssi/aspeed_smc.c           |   6 +-
>  9 files changed, 209 insertions(+), 95 deletions(-)
>
> --
> 2.25.4
>

Re: [PATCH v2 00/21] aspeed: cleanups and some extensions

[Cédric Le Goater]

On 8/25/20 8:01 AM, Joel Stanley wrote:
> On Wed, 19 Aug 2020 at 10:10, Cédric Le Goater <clg@kaod.org> wrote:
>>
>> Hello,
>>
>> This series includes various fixes improving the support of Aspeed
>> machines. Extra attention was given to the robustness of the ftgmac100
>> model. A small kernel module tester was created for this purpose :
>>
>>    https://github.com/legoater/ftgmac100-test/
> 
> I gave this a test and it successfully broke the machine for me
> without your fixes.

The network stack is busted. yes. Sometimes it survives or does a reset.

HW always survives which is surprising.

Thanks, 

C.

> I tried to test this series but the new build system stopped me from
> being able to complete a build. It failed with:
> 
> Found ninjatool-1.8 at qemu/upstream/build/ninjatool
> ./ninjatool -t ninja2make --omit clean dist uninstall < build.ninja >
> Makefile.ninja
> /bin/sh: build.ninja: No such file or directory
> 
> :(
> 
>>
>> Changes in v2 :
>>
>>  - definitions for some new flash models in m25p80 by Igor
>>  - All Joel's comments should have been addressed
>>  - A better fix of the integer overflow in ftgmac100_do_tx suggested
>>    by Peter.
>>
>>
>> This needs a couple more reviewed-by before I can send a PR.
> 
> I have read through all the patches and I have no objections.
> 
> Cheers,
> 
> Joel
> 
>>
>> Thanks,
>>
>> C.
>>
>> Cédric Le Goater (16):
>>   m25p80: Return the JEDEC ID twice for mx25l25635e
>>   m25p80: Add support for mx25l25635f
>>   m25p80: Add support for n25q512ax3
>>   aspeed/scu: Fix valid access size on AST2400
>>   aspeed/smc: Fix MemoryRegionOps definition
>>   aspeed/smc: Fix max_slaves of the legacy SMC device
>>   aspeed/sdhci: Fix reset sequence
>>   ftgmac100: Fix registers that can be read
>>   ftgmac100: Fix interrupt status "Packet transmitted on ethernet"
>>   ftgmac100: Fix interrupt status "Packet moved to RX FIFO"
>>   ftgmac100: Change interrupt status when a DMA error occurs
>>   ftgmac100: Check for invalid len and address before doing a DMA
>>     transfer
>>   ftgmac100: Fix integer overflow in ftgmac100_do_tx()
>>   ftgmac100: Improve software reset
>>   aspeed/sdmc: Simplify calculation of RAM bits
>>   aspeed/smc: Open AHB window of the second chip of the AST2600 FMC
>>     controller
>>
>> Igor Kononenko (2):
>>   arm: aspeed: add strap define `25HZ` of AST2500
>>   hw: add a number of SPI-flash's of m25p80 family
>>
>> Joel Stanley (2):
>>   aspeed/sdmc: Perform memory training
>>   aspeed/sdmc: Allow writes to unprotected registers
>>
>> erik-smit (1):
>>   hw/arm/aspeed: Add board model for Supermicro X11 BMC
>>
>>  include/hw/misc/aspeed_scu.h  |   1 +
>>  include/hw/misc/aspeed_sdmc.h |  13 +++-
>>  hw/arm/aspeed.c               |  35 ++++++++++
>>  hw/block/m25p80.c             |   6 +-
>>  hw/misc/aspeed_scu.c          |   9 +--
>>  hw/misc/aspeed_sdmc.c         | 125 +++++++++++++++++++---------------
>>  hw/net/ftgmac100.c            |  95 ++++++++++++++++++--------
>>  hw/sd/aspeed_sdhci.c          |  14 +++-
>>  hw/ssi/aspeed_smc.c           |   6 +-
>>  9 files changed, 209 insertions(+), 95 deletions(-)
>>
>> --
>> 2.25.4
>>

Re: [PATCH v2 05/21] hw/arm/aspeed: Add board model for Supermicro X11 BMC

[Peter Maydell]

On Wed, 19 Aug 2020 at 11:10, Cédric Le Goater <clg@kaod.org> wrote:
>
> From: erik-smit <erik.lucas.smit@gmail.com>
>
> The BMC Firmware can be downloaded from :
>
>   https://www.supermicro.com/en/products/motherboard/X11SSL-F
>
> Signed-off-by: erik-smit <erik.lucas.smit@gmail.com>

Should the name in the From and Signed-off-by: here be
"Erik Smit" rather than "erik-smit" ?

thanks
-- PMM

Re: [PATCH v2 05/21] hw/arm/aspeed: Add board model for Supermicro X11 BMC

[Erik Smit]

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

On Tue, Aug 25, 2020, 4:33 PM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Wed, 19 Aug 2020 at 11:10, Cédric Le Goater <clg@kaod.org> wrote:
> >
> > From: erik-smit <erik.lucas.smit@gmail.com>
> >
> > The BMC Firmware can be downloaded from :
> >
> >   https://www.supermicro.com/en/products/motherboard/X11SSL-F
> >
> > Signed-off-by: erik-smit <erik.lucas.smit@gmail.com>
>
> Should the name in the From and Signed-off-by: here be
> "Erik Smit" rather than "erik-smit" ?
>

I don't know if it matters. I'm fine with either.

-- 
Erik Smit

>

[-- Attachment #2: Type: text/html, Size: 1568 bytes --]

Re: [PATCH v2 05/21] hw/arm/aspeed: Add board model for Supermicro X11 BMC

[Peter Maydell]

On Tue, 25 Aug 2020 at 15:37, Erik Smit <erik.lucas.smit@gmail.com> wrote:
> On Tue, Aug 25, 2020, 4:33 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>>
>> On Wed, 19 Aug 2020 at 11:10, Cédric Le Goater <clg@kaod.org> wrote:
>> >
>> > From: erik-smit <erik.lucas.smit@gmail.com>
>> >
>> > The BMC Firmware can be downloaded from :
>> >
>> >   https://www.supermicro.com/en/products/motherboard/X11SSL-F
>> >
>> > Signed-off-by: erik-smit <erik.lucas.smit@gmail.com>
>>
>> Should the name in the From and Signed-off-by: here be
>> "Erik Smit" rather than "erik-smit" ?
>
> I don't know if it matters. I'm fine with either.

It's supposed to be "your real name" (ie not a pseudonym,
email address or username); obviously for some people that
really is a single word or all-lower-case, but I usually
check because separate-words-with-caps is the more common.

thanks
-- PMM

Re: [PATCH v2 05/21] hw/arm/aspeed: Add board model for Supermicro X11 BMC

[Cédric Le Goater]

On 8/25/20 5:18 PM, Peter Maydell wrote:
> On Tue, 25 Aug 2020 at 15:37, Erik Smit <erik.lucas.smit@gmail.com> wrote:
>> On Tue, Aug 25, 2020, 4:33 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>>>
>>> On Wed, 19 Aug 2020 at 11:10, Cédric Le Goater <clg@kaod.org> wrote:
>>>>
>>>> From: erik-smit <erik.lucas.smit@gmail.com>
>>>>
>>>> The BMC Firmware can be downloaded from :
>>>>
>>>>   https://www.supermicro.com/en/products/motherboard/X11SSL-F
>>>>
>>>> Signed-off-by: erik-smit <erik.lucas.smit@gmail.com>
>>>
>>> Should the name in the From and Signed-off-by: here be
>>> "Erik Smit" rather than "erik-smit" ?
>>
>> I don't know if it matters. I'm fine with either.
> 
> It's supposed to be "your real name" (ie not a pseudonym,
> email address or username); obviously for some people that
> really is a single word or all-lower-case, but I usually
> check because separate-words-with-caps is the more common.


I have changed it to "Erik Smit" in the patchset.

Thanks,

C.

Re: [PATCH v2 02/21] m25p80: Add support for mx25l25635f

[Cédric Le Goater]

On 8/19/20 12:09 PM, Cédric Le Goater wrote:
> The mx25l25635f is an extenstion of the mx25l25635e. It includes QPI
> support, 4-Byte Address Command Set and faster transfers. See this
> document for more details :
> 
> https://www.macronix.com/Lists/ApplicationNote/Attachments/1892/AN0200V1_MGRT_MX25L25635E_25735E%20to%20MX25L25635F_25735F.pdf
> 
> Both devices have the same 3bytes JEDEC ID: 0xc22019. They can be
> distinguished with the QPIID command which is only available on
> mx25l25635f and their Serial Flash Discoverable Parameters. However,
> some FW use the JEDEC ID. For instance, on a SuperMicro P9 Boston, the
> BMC FW reports :
> 
>   BMC flash ID: 0xc21920c2
>   jedec_id: 0xc21920c2
>   flash type: MX25L25635F
>   ReadClk=0x32, WriteClk=0x85, EraseClk=0x85
>   [smcfw_spi] cpuclk: 198000000 MHz, RefCLK: 24000000 MHz, AXI-AHB ratio: 2:1
>   platform_flash: MX25L25635F (32768 Kbytes)
> 
> Define the mx25l25635f with an extended JEDEC ID.

I am starting to think this is a broken driver or a very exotic HW. 
We have a couple of mx25l25635f on other openpower systems and the 
returned JEDEC ID is 0xc21920c21920.

I will keep this patch on the side for now.

Thanks,

C. 

> 
> Reviewed-by: Joel Stanley <joel@jms.id.au>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  hw/block/m25p80.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 605ff55c6756..1696ab1f7821 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -218,6 +218,7 @@ static const FlashPartInfo known_devices[] = {
>      { INFO("mx25l12805d", 0xc22018,      0,  64 << 10, 256, 0) },
>      { INFO("mx25l12855e", 0xc22618,      0,  64 << 10, 256, 0) },
>      { INFO6("mx25l25635e", 0xc22019,     0xc22019,  64 << 10, 512, 0) },
> +    { INFO("mx25l25635f", 0xc22019,      0xc200,  64 << 10, 512, 0) },
>      { INFO("mx25l25655e", 0xc22619,      0,  64 << 10, 512, 0) },
>      { INFO("mx66u51235f", 0xc2253a,      0,  64 << 10, 1024, ER_4K | ER_32K) },
>      { INFO("mx66u1g45g",  0xc2253b,      0,  64 << 10, 2048, ER_4K | ER_32K) },
>