diff mbox series

[RFC,v4,08/15] tests/qtest: Adjust test-hmp to always pass -cpu option

Message ID 20230119135424.5417-9-farosas@suse.de (mailing list archive)
State New, archived
Headers show
Series target/arm: Allow CONFIG_TCG=n builds | expand

Commit Message

Fabiano Rosas Jan. 19, 2023, 1:54 p.m. UTC
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 tests/qtest/test-hmp.c | 34 +++++++++++++++++++++++++++++++---
 1 file changed, 31 insertions(+), 3 deletions(-)

Comments

Dr. David Alan Gilbert Jan. 19, 2023, 2:06 p.m. UTC | #1
* Fabiano Rosas (farosas@suse.de) wrote:
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>  tests/qtest/test-hmp.c | 34 +++++++++++++++++++++++++++++++---
>  1 file changed, 31 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/qtest/test-hmp.c b/tests/qtest/test-hmp.c
> index f8b22abe4c..c367612d4a 100644
> --- a/tests/qtest/test-hmp.c
> +++ b/tests/qtest/test-hmp.c
> @@ -121,21 +121,49 @@ static void test_info_commands(QTestState *qts)
>      g_free(info_buf);
>  }
>  
> +static const char *arch_get_cpu(const char *machine)
> +{
> +    const char *arch = qtest_get_arch();
> +
> +    if (g_str_equal(arch, "aarch64")) {
> +        if (!strncmp(machine, "virt", 4)) {
> +            return "cortex-a57";

Won't that break on a kvm host on a different CPU?
Would -cpu max   work on everything?

Dave

> +        }
> +    }
> +
> +    return NULL;
> +}
> +
>  static void test_machine(gconstpointer data)
>  {
>      const char *machine = data;
>      char *args;
>      QTestState *qts;
>  
> -    args = g_strdup_printf("-S -M %s", machine);
> +    args = qtest_get_machine_args(machine, arch_get_cpu(machine), "-S");
>      qts = qtest_init(args);
>  
>      test_info_commands(qts);
>      test_commands(qts);
>  
>      qtest_quit(qts);
> -    g_free(args);
>      g_free((void *)data);
> +    g_free((void *)args);
> +}
> +
> +static void test_none_with_memory(void)
> +{
> +    QTestState *qts;
> +    char *args;
> +
> +    args = qtest_get_machine_args("none", NULL, "-S -m 2");
> +    qts = qtest_init(args);
> +
> +    test_info_commands(qts);
> +    test_commands(qts);
> +
> +    qtest_quit(qts);
> +    g_free((void *)args);
>  }
>  
>  static void add_machine_test_case(const char *mname)
> @@ -160,7 +188,7 @@ int main(int argc, char **argv)
>      qtest_cb_for_every_machine(add_machine_test_case, g_test_quick());
>  
>      /* as none machine has no memory by default, add a test case with memory */
> -    qtest_add_data_func("hmp/none+2MB", g_strdup("none -m 2"), test_machine);
> +    qtest_add_func("hmp/none+2MB", test_none_with_memory);
>  
>      return g_test_run();
>  }
> -- 
> 2.35.3
>
Fabiano Rosas Jan. 19, 2023, 2:39 p.m. UTC | #2
"Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:

> * Fabiano Rosas (farosas@suse.de) wrote:
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> ---
>>  tests/qtest/test-hmp.c | 34 +++++++++++++++++++++++++++++++---
>>  1 file changed, 31 insertions(+), 3 deletions(-)
>> 
>> diff --git a/tests/qtest/test-hmp.c b/tests/qtest/test-hmp.c
>> index f8b22abe4c..c367612d4a 100644
>> --- a/tests/qtest/test-hmp.c
>> +++ b/tests/qtest/test-hmp.c
>> @@ -121,21 +121,49 @@ static void test_info_commands(QTestState *qts)
>>      g_free(info_buf);
>>  }
>>  
>> +static const char *arch_get_cpu(const char *machine)
>> +{
>> +    const char *arch = qtest_get_arch();
>> +
>> +    if (g_str_equal(arch, "aarch64")) {
>> +        if (!strncmp(machine, "virt", 4)) {
>> +            return "cortex-a57";
>
> Won't that break on a kvm host on a different CPU?
> Would -cpu max   work on everything?

These tests run with -accel qtest only. No TCG or KVM. So this won't
break a KVM host with a different CPU. But we also cannot expect that
cpu to do anything useful. This is more to avoid initial configuration
code to break due to lack of cpu.

About -cpu max, before this series, it works because of a nuance of the
code which does:

aarch64_max_initfn:
if kvm || hvf
    host
else
    cortex-a57
    extra_features that depend on TCG

After this series this becomes roughly (see patch 01):

aarch64_max_initfn:
if kvm || hvf
    host
else if qtest
    cortex-a57
#ifdef CONFIG_TCG    
else if tcg
    cortex-a57
    extra_features that depend on TCG
#endif

The above routine causes us to have two different -cpu max depending on
whether TCG is built in or not, so I'm basically bypassing it and
hardcoding cortex-a57 to be consistent.
diff mbox series

Patch

diff --git a/tests/qtest/test-hmp.c b/tests/qtest/test-hmp.c
index f8b22abe4c..c367612d4a 100644
--- a/tests/qtest/test-hmp.c
+++ b/tests/qtest/test-hmp.c
@@ -121,21 +121,49 @@  static void test_info_commands(QTestState *qts)
     g_free(info_buf);
 }
 
+static const char *arch_get_cpu(const char *machine)
+{
+    const char *arch = qtest_get_arch();
+
+    if (g_str_equal(arch, "aarch64")) {
+        if (!strncmp(machine, "virt", 4)) {
+            return "cortex-a57";
+        }
+    }
+
+    return NULL;
+}
+
 static void test_machine(gconstpointer data)
 {
     const char *machine = data;
     char *args;
     QTestState *qts;
 
-    args = g_strdup_printf("-S -M %s", machine);
+    args = qtest_get_machine_args(machine, arch_get_cpu(machine), "-S");
     qts = qtest_init(args);
 
     test_info_commands(qts);
     test_commands(qts);
 
     qtest_quit(qts);
-    g_free(args);
     g_free((void *)data);
+    g_free((void *)args);
+}
+
+static void test_none_with_memory(void)
+{
+    QTestState *qts;
+    char *args;
+
+    args = qtest_get_machine_args("none", NULL, "-S -m 2");
+    qts = qtest_init(args);
+
+    test_info_commands(qts);
+    test_commands(qts);
+
+    qtest_quit(qts);
+    g_free((void *)args);
 }
 
 static void add_machine_test_case(const char *mname)
@@ -160,7 +188,7 @@  int main(int argc, char **argv)
     qtest_cb_for_every_machine(add_machine_test_case, g_test_quick());
 
     /* as none machine has no memory by default, add a test case with memory */
-    qtest_add_data_func("hmp/none+2MB", g_strdup("none -m 2"), test_machine);
+    qtest_add_func("hmp/none+2MB", test_none_with_memory);
 
     return g_test_run();
 }