[Qemu-devel] [PATCH v2 0/5] fw_cfg_test refactor and add two test cases

[Qemu-devel] [PATCH v2 0/5] fw_cfg_test refactor and add two test cases

[Li Qiang]

In the disscuss of adding reboot timeout test case:
https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg03304.html

Philippe suggested we should uses the only related option for one
specific test. However currently we uses one QTestState for all the
test cases. In order to achieve Philippe's idea, I split the test case
for its own QTestState. As this patchset has changed a lot, I don't bump
the version.

Change since v1:
Add a patch to store the reboot_timeout as little endian
Fix the endian issue per Thomas's review

Li Qiang (5):
  tests: refactor fw_cfg_test
  tests: fw_cfg: add a function to get the fw_cfg file
  fw_cfg: reboot: store reboot-timeout as little endian
  tests: fw_cfg: add reboot_timeout test case
  tests: fw_cfg: add splash time test case

 hw/nvram/fw_cfg.c     |   4 +-
 tests/fw_cfg-test.c   | 125 +++++++++++++++++++++++++++++++++++++++---
 tests/libqos/fw_cfg.c |  55 +++++++++++++++++++
 tests/libqos/fw_cfg.h |   9 +++
 4 files changed, 184 insertions(+), 9 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH v2 0/5] fw_cfg_test refactor and add two test cases

[Li Qiang]

In the disscuss of adding reboot timeout test case:
https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg03304.html

Philippe suggested we should uses the only related option for one
specific test. However currently we uses one QTestState for all the
test cases. In order to achieve Philippe's idea, I split the test case
for its own QTestState. As this patchset has changed a lot, I don't bump
the version.

Change since v1:
Add a patch to store the reboot_timeout as little endian
Fix the endian issue per Thomas's review

Li Qiang (5):
  tests: refactor fw_cfg_test
  tests: fw_cfg: add a function to get the fw_cfg file
  fw_cfg: reboot: store reboot-timeout as little endian
  tests: fw_cfg: add reboot_timeout test case
  tests: fw_cfg: add splash time test case

 hw/nvram/fw_cfg.c     |   4 +-
 tests/fw_cfg-test.c   | 125 +++++++++++++++++++++++++++++++++++++++---
 tests/libqos/fw_cfg.c |  55 +++++++++++++++++++
 tests/libqos/fw_cfg.h |   9 +++
 4 files changed, 184 insertions(+), 9 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH v2 1/5] tests: refactor fw_cfg_test

[Li Qiang]

Currently, fw_cfg_test uses one QTestState for every test case.
This will add all command lines for every test case and
this is unnecessary. This patch split the test cases and for
every test case it uses his own QTestState. This patch does following
things:

1. Get rid of the global 'fw_cfg', this need add a uninit function

2. Convert every test case in a separate QTestState

After this patch, we can add fw_cfg test case freely and will not
have effect on other test cases.

Signed-off-by: Li Qiang <liq3ea@163.com>
Acked-by: Thomas Huth <thuth@redhat.com>
---
 tests/fw_cfg-test.c   | 86 ++++++++++++++++++++++++++++++++++++++-----
 tests/libqos/fw_cfg.c | 10 +++++
 tests/libqos/fw_cfg.h |  7 ++++
 3 files changed, 94 insertions(+), 9 deletions(-)

diff --git a/tests/fw_cfg-test.c b/tests/fw_cfg-test.c
index 1c5103fe1c..c22503619f 100644
--- a/tests/fw_cfg-test.c
+++ b/tests/fw_cfg-test.c
@@ -21,62 +21,127 @@ static uint16_t nb_cpus = 1;
 static uint16_t max_cpus = 1;
 static uint64_t nb_nodes = 0;
 static uint16_t boot_menu = 0;
-static QFWCFG *fw_cfg = NULL;
 
 static void test_fw_cfg_signature(void)
 {
+    QFWCFG *fw_cfg;
+    QTestState *s;
     char buf[5];
 
+    s = qtest_init("");
+    fw_cfg = pc_fw_cfg_init(s);
+
     qfw_cfg_get(fw_cfg, FW_CFG_SIGNATURE, buf, 4);
     buf[4] = 0;
 
     g_assert_cmpstr(buf, ==, "QEMU");
+    pc_fw_cfg_uninit(fw_cfg);
+    qtest_quit(s);
 }
 
 static void test_fw_cfg_id(void)
 {
-    uint32_t id = qfw_cfg_get_u32(fw_cfg, FW_CFG_ID);
+    QFWCFG *fw_cfg;
+    QTestState *s;
+    uint32_t id;
+
+    s = qtest_init("");
+    fw_cfg = pc_fw_cfg_init(s);
+
+    id = qfw_cfg_get_u32(fw_cfg, FW_CFG_ID);
     g_assert((id == 1) ||
              (id == 3));
+    pc_fw_cfg_uninit(fw_cfg);
+    qtest_quit(s);
 }
 
 static void test_fw_cfg_uuid(void)
 {
+    QFWCFG *fw_cfg;
+    QTestState *s;
+
     uint8_t buf[16];
     static const uint8_t uuid[16] = {
         0x46, 0x00, 0xcb, 0x32, 0x38, 0xec, 0x4b, 0x2f,
         0x8a, 0xcb, 0x81, 0xc6, 0xea, 0x54, 0xf2, 0xd8,
     };
 
+    s = qtest_init("-uuid 4600cb32-38ec-4b2f-8acb-81c6ea54f2d8");
+    fw_cfg = pc_fw_cfg_init(s);
+
     qfw_cfg_get(fw_cfg, FW_CFG_UUID, buf, 16);
     g_assert(memcmp(buf, uuid, sizeof(buf)) == 0);
+
+    pc_fw_cfg_uninit(fw_cfg);
+    qtest_quit(s);
+
 }
 
 static void test_fw_cfg_ram_size(void)
 {
+    QFWCFG *fw_cfg;
+    QTestState *s;
+
+    s = qtest_init("");
+    fw_cfg = pc_fw_cfg_init(s);
+
     g_assert_cmpint(qfw_cfg_get_u64(fw_cfg, FW_CFG_RAM_SIZE), ==, ram_size);
+
+    pc_fw_cfg_uninit(fw_cfg);
+    qtest_quit(s);
 }
 
 static void test_fw_cfg_nographic(void)
 {
+    QFWCFG *fw_cfg;
+    QTestState *s;
+
+    s = qtest_init("");
+    fw_cfg = pc_fw_cfg_init(s);
+
     g_assert_cmpint(qfw_cfg_get_u16(fw_cfg, FW_CFG_NOGRAPHIC), ==, 0);
+
+    pc_fw_cfg_uninit(fw_cfg);
+    qtest_quit(s);
 }
 
 static void test_fw_cfg_nb_cpus(void)
 {
+    QFWCFG *fw_cfg;
+    QTestState *s;
+
+    s = qtest_init("");
+    fw_cfg = pc_fw_cfg_init(s);
+
     g_assert_cmpint(qfw_cfg_get_u16(fw_cfg, FW_CFG_NB_CPUS), ==, nb_cpus);
+
+    pc_fw_cfg_uninit(fw_cfg);
+    qtest_quit(s);
 }
 
 static void test_fw_cfg_max_cpus(void)
 {
+    QFWCFG *fw_cfg;
+    QTestState *s;
+
+    s = qtest_init("");
+    fw_cfg = pc_fw_cfg_init(s);
+
     g_assert_cmpint(qfw_cfg_get_u16(fw_cfg, FW_CFG_MAX_CPUS), ==, max_cpus);
+    pc_fw_cfg_uninit(fw_cfg);
+    qtest_quit(s);
 }
 
 static void test_fw_cfg_numa(void)
 {
+    QFWCFG *fw_cfg;
+    QTestState *s;
     uint64_t *cpu_mask;
     uint64_t *node_mask;
 
+    s = qtest_init("");
+    fw_cfg = pc_fw_cfg_init(s);
+
     g_assert_cmpint(qfw_cfg_get_u64(fw_cfg, FW_CFG_NUMA), ==, nb_nodes);
 
     cpu_mask = g_new0(uint64_t, max_cpus);
@@ -92,24 +157,29 @@ static void test_fw_cfg_numa(void)
 
     g_free(node_mask);
     g_free(cpu_mask);
+    pc_fw_cfg_uninit(fw_cfg);
+    qtest_quit(s);
 }
 
 static void test_fw_cfg_boot_menu(void)
 {
+    QFWCFG *fw_cfg;
+    QTestState *s;
+
+    s = qtest_init("");
+    fw_cfg = pc_fw_cfg_init(s);
+
     g_assert_cmpint(qfw_cfg_get_u16(fw_cfg, FW_CFG_BOOT_MENU), ==, boot_menu);
+    pc_fw_cfg_uninit(fw_cfg);
+    qtest_quit(s);
 }
 
 int main(int argc, char **argv)
 {
-    QTestState *s;
     int ret;
 
     g_test_init(&argc, &argv, NULL);
 
-    s = qtest_init("-uuid 4600cb32-38ec-4b2f-8acb-81c6ea54f2d8");
-
-    fw_cfg = pc_fw_cfg_init(s);
-
     qtest_add_func("fw_cfg/signature", test_fw_cfg_signature);
     qtest_add_func("fw_cfg/id", test_fw_cfg_id);
     qtest_add_func("fw_cfg/uuid", test_fw_cfg_uuid);
@@ -128,7 +198,5 @@ int main(int argc, char **argv)
 
     ret = g_test_run();
 
-    qtest_quit(s);
-
     return ret;
 }
diff --git a/tests/libqos/fw_cfg.c b/tests/libqos/fw_cfg.c
index d0889d1e22..c6839c53c8 100644
--- a/tests/libqos/fw_cfg.c
+++ b/tests/libqos/fw_cfg.c
@@ -81,6 +81,11 @@ QFWCFG *mm_fw_cfg_init(QTestState *qts, uint64_t base)
     return fw_cfg;
 }
 
+void mm_fw_cfg_uninit(QFWCFG *fw_cfg)
+{
+    g_free(fw_cfg);
+}
+
 static void io_fw_cfg_select(QFWCFG *fw_cfg, uint16_t key)
 {
     qtest_outw(fw_cfg->qts, fw_cfg->base, key);
@@ -107,3 +112,8 @@ QFWCFG *io_fw_cfg_init(QTestState *qts, uint16_t base)
 
     return fw_cfg;
 }
+
+void io_fw_cfg_uninit(QFWCFG *fw_cfg)
+{
+    g_free(fw_cfg);
+}
diff --git a/tests/libqos/fw_cfg.h b/tests/libqos/fw_cfg.h
index 0353416af0..60de81e863 100644
--- a/tests/libqos/fw_cfg.h
+++ b/tests/libqos/fw_cfg.h
@@ -33,11 +33,18 @@ uint32_t qfw_cfg_get_u32(QFWCFG *fw_cfg, uint16_t key);
 uint64_t qfw_cfg_get_u64(QFWCFG *fw_cfg, uint16_t key);
 
 QFWCFG *mm_fw_cfg_init(QTestState *qts, uint64_t base);
+void mm_fw_cfg_uninit(QFWCFG *fw_cfg);
 QFWCFG *io_fw_cfg_init(QTestState *qts, uint16_t base);
+void io_fw_cfg_uninit(QFWCFG *fw_cfg);
 
 static inline QFWCFG *pc_fw_cfg_init(QTestState *qts)
 {
     return io_fw_cfg_init(qts, 0x510);
 }
 
+static inline void pc_fw_cfg_uninit(QFWCFG *fw_cfg)
+{
+    io_fw_cfg_uninit(fw_cfg);
+}
+
 #endif
-- 
2.17.1

[Qemu-devel] [PATCH v2 1/5] tests: refactor fw_cfg_test

[Li Qiang]

Currently, fw_cfg_test uses one QTestState for every test case.
This will add all command lines for every test case and
this is unnecessary. This patch split the test cases and for
every test case it uses his own QTestState. This patch does following
things:

1. Get rid of the global 'fw_cfg', this need add a uninit function

2. Convert every test case in a separate QTestState

After this patch, we can add fw_cfg test case freely and will not
have effect on other test cases.

Signed-off-by: Li Qiang <liq3ea@163.com>
Acked-by: Thomas Huth <thuth@redhat.com>
---
 tests/fw_cfg-test.c   | 86 ++++++++++++++++++++++++++++++++++++++-----
 tests/libqos/fw_cfg.c | 10 +++++
 tests/libqos/fw_cfg.h |  7 ++++
 3 files changed, 94 insertions(+), 9 deletions(-)

diff --git a/tests/fw_cfg-test.c b/tests/fw_cfg-test.c
index 1c5103fe1c..c22503619f 100644
--- a/tests/fw_cfg-test.c
+++ b/tests/fw_cfg-test.c
@@ -21,62 +21,127 @@ static uint16_t nb_cpus = 1;
 static uint16_t max_cpus = 1;
 static uint64_t nb_nodes = 0;
 static uint16_t boot_menu = 0;
-static QFWCFG *fw_cfg = NULL;
 
 static void test_fw_cfg_signature(void)
 {
+    QFWCFG *fw_cfg;
+    QTestState *s;
     char buf[5];
 
+    s = qtest_init("");
+    fw_cfg = pc_fw_cfg_init(s);
+
     qfw_cfg_get(fw_cfg, FW_CFG_SIGNATURE, buf, 4);
     buf[4] = 0;
 
     g_assert_cmpstr(buf, ==, "QEMU");
+    pc_fw_cfg_uninit(fw_cfg);
+    qtest_quit(s);
 }
 
 static void test_fw_cfg_id(void)
 {
-    uint32_t id = qfw_cfg_get_u32(fw_cfg, FW_CFG_ID);
+    QFWCFG *fw_cfg;
+    QTestState *s;
+    uint32_t id;
+
+    s = qtest_init("");
+    fw_cfg = pc_fw_cfg_init(s);
+
+    id = qfw_cfg_get_u32(fw_cfg, FW_CFG_ID);
     g_assert((id == 1) ||
              (id == 3));
+    pc_fw_cfg_uninit(fw_cfg);
+    qtest_quit(s);
 }
 
 static void test_fw_cfg_uuid(void)
 {
+    QFWCFG *fw_cfg;
+    QTestState *s;
+
     uint8_t buf[16];
     static const uint8_t uuid[16] = {
         0x46, 0x00, 0xcb, 0x32, 0x38, 0xec, 0x4b, 0x2f,
         0x8a, 0xcb, 0x81, 0xc6, 0xea, 0x54, 0xf2, 0xd8,
     };
 
+    s = qtest_init("-uuid 4600cb32-38ec-4b2f-8acb-81c6ea54f2d8");
+    fw_cfg = pc_fw_cfg_init(s);
+
     qfw_cfg_get(fw_cfg, FW_CFG_UUID, buf, 16);
     g_assert(memcmp(buf, uuid, sizeof(buf)) == 0);
+
+    pc_fw_cfg_uninit(fw_cfg);
+    qtest_quit(s);
+
 }
 
 static void test_fw_cfg_ram_size(void)
 {
+    QFWCFG *fw_cfg;
+    QTestState *s;
+
+    s = qtest_init("");
+    fw_cfg = pc_fw_cfg_init(s);
+
     g_assert_cmpint(qfw_cfg_get_u64(fw_cfg, FW_CFG_RAM_SIZE), ==, ram_size);
+
+    pc_fw_cfg_uninit(fw_cfg);
+    qtest_quit(s);
 }
 
 static void test_fw_cfg_nographic(void)
 {
+    QFWCFG *fw_cfg;
+    QTestState *s;
+
+    s = qtest_init("");
+    fw_cfg = pc_fw_cfg_init(s);
+
     g_assert_cmpint(qfw_cfg_get_u16(fw_cfg, FW_CFG_NOGRAPHIC), ==, 0);
+
+    pc_fw_cfg_uninit(fw_cfg);
+    qtest_quit(s);
 }
 
 static void test_fw_cfg_nb_cpus(void)
 {
+    QFWCFG *fw_cfg;
+    QTestState *s;
+
+    s = qtest_init("");
+    fw_cfg = pc_fw_cfg_init(s);
+
     g_assert_cmpint(qfw_cfg_get_u16(fw_cfg, FW_CFG_NB_CPUS), ==, nb_cpus);
+
+    pc_fw_cfg_uninit(fw_cfg);
+    qtest_quit(s);
 }
 
 static void test_fw_cfg_max_cpus(void)
 {
+    QFWCFG *fw_cfg;
+    QTestState *s;
+
+    s = qtest_init("");
+    fw_cfg = pc_fw_cfg_init(s);
+
     g_assert_cmpint(qfw_cfg_get_u16(fw_cfg, FW_CFG_MAX_CPUS), ==, max_cpus);
+    pc_fw_cfg_uninit(fw_cfg);
+    qtest_quit(s);
 }
 
 static void test_fw_cfg_numa(void)
 {
+    QFWCFG *fw_cfg;
+    QTestState *s;
     uint64_t *cpu_mask;
     uint64_t *node_mask;
 
+    s = qtest_init("");
+    fw_cfg = pc_fw_cfg_init(s);
+
     g_assert_cmpint(qfw_cfg_get_u64(fw_cfg, FW_CFG_NUMA), ==, nb_nodes);
 
     cpu_mask = g_new0(uint64_t, max_cpus);
@@ -92,24 +157,29 @@ static void test_fw_cfg_numa(void)
 
     g_free(node_mask);
     g_free(cpu_mask);
+    pc_fw_cfg_uninit(fw_cfg);
+    qtest_quit(s);
 }
 
 static void test_fw_cfg_boot_menu(void)
 {
+    QFWCFG *fw_cfg;
+    QTestState *s;
+
+    s = qtest_init("");
+    fw_cfg = pc_fw_cfg_init(s);
+
     g_assert_cmpint(qfw_cfg_get_u16(fw_cfg, FW_CFG_BOOT_MENU), ==, boot_menu);
+    pc_fw_cfg_uninit(fw_cfg);
+    qtest_quit(s);
 }
 
 int main(int argc, char **argv)
 {
-    QTestState *s;
     int ret;
 
     g_test_init(&argc, &argv, NULL);
 
-    s = qtest_init("-uuid 4600cb32-38ec-4b2f-8acb-81c6ea54f2d8");
-
-    fw_cfg = pc_fw_cfg_init(s);
-
     qtest_add_func("fw_cfg/signature", test_fw_cfg_signature);
     qtest_add_func("fw_cfg/id", test_fw_cfg_id);
     qtest_add_func("fw_cfg/uuid", test_fw_cfg_uuid);
@@ -128,7 +198,5 @@ int main(int argc, char **argv)
 
     ret = g_test_run();
 
-    qtest_quit(s);
-
     return ret;
 }
diff --git a/tests/libqos/fw_cfg.c b/tests/libqos/fw_cfg.c
index d0889d1e22..c6839c53c8 100644
--- a/tests/libqos/fw_cfg.c
+++ b/tests/libqos/fw_cfg.c
@@ -81,6 +81,11 @@ QFWCFG *mm_fw_cfg_init(QTestState *qts, uint64_t base)
     return fw_cfg;
 }
 
+void mm_fw_cfg_uninit(QFWCFG *fw_cfg)
+{
+    g_free(fw_cfg);
+}
+
 static void io_fw_cfg_select(QFWCFG *fw_cfg, uint16_t key)
 {
     qtest_outw(fw_cfg->qts, fw_cfg->base, key);
@@ -107,3 +112,8 @@ QFWCFG *io_fw_cfg_init(QTestState *qts, uint16_t base)
 
     return fw_cfg;
 }
+
+void io_fw_cfg_uninit(QFWCFG *fw_cfg)
+{
+    g_free(fw_cfg);
+}
diff --git a/tests/libqos/fw_cfg.h b/tests/libqos/fw_cfg.h
index 0353416af0..60de81e863 100644
--- a/tests/libqos/fw_cfg.h
+++ b/tests/libqos/fw_cfg.h
@@ -33,11 +33,18 @@ uint32_t qfw_cfg_get_u32(QFWCFG *fw_cfg, uint16_t key);
 uint64_t qfw_cfg_get_u64(QFWCFG *fw_cfg, uint16_t key);
 
 QFWCFG *mm_fw_cfg_init(QTestState *qts, uint64_t base);
+void mm_fw_cfg_uninit(QFWCFG *fw_cfg);
 QFWCFG *io_fw_cfg_init(QTestState *qts, uint16_t base);
+void io_fw_cfg_uninit(QFWCFG *fw_cfg);
 
 static inline QFWCFG *pc_fw_cfg_init(QTestState *qts)
 {
     return io_fw_cfg_init(qts, 0x510);
 }
 
+static inline void pc_fw_cfg_uninit(QFWCFG *fw_cfg)
+{
+    io_fw_cfg_uninit(fw_cfg);
+}
+
 #endif
-- 
2.17.1

[Qemu-devel] [PATCH v2 2/5] tests: fw_cfg: add a function to get the fw_cfg file

[Li Qiang]

This is useful to write qtest about fw_cfg file entry.

Signed-off-by: Li Qiang <liq3ea@163.com>
Acked-by: Thomas Huth <thuth@redhat.com>
---
 tests/libqos/fw_cfg.c | 45 +++++++++++++++++++++++++++++++++++++++++++
 tests/libqos/fw_cfg.h |  2 ++
 2 files changed, 47 insertions(+)

diff --git a/tests/libqos/fw_cfg.c b/tests/libqos/fw_cfg.c
index c6839c53c8..1f46258f96 100644
--- a/tests/libqos/fw_cfg.c
+++ b/tests/libqos/fw_cfg.c
@@ -16,6 +16,7 @@
 #include "libqos/fw_cfg.h"
 #include "libqtest.h"
 #include "qemu/bswap.h"
+#include "hw/nvram/fw_cfg.h"
 
 void qfw_cfg_select(QFWCFG *fw_cfg, uint16_t key)
 {
@@ -59,6 +60,50 @@ static void mm_fw_cfg_select(QFWCFG *fw_cfg, uint16_t key)
     qtest_writew(fw_cfg->qts, fw_cfg->base, key);
 }
 
+/*
+ * The caller need check the return value. When the return value is
+ * nonzero, it means that some bytes have been transferred.
+ *
+ * If the fw_cfg file in question is smaller than the allocated & passed-in
+ * buffer, then the buffer has been populated only in part.
+ *
+ * If the fw_cfg file in question is larger than the passed-in
+ * buffer, then the return value explains how much room would have been
+ * necessary in total. And, while the caller's buffer has been fully
+ * populated, it has received only a starting slice of the fw_cfg file.
+ */
+size_t qfw_cfg_get_file(QFWCFG *fw_cfg, const char *filename,
+                      void *data, size_t buflen)
+{
+    uint32_t count;
+    uint32_t i;
+    unsigned char *filesbuf = NULL;
+    size_t dsize;
+    FWCfgFile *pdir_entry;
+    size_t filesize = 0;
+
+    qfw_cfg_get(fw_cfg, FW_CFG_FILE_DIR, &count, sizeof(count));
+    count = be32_to_cpu(count);
+    dsize = sizeof(uint32_t) + count * sizeof(struct fw_cfg_file);
+    filesbuf = g_malloc(dsize);
+    qfw_cfg_get(fw_cfg, FW_CFG_FILE_DIR, filesbuf, dsize);
+    pdir_entry = (FWCfgFile *)(filesbuf + sizeof(uint32_t));
+    for (i = 0; i < count; ++i, ++pdir_entry) {
+        if (!strcmp(pdir_entry->name, filename)) {
+            uint32_t len = be32_to_cpu(pdir_entry->size);
+            uint16_t sel = be16_to_cpu(pdir_entry->select);
+            filesize = len;
+            if (len > buflen) {
+                len = buflen;
+            }
+            qfw_cfg_get(fw_cfg, sel, data, len);
+            break;
+        }
+    }
+    g_free(filesbuf);
+    return filesize;
+}
+
 static void mm_fw_cfg_read(QFWCFG *fw_cfg, void *data, size_t len)
 {
     uint8_t *ptr = data;
diff --git a/tests/libqos/fw_cfg.h b/tests/libqos/fw_cfg.h
index 60de81e863..13325cc4ff 100644
--- a/tests/libqos/fw_cfg.h
+++ b/tests/libqos/fw_cfg.h
@@ -31,6 +31,8 @@ void qfw_cfg_get(QFWCFG *fw_cfg, uint16_t key, void *data, size_t len);
 uint16_t qfw_cfg_get_u16(QFWCFG *fw_cfg, uint16_t key);
 uint32_t qfw_cfg_get_u32(QFWCFG *fw_cfg, uint16_t key);
 uint64_t qfw_cfg_get_u64(QFWCFG *fw_cfg, uint16_t key);
+size_t qfw_cfg_get_file(QFWCFG *fw_cfg, const char *filename,
+                        void *data, size_t buflen);
 
 QFWCFG *mm_fw_cfg_init(QTestState *qts, uint64_t base);
 void mm_fw_cfg_uninit(QFWCFG *fw_cfg);
-- 
2.17.1

[Qemu-devel] [PATCH v2 2/5] tests: fw_cfg: add a function to get the fw_cfg file

[Li Qiang]

This is useful to write qtest about fw_cfg file entry.

Signed-off-by: Li Qiang <liq3ea@163.com>
Acked-by: Thomas Huth <thuth@redhat.com>
---
 tests/libqos/fw_cfg.c | 45 +++++++++++++++++++++++++++++++++++++++++++
 tests/libqos/fw_cfg.h |  2 ++
 2 files changed, 47 insertions(+)

diff --git a/tests/libqos/fw_cfg.c b/tests/libqos/fw_cfg.c
index c6839c53c8..1f46258f96 100644
--- a/tests/libqos/fw_cfg.c
+++ b/tests/libqos/fw_cfg.c
@@ -16,6 +16,7 @@
 #include "libqos/fw_cfg.h"
 #include "libqtest.h"
 #include "qemu/bswap.h"
+#include "hw/nvram/fw_cfg.h"
 
 void qfw_cfg_select(QFWCFG *fw_cfg, uint16_t key)
 {
@@ -59,6 +60,50 @@ static void mm_fw_cfg_select(QFWCFG *fw_cfg, uint16_t key)
     qtest_writew(fw_cfg->qts, fw_cfg->base, key);
 }
 
+/*
+ * The caller need check the return value. When the return value is
+ * nonzero, it means that some bytes have been transferred.
+ *
+ * If the fw_cfg file in question is smaller than the allocated & passed-in
+ * buffer, then the buffer has been populated only in part.
+ *
+ * If the fw_cfg file in question is larger than the passed-in
+ * buffer, then the return value explains how much room would have been
+ * necessary in total. And, while the caller's buffer has been fully
+ * populated, it has received only a starting slice of the fw_cfg file.
+ */
+size_t qfw_cfg_get_file(QFWCFG *fw_cfg, const char *filename,
+                      void *data, size_t buflen)
+{
+    uint32_t count;
+    uint32_t i;
+    unsigned char *filesbuf = NULL;
+    size_t dsize;
+    FWCfgFile *pdir_entry;
+    size_t filesize = 0;
+
+    qfw_cfg_get(fw_cfg, FW_CFG_FILE_DIR, &count, sizeof(count));
+    count = be32_to_cpu(count);
+    dsize = sizeof(uint32_t) + count * sizeof(struct fw_cfg_file);
+    filesbuf = g_malloc(dsize);
+    qfw_cfg_get(fw_cfg, FW_CFG_FILE_DIR, filesbuf, dsize);
+    pdir_entry = (FWCfgFile *)(filesbuf + sizeof(uint32_t));
+    for (i = 0; i < count; ++i, ++pdir_entry) {
+        if (!strcmp(pdir_entry->name, filename)) {
+            uint32_t len = be32_to_cpu(pdir_entry->size);
+            uint16_t sel = be16_to_cpu(pdir_entry->select);
+            filesize = len;
+            if (len > buflen) {
+                len = buflen;
+            }
+            qfw_cfg_get(fw_cfg, sel, data, len);
+            break;
+        }
+    }
+    g_free(filesbuf);
+    return filesize;
+}
+
 static void mm_fw_cfg_read(QFWCFG *fw_cfg, void *data, size_t len)
 {
     uint8_t *ptr = data;
diff --git a/tests/libqos/fw_cfg.h b/tests/libqos/fw_cfg.h
index 60de81e863..13325cc4ff 100644
--- a/tests/libqos/fw_cfg.h
+++ b/tests/libqos/fw_cfg.h
@@ -31,6 +31,8 @@ void qfw_cfg_get(QFWCFG *fw_cfg, uint16_t key, void *data, size_t len);
 uint16_t qfw_cfg_get_u16(QFWCFG *fw_cfg, uint16_t key);
 uint32_t qfw_cfg_get_u32(QFWCFG *fw_cfg, uint16_t key);
 uint64_t qfw_cfg_get_u64(QFWCFG *fw_cfg, uint16_t key);
+size_t qfw_cfg_get_file(QFWCFG *fw_cfg, const char *filename,
+                        void *data, size_t buflen);
 
 QFWCFG *mm_fw_cfg_init(QTestState *qts, uint64_t base);
 void mm_fw_cfg_uninit(QFWCFG *fw_cfg);
-- 
2.17.1

[Qemu-devel] [PATCH v2 3/5] fw_cfg: reboot: store reboot-timeout as little endian

[Li Qiang]

So that if the guest and host endianess does not match it
can still work as expection.

Signed-off-by: Li Qiang <liq3ea@163.com>
---
 hw/nvram/fw_cfg.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 5c3a46ce6f..df4242fc9c 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -178,6 +178,7 @@ static void fw_cfg_reboot(FWCfgState *s)
 {
     const char *reboot_timeout = NULL;
     int64_t rt_val = -1;
+    uint32_t rt_le32;
 
     /* get user configuration */
     QemuOptsList *plist = qemu_find_opts("boot-opts");
@@ -194,7 +195,8 @@ static void fw_cfg_reboot(FWCfgState *s)
         }
     }
 
-    fw_cfg_add_file(s, "etc/boot-fail-wait", g_memdup(&rt_val, 4), 4);
+    rt_le32 = cpu_to_le32(rt_val);
+    fw_cfg_add_file(s, "etc/boot-fail-wait", g_memdup(&rt_le32, 4), 4);
 }
 
 static void fw_cfg_write(FWCfgState *s, uint8_t value)
-- 
2.17.1

[Qemu-devel] [PATCH v2 3/5] fw_cfg: reboot: store reboot-timeout as little endian

[Li Qiang]

So that if the guest and host endianess does not match it
can still work as expection.

Signed-off-by: Li Qiang <liq3ea@163.com>
---
 hw/nvram/fw_cfg.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 5c3a46ce6f..df4242fc9c 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -178,6 +178,7 @@ static void fw_cfg_reboot(FWCfgState *s)
 {
     const char *reboot_timeout = NULL;
     int64_t rt_val = -1;
+    uint32_t rt_le32;
 
     /* get user configuration */
     QemuOptsList *plist = qemu_find_opts("boot-opts");
@@ -194,7 +195,8 @@ static void fw_cfg_reboot(FWCfgState *s)
         }
     }
 
-    fw_cfg_add_file(s, "etc/boot-fail-wait", g_memdup(&rt_val, 4), 4);
+    rt_le32 = cpu_to_le32(rt_val);
+    fw_cfg_add_file(s, "etc/boot-fail-wait", g_memdup(&rt_le32, 4), 4);
 }
 
 static void fw_cfg_write(FWCfgState *s, uint8_t value)
-- 
2.17.1

[Qemu-devel] [PATCH v2 4/5] tests: fw_cfg: add reboot_timeout test case

[Li Qiang]

Signed-off-by: Li Qiang <liq3ea@163.com>
---
Change since v1:
Converting little endian reboot_timeout to cpu endian

 tests/fw_cfg-test.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/tests/fw_cfg-test.c b/tests/fw_cfg-test.c
index c22503619f..6c6add54db 100644
--- a/tests/fw_cfg-test.c
+++ b/tests/fw_cfg-test.c
@@ -15,6 +15,7 @@
 #include "libqtest.h"
 #include "standard-headers/linux/qemu_fw_cfg.h"
 #include "libqos/fw_cfg.h"
+#include "qemu/bswap.h"
 
 static uint64_t ram_size = 128 << 20;
 static uint16_t nb_cpus = 1;
@@ -174,6 +175,25 @@ static void test_fw_cfg_boot_menu(void)
     qtest_quit(s);
 }
 
+static void test_fw_cfg_reboot_timeout(void)
+{
+    QFWCFG *fw_cfg;
+    QTestState *s;
+    uint32_t reboot_timeout = 0;
+    size_t filesize;
+
+    s = qtest_init("-boot reboot-timeout=15");
+    fw_cfg = pc_fw_cfg_init(s);
+
+    filesize = qfw_cfg_get_file(fw_cfg, "etc/boot-fail-wait",
+                     &reboot_timeout, sizeof(reboot_timeout));
+    g_assert_cmpint(filesize, ==, sizeof(reboot_timeout));
+    reboot_timeout = le32_to_cpu(reboot_timeout);
+    g_assert_cmpint(reboot_timeout, ==, 15);
+    pc_fw_cfg_uninit(fw_cfg);
+    qtest_quit(s);
+}
+
 int main(int argc, char **argv)
 {
     int ret;
@@ -195,6 +215,7 @@ int main(int argc, char **argv)
     qtest_add_func("fw_cfg/max_cpus", test_fw_cfg_max_cpus);
     qtest_add_func("fw_cfg/numa", test_fw_cfg_numa);
     qtest_add_func("fw_cfg/boot_menu", test_fw_cfg_boot_menu);
+    qtest_add_func("fw_cfg/reboot_timeout", test_fw_cfg_reboot_timeout);
 
     ret = g_test_run();
 
-- 
2.17.1

[Qemu-devel] [PATCH v2 4/5] tests: fw_cfg: add reboot_timeout test case

[Li Qiang]

Signed-off-by: Li Qiang <liq3ea@163.com>
---
Change since v1:
Converting little endian reboot_timeout to cpu endian

 tests/fw_cfg-test.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/tests/fw_cfg-test.c b/tests/fw_cfg-test.c
index c22503619f..6c6add54db 100644
--- a/tests/fw_cfg-test.c
+++ b/tests/fw_cfg-test.c
@@ -15,6 +15,7 @@
 #include "libqtest.h"
 #include "standard-headers/linux/qemu_fw_cfg.h"
 #include "libqos/fw_cfg.h"
+#include "qemu/bswap.h"
 
 static uint64_t ram_size = 128 << 20;
 static uint16_t nb_cpus = 1;
@@ -174,6 +175,25 @@ static void test_fw_cfg_boot_menu(void)
     qtest_quit(s);
 }
 
+static void test_fw_cfg_reboot_timeout(void)
+{
+    QFWCFG *fw_cfg;
+    QTestState *s;
+    uint32_t reboot_timeout = 0;
+    size_t filesize;
+
+    s = qtest_init("-boot reboot-timeout=15");
+    fw_cfg = pc_fw_cfg_init(s);
+
+    filesize = qfw_cfg_get_file(fw_cfg, "etc/boot-fail-wait",
+                     &reboot_timeout, sizeof(reboot_timeout));
+    g_assert_cmpint(filesize, ==, sizeof(reboot_timeout));
+    reboot_timeout = le32_to_cpu(reboot_timeout);
+    g_assert_cmpint(reboot_timeout, ==, 15);
+    pc_fw_cfg_uninit(fw_cfg);
+    qtest_quit(s);
+}
+
 int main(int argc, char **argv)
 {
     int ret;
@@ -195,6 +215,7 @@ int main(int argc, char **argv)
     qtest_add_func("fw_cfg/max_cpus", test_fw_cfg_max_cpus);
     qtest_add_func("fw_cfg/numa", test_fw_cfg_numa);
     qtest_add_func("fw_cfg/boot_menu", test_fw_cfg_boot_menu);
+    qtest_add_func("fw_cfg/reboot_timeout", test_fw_cfg_reboot_timeout);
 
     ret = g_test_run();
 
-- 
2.17.1

[Qemu-devel] [PATCH v2 5/5] tests: fw_cfg: add splash time test case

[Li Qiang]

Signed-off-by: Li Qiang <liq3ea@163.com>
---
Change since v1:
Converting little endian splash time to cpu endian

 tests/fw_cfg-test.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/tests/fw_cfg-test.c b/tests/fw_cfg-test.c
index 6c6add54db..c6a3d6bb23 100644
--- a/tests/fw_cfg-test.c
+++ b/tests/fw_cfg-test.c
@@ -194,6 +194,25 @@ static void test_fw_cfg_reboot_timeout(void)
     qtest_quit(s);
 }
 
+static void test_fw_cfg_splash_time(void)
+{
+    QFWCFG *fw_cfg;
+    QTestState *s;
+    uint16_t splash_time = 0;
+    size_t filesize;
+
+    s = qtest_init("-boot splash-time=12");
+    fw_cfg = pc_fw_cfg_init(s);
+
+    filesize = qfw_cfg_get_file(fw_cfg, "etc/boot-menu-wait",
+                     &splash_time, sizeof(splash_time));
+    g_assert_cmpint(filesize, ==, sizeof(splash_time));
+    splash_time = le16_to_cpu(splash_time);
+    g_assert_cmpint(splash_time, ==, 12);
+    pc_fw_cfg_uninit(fw_cfg);
+    qtest_quit(s);
+}
+
 int main(int argc, char **argv)
 {
     int ret;
@@ -216,6 +235,7 @@ int main(int argc, char **argv)
     qtest_add_func("fw_cfg/numa", test_fw_cfg_numa);
     qtest_add_func("fw_cfg/boot_menu", test_fw_cfg_boot_menu);
     qtest_add_func("fw_cfg/reboot_timeout", test_fw_cfg_reboot_timeout);
+    qtest_add_func("fw_cfg/splash_time", test_fw_cfg_splash_time);
 
     ret = g_test_run();
 
-- 
2.17.1

[Qemu-devel] [PATCH v2 5/5] tests: fw_cfg: add splash time test case

[Li Qiang]

Signed-off-by: Li Qiang <liq3ea@163.com>
---
Change since v1:
Converting little endian splash time to cpu endian

 tests/fw_cfg-test.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/tests/fw_cfg-test.c b/tests/fw_cfg-test.c
index 6c6add54db..c6a3d6bb23 100644
--- a/tests/fw_cfg-test.c
+++ b/tests/fw_cfg-test.c
@@ -194,6 +194,25 @@ static void test_fw_cfg_reboot_timeout(void)
     qtest_quit(s);
 }
 
+static void test_fw_cfg_splash_time(void)
+{
+    QFWCFG *fw_cfg;
+    QTestState *s;
+    uint16_t splash_time = 0;
+    size_t filesize;
+
+    s = qtest_init("-boot splash-time=12");
+    fw_cfg = pc_fw_cfg_init(s);
+
+    filesize = qfw_cfg_get_file(fw_cfg, "etc/boot-menu-wait",
+                     &splash_time, sizeof(splash_time));
+    g_assert_cmpint(filesize, ==, sizeof(splash_time));
+    splash_time = le16_to_cpu(splash_time);
+    g_assert_cmpint(splash_time, ==, 12);
+    pc_fw_cfg_uninit(fw_cfg);
+    qtest_quit(s);
+}
+
 int main(int argc, char **argv)
 {
     int ret;
@@ -216,6 +235,7 @@ int main(int argc, char **argv)
     qtest_add_func("fw_cfg/numa", test_fw_cfg_numa);
     qtest_add_func("fw_cfg/boot_menu", test_fw_cfg_boot_menu);
     qtest_add_func("fw_cfg/reboot_timeout", test_fw_cfg_reboot_timeout);
+    qtest_add_func("fw_cfg/splash_time", test_fw_cfg_splash_time);
 
     ret = g_test_run();
 
-- 
2.17.1

Re: [Qemu-devel] [PATCH v2 0/5] fw_cfg_test refactor and add two test cases

[Thomas Huth]

On 24/04/2019 16.06, Li Qiang wrote:
> In the disscuss of adding reboot timeout test case:
> https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg03304.html
> 
> Philippe suggested we should uses the only related option for one
> specific test. However currently we uses one QTestState for all the
> test cases. In order to achieve Philippe's idea, I split the test case
> for its own QTestState. As this patchset has changed a lot, I don't bump
> the version.
> 
> Change since v1:
> Add a patch to store the reboot_timeout as little endian
> Fix the endian issue per Thomas's review

The test still aborts on a big endian host:

$ QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 tests/fw_cfg-test
/x86_64/fw_cfg/signature: OK
/x86_64/fw_cfg/id: OK
/x86_64/fw_cfg/uuid: OK
/x86_64/fw_cfg/ram_size: OK
/x86_64/fw_cfg/nographic: OK
/x86_64/fw_cfg/nb_cpus: OK
/x86_64/fw_cfg/max_cpus: OK
/x86_64/fw_cfg/numa: OK
/x86_64/fw_cfg/boot_menu: OK
/x86_64/fw_cfg/reboot_timeout: **
ERROR:/home/thuth/devel/qemu/tests/fw_cfg-test.c:190:test_fw_cfg_reboot_timeout:
assertion failed (reboot_timeout == 15): (251658240 == 15)
Aborted

251658240 is 0x0F000000, i.e. a byte-swapped 0xf = 15 ... i.e. you still
got an endianess issue somewhere in the code.

 Thomas

Re: [Qemu-devel] [PATCH v2 0/5] fw_cfg_test refactor and add two test cases

[Thomas Huth]

On 24/04/2019 16.06, Li Qiang wrote:
> In the disscuss of adding reboot timeout test case:
> https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg03304.html
> 
> Philippe suggested we should uses the only related option for one
> specific test. However currently we uses one QTestState for all the
> test cases. In order to achieve Philippe's idea, I split the test case
> for its own QTestState. As this patchset has changed a lot, I don't bump
> the version.
> 
> Change since v1:
> Add a patch to store the reboot_timeout as little endian
> Fix the endian issue per Thomas's review

The test still aborts on a big endian host:

$ QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 tests/fw_cfg-test
/x86_64/fw_cfg/signature: OK
/x86_64/fw_cfg/id: OK
/x86_64/fw_cfg/uuid: OK
/x86_64/fw_cfg/ram_size: OK
/x86_64/fw_cfg/nographic: OK
/x86_64/fw_cfg/nb_cpus: OK
/x86_64/fw_cfg/max_cpus: OK
/x86_64/fw_cfg/numa: OK
/x86_64/fw_cfg/boot_menu: OK
/x86_64/fw_cfg/reboot_timeout: **
ERROR:/home/thuth/devel/qemu/tests/fw_cfg-test.c:190:test_fw_cfg_reboot_timeout:
assertion failed (reboot_timeout == 15): (251658240 == 15)
Aborted

251658240 is 0x0F000000, i.e. a byte-swapped 0xf = 15 ... i.e. you still
got an endianess issue somewhere in the code.

 Thomas

Re: [Qemu-devel] [PATCH v2 0/5] fw_cfg_test refactor and add two test cases

[Li Qiang]

Thomas Huth <thuth@redhat.com> 于2019年4月25日周四 下午5:57写道：

> On 24/04/2019 16.06, Li Qiang wrote:
> > In the disscuss of adding reboot timeout test case:
> > https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg03304.html
> >
> > Philippe suggested we should uses the only related option for one
> > specific test. However currently we uses one QTestState for all the
> > test cases. In order to achieve Philippe's idea, I split the test case
> > for its own QTestState. As this patchset has changed a lot, I don't bump
> > the version.
> >
> > Change since v1:
> > Add a patch to store the reboot_timeout as little endian
> > Fix the endian issue per Thomas's review
>
> The test still aborts on a big endian host:
>
> $ QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 tests/fw_cfg-test
> /x86_64/fw_cfg/signature: OK
> /x86_64/fw_cfg/id: OK
> /x86_64/fw_cfg/uuid: OK
> /x86_64/fw_cfg/ram_size: OK
> /x86_64/fw_cfg/nographic: OK
> /x86_64/fw_cfg/nb_cpus: OK
> /x86_64/fw_cfg/max_cpus: OK
> /x86_64/fw_cfg/numa: OK
> /x86_64/fw_cfg/boot_menu: OK
> /x86_64/fw_cfg/reboot_timeout: **
>
> ERROR:/home/thuth/devel/qemu/tests/fw_cfg-test.c:190:test_fw_cfg_reboot_timeout:
> assertion failed (reboot_timeout == 15): (251658240 == 15)
> Aborted
>
> 251658240 is 0x0F000000, i.e. a byte-swapped 0xf = 15 ... i.e. you still
> got an endianess issue somewhere in the code.
>


Hmmmm,

I have thought a long time, still can't point where is wrong.

Let's from the result:
0x0f000000 in the big endian laid as this:
low ---> high
0x0f 00 00 00

As I have swapped before the compare so it is read as this:
low ---> high
00 00 00 0x0f

However from the store side:
the 15 in big endian is:
low ---> high
00 00 00 0x0f

But Before I store it, I convert it to little endian, so following should
be stored:
low ---> high
0x0f 00 00 00

Do you apply the patch 3 and recompile the qemu binary?
If it is, I may need your help as I have no big endian host device.

You can debug and  inspect the memory layout and point out where is wrong.

Thanks,
Li Qiang







>
>  Thomas
>

Re: [Qemu-devel] [PATCH v2 0/5] fw_cfg_test refactor and add two test cases

[Li Qiang]

Thomas Huth <thuth@redhat.com> 于2019年4月25日周四 下午5:57写道：

> On 24/04/2019 16.06, Li Qiang wrote:
> > In the disscuss of adding reboot timeout test case:
> > https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg03304.html
> >
> > Philippe suggested we should uses the only related option for one
> > specific test. However currently we uses one QTestState for all the
> > test cases. In order to achieve Philippe's idea, I split the test case
> > for its own QTestState. As this patchset has changed a lot, I don't bump
> > the version.
> >
> > Change since v1:
> > Add a patch to store the reboot_timeout as little endian
> > Fix the endian issue per Thomas's review
>
> The test still aborts on a big endian host:
>
> $ QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 tests/fw_cfg-test
> /x86_64/fw_cfg/signature: OK
> /x86_64/fw_cfg/id: OK
> /x86_64/fw_cfg/uuid: OK
> /x86_64/fw_cfg/ram_size: OK
> /x86_64/fw_cfg/nographic: OK
> /x86_64/fw_cfg/nb_cpus: OK
> /x86_64/fw_cfg/max_cpus: OK
> /x86_64/fw_cfg/numa: OK
> /x86_64/fw_cfg/boot_menu: OK
> /x86_64/fw_cfg/reboot_timeout: **
>
> ERROR:/home/thuth/devel/qemu/tests/fw_cfg-test.c:190:test_fw_cfg_reboot_timeout:
> assertion failed (reboot_timeout == 15): (251658240 == 15)
> Aborted
>
> 251658240 is 0x0F000000, i.e. a byte-swapped 0xf = 15 ... i.e. you still
> got an endianess issue somewhere in the code.
>


Hmmmm,

I have thought a long time, still can't point where is wrong.

Let's from the result:
0x0f000000 in the big endian laid as this:
low ---> high
0x0f 00 00 00

As I have swapped before the compare so it is read as this:
low ---> high
00 00 00 0x0f

However from the store side:
the 15 in big endian is:
low ---> high
00 00 00 0x0f

But Before I store it, I convert it to little endian, so following should
be stored:
low ---> high
0x0f 00 00 00

Do you apply the patch 3 and recompile the qemu binary?
If it is, I may need your help as I have no big endian host device.

You can debug and  inspect the memory layout and point out where is wrong.

Thanks,
Li Qiang







>
>  Thomas
>

Re: [Qemu-devel] [PATCH v2 0/5] fw_cfg_test refactor and add two test cases

[Li Qiang]

Li Qiang <liq3ea@gmail.com> 于2019年4月25日周四 下午10:29写道：

>
>
> Thomas Huth <thuth@redhat.com> 于2019年4月25日周四 下午5:57写道：
>
>> On 24/04/2019 16.06, Li Qiang wrote:
>> > In the disscuss of adding reboot timeout test case:
>> > https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg03304.html
>> >
>> > Philippe suggested we should uses the only related option for one
>> > specific test. However currently we uses one QTestState for all the
>> > test cases. In order to achieve Philippe's idea, I split the test case
>> > for its own QTestState. As this patchset has changed a lot, I don't bump
>> > the version.
>> >
>> > Change since v1:
>> > Add a patch to store the reboot_timeout as little endian
>> > Fix the endian issue per Thomas's review
>>
>> The test still aborts on a big endian host:
>>
>> $ QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 tests/fw_cfg-test
>> /x86_64/fw_cfg/signature: OK
>> /x86_64/fw_cfg/id: OK
>> /x86_64/fw_cfg/uuid: OK
>> /x86_64/fw_cfg/ram_size: OK
>> /x86_64/fw_cfg/nographic: OK
>> /x86_64/fw_cfg/nb_cpus: OK
>> /x86_64/fw_cfg/max_cpus: OK
>> /x86_64/fw_cfg/numa: OK
>> /x86_64/fw_cfg/boot_menu: OK
>> /x86_64/fw_cfg/reboot_timeout: **
>>
>> ERROR:/home/thuth/devel/qemu/tests/fw_cfg-test.c:190:test_fw_cfg_reboot_timeout:
>> assertion failed (reboot_timeout == 15): (251658240 == 15)
>> Aborted
>>
>> 251658240 is 0x0F000000, i.e. a byte-swapped 0xf = 15 ... i.e. you still
>> got an endianess issue somewhere in the code.
>>
>
>
> Hmmmm,
>
> I have thought a long time, still can't point where is wrong.
>
> Let's from the result:
> 0x0f000000 in the big endian laid as this:
> low ---> high
> 0x0f 00 00 00
>
> As I have swapped before the compare so it is read as this:
> low ---> high
> 00 00 00 0x0f
>
> However from the store side:
> the 15 in big endian is:
> low ---> high
> 00 00 00 0x0f
>
> But Before I store it, I convert it to little endian, so following should
> be stored:
> low ---> high
> 0x0f 00 00 00
>
> Do you apply the patch 3 and recompile the qemu binary?
>


Hello Thomas,
I have tested again this and just store it as big endian(so that the
store/load has different endianness),
I don't see any error.

Also, can we add these test sceneries(big-endian host) in our CI? so that
the bot can report for every commit.


Thanks,
Li Qiang



If it is, I may need your help as I have no big endian host device.
>
> You can debug and  inspect the memory layout and point out where is wrong.
>
> Thanks,
> Li Qiang
>
>
>
>
>
>
>
>>
>>  Thomas
>>
>

Re: [Qemu-devel] [PATCH v2 0/5] fw_cfg_test refactor and add two test cases

[Li Qiang]

Li Qiang <liq3ea@gmail.com> 于2019年4月25日周四 下午10:29写道：

>
>
> Thomas Huth <thuth@redhat.com> 于2019年4月25日周四 下午5:57写道：
>
>> On 24/04/2019 16.06, Li Qiang wrote:
>> > In the disscuss of adding reboot timeout test case:
>> > https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg03304.html
>> >
>> > Philippe suggested we should uses the only related option for one
>> > specific test. However currently we uses one QTestState for all the
>> > test cases. In order to achieve Philippe's idea, I split the test case
>> > for its own QTestState. As this patchset has changed a lot, I don't bump
>> > the version.
>> >
>> > Change since v1:
>> > Add a patch to store the reboot_timeout as little endian
>> > Fix the endian issue per Thomas's review
>>
>> The test still aborts on a big endian host:
>>
>> $ QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 tests/fw_cfg-test
>> /x86_64/fw_cfg/signature: OK
>> /x86_64/fw_cfg/id: OK
>> /x86_64/fw_cfg/uuid: OK
>> /x86_64/fw_cfg/ram_size: OK
>> /x86_64/fw_cfg/nographic: OK
>> /x86_64/fw_cfg/nb_cpus: OK
>> /x86_64/fw_cfg/max_cpus: OK
>> /x86_64/fw_cfg/numa: OK
>> /x86_64/fw_cfg/boot_menu: OK
>> /x86_64/fw_cfg/reboot_timeout: **
>>
>> ERROR:/home/thuth/devel/qemu/tests/fw_cfg-test.c:190:test_fw_cfg_reboot_timeout:
>> assertion failed (reboot_timeout == 15): (251658240 == 15)
>> Aborted
>>
>> 251658240 is 0x0F000000, i.e. a byte-swapped 0xf = 15 ... i.e. you still
>> got an endianess issue somewhere in the code.
>>
>
>
> Hmmmm,
>
> I have thought a long time, still can't point where is wrong.
>
> Let's from the result:
> 0x0f000000 in the big endian laid as this:
> low ---> high
> 0x0f 00 00 00
>
> As I have swapped before the compare so it is read as this:
> low ---> high
> 00 00 00 0x0f
>
> However from the store side:
> the 15 in big endian is:
> low ---> high
> 00 00 00 0x0f
>
> But Before I store it, I convert it to little endian, so following should
> be stored:
> low ---> high
> 0x0f 00 00 00
>
> Do you apply the patch 3 and recompile the qemu binary?
>


Hello Thomas,
I have tested again this and just store it as big endian(so that the
store/load has different endianness),
I don't see any error.

Also, can we add these test sceneries(big-endian host) in our CI? so that
the bot can report for every commit.


Thanks,
Li Qiang



If it is, I may need your help as I have no big endian host device.
>
> You can debug and  inspect the memory layout and point out where is wrong.
>
> Thanks,
> Li Qiang
>
>
>
>
>
>
>
>>
>>  Thomas
>>
>

Re: [Qemu-devel] [PATCH v2 0/5] fw_cfg_test refactor and add two test cases

[Thomas Huth]

On 29/04/2019 07.09, Li Qiang wrote:
> 
> 
> Li Qiang <liq3ea@gmail.com <mailto:liq3ea@gmail.com>> 于2019年4月25日周
> 四 下午10:29写道：
> 
> 
> 
>     Thomas Huth <thuth@redhat.com <mailto:thuth@redhat.com>> 于2019年4月
>     25日周四 下午5:57写道：
> 
>         On 24/04/2019 16.06, Li Qiang wrote:
>         > In the disscuss of adding reboot timeout test case:
>         >
>         https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg03304.html
>         >
>         > Philippe suggested we should uses the only related option for one
>         > specific test. However currently we uses one QTestState for
>         all the
>         > test cases. In order to achieve Philippe's idea, I split the
>         test case
>         > for its own QTestState. As this patchset has changed a lot, I
>         don't bump
>         > the version.
>         >
>         > Change since v1:
>         > Add a patch to store the reboot_timeout as little endian
>         > Fix the endian issue per Thomas's review
> 
>         The test still aborts on a big endian host:
> 
>         $ QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64
>         tests/fw_cfg-test
>         /x86_64/fw_cfg/signature: OK
>         /x86_64/fw_cfg/id: OK
>         /x86_64/fw_cfg/uuid: OK
>         /x86_64/fw_cfg/ram_size: OK
>         /x86_64/fw_cfg/nographic: OK
>         /x86_64/fw_cfg/nb_cpus: OK
>         /x86_64/fw_cfg/max_cpus: OK
>         /x86_64/fw_cfg/numa: OK
>         /x86_64/fw_cfg/boot_menu: OK
>         /x86_64/fw_cfg/reboot_timeout: **
>         ERROR:/home/thuth/devel/qemu/tests/fw_cfg-test.c:190:test_fw_cfg_reboot_timeout:
>         assertion failed (reboot_timeout == 15): (251658240 == 15)
>         Aborted
> 
>         251658240 is 0x0F000000, i.e. a byte-swapped 0xf = 15 ... i.e.
>         you still
>         got an endianess issue somewhere in the code.
> 
> 
> 
>     Hmmmm,
> 
>     I have thought a long time, still can't point where is wrong.
> 
>     Let's from the result: 
>     0x0f000000 in the big endian laid as this:
>     low ---> high
>     0x0f 00 00 00
> 
>     As I have swapped before the compare so it is read as this:
>     low ---> high
>     00 00 00 0x0f
> 
>     However from the store side:
>     the 15 in big endian is:
>     low ---> high
>     00 00 00 0x0f
> 
>     But Before I store it, I convert it to little endian, so following
>     should be stored:
>     low ---> high
>     0x0f 00 00 00
> 
>     Do you apply the patch 3 and recompile the qemu binary?
> 
> 
> 
> Hello Thomas, 
> I have tested again this and just store it as big endian(so that the
> store/load has different endianness), 
> I don't see any error. 

Uh, now this is embarrassing... I just tried again to see whether I
could find the issue, but now the test passes without problems!
I guess I simply only did a "make tests/fw_cfg-test" before and forgot
to recompile qemu itself. Big sorry for this!

Anyway, patch series works fine for me, so for the series:

Tested-by: Thomas Huth <thuth@redhat.com>

> Also, can we add these test sceneries(big-endian host) in our CI? so
> that the bot can report for every commit.

Patchew only runs on x86, but Peter has some big endian hosts for his
acceptance tests - so problems should be found during PULL requests at
least.

 Thomas

Re: [Qemu-devel] [PATCH v2 0/5] fw_cfg_test refactor and add two test cases

[Thomas Huth]

On 29/04/2019 07.09, Li Qiang wrote:
> 
> 
> Li Qiang <liq3ea@gmail.com <mailto:liq3ea@gmail.com>> 于2019年4月25日周
> 四 下午10:29写道：
> 
> 
> 
>     Thomas Huth <thuth@redhat.com <mailto:thuth@redhat.com>> 于2019年4月
>     25日周四 下午5:57写道：
> 
>         On 24/04/2019 16.06, Li Qiang wrote:
>         > In the disscuss of adding reboot timeout test case:
>         >
>         https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg03304.html
>         >
>         > Philippe suggested we should uses the only related option for one
>         > specific test. However currently we uses one QTestState for
>         all the
>         > test cases. In order to achieve Philippe's idea, I split the
>         test case
>         > for its own QTestState. As this patchset has changed a lot, I
>         don't bump
>         > the version.
>         >
>         > Change since v1:
>         > Add a patch to store the reboot_timeout as little endian
>         > Fix the endian issue per Thomas's review
> 
>         The test still aborts on a big endian host:
> 
>         $ QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64
>         tests/fw_cfg-test
>         /x86_64/fw_cfg/signature: OK
>         /x86_64/fw_cfg/id: OK
>         /x86_64/fw_cfg/uuid: OK
>         /x86_64/fw_cfg/ram_size: OK
>         /x86_64/fw_cfg/nographic: OK
>         /x86_64/fw_cfg/nb_cpus: OK
>         /x86_64/fw_cfg/max_cpus: OK
>         /x86_64/fw_cfg/numa: OK
>         /x86_64/fw_cfg/boot_menu: OK
>         /x86_64/fw_cfg/reboot_timeout: **
>         ERROR:/home/thuth/devel/qemu/tests/fw_cfg-test.c:190:test_fw_cfg_reboot_timeout:
>         assertion failed (reboot_timeout == 15): (251658240 == 15)
>         Aborted
> 
>         251658240 is 0x0F000000, i.e. a byte-swapped 0xf = 15 ... i.e.
>         you still
>         got an endianess issue somewhere in the code.
> 
> 
> 
>     Hmmmm,
> 
>     I have thought a long time, still can't point where is wrong.
> 
>     Let's from the result: 
>     0x0f000000 in the big endian laid as this:
>     low ---> high
>     0x0f 00 00 00
> 
>     As I have swapped before the compare so it is read as this:
>     low ---> high
>     00 00 00 0x0f
> 
>     However from the store side:
>     the 15 in big endian is:
>     low ---> high
>     00 00 00 0x0f
> 
>     But Before I store it, I convert it to little endian, so following
>     should be stored:
>     low ---> high
>     0x0f 00 00 00
> 
>     Do you apply the patch 3 and recompile the qemu binary?
> 
> 
> 
> Hello Thomas, 
> I have tested again this and just store it as big endian(so that the
> store/load has different endianness), 
> I don't see any error. 

Uh, now this is embarrassing... I just tried again to see whether I
could find the issue, but now the test passes without problems!
I guess I simply only did a "make tests/fw_cfg-test" before and forgot
to recompile qemu itself. Big sorry for this!

Anyway, patch series works fine for me, so for the series:

Tested-by: Thomas Huth <thuth@redhat.com>

> Also, can we add these test sceneries(big-endian host) in our CI? so
> that the bot can report for every commit.

Patchew only runs on x86, but Peter has some big endian hosts for his
acceptance tests - so problems should be found during PULL requests at
least.

 Thomas

Re: [Qemu-devel] [PATCH v2 0/5] fw_cfg_test refactor and add two test cases

[Li Qiang]

Thomas Huth <thuth@redhat.com> 于2019年4月29日周一 下午9:18写道：

> On 29/04/2019 07.09, Li Qiang wrote:
> >
> >
> > Li Qiang <liq3ea@gmail.com <mailto:liq3ea@gmail.com>> 于2019年4月25日周
> > 四 下午10:29写道：
> >
> >
> >
> >     Thomas Huth <thuth@redhat.com <mailto:thuth@redhat.com>> 于2019年4月
> >     25日周四 下午5:57写道：
> >
> >         On 24/04/2019 16.06, Li Qiang wrote:
> >         > In the disscuss of adding reboot timeout test case:
> >         >
> >
> https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg03304.html
> >         >
> >         > Philippe suggested we should uses the only related option for
> one
> >         > specific test. However currently we uses one QTestState for
> >         all the
> >         > test cases. In order to achieve Philippe's idea, I split the
> >         test case
> >         > for its own QTestState. As this patchset has changed a lot, I
> >         don't bump
> >         > the version.
> >         >
> >         > Change since v1:
> >         > Add a patch to store the reboot_timeout as little endian
> >         > Fix the endian issue per Thomas's review
> >
> >         The test still aborts on a big endian host:
> >
> >         $ QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64
> >         tests/fw_cfg-test
> >         /x86_64/fw_cfg/signature: OK
> >         /x86_64/fw_cfg/id: OK
> >         /x86_64/fw_cfg/uuid: OK
> >         /x86_64/fw_cfg/ram_size: OK
> >         /x86_64/fw_cfg/nographic: OK
> >         /x86_64/fw_cfg/nb_cpus: OK
> >         /x86_64/fw_cfg/max_cpus: OK
> >         /x86_64/fw_cfg/numa: OK
> >         /x86_64/fw_cfg/boot_menu: OK
> >         /x86_64/fw_cfg/reboot_timeout: **
> >
>  ERROR:/home/thuth/devel/qemu/tests/fw_cfg-test.c:190:test_fw_cfg_reboot_timeout:
> >         assertion failed (reboot_timeout == 15): (251658240 == 15)
> >         Aborted
> >
> >         251658240 is 0x0F000000, i.e. a byte-swapped 0xf = 15 ... i.e.
> >         you still
> >         got an endianess issue somewhere in the code.
> >
> >
> >
> >     Hmmmm,
> >
> >     I have thought a long time, still can't point where is wrong.
> >
> >     Let's from the result:
> >     0x0f000000 in the big endian laid as this:
> >     low ---> high
> >     0x0f 00 00 00
> >
> >     As I have swapped before the compare so it is read as this:
> >     low ---> high
> >     00 00 00 0x0f
> >
> >     However from the store side:
> >     the 15 in big endian is:
> >     low ---> high
> >     00 00 00 0x0f
> >
> >     But Before I store it, I convert it to little endian, so following
> >     should be stored:
> >     low ---> high
> >     0x0f 00 00 00
> >
> >     Do you apply the patch 3 and recompile the qemu binary?
> >
> >
> >
> > Hello Thomas,
> > I have tested again this and just store it as big endian(so that the
> > store/load has different endianness),
> > I don't see any error.
>
> Uh, now this is embarrassing... I just tried again to see whether I
> could find the issue, but now the test passes without problems!
> I guess I simply only did a "make tests/fw_cfg-test" before and forgot
> to recompile qemu itself. Big sorry for this!
>
> Anyway, patch series works fine for me, so for the series:
>
> Tested-by: Thomas Huth <thuth@redhat.com>
>
>

OK, Thanks Thomas.

Philippe maybe you can take a look at this series and merge it.

Thanks,
Li Qiang




> > Also, can we add these test sceneries(big-endian host) in our CI? so
> > that the bot can report for every commit.
>
> Patchew only runs on x86, but Peter has some big endian hosts for his
> acceptance tests - so problems should be found during PULL requests at
> least.
>
>  Thomas
>

Re: [Qemu-devel] [PATCH v2 0/5] fw_cfg_test refactor and add two test cases

[Li Qiang]

Thomas Huth <thuth@redhat.com> 于2019年4月29日周一 下午9:18写道：

> On 29/04/2019 07.09, Li Qiang wrote:
> >
> >
> > Li Qiang <liq3ea@gmail.com <mailto:liq3ea@gmail.com>> 于2019年4月25日周
> > 四 下午10:29写道：
> >
> >
> >
> >     Thomas Huth <thuth@redhat.com <mailto:thuth@redhat.com>> 于2019年4月
> >     25日周四 下午5:57写道：
> >
> >         On 24/04/2019 16.06, Li Qiang wrote:
> >         > In the disscuss of adding reboot timeout test case:
> >         >
> >
> https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg03304.html
> >         >
> >         > Philippe suggested we should uses the only related option for
> one
> >         > specific test. However currently we uses one QTestState for
> >         all the
> >         > test cases. In order to achieve Philippe's idea, I split the
> >         test case
> >         > for its own QTestState. As this patchset has changed a lot, I
> >         don't bump
> >         > the version.
> >         >
> >         > Change since v1:
> >         > Add a patch to store the reboot_timeout as little endian
> >         > Fix the endian issue per Thomas's review
> >
> >         The test still aborts on a big endian host:
> >
> >         $ QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64
> >         tests/fw_cfg-test
> >         /x86_64/fw_cfg/signature: OK
> >         /x86_64/fw_cfg/id: OK
> >         /x86_64/fw_cfg/uuid: OK
> >         /x86_64/fw_cfg/ram_size: OK
> >         /x86_64/fw_cfg/nographic: OK
> >         /x86_64/fw_cfg/nb_cpus: OK
> >         /x86_64/fw_cfg/max_cpus: OK
> >         /x86_64/fw_cfg/numa: OK
> >         /x86_64/fw_cfg/boot_menu: OK
> >         /x86_64/fw_cfg/reboot_timeout: **
> >
>  ERROR:/home/thuth/devel/qemu/tests/fw_cfg-test.c:190:test_fw_cfg_reboot_timeout:
> >         assertion failed (reboot_timeout == 15): (251658240 == 15)
> >         Aborted
> >
> >         251658240 is 0x0F000000, i.e. a byte-swapped 0xf = 15 ... i.e.
> >         you still
> >         got an endianess issue somewhere in the code.
> >
> >
> >
> >     Hmmmm,
> >
> >     I have thought a long time, still can't point where is wrong.
> >
> >     Let's from the result:
> >     0x0f000000 in the big endian laid as this:
> >     low ---> high
> >     0x0f 00 00 00
> >
> >     As I have swapped before the compare so it is read as this:
> >     low ---> high
> >     00 00 00 0x0f
> >
> >     However from the store side:
> >     the 15 in big endian is:
> >     low ---> high
> >     00 00 00 0x0f
> >
> >     But Before I store it, I convert it to little endian, so following
> >     should be stored:
> >     low ---> high
> >     0x0f 00 00 00
> >
> >     Do you apply the patch 3 and recompile the qemu binary?
> >
> >
> >
> > Hello Thomas,
> > I have tested again this and just store it as big endian(so that the
> > store/load has different endianness),
> > I don't see any error.
>
> Uh, now this is embarrassing... I just tried again to see whether I
> could find the issue, but now the test passes without problems!
> I guess I simply only did a "make tests/fw_cfg-test" before and forgot
> to recompile qemu itself. Big sorry for this!
>
> Anyway, patch series works fine for me, so for the series:
>
> Tested-by: Thomas Huth <thuth@redhat.com>
>
>

OK, Thanks Thomas.

Philippe maybe you can take a look at this series and merge it.

Thanks,
Li Qiang




> > Also, can we add these test sceneries(big-endian host) in our CI? so
> > that the bot can report for every commit.
>
> Patchew only runs on x86, but Peter has some big endian hosts for his
> acceptance tests - so problems should be found during PULL requests at
> least.
>
>  Thomas
>

Re: [Qemu-devel] [PATCH v2 0/5] fw_cfg_test refactor and add two test cases

[Li Qiang]

Ping.... this serials.

Thanks,
Li Qiang

Li Qiang <liq3ea@163.com> 于2019年4月24日周三 下午10:07写道：

> In the disscuss of adding reboot timeout test case:
> https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg03304.html
>
> Philippe suggested we should uses the only related option for one
> specific test. However currently we uses one QTestState for all the
> test cases. In order to achieve Philippe's idea, I split the test case
> for its own QTestState. As this patchset has changed a lot, I don't bump
> the version.
>
> Change since v1:
> Add a patch to store the reboot_timeout as little endian
> Fix the endian issue per Thomas's review
>
> Li Qiang (5):
>   tests: refactor fw_cfg_test
>   tests: fw_cfg: add a function to get the fw_cfg file
>   fw_cfg: reboot: store reboot-timeout as little endian
>   tests: fw_cfg: add reboot_timeout test case
>   tests: fw_cfg: add splash time test case
>
>  hw/nvram/fw_cfg.c     |   4 +-
>  tests/fw_cfg-test.c   | 125 +++++++++++++++++++++++++++++++++++++++---
>  tests/libqos/fw_cfg.c |  55 +++++++++++++++++++
>  tests/libqos/fw_cfg.h |   9 +++
>  4 files changed, 184 insertions(+), 9 deletions(-)
>
> --
> 2.17.1
>
>
>

Re: [Qemu-devel] [PATCH v2 0/5] fw_cfg_test refactor and add two test cases

[Li Qiang]

Ping.....

Li Qiang <liq3ea@gmail.com> 于2019年5月9日周四 下午5:57写道：

> Ping.... this serials.
>
> Thanks,
> Li Qiang
>
> Li Qiang <liq3ea@163.com> 于2019年4月24日周三 下午10:07写道：
>
>> In the disscuss of adding reboot timeout test case:
>> https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg03304.html
>>
>> Philippe suggested we should uses the only related option for one
>> specific test. However currently we uses one QTestState for all the
>> test cases. In order to achieve Philippe's idea, I split the test case
>> for its own QTestState. As this patchset has changed a lot, I don't bump
>> the version.
>>
>> Change since v1:
>> Add a patch to store the reboot_timeout as little endian
>> Fix the endian issue per Thomas's review
>>
>> Li Qiang (5):
>>   tests: refactor fw_cfg_test
>>   tests: fw_cfg: add a function to get the fw_cfg file
>>   fw_cfg: reboot: store reboot-timeout as little endian
>>   tests: fw_cfg: add reboot_timeout test case
>>   tests: fw_cfg: add splash time test case
>>
>>  hw/nvram/fw_cfg.c     |   4 +-
>>  tests/fw_cfg-test.c   | 125 +++++++++++++++++++++++++++++++++++++++---
>>  tests/libqos/fw_cfg.c |  55 +++++++++++++++++++
>>  tests/libqos/fw_cfg.h |   9 +++
>>  4 files changed, 184 insertions(+), 9 deletions(-)
>>
>> --
>> 2.17.1
>>
>>
>>

Re: [Qemu-devel] [PATCH v2 0/5] fw_cfg_test refactor and add two test cases

[Philippe Mathieu-Daudé]

Hi Li,

On 5/17/19 4:28 AM, Li Qiang wrote:
> Ping.....
> 
> Li Qiang <liq3ea@gmail.com <mailto:liq3ea@gmail.com>> 于2019年5月9日周四
> 下午5:57写道：
> 
>     Ping.... this serials.

I apologize I hold this series for too long.
With your v1 I wanted to clarify the commit descriptions without asking
you to send a v2, then I reword your patches and the same day you sent
your v2, then I had mixed feeling about how to do to not frustrate you
asking to respin again, but I ended it worst :(
I adapted the descriptions on your v2 and will repost as v3, then merge
if you are OK with v3.

Regards,

Phil.

> 
>     Thanks,
>     Li Qiang
> 
>     Li Qiang <liq3ea@163.com <mailto:liq3ea@163.com>> 于2019年4月24日周
>     三 下午10:07写道：
> 
>         In the disscuss of adding reboot timeout test case:
>         https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg03304.html
> 
>         Philippe suggested we should uses the only related option for one
>         specific test. However currently we uses one QTestState for all the
>         test cases. In order to achieve Philippe's idea, I split the
>         test case
>         for its own QTestState. As this patchset has changed a lot, I
>         don't bump
>         the version.
> 
>         Change since v1:
>         Add a patch to store the reboot_timeout as little endian
>         Fix the endian issue per Thomas's review
> 
>         Li Qiang (5):
>           tests: refactor fw_cfg_test
>           tests: fw_cfg: add a function to get the fw_cfg file
>           fw_cfg: reboot: store reboot-timeout as little endian
>           tests: fw_cfg: add reboot_timeout test case
>           tests: fw_cfg: add splash time test case
> 
>          hw/nvram/fw_cfg.c     |   4 +-
>          tests/fw_cfg-test.c   | 125
>         +++++++++++++++++++++++++++++++++++++++---
>          tests/libqos/fw_cfg.c |  55 +++++++++++++++++++
>          tests/libqos/fw_cfg.h |   9 +++
>          4 files changed, 184 insertions(+), 9 deletions(-)
> 
>         -- 
>         2.17.1
> 
> 

Re: [Qemu-devel] [PATCH v2 0/5] fw_cfg_test refactor and add two test cases

[Li Qiang]

Philippe Mathieu-Daudé <philmd@redhat.com> 于2019年5月21日周二 上午5:29写道：

> Hi Li,
>
> On 5/17/19 4:28 AM, Li Qiang wrote:
> > Ping.....
> >
> > Li Qiang <liq3ea@gmail.com <mailto:liq3ea@gmail.com>> 于2019年5月9日周四
> > 下午5:57写道：
> >
> >     Ping.... this serials.
>
> I apologize I hold this series for too long.
> With your v1 I wanted to clarify the commit descriptions without asking
> you to send a v2, then I reword your patches and the same day you sent
> your v2, then I had mixed feeling about how to do to not frustrate you
> asking to respin again, but I ended it worst :(
>


Hi Philippe, not afraid to frustrate me next time, just send out the review
email. I don't mind to make
revisions to improve the patches.



> I adapted the descriptions on your v2 and will repost as v3, then merge
> if you are OK with v3.
>
>

I have no objection for this, just merge it.

Thanks,
Li Qiang




> Regards,
>
> Phil.
>
> >
> >     Thanks,
> >     Li Qiang
> >
> >     Li Qiang <liq3ea@163.com <mailto:liq3ea@163.com>> 于2019年4月24日周
> >     三 下午10:07写道：
> >
> >         In the disscuss of adding reboot timeout test case:
> >
> https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg03304.html
> >
> >         Philippe suggested we should uses the only related option for one
> >         specific test. However currently we uses one QTestState for all
> the
> >         test cases. In order to achieve Philippe's idea, I split the
> >         test case
> >         for its own QTestState. As this patchset has changed a lot, I
> >         don't bump
> >         the version.
> >
> >         Change since v1:
> >         Add a patch to store the reboot_timeout as little endian
> >         Fix the endian issue per Thomas's review
> >
> >         Li Qiang (5):
> >           tests: refactor fw_cfg_test
> >           tests: fw_cfg: add a function to get the fw_cfg file
> >           fw_cfg: reboot: store reboot-timeout as little endian
> >           tests: fw_cfg: add reboot_timeout test case
> >           tests: fw_cfg: add splash time test case
> >
> >          hw/nvram/fw_cfg.c     |   4 +-
> >          tests/fw_cfg-test.c   | 125
> >         +++++++++++++++++++++++++++++++++++++++---
> >          tests/libqos/fw_cfg.c |  55 +++++++++++++++++++
> >          tests/libqos/fw_cfg.h |   9 +++
> >          4 files changed, 184 insertions(+), 9 deletions(-)
> >
> >         --
> >         2.17.1
> >
> >
>