[PATCH 0/5] tests: Enable fw_cfg tests on AArch64

[PATCH 0/5] tests: Enable fw_cfg tests on AArch64

[Xiang Zheng]

There are quite a few tests disabled on AArch64 such as fw_cfg-tests.
This patch series fix some problems in test code and adapt it to
virt machine.

Xiang Zheng (5):
  tests: fw_cfg: Rename pc_fw_cfg_* to fw_cfg_*
  tests: fw_cfg: Support read/write of fw_cfg registers on aarch64
  tests: fw_cfg: Use virt as default machine in fw_cfg-test.c
  hw/arm/virt: Add FW_CFG_RAM_SIZE and FW_CFG_MAX_CPUS into fw_cfg
  tests: Enable fw_cfg test on aarch64

 hw/arm/virt.c            |   3 ++
 tests/Makefile.include   |   1 +
 tests/fw_cfg-test.c      | 113 ++++++++++++++++++++++++++-------------
 tests/hd-geo-test.c      |   6 +--
 tests/libqos/fw_cfg.c    |  17 +++++-
 tests/libqos/fw_cfg.h    |  20 +++++--
 tests/libqos/malloc-pc.c |   4 +-
 7 files changed, 115 insertions(+), 49 deletions(-)

-- 
2.19.1

[PATCH 1/5] tests: fw_cfg: Rename pc_fw_cfg_* to fw_cfg_*

[Xiang Zheng]

Rename pc_fw_cfg_* to fw_cfg_* to make them common for other
architectures so that we can run fw_cfg tests on aarch64.

Signed-off-by: Xiang Zheng <zhengxiang9@huawei.com>
---
 tests/fw_cfg-test.c      | 48 ++++++++++++++++++++--------------------
 tests/hd-geo-test.c      |  6 ++---
 tests/libqos/fw_cfg.h    | 20 +++++++++++++----
 tests/libqos/malloc-pc.c |  4 ++--
 4 files changed, 45 insertions(+), 33 deletions(-)

diff --git a/tests/fw_cfg-test.c b/tests/fw_cfg-test.c
index 5dc807ba23..5a5342fa9d 100644
--- a/tests/fw_cfg-test.c
+++ b/tests/fw_cfg-test.c
@@ -30,13 +30,13 @@ static void test_fw_cfg_signature(void)
     char buf[5];
 
     s = qtest_init("");
-    fw_cfg = pc_fw_cfg_init(s);
+    fw_cfg = 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);
+    fw_cfg_uninit(fw_cfg);
     qtest_quit(s);
 }
 
@@ -47,12 +47,12 @@ static void test_fw_cfg_id(void)
     uint32_t id;
 
     s = qtest_init("");
-    fw_cfg = pc_fw_cfg_init(s);
+    fw_cfg = 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);
+    fw_cfg_uninit(fw_cfg);
     qtest_quit(s);
 }
 
@@ -68,12 +68,12 @@ static void test_fw_cfg_uuid(void)
     };
 
     s = qtest_init("-uuid 4600cb32-38ec-4b2f-8acb-81c6ea54f2d8");
-    fw_cfg = pc_fw_cfg_init(s);
+    fw_cfg = 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);
+    fw_cfg_uninit(fw_cfg);
     qtest_quit(s);
 
 }
@@ -84,11 +84,11 @@ static void test_fw_cfg_ram_size(void)
     QTestState *s;
 
     s = qtest_init("");
-    fw_cfg = pc_fw_cfg_init(s);
+    fw_cfg = 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);
+    fw_cfg_uninit(fw_cfg);
     qtest_quit(s);
 }
 
@@ -98,11 +98,11 @@ static void test_fw_cfg_nographic(void)
     QTestState *s;
 
     s = qtest_init("");
-    fw_cfg = pc_fw_cfg_init(s);
+    fw_cfg = fw_cfg_init(s);
 
     g_assert_cmpint(qfw_cfg_get_u16(fw_cfg, FW_CFG_NOGRAPHIC), ==, 0);
 
-    pc_fw_cfg_uninit(fw_cfg);
+    fw_cfg_uninit(fw_cfg);
     qtest_quit(s);
 }
 
@@ -112,11 +112,11 @@ static void test_fw_cfg_nb_cpus(void)
     QTestState *s;
 
     s = qtest_init("");
-    fw_cfg = pc_fw_cfg_init(s);
+    fw_cfg = 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);
+    fw_cfg_uninit(fw_cfg);
     qtest_quit(s);
 }
 
@@ -126,10 +126,10 @@ static void test_fw_cfg_max_cpus(void)
     QTestState *s;
 
     s = qtest_init("");
-    fw_cfg = pc_fw_cfg_init(s);
+    fw_cfg = 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);
+    fw_cfg_uninit(fw_cfg);
     qtest_quit(s);
 }
 
@@ -141,7 +141,7 @@ static void test_fw_cfg_numa(void)
     uint64_t *node_mask;
 
     s = qtest_init("");
-    fw_cfg = pc_fw_cfg_init(s);
+    fw_cfg = fw_cfg_init(s);
 
     g_assert_cmpint(qfw_cfg_get_u64(fw_cfg, FW_CFG_NUMA), ==, nb_nodes);
 
@@ -158,7 +158,7 @@ static void test_fw_cfg_numa(void)
 
     g_free(node_mask);
     g_free(cpu_mask);
-    pc_fw_cfg_uninit(fw_cfg);
+    fw_cfg_uninit(fw_cfg);
     qtest_quit(s);
 }
 
@@ -168,10 +168,10 @@ static void test_fw_cfg_boot_menu(void)
     QTestState *s;
 
     s = qtest_init("");
-    fw_cfg = pc_fw_cfg_init(s);
+    fw_cfg = 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);
+    fw_cfg_uninit(fw_cfg);
     qtest_quit(s);
 }
 
@@ -183,14 +183,14 @@ static void test_fw_cfg_reboot_timeout(void)
     size_t filesize;
 
     s = qtest_init("-boot reboot-timeout=15");
-    fw_cfg = pc_fw_cfg_init(s);
+    fw_cfg = 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);
+    fw_cfg_uninit(fw_cfg);
     qtest_quit(s);
 }
 
@@ -203,14 +203,14 @@ static void test_fw_cfg_no_reboot_timeout(void)
 
     /* Special value -1 means "don't reboot" */
     s = qtest_init("-boot reboot-timeout=-1");
-    fw_cfg = pc_fw_cfg_init(s);
+    fw_cfg = 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, ==, UINT32_MAX);
-    pc_fw_cfg_uninit(fw_cfg);
+    fw_cfg_uninit(fw_cfg);
     qtest_quit(s);
 }
 
@@ -222,14 +222,14 @@ static void test_fw_cfg_splash_time(void)
     size_t filesize;
 
     s = qtest_init("-boot splash-time=12");
-    fw_cfg = pc_fw_cfg_init(s);
+    fw_cfg = 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);
+    fw_cfg_uninit(fw_cfg);
     qtest_quit(s);
 }
 
diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c
index 7e86c5416c..a9b8a07403 100644
--- a/tests/hd-geo-test.c
+++ b/tests/hd-geo-test.c
@@ -693,7 +693,7 @@ static void test_override(TestArgs *args, CHSResult expected[])
     joined_args = g_strjoinv(" ", args->argv);
 
     qts = qtest_init(joined_args);
-    fw_cfg = pc_fw_cfg_init(qts);
+    fw_cfg = fw_cfg_init(qts);
 
     read_bootdevices(fw_cfg, expected);
 
@@ -829,7 +829,7 @@ static void test_override_scsi_hot_unplug(void)
     joined_args = g_strjoinv(" ", args->argv);
 
     qts = qtest_init(joined_args);
-    fw_cfg = pc_fw_cfg_init(qts);
+    fw_cfg = fw_cfg_init(qts);
 
     read_bootdevices(fw_cfg, expected);
 
@@ -889,7 +889,7 @@ static void test_override_virtio_hot_unplug(void)
     joined_args = g_strjoinv(" ", args->argv);
 
     qts = qtest_init(joined_args);
-    fw_cfg = pc_fw_cfg_init(qts);
+    fw_cfg = fw_cfg_init(qts);
 
     read_bootdevices(fw_cfg, expected);
 
diff --git a/tests/libqos/fw_cfg.h b/tests/libqos/fw_cfg.h
index 13325cc4ff..708aa922ed 100644
--- a/tests/libqos/fw_cfg.h
+++ b/tests/libqos/fw_cfg.h
@@ -39,14 +39,26 @@ 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)
+static inline QFWCFG *fw_cfg_init(QTestState *qts)
 {
-    return io_fw_cfg_init(qts, 0x510);
+    const char *arch = qtest_get_arch();
+
+    if (!strcmp(arch, "aarch64")) {
+        return mm_fw_cfg_init(qts, 0x09020000);
+    } else {
+        return io_fw_cfg_init(qts, 0x510);
+    }
 }
 
-static inline void pc_fw_cfg_uninit(QFWCFG *fw_cfg)
+static inline void fw_cfg_uninit(QFWCFG *fw_cfg)
 {
-    io_fw_cfg_uninit(fw_cfg);
+    const char *arch = qtest_get_arch();
+
+    if (!strcmp(arch, "aarch64")) {
+        mm_fw_cfg_uninit(fw_cfg);
+    } else {
+        io_fw_cfg_uninit(fw_cfg);
+    }
 }
 
 #endif
diff --git a/tests/libqos/malloc-pc.c b/tests/libqos/malloc-pc.c
index 6f92ce4135..5c58bf6e88 100644
--- a/tests/libqos/malloc-pc.c
+++ b/tests/libqos/malloc-pc.c
@@ -23,11 +23,11 @@
 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 = fw_cfg_init(qts);
 
     ram_size = qfw_cfg_get_u64(fw_cfg, FW_CFG_RAM_SIZE);
     alloc_init(s, flags, 1 << 20, MIN(ram_size, 0xE0000000), PAGE_SIZE);
 
     /* clean-up */
-    pc_fw_cfg_uninit(fw_cfg);
+    fw_cfg_uninit(fw_cfg);
 }
-- 
2.19.1

Re: [PATCH 1/5] tests: fw_cfg: Rename pc_fw_cfg_* to fw_cfg_*

[Peter Maydell]

On Tue, 3 Dec 2019 at 12:29, Xiang Zheng <zhengxiang9@huawei.com> wrote:
>
> Rename pc_fw_cfg_* to fw_cfg_* to make them common for other
> architectures so that we can run fw_cfg tests on aarch64.
>
> Signed-off-by: Xiang Zheng <zhengxiang9@huawei.com>

> -static inline QFWCFG *pc_fw_cfg_init(QTestState *qts)
> +static inline QFWCFG *fw_cfg_init(QTestState *qts)
>  {
> -    return io_fw_cfg_init(qts, 0x510);
> +    const char *arch = qtest_get_arch();
> +
> +    if (!strcmp(arch, "aarch64")) {
> +        return mm_fw_cfg_init(qts, 0x09020000);
> +    } else {
> +        return io_fw_cfg_init(qts, 0x510);
> +    }

Presence and address of the fw_cfg device depends
on the machine type, not the architecture, so is
it possible to write this so that it varies by
machine type, rather than by guest arch ?
There should also presumably be a fallback path
for "fw_cfg not present here", I suppose.

thanks
-- PMM

Re: [PATCH 1/5] tests: fw_cfg: Rename pc_fw_cfg_* to fw_cfg_*

[Xiang Zheng]

On 2019/12/3 20:34, Peter Maydell wrote:
> On Tue, 3 Dec 2019 at 12:29, Xiang Zheng <zhengxiang9@huawei.com> wrote:
>>
>> Rename pc_fw_cfg_* to fw_cfg_* to make them common for other
>> architectures so that we can run fw_cfg tests on aarch64.
>>
>> Signed-off-by: Xiang Zheng <zhengxiang9@huawei.com>
> 
>> -static inline QFWCFG *pc_fw_cfg_init(QTestState *qts)
>> +static inline QFWCFG *fw_cfg_init(QTestState *qts)
>>  {
>> -    return io_fw_cfg_init(qts, 0x510);
>> +    const char *arch = qtest_get_arch();
>> +
>> +    if (!strcmp(arch, "aarch64")) {
>> +        return mm_fw_cfg_init(qts, 0x09020000);
>> +    } else {
>> +        return io_fw_cfg_init(qts, 0x510);
>> +    }
> 
> Presence and address of the fw_cfg device depends
> on the machine type, not the architecture, so is
> it possible to write this so that it varies by
> machine type, rather than by guest arch ?
> There should also presumably be a fallback path
> for "fw_cfg not present here", I suppose.
> 

Yes, "0x09020000" is the address of the fw_cfg device on virt machine, I
should have noticed it. I will have a try for varying the addresses
by machine type.

-- 

Thanks,
Xiang

[PATCH 2/5] tests: fw_cfg: Support read/write of fw_cfg registers on aarch64

[Xiang Zheng]

Refer to the fw_cfg registers locations of x86 and arm in
docs/specs/fw_cfg.txt, the test codes need to differ on the addresses
for read/write.

Besides, fix the endian problems in mm_fw_cfg_select().

Signed-off-by: Xiang Zheng <zhengxiang9@huawei.com>
---
 tests/libqos/fw_cfg.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/tests/libqos/fw_cfg.c b/tests/libqos/fw_cfg.c
index 1f46258f96..c1518c5e81 100644
--- a/tests/libqos/fw_cfg.c
+++ b/tests/libqos/fw_cfg.c
@@ -57,7 +57,14 @@ uint64_t qfw_cfg_get_u64(QFWCFG *fw_cfg, uint16_t key)
 
 static void mm_fw_cfg_select(QFWCFG *fw_cfg, uint16_t key)
 {
-    qtest_writew(fw_cfg->qts, fw_cfg->base, key);
+    const char *arch = qtest_get_arch();
+    uint64_t offset = 0;
+
+    if (!strcmp(arch, "aarch64")) {
+        offset = 8;
+    }
+
+    qtest_writew(fw_cfg->qts, fw_cfg->base + offset, cpu_to_be16(key));
 }
 
 /*
@@ -108,9 +115,15 @@ static void mm_fw_cfg_read(QFWCFG *fw_cfg, void *data, size_t len)
 {
     uint8_t *ptr = data;
     int i;
+    uint64_t offset = 2;
+    const char *arch = qtest_get_arch();
+
+    if (!strcmp(arch, "aarch64")) {
+        offset = 0;
+    }
 
     for (i = 0; i < len; i++) {
-        ptr[i] = qtest_readb(fw_cfg->qts, fw_cfg->base + 2);
+        ptr[i] = qtest_readb(fw_cfg->qts, fw_cfg->base + offset);
     }
 }
 
-- 
2.19.1

[PATCH 3/5] tests: fw_cfg: Use virt as default machine in fw_cfg-test.c

[Xiang Zheng]

The default machine type on aarch64 is not set which causes error in
qtest_init(). Here we use the "virt" machine as the default machine
type on aarch64.

Signed-off-by: Xiang Zheng <zhengxiang9@huawei.com>
---
 tests/fw_cfg-test.c | 65 +++++++++++++++++++++++++++++++++++----------
 1 file changed, 51 insertions(+), 14 deletions(-)

diff --git a/tests/fw_cfg-test.c b/tests/fw_cfg-test.c
index 5a5342fa9d..a3a67d1099 100644
--- a/tests/fw_cfg-test.c
+++ b/tests/fw_cfg-test.c
@@ -23,13 +23,28 @@ static uint16_t max_cpus = 1;
 static uint64_t nb_nodes = 0;
 static uint16_t boot_menu = 0;
 
+static char *make_extra_args(const char *args)
+{
+    const char *arch = qtest_get_arch();
+    const char *machine_arg = NULL;
+
+    if (strcmp(arch, "aarch64") == 0) {
+        machine_arg = "-machine virt";
+    } else {
+        machine_arg = "";
+    }
+
+    return g_strdup_printf("%s %s", machine_arg, args);
+}
+
 static void test_fw_cfg_signature(void)
 {
     QFWCFG *fw_cfg;
     QTestState *s;
     char buf[5];
+    char *cli = make_extra_args("");
 
-    s = qtest_init("");
+    s = qtest_init(cli);
     fw_cfg = fw_cfg_init(s);
 
     qfw_cfg_get(fw_cfg, FW_CFG_SIGNATURE, buf, 4);
@@ -38,6 +53,7 @@ static void test_fw_cfg_signature(void)
     g_assert_cmpstr(buf, ==, "QEMU");
     fw_cfg_uninit(fw_cfg);
     qtest_quit(s);
+    g_free(cli);
 }
 
 static void test_fw_cfg_id(void)
@@ -45,8 +61,9 @@ static void test_fw_cfg_id(void)
     QFWCFG *fw_cfg;
     QTestState *s;
     uint32_t id;
+    char *cli = make_extra_args("");
 
-    s = qtest_init("");
+    s = qtest_init(cli);
     fw_cfg = fw_cfg_init(s);
 
     id = qfw_cfg_get_u32(fw_cfg, FW_CFG_ID);
@@ -54,6 +71,7 @@ static void test_fw_cfg_id(void)
              (id == 3));
     fw_cfg_uninit(fw_cfg);
     qtest_quit(s);
+    g_free(cli);
 }
 
 static void test_fw_cfg_uuid(void)
@@ -66,8 +84,9 @@ static void test_fw_cfg_uuid(void)
         0x46, 0x00, 0xcb, 0x32, 0x38, 0xec, 0x4b, 0x2f,
         0x8a, 0xcb, 0x81, 0xc6, 0xea, 0x54, 0xf2, 0xd8,
     };
+    char *cli = make_extra_args("-uuid 4600cb32-38ec-4b2f-8acb-81c6ea54f2d8");
 
-    s = qtest_init("-uuid 4600cb32-38ec-4b2f-8acb-81c6ea54f2d8");
+    s = qtest_init(cli);
     fw_cfg = fw_cfg_init(s);
 
     qfw_cfg_get(fw_cfg, FW_CFG_UUID, buf, 16);
@@ -75,62 +94,70 @@ static void test_fw_cfg_uuid(void)
 
     fw_cfg_uninit(fw_cfg);
     qtest_quit(s);
-
+    g_free(cli);
 }
 
 static void test_fw_cfg_ram_size(void)
 {
     QFWCFG *fw_cfg;
     QTestState *s;
+    char *cli = make_extra_args("");
 
-    s = qtest_init("");
+    s = qtest_init(cli);
     fw_cfg = fw_cfg_init(s);
 
     g_assert_cmpint(qfw_cfg_get_u64(fw_cfg, FW_CFG_RAM_SIZE), ==, ram_size);
 
     fw_cfg_uninit(fw_cfg);
     qtest_quit(s);
+    g_free(cli);
 }
 
 static void test_fw_cfg_nographic(void)
 {
     QFWCFG *fw_cfg;
     QTestState *s;
+    char *cli = make_extra_args("");
 
-    s = qtest_init("");
+    s = qtest_init(cli);
     fw_cfg = fw_cfg_init(s);
 
     g_assert_cmpint(qfw_cfg_get_u16(fw_cfg, FW_CFG_NOGRAPHIC), ==, 0);
 
     fw_cfg_uninit(fw_cfg);
     qtest_quit(s);
+    g_free(cli);
 }
 
 static void test_fw_cfg_nb_cpus(void)
 {
     QFWCFG *fw_cfg;
     QTestState *s;
+    char *cli = make_extra_args("");
 
-    s = qtest_init("");
+    s = qtest_init(cli);
     fw_cfg = fw_cfg_init(s);
 
     g_assert_cmpint(qfw_cfg_get_u16(fw_cfg, FW_CFG_NB_CPUS), ==, nb_cpus);
 
     fw_cfg_uninit(fw_cfg);
     qtest_quit(s);
+    g_free(cli);
 }
 
 static void test_fw_cfg_max_cpus(void)
 {
     QFWCFG *fw_cfg;
     QTestState *s;
+    char *cli = make_extra_args("");
 
-    s = qtest_init("");
+    s = qtest_init(cli);
     fw_cfg = fw_cfg_init(s);
 
     g_assert_cmpint(qfw_cfg_get_u16(fw_cfg, FW_CFG_MAX_CPUS), ==, max_cpus);
     fw_cfg_uninit(fw_cfg);
     qtest_quit(s);
+    g_free(cli);
 }
 
 static void test_fw_cfg_numa(void)
@@ -139,8 +166,9 @@ static void test_fw_cfg_numa(void)
     QTestState *s;
     uint64_t *cpu_mask;
     uint64_t *node_mask;
+    char *cli = make_extra_args("");
 
-    s = qtest_init("");
+    s = qtest_init(cli);
     fw_cfg = fw_cfg_init(s);
 
     g_assert_cmpint(qfw_cfg_get_u64(fw_cfg, FW_CFG_NUMA), ==, nb_nodes);
@@ -160,19 +188,22 @@ static void test_fw_cfg_numa(void)
     g_free(cpu_mask);
     fw_cfg_uninit(fw_cfg);
     qtest_quit(s);
+    g_free(cli);
 }
 
 static void test_fw_cfg_boot_menu(void)
 {
     QFWCFG *fw_cfg;
     QTestState *s;
+    char *cli = make_extra_args("");
 
-    s = qtest_init("");
+    s = qtest_init(cli);
     fw_cfg = fw_cfg_init(s);
 
     g_assert_cmpint(qfw_cfg_get_u16(fw_cfg, FW_CFG_BOOT_MENU), ==, boot_menu);
     fw_cfg_uninit(fw_cfg);
     qtest_quit(s);
+    g_free(cli);
 }
 
 static void test_fw_cfg_reboot_timeout(void)
@@ -181,8 +212,9 @@ static void test_fw_cfg_reboot_timeout(void)
     QTestState *s;
     uint32_t reboot_timeout = 0;
     size_t filesize;
+    char *cli = make_extra_args("-boot reboot-timeout=15");
 
-    s = qtest_init("-boot reboot-timeout=15");
+    s = qtest_init(cli);
     fw_cfg = fw_cfg_init(s);
 
     filesize = qfw_cfg_get_file(fw_cfg, "etc/boot-fail-wait",
@@ -192,6 +224,7 @@ static void test_fw_cfg_reboot_timeout(void)
     g_assert_cmpint(reboot_timeout, ==, 15);
     fw_cfg_uninit(fw_cfg);
     qtest_quit(s);
+    g_free(cli);
 }
 
 static void test_fw_cfg_no_reboot_timeout(void)
@@ -200,9 +233,10 @@ static void test_fw_cfg_no_reboot_timeout(void)
     QTestState *s;
     uint32_t reboot_timeout = 0;
     size_t filesize;
-
     /* Special value -1 means "don't reboot" */
-    s = qtest_init("-boot reboot-timeout=-1");
+    char *cli = make_extra_args("-boot reboot-timeout=-1");
+
+    s = qtest_init(cli);
     fw_cfg = fw_cfg_init(s);
 
     filesize = qfw_cfg_get_file(fw_cfg, "etc/boot-fail-wait",
@@ -212,6 +246,7 @@ static void test_fw_cfg_no_reboot_timeout(void)
     g_assert_cmpint(reboot_timeout, ==, UINT32_MAX);
     fw_cfg_uninit(fw_cfg);
     qtest_quit(s);
+    g_free(cli);
 }
 
 static void test_fw_cfg_splash_time(void)
@@ -220,8 +255,9 @@ static void test_fw_cfg_splash_time(void)
     QTestState *s;
     uint16_t splash_time = 0;
     size_t filesize;
+    char *cli = make_extra_args("-boot splash-time=12");
 
-    s = qtest_init("-boot splash-time=12");
+    s = qtest_init(cli);
     fw_cfg = fw_cfg_init(s);
 
     filesize = qfw_cfg_get_file(fw_cfg, "etc/boot-menu-wait",
@@ -231,6 +267,7 @@ static void test_fw_cfg_splash_time(void)
     g_assert_cmpint(splash_time, ==, 12);
     fw_cfg_uninit(fw_cfg);
     qtest_quit(s);
+    g_free(cli);
 }
 
 int main(int argc, char **argv)
-- 
2.19.1

[PATCH 4/5] hw/arm/virt: Add FW_CFG_RAM_SIZE and FW_CFG_MAX_CPUS into fw_cfg

[Xiang Zheng]

I'm not sure whether it's neccesary to add FW_CFG_RAM_SIZE and
FW_CFG_MAX_CPUS into fw_cfg on virt machine. This patch just makes
the fw_cfg-test happy.

Signed-off-by: Xiang Zheng <zhengxiang9@huawei.com>
---
 hw/arm/virt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index d4bedc2607..26a4183775 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1084,6 +1084,9 @@ static FWCfgState *create_fw_cfg(const VirtMachineState *vms, AddressSpace *as)
     fw_cfg = fw_cfg_init_mem_wide(base + 8, base, 8, base + 16, as);
     fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)ms->smp.cpus);
 
+    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
+    fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)ms->smp.max_cpus);
+
     nodename = g_strdup_printf("/fw-cfg@%" PRIx64, base);
     qemu_fdt_add_subnode(vms->fdt, nodename);
     qemu_fdt_setprop_string(vms->fdt, nodename,
-- 
2.19.1

Re: [PATCH 4/5] hw/arm/virt: Add FW_CFG_RAM_SIZE and FW_CFG_MAX_CPUS into fw_cfg

[Peter Maydell]

On Tue, 3 Dec 2019 at 12:29, Xiang Zheng <zhengxiang9@huawei.com> wrote:
>
> I'm not sure whether it's neccesary to add FW_CFG_RAM_SIZE and
> FW_CFG_MAX_CPUS into fw_cfg on virt machine. This patch just makes
> the fw_cfg-test happy.
>
> Signed-off-by: Xiang Zheng <zhengxiang9@huawei.com>
> ---
>  hw/arm/virt.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index d4bedc2607..26a4183775 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1084,6 +1084,9 @@ static FWCfgState *create_fw_cfg(const VirtMachineState *vms, AddressSpace *as)
>      fw_cfg = fw_cfg_init_mem_wide(base + 8, base, 8, base + 16, as);
>      fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)ms->smp.cpus);
>
> +    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> +    fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)ms->smp.max_cpus);
> +
>      nodename = g_strdup_printf("/fw-cfg@%" PRIx64, base);
>      qemu_fdt_add_subnode(vms->fdt, nodename);
>      qemu_fdt_setprop_string(vms->fdt, nodename,
> --

Is there a spec anywhere that defines the meaning of these
FW_CFG entries ? docs/specs/fw_cfg.txt defines the
device interface but not what the 'standard' keys mean.
I'd prefer not to add them to the virt board without knowing
what they mean and why we have them.

thanks
-- PMM

Re: [PATCH 4/5] hw/arm/virt: Add FW_CFG_RAM_SIZE and FW_CFG_MAX_CPUS into fw_cfg

[Xiang Zheng]

On 2019/12/3 20:32, Peter Maydell wrote:
> On Tue, 3 Dec 2019 at 12:29, Xiang Zheng <zhengxiang9@huawei.com> wrote:
>>
>> I'm not sure whether it's neccesary to add FW_CFG_RAM_SIZE and
>> FW_CFG_MAX_CPUS into fw_cfg on virt machine. This patch just makes
>> the fw_cfg-test happy.
>>
>> Signed-off-by: Xiang Zheng <zhengxiang9@huawei.com>
>> ---
>>  hw/arm/virt.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index d4bedc2607..26a4183775 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -1084,6 +1084,9 @@ static FWCfgState *create_fw_cfg(const VirtMachineState *vms, AddressSpace *as)
>>      fw_cfg = fw_cfg_init_mem_wide(base + 8, base, 8, base + 16, as);
>>      fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)ms->smp.cpus);
>>
>> +    fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
>> +    fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)ms->smp.max_cpus);
>> +
>>      nodename = g_strdup_printf("/fw-cfg@%" PRIx64, base);
>>      qemu_fdt_add_subnode(vms->fdt, nodename);
>>      qemu_fdt_setprop_string(vms->fdt, nodename,
>> --
> 
> Is there a spec anywhere that defines the meaning of these
> FW_CFG entries ? docs/specs/fw_cfg.txt defines the
> device interface but not what the 'standard' keys mean.
> I'd prefer not to add them to the virt board without knowing
> what they mean and why we have them.
> 

I cannot find a spec or doc defines these FW_CFG entries. It seems that they
are used on x86 machines to limit APIC ID values but useless on virt machine.
I will drop this patch.

-- 

Thanks,
Xiang

[PATCH 5/5] tests: Enable fw_cfg test on aarch64

[Xiang Zheng]

Now turn on the fw_cfg test for aarch64.

Signed-off-by: Xiang Zheng <zhengxiang9@huawei.com>
---
 tests/Makefile.include | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 8566f5f119..180e0ed2b7 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -276,6 +276,7 @@ check-qtest-aarch64-y += tests/arm-cpu-features$(EXESUF)
 check-qtest-aarch64-y += tests/numa-test$(EXESUF)
 check-qtest-aarch64-y += tests/boot-serial-test$(EXESUF)
 check-qtest-aarch64-y += tests/migration-test$(EXESUF)
+check-qtest-aarch64-y += tests/fw_cfg-test$(EXESUF)
 # TODO: once aarch64 TCG is fixed on ARM 32 bit host, make test unconditional
 ifneq ($(ARCH),arm)
 check-qtest-aarch64-y += tests/bios-tables-test$(EXESUF)
-- 
2.19.1

Re: [PATCH 0/5] tests: Enable fw_cfg tests on AArch64

[Thomas Huth]

On 03/12/2019 13.27, Xiang Zheng wrote:
> There are quite a few tests disabled on AArch64 such as fw_cfg-tests.
> This patch series fix some problems in test code and adapt it to
> virt machine.
> 
> Xiang Zheng (5):
>   tests: fw_cfg: Rename pc_fw_cfg_* to fw_cfg_*
>   tests: fw_cfg: Support read/write of fw_cfg registers on aarch64
>   tests: fw_cfg: Use virt as default machine in fw_cfg-test.c
>   hw/arm/virt: Add FW_CFG_RAM_SIZE and FW_CFG_MAX_CPUS into fw_cfg
>   tests: Enable fw_cfg test on aarch64

 Hi,

this breaks "make check-qtest-ppc64":

  TEST    check-qtest-ppc64: tests/boot-order-test
**
ERROR:tests/boot-order-test.c:40:test_a_boot_order: assertion failed
(actual == expected_boot): (0x00000000 == 0x00000063)

Please make sure that "make check" continuous to work with all other
targets, too.

 Thanks,
  Thomas

Re: [PATCH 0/5] tests: Enable fw_cfg tests on AArch64

[Xiang Zheng]

On 2019/12/3 21:01, Thomas Huth wrote:
> On 03/12/2019 13.27, Xiang Zheng wrote:
>> There are quite a few tests disabled on AArch64 such as fw_cfg-tests.
>> This patch series fix some problems in test code and adapt it to
>> virt machine.
>>
>> Xiang Zheng (5):
>>   tests: fw_cfg: Rename pc_fw_cfg_* to fw_cfg_*
>>   tests: fw_cfg: Support read/write of fw_cfg registers on aarch64
>>   tests: fw_cfg: Use virt as default machine in fw_cfg-test.c
>>   hw/arm/virt: Add FW_CFG_RAM_SIZE and FW_CFG_MAX_CPUS into fw_cfg
>>   tests: Enable fw_cfg test on aarch64
> 
>  Hi,
> 
> this breaks "make check-qtest-ppc64":
> 
>   TEST    check-qtest-ppc64: tests/boot-order-test
> **
> ERROR:tests/boot-order-test.c:40:test_a_boot_order: assertion failed
> (actual == expected_boot): (0x00000000 == 0x00000063)
> 
> Please make sure that "make check" continuous to work with all other
> targets, too.
> 

Hi Thomas,

Thanks for your suggestion, I will make sure for that. :)

-- 

Thanks,
Xiang