[PATCH v1 0/3] tests/qtest/aspeed: Add Data FIFO qtest coverage for AST2700 SMC

[PATCH v1 0/3] tests/qtest/aspeed: Add Data FIFO qtest coverage for AST2700 SMC

[Jamin Lin]

This series depends on https://github.com/legoater/qemu.git at aspeed-next branch.

v1
 1. Add Data FIFO mode test
 2. Clarify SCU/SCUIO/FMC fields are shared with PSP SoC to AST2700
 3. Bump vmstate version after ASPEED_SMC_R_MAX increase
 
Jamin Lin (3):
  tests/qtest/ast2700-smc-test: Add Data FIFO mode test
  hw/arm/aspeed_coprocessor: Clarify SCU/SCUIO/FMC fields are shared
    with PSP SoC to AST2700
  hw/ssi/aspeed_smc: Bump vmstate version after ASPEED_SMC_R_MAX
    increase

 include/hw/arm/aspeed_coprocessor.h |   5 ++
 tests/qtest/aspeed-smc-utils.h      |   4 ++
 hw/ssi/aspeed_smc.c                 |   4 +-
 tests/qtest/aspeed-smc-utils.c      | 102 ++++++++++++++++++++++++++++
 tests/qtest/ast2700-smc-test.c      |   4 ++
 5 files changed, 117 insertions(+), 2 deletions(-)

-- 
2.43.0

[PATCH v1 1/3] tests/qtest/ast2700-smc-test: Add Data FIFO mode test

[Jamin Lin]

Add two qtest cases exercising the new AST2700 Data FIFO-based flash
access path (R_DATA_FIFO at spi_base + 0x200).

Write_page_datafifo sends the page-program command and data through
the FIFO port, then verifies the result via the regular read path.
Read_page_datafifo writes a page the regular way, then reads it back
through the FIFO port, so both directions are checked independently.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 tests/qtest/aspeed-smc-utils.h |   4 ++
 tests/qtest/aspeed-smc-utils.c | 102 +++++++++++++++++++++++++++++++++
 tests/qtest/ast2700-smc-test.c |   4 ++
 3 files changed, 110 insertions(+)

diff --git a/tests/qtest/aspeed-smc-utils.h b/tests/qtest/aspeed-smc-utils.h
index e2fd8ff1bd..485cb92ede 100644
--- a/tests/qtest/aspeed-smc-utils.h
+++ b/tests/qtest/aspeed-smc-utils.h
@@ -45,6 +45,8 @@
 #define   CTRL_WRITEMODE       0x2
 #define   CTRL_USERMODE        0x3
 #define SR_WEL BIT(1)
+/* Data fifo */
+#define R_DATA_FIFO 0x200
 
 /*
  * Flash commands
@@ -90,5 +92,7 @@ void aspeed_smc_test_status_reg_write_protection(const void *data);
 void aspeed_smc_test_write_block_protect(const void *data);
 void aspeed_smc_test_write_block_protect_bottom_bit(const void *data);
 void aspeed_smc_test_write_page_qpi(const void *data);
+void aspeed_smc_test_write_page_datafifo(const void *data);
+void aspeed_smc_test_read_page_datafifo(const void *data);
 
 #endif /* TESTS_ASPEED_SMC_UTILS_H */
diff --git a/tests/qtest/aspeed-smc-utils.c b/tests/qtest/aspeed-smc-utils.c
index c27d09e767..a1be4e02f8 100644
--- a/tests/qtest/aspeed-smc-utils.c
+++ b/tests/qtest/aspeed-smc-utils.c
@@ -73,6 +73,28 @@ static inline uint32_t flash_readl(const AspeedSMCTestData *data,
     return qtest_readl(data->s, data->flash_base + offset);
 }
 
+/*
+ * Data FIFO port, in spi_base's register bank (not flash_base). Accesses
+ * through the FIFO require the complete user-mode transaction (opcode,
+ * address, and data). Assumes CS0, whose FIFO slot is at R_DATA_FIFO.
+ */
+static inline void datafifo_writeb(const AspeedSMCTestData *data,
+                                   uint8_t value)
+{
+    qtest_writeb(data->s, data->spi_base + R_DATA_FIFO, value);
+}
+
+static inline void datafifo_writel(const AspeedSMCTestData *data,
+                                   uint32_t value)
+{
+    spi_writel(data, R_DATA_FIFO, value);
+}
+
+static inline uint32_t datafifo_readl(const AspeedSMCTestData *data)
+{
+    return spi_readl(data, R_DATA_FIFO);
+}
+
 static void spi_conf(const AspeedSMCTestData *data, uint32_t value)
 {
     uint32_t conf = spi_readl(data, R_CONF);
@@ -684,3 +706,83 @@ void aspeed_smc_test_write_page_qpi(const void *data)
     flash_reset(test_data);
 }
 
+void aspeed_smc_test_write_page_datafifo(const void *data)
+{
+    const AspeedSMCTestData *test_data = (const AspeedSMCTestData *)data;
+    uint32_t my_page_addr = test_data->page_addr;
+    uint32_t some_page_addr = my_page_addr + FLASH_PAGE_SIZE;
+    uint32_t page[FLASH_PAGE_SIZE / 4];
+    int i;
+
+    spi_conf(test_data, 1 << (CONF_ENABLE_W0 + test_data->cs));
+
+    /*
+     * Send the complete user-mode transaction (opcode, address, data)
+     * through the Data FIFO port.
+     */
+    spi_ctrl_start_user(test_data);
+    datafifo_writeb(test_data, EN_4BYTE_ADDR);
+    datafifo_writeb(test_data, WREN);
+    datafifo_writeb(test_data, PP);
+    datafifo_writel(test_data, make_be32(my_page_addr));
+
+    for (i = 0; i < FLASH_PAGE_SIZE / 4; i++) {
+        datafifo_writel(test_data, make_be32(my_page_addr + i * 4));
+    }
+    spi_ctrl_stop_user(test_data);
+
+    /* Check what was written, using the regular read path */
+    read_page(test_data, my_page_addr, page);
+    for (i = 0; i < FLASH_PAGE_SIZE / 4; i++) {
+        g_assert_cmphex(page[i], ==, my_page_addr + i * 4);
+    }
+
+    /* Check some other page. It should be full of 0xff */
+    read_page(test_data, some_page_addr, page);
+    for (i = 0; i < FLASH_PAGE_SIZE / 4; i++) {
+        g_assert_cmphex(page[i], ==, 0xffffffff);
+    }
+
+    flash_reset(test_data);
+}
+
+void aspeed_smc_test_read_page_datafifo(const void *data)
+{
+    const AspeedSMCTestData *test_data = (const AspeedSMCTestData *)data;
+    uint32_t my_page_addr = test_data->page_addr;
+    uint32_t page[FLASH_PAGE_SIZE / 4];
+    int i;
+
+    spi_conf(test_data, 1 << (CONF_ENABLE_W0 + test_data->cs));
+
+    /* Write the page the regular way */
+    spi_ctrl_start_user(test_data);
+    flash_writeb(test_data, 0, EN_4BYTE_ADDR);
+    flash_writeb(test_data, 0, WREN);
+    flash_writeb(test_data, 0, PP);
+    flash_writel(test_data, 0, make_be32(my_page_addr));
+    for (i = 0; i < FLASH_PAGE_SIZE / 4; i++) {
+        flash_writel(test_data, 0, make_be32(my_page_addr + i * 4));
+    }
+    spi_ctrl_stop_user(test_data);
+
+    /*
+     * Read it back through the data FIFO port, again sending the whole
+     * transaction (opcode, address, data) through it.
+     */
+    spi_ctrl_start_user(test_data);
+    datafifo_writeb(test_data, EN_4BYTE_ADDR);
+    datafifo_writeb(test_data, READ);
+    datafifo_writel(test_data, make_be32(my_page_addr));
+    for (i = 0; i < FLASH_PAGE_SIZE / 4; i++) {
+        page[i] = make_be32(datafifo_readl(test_data));
+    }
+    spi_ctrl_stop_user(test_data);
+
+    for (i = 0; i < FLASH_PAGE_SIZE / 4; i++) {
+        g_assert_cmphex(page[i], ==, my_page_addr + i * 4);
+    }
+
+    flash_reset(test_data);
+}
+
diff --git a/tests/qtest/ast2700-smc-test.c b/tests/qtest/ast2700-smc-test.c
index 33fc47230e..58f796ce0b 100644
--- a/tests/qtest/ast2700-smc-test.c
+++ b/tests/qtest/ast2700-smc-test.c
@@ -52,6 +52,10 @@ static void test_ast2700_evb(AspeedSMCTestData *data)
                         data, aspeed_smc_test_read_status_reg);
     qtest_add_data_func("/ast2700/smc/write_page_qpi",
                         data, aspeed_smc_test_write_page_qpi);
+    qtest_add_data_func("/ast2700/smc/write_page_datafifo",
+                        data, aspeed_smc_test_write_page_datafifo);
+    qtest_add_data_func("/ast2700/smc/read_page_datafifo",
+                        data, aspeed_smc_test_read_page_datafifo);
 }
 
 int main(int argc, char **argv)
-- 
2.43.0

[PATCH v1 2/3] hw/arm/aspeed_coprocessor: Clarify SCU/SCUIO/FMC fields are shared with PSP SoC to AST2700

[Jamin Lin]

Add a comment above Aspeed27x0CoprocessorState's scu_alias/scuio_alias/
fmc_alias and scu/scuio/fmc fields noting that they are not owned by
the coprocessor: they point at resources owned by the main PSP SoC,
only aliased into this coprocessor's own address space.

No functional change.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 include/hw/arm/aspeed_coprocessor.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/hw/arm/aspeed_coprocessor.h b/include/hw/arm/aspeed_coprocessor.h
index 23c3b97f06..ff359ab532 100644
--- a/include/hw/arm/aspeed_coprocessor.h
+++ b/include/hw/arm/aspeed_coprocessor.h
@@ -49,6 +49,11 @@ struct Aspeed27x0CoprocessorState {
 
     ARMv7MState armv7m;
 
+   /*
+    * SCU, SCUIO and FMC are not owned by this coprocessor: they are
+    * shared with the main PSP SoC, and only aliased into this
+    * coprocessor's own address space here.
+    */
     MemoryRegion scu_alias;
     MemoryRegion scuio_alias;
     MemoryRegion fmc_alias;
-- 
2.43.0

[PATCH v1 3/3] hw/ssi/aspeed_smc: Bump vmstate version after ASPEED_SMC_R_MAX increase

[Jamin Lin]

ASPEED_SMC_R_MAX grew from (0x100 / 4) to (0x300 / 4) to cover the new
AST2700 data fifo registers.

This breaks migration compatibility with older QEMU builds for the
affected models, even though Aspeed machines are not officially
covered by migration compatibility guarantees.

Bump version_id to 4 and minimum_version_id to 2 to reflect the
incompatible format.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/ssi/aspeed_smc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index 5cc4dd5e42..bf596f7b2d 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -1218,8 +1218,8 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
 
 static const VMStateDescription vmstate_aspeed_smc = {
     .name = "aspeed.smc",
-    .version_id = 3,
-    .minimum_version_id = 1,
+    .version_id = 4,
+    .minimum_version_id = 2,
     .fields = (const VMStateField[]) {
         VMSTATE_UINT32_ARRAY(regs, AspeedSMCState, ASPEED_SMC_R_MAX),
         VMSTATE_UNUSED_V(2, 2), /* was snoop_index/snoop_dummies */
-- 
2.43.0