Philippe Mathieu-Daudé <phi...@redhat.com> 于2019年10月7日周一 下午11:20写道：
> 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 <ler...@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com>
>
Reviewed-by: Li Qiang <liq...@gmail.com>
> ---
> 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
>
>
