[PATCH v2 5/7] tests/libqos/fw_cfg: Pass QTestState as argument

[Philippe Mathieu-Daudé]

Since a QFWCFG object is not tied to a particular test, we can
call *_fw_cfg_init() once before creating QTests and use the same
for all the tests, then release the object with g_free() once all
the tests are run.

Refactor the qfw_cfg* API to take QTestState as argument.

Reviewed-by: Laszlo Ersek <address@hidden>
Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
---
 tests/boot-order-test.c  | 12 ++++----
 tests/fw_cfg-test.c      | 49 ++++++++++++++++----------------
 tests/libqos/fw_cfg.c    | 61 ++++++++++++++++++++--------------------
 tests/libqos/fw_cfg.h    | 31 +++++++++-----------
 tests/libqos/malloc-pc.c |  4 +--
 5 files changed, 78 insertions(+), 79 deletions(-)

diff --git a/tests/boot-order-test.c b/tests/boot-order-test.c
index a725bce729..e2d1b7940f 100644
--- a/tests/boot-order-test.c
+++ b/tests/boot-order-test.c
@@ -134,9 +134,9 @@ static void test_prep_boot_order(void)
 
 static uint64_t read_boot_order_pmac(QTestState *qts)
 {
-    QFWCFG *fw_cfg = mm_fw_cfg_init(qts, 0xf0000510);
+    QFWCFG *fw_cfg = mm_fw_cfg_init(0xf0000510);
 
-    return qfw_cfg_get_u16(fw_cfg, FW_CFG_BOOT_DEVICE);
+    return qfw_cfg_get_u16(qts, fw_cfg, FW_CFG_BOOT_DEVICE);
 }
 
 static const boot_order_test test_cases_fw_cfg[] = {
@@ -159,9 +159,9 @@ static void test_pmac_newworld_boot_order(void)
 
 static uint64_t read_boot_order_sun4m(QTestState *qts)
 {
-    QFWCFG *fw_cfg = mm_fw_cfg_init(qts, 0xd00000510ULL);
+    QFWCFG *fw_cfg = mm_fw_cfg_init(0xd00000510ULL);
 
-    return qfw_cfg_get_u16(fw_cfg, FW_CFG_BOOT_DEVICE);
+    return qfw_cfg_get_u16(qts, fw_cfg, FW_CFG_BOOT_DEVICE);
 }
 
 static void test_sun4m_boot_order(void)
@@ -171,9 +171,9 @@ static void test_sun4m_boot_order(void)
 
 static uint64_t read_boot_order_sun4u(QTestState *qts)
 {
-    QFWCFG *fw_cfg = io_fw_cfg_init(qts, 0x510);
+    QFWCFG *fw_cfg = io_fw_cfg_init(0x510);
 
-    return qfw_cfg_get_u16(fw_cfg, FW_CFG_BOOT_DEVICE);
+    return qfw_cfg_get_u16(qts, fw_cfg, FW_CFG_BOOT_DEVICE);
 }
 
 static void test_sun4u_boot_order(void)
diff --git a/tests/fw_cfg-test.c b/tests/fw_cfg-test.c
index 65785bca73..dda9a9fb07 100644
--- a/tests/fw_cfg-test.c
+++ b/tests/fw_cfg-test.c
@@ -35,9 +35,9 @@ static void test_fw_cfg_signature(const void *opaque)
     char buf[5];
 
     s = qtest_initf("-M %s", ctx->machine_name);
-    fw_cfg = pc_fw_cfg_init(s);
+    fw_cfg = pc_fw_cfg_init();
 
-    qfw_cfg_get(fw_cfg, FW_CFG_SIGNATURE, buf, 4);
+    qfw_cfg_get(s, fw_cfg, FW_CFG_SIGNATURE, buf, 4);
     buf[4] = 0;
 
     g_assert_cmpstr(buf, ==, "QEMU");
@@ -53,9 +53,9 @@ static void test_fw_cfg_id(const void *opaque)
     uint32_t id;
 
     s = qtest_initf("-M %s", ctx->machine_name);
-    fw_cfg = pc_fw_cfg_init(s);
+    fw_cfg = pc_fw_cfg_init();
 
-    id = qfw_cfg_get_u32(fw_cfg, FW_CFG_ID);
+    id = qfw_cfg_get_u32(s, fw_cfg, FW_CFG_ID);
     g_assert((id == 1) ||
              (id == 3));
     g_free(fw_cfg);
@@ -76,9 +76,9 @@ static void test_fw_cfg_uuid(const void *opaque)
 
     s = qtest_initf("-M %s -uuid 4600cb32-38ec-4b2f-8acb-81c6ea54f2d8",
                     ctx->machine_name);
-    fw_cfg = pc_fw_cfg_init(s);
+    fw_cfg = pc_fw_cfg_init();
 
-    qfw_cfg_get(fw_cfg, FW_CFG_UUID, buf, 16);
+    qfw_cfg_get(s, fw_cfg, FW_CFG_UUID, buf, 16);
     g_assert(memcmp(buf, uuid, sizeof(buf)) == 0);
 
     g_free(fw_cfg);
@@ -93,9 +93,9 @@ static void test_fw_cfg_ram_size(const void *opaque)
     QTestState *s;
 
     s = qtest_initf("-M %s", ctx->machine_name);
-    fw_cfg = pc_fw_cfg_init(s);
+    fw_cfg = pc_fw_cfg_init();
 
-    g_assert_cmpint(qfw_cfg_get_u64(fw_cfg, FW_CFG_RAM_SIZE), ==, ram_size);
+    g_assert_cmpint(qfw_cfg_get_u64(s, fw_cfg, FW_CFG_RAM_SIZE), ==, ram_size);
 
     g_free(fw_cfg);
     qtest_quit(s);
@@ -108,9 +108,9 @@ static void test_fw_cfg_nographic(const void *opaque)
     QTestState *s;
 
     s = qtest_initf("-M %s", ctx->machine_name);
-    fw_cfg = pc_fw_cfg_init(s);
+    fw_cfg = pc_fw_cfg_init();
 
-    g_assert_cmpint(qfw_cfg_get_u16(fw_cfg, FW_CFG_NOGRAPHIC), ==, 0);
+    g_assert_cmpint(qfw_cfg_get_u16(s, fw_cfg, FW_CFG_NOGRAPHIC), ==, 0);
 
     g_free(fw_cfg);
     qtest_quit(s);
@@ -123,9 +123,9 @@ static void test_fw_cfg_nb_cpus(const void *opaque)
     QTestState *s;
 
     s = qtest_initf("-M %s", ctx->machine_name);
-    fw_cfg = pc_fw_cfg_init(s);
+    fw_cfg = pc_fw_cfg_init();
 
-    g_assert_cmpint(qfw_cfg_get_u16(fw_cfg, FW_CFG_NB_CPUS), ==, nb_cpus);
+    g_assert_cmpint(qfw_cfg_get_u16(s, fw_cfg, FW_CFG_NB_CPUS), ==, nb_cpus);
 
     g_free(fw_cfg);
     qtest_quit(s);
@@ -138,9 +138,9 @@ static void test_fw_cfg_max_cpus(const void *opaque)
     QTestState *s;
 
     s = qtest_initf("-M %s", ctx->machine_name);
-    fw_cfg = pc_fw_cfg_init(s);
+    fw_cfg = pc_fw_cfg_init();
 
-    g_assert_cmpint(qfw_cfg_get_u16(fw_cfg, FW_CFG_MAX_CPUS), ==, max_cpus);
+    g_assert_cmpint(qfw_cfg_get_u16(s, fw_cfg, FW_CFG_MAX_CPUS), ==, max_cpus);
     g_free(fw_cfg);
     qtest_quit(s);
 }
@@ -154,15 +154,15 @@ static void test_fw_cfg_numa(const void *opaque)
     uint64_t *node_mask;
 
     s = qtest_initf("-M %s", ctx->machine_name);
-    fw_cfg = pc_fw_cfg_init(s);
+    fw_cfg = pc_fw_cfg_init();
 
-    g_assert_cmpint(qfw_cfg_get_u64(fw_cfg, FW_CFG_NUMA), ==, nb_nodes);
+    g_assert_cmpint(qfw_cfg_get_u64(s, fw_cfg, FW_CFG_NUMA), ==, nb_nodes);
 
     cpu_mask = g_new0(uint64_t, max_cpus);
     node_mask = g_new0(uint64_t, nb_nodes);
 
-    qfw_cfg_read_data(fw_cfg, cpu_mask, sizeof(uint64_t) * max_cpus);
-    qfw_cfg_read_data(fw_cfg, node_mask, sizeof(uint64_t) * nb_nodes);
+    qfw_cfg_read_data(s, fw_cfg, cpu_mask, sizeof(uint64_t) * max_cpus);
+    qfw_cfg_read_data(s, fw_cfg, node_mask, sizeof(uint64_t) * nb_nodes);
 
     if (nb_nodes) {
         g_assert(cpu_mask[0] & 0x01);
@@ -182,9 +182,10 @@ static void test_fw_cfg_boot_menu(const void *opaque)
     QTestState *s;
 
     s = qtest_initf("-M %s", ctx->machine_name);
-    fw_cfg = pc_fw_cfg_init(s);
+    fw_cfg = pc_fw_cfg_init();
 
-    g_assert_cmpint(qfw_cfg_get_u16(fw_cfg, FW_CFG_BOOT_MENU), ==, boot_menu);
+    g_assert_cmpint(qfw_cfg_get_u16(s, fw_cfg, FW_CFG_BOOT_MENU),
+                    ==, boot_menu);
     g_free(fw_cfg);
     qtest_quit(s);
 }
@@ -198,9 +199,9 @@ static void test_fw_cfg_reboot_timeout(const void *opaque)
     size_t filesize;
 
     s = qtest_initf("-M %s -boot reboot-timeout=15", ctx->machine_name);
-    fw_cfg = pc_fw_cfg_init(s);
+    fw_cfg = pc_fw_cfg_init();
 
-    filesize = qfw_cfg_get_file(fw_cfg, "etc/boot-fail-wait",
+    filesize = qfw_cfg_get_file(s, 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);
@@ -218,9 +219,9 @@ static void test_fw_cfg_splash_time(const void *opaque)
     size_t filesize;
 
     s = qtest_initf("-M %s -boot splash-time=12", ctx->machine_name);
-    fw_cfg = pc_fw_cfg_init(s);
+    fw_cfg = pc_fw_cfg_init();
 
-    filesize = qfw_cfg_get_file(fw_cfg, "etc/boot-menu-wait",
+    filesize = qfw_cfg_get_file(s, 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);
diff --git a/tests/libqos/fw_cfg.c b/tests/libqos/fw_cfg.c
index ddeec821db..d25a367194 100644
--- a/tests/libqos/fw_cfg.c
+++ b/tests/libqos/fw_cfg.c
@@ -18,46 +18,47 @@
 #include "qemu/bswap.h"
 #include "hw/nvram/fw_cfg.h"
 
-void qfw_cfg_select(QFWCFG *fw_cfg, uint16_t key)
+void qfw_cfg_select(QTestState *qts, QFWCFG *fw_cfg, uint16_t key)
 {
-    fw_cfg->select(fw_cfg, key);
+    fw_cfg->select(qts, fw_cfg, key);
 }
 
-void qfw_cfg_read_data(QFWCFG *fw_cfg, void *data, size_t len)
+void qfw_cfg_read_data(QTestState *qts, QFWCFG *fw_cfg, void *data, size_t len)
 {
-    fw_cfg->read(fw_cfg, data, len);
+    fw_cfg->read(qts, fw_cfg, data, len);
 }
 
-void qfw_cfg_get(QFWCFG *fw_cfg, uint16_t key, void *data, size_t len)
+void qfw_cfg_get(QTestState *qts, QFWCFG *fw_cfg, uint16_t key,
+                 void *data, size_t len)
 {
-    qfw_cfg_select(fw_cfg, key);
-    qfw_cfg_read_data(fw_cfg, data, len);
+    qfw_cfg_select(qts, fw_cfg, key);
+    qfw_cfg_read_data(qts, fw_cfg, data, len);
 }
 
-uint16_t qfw_cfg_get_u16(QFWCFG *fw_cfg, uint16_t key)
+uint16_t qfw_cfg_get_u16(QTestState *qts, QFWCFG *fw_cfg, uint16_t key)
 {
     uint16_t value;
-    qfw_cfg_get(fw_cfg, key, &value, sizeof(value));
+    qfw_cfg_get(qts, fw_cfg, key, &value, sizeof(value));
     return le16_to_cpu(value);
 }
 
-uint32_t qfw_cfg_get_u32(QFWCFG *fw_cfg, uint16_t key)
+uint32_t qfw_cfg_get_u32(QTestState *qts, QFWCFG *fw_cfg, uint16_t key)
 {
     uint32_t value;
-    qfw_cfg_get(fw_cfg, key, &value, sizeof(value));
+    qfw_cfg_get(qts, fw_cfg, key, &value, sizeof(value));
     return le32_to_cpu(value);
 }
 
-uint64_t qfw_cfg_get_u64(QFWCFG *fw_cfg, uint16_t key)
+uint64_t qfw_cfg_get_u64(QTestState *qts, QFWCFG *fw_cfg, uint16_t key)
 {
     uint64_t value;
-    qfw_cfg_get(fw_cfg, key, &value, sizeof(value));
+    qfw_cfg_get(qts, fw_cfg, key, &value, sizeof(value));
     return le64_to_cpu(value);
 }
 
-static void mm_fw_cfg_select(QFWCFG *fw_cfg, uint16_t key)
+static void mm_fw_cfg_select(QTestState *qts, QFWCFG *fw_cfg, uint16_t key)
 {
-    qtest_writew(fw_cfg->qts, fw_cfg->base, key);
+    qtest_writew(qts, fw_cfg->base, key);
 }
 
 /*
@@ -72,8 +73,8 @@ static void mm_fw_cfg_select(QFWCFG *fw_cfg, uint16_t key)
  * 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)
+size_t qfw_cfg_get_file(QTestState *qts, QFWCFG *fw_cfg, const char *filename,
+                        void *data, size_t buflen)
 {
     uint32_t count;
     uint32_t i;
@@ -82,11 +83,11 @@ size_t qfw_cfg_get_file(QFWCFG *fw_cfg, const char 
*filename,
     FWCfgFile *pdir_entry;
     size_t filesize = 0;
 
-    qfw_cfg_get(fw_cfg, FW_CFG_FILE_DIR, &count, sizeof(count));
+    qfw_cfg_get(qts, 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);
+    qfw_cfg_get(qts, 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)) {
@@ -96,7 +97,7 @@ size_t qfw_cfg_get_file(QFWCFG *fw_cfg, const char *filename,
             if (len > buflen) {
                 len = buflen;
             }
-            qfw_cfg_get(fw_cfg, sel, data, len);
+            qfw_cfg_get(qts, fw_cfg, sel, data, len);
             break;
         }
     }
@@ -104,49 +105,49 @@ size_t qfw_cfg_get_file(QFWCFG *fw_cfg, const char 
*filename,
     return filesize;
 }
 
-static void mm_fw_cfg_read(QFWCFG *fw_cfg, void *data, size_t len)
+static void mm_fw_cfg_read(QTestState *qts, QFWCFG *fw_cfg,
+                           void *data, size_t len)
 {
     uint8_t *ptr = data;
     int i;
 
     for (i = 0; i < len; i++) {
-        ptr[i] = qtest_readb(fw_cfg->qts, fw_cfg->base + 2);
+        ptr[i] = qtest_readb(qts, fw_cfg->base + 2);
     }
 }
 
-QFWCFG *mm_fw_cfg_init(QTestState *qts, uint64_t base)
+QFWCFG *mm_fw_cfg_init(uint64_t base)
 {
     QFWCFG *fw_cfg = g_malloc0(sizeof(*fw_cfg));
 
     fw_cfg->base = base;
-    fw_cfg->qts = qts;
     fw_cfg->select = mm_fw_cfg_select;
     fw_cfg->read = mm_fw_cfg_read;
 
     return fw_cfg;
 }
 
-static void io_fw_cfg_select(QFWCFG *fw_cfg, uint16_t key)
+static void io_fw_cfg_select(QTestState *qts, QFWCFG *fw_cfg, uint16_t key)
 {
-    qtest_outw(fw_cfg->qts, fw_cfg->base, key);
+    qtest_outw(qts, fw_cfg->base, key);
 }
 
-static void io_fw_cfg_read(QFWCFG *fw_cfg, void *data, size_t len)
+static void io_fw_cfg_read(QTestState *qts, QFWCFG *fw_cfg,
+                           void *data, size_t len)
 {
     uint8_t *ptr = data;
     int i;
 
     for (i = 0; i < len; i++) {
-        ptr[i] = qtest_inb(fw_cfg->qts, fw_cfg->base + 1);
+        ptr[i] = qtest_inb(qts, fw_cfg->base + 1);
     }
 }
 
-QFWCFG *io_fw_cfg_init(QTestState *qts, uint16_t base)
+QFWCFG *io_fw_cfg_init(uint16_t base)
 {
     QFWCFG *fw_cfg = g_malloc0(sizeof(*fw_cfg));
 
     fw_cfg->base = base;
-    fw_cfg->qts = qts;
     fw_cfg->select = io_fw_cfg_select;
     fw_cfg->read = io_fw_cfg_read;
 
diff --git a/tests/libqos/fw_cfg.h b/tests/libqos/fw_cfg.h
index 6316f4c354..f9e69be450 100644
--- a/tests/libqos/fw_cfg.h
+++ b/tests/libqos/fw_cfg.h
@@ -20,52 +20,49 @@ typedef struct QFWCFG QFWCFG;
 struct QFWCFG
 {
     uint64_t base;
-    QTestState *qts;
-    void (*select)(QFWCFG *fw_cfg, uint16_t key);
-    void (*read)(QFWCFG *fw_cfg, void *data, size_t len);
+    void (*select)(QTestState *qts, QFWCFG *fw_cfg, uint16_t key);
+    void (*read)(QTestState *qts, QFWCFG *fw_cfg, void *data, size_t len);
 };
 
-void qfw_cfg_select(QFWCFG *fw_cfg, uint16_t key);
-void qfw_cfg_read_data(QFWCFG *fw_cfg, void *data, size_t len);
-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 qfw_cfg_select(QTestState *qts, QFWCFG *fw_cfg, uint16_t key);
+void qfw_cfg_read_data(QTestState *qts, QFWCFG *fw_cfg, void *data, size_t 
len);
+void qfw_cfg_get(QTestState *qts, QFWCFG *fw_cfg, uint16_t key,
+                 void *data, size_t len);
+uint16_t qfw_cfg_get_u16(QTestState *qts, QFWCFG *fw_cfg, uint16_t key);
+uint32_t qfw_cfg_get_u32(QTestState *qts, QFWCFG *fw_cfg, uint16_t key);
+uint64_t qfw_cfg_get_u64(QTestState *qts, QFWCFG *fw_cfg, uint16_t key);
+size_t qfw_cfg_get_file(QTestState *qts, QFWCFG *fw_cfg, const char *filename,
                         void *data, size_t buflen);
 
 /**
  * mm_fw_cfg_init():
- * @qts: The #QTestState that will be referred to by the QFWCFG object.
  * @base: The MMIO base address of the fw_cfg device in the guest.
  *
  * Returns a newly allocated QFWCFG object which must be released
  * with a call to g_free() when no longer required.
  */
-QFWCFG *mm_fw_cfg_init(QTestState *qts, uint64_t base);
+QFWCFG *mm_fw_cfg_init(uint64_t base);
 
 /**
  * io_fw_cfg_init():
- * @qts: The #QTestState that will be referred to by the QFWCFG object.
  * @base: The I/O address of the fw_cfg device in the guest.
  *
  * Returns a newly allocated QFWCFG object which must be released
  * with a call to g_free() when no longer required.
  */
-QFWCFG *io_fw_cfg_init(QTestState *qts, uint16_t base);
+QFWCFG *io_fw_cfg_init(uint16_t base);
 
 /**
  * pc_fw_cfg_init():
- * @qts: The #QTestState that will be referred to by the QFWCFG object.
  *
  * This function is specific to the PC machine (X86).
  *
  * Returns a newly allocated QFWCFG object which must be released
  * with a call to g_free() when no longer required.
  */
-static inline QFWCFG *pc_fw_cfg_init(QTestState *qts)
+static inline QFWCFG *pc_fw_cfg_init(void)
 {
-    return io_fw_cfg_init(qts, 0x510);
+    return io_fw_cfg_init(0x510);
 }
 
 #endif
diff --git a/tests/libqos/malloc-pc.c b/tests/libqos/malloc-pc.c
index 949a99361d..4932ae092d 100644
--- a/tests/libqos/malloc-pc.c
+++ b/tests/libqos/malloc-pc.c
@@ -23,9 +23,9 @@
 void pc_alloc_init(QGuestAllocator *s, QTestState *qts, QAllocOpts flags)
 {
     uint64_t ram_size;
-    QFWCFG *fw_cfg = pc_fw_cfg_init(qts);
+    QFWCFG *fw_cfg = pc_fw_cfg_init();
 
-    ram_size = qfw_cfg_get_u64(fw_cfg, FW_CFG_RAM_SIZE);
+    ram_size = qfw_cfg_get_u64(qts, fw_cfg, FW_CFG_RAM_SIZE);
     alloc_init(s, flags, 1 << 20, MIN(ram_size, 0xE0000000), PAGE_SIZE);
 
     /* clean-up */
-- 
2.21.0