Re: [RFC PATCH 9/9] tests/qtest/arm-cpu-features: Restrict TCG-only tests
On 2/5/21 4:30 PM, Philippe Mathieu-Daudé wrote:
> On 2/5/21 4:20 PM, Claudio Fontana wrote:
>> On 2/5/21 3:43 PM, Philippe Mathieu-Daudé wrote:
>>> Some tests explicitly request the TCG accelerator. As these
>>> tests will obviously fails if TCG is not present, disable
>>> them in such case.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé
>>> ---
>>> Cc: Roman Bolshakov
>>> Cc: Claudio Fontana
>>>
>>> RFC because of the TODO.
>>>
>>> Roman posted a series to have a QMP command to query enabled
>>> accelerators.
>>> ---
>>> tests/qtest/arm-cpu-features.c | 33 +
>>> 1 file changed, 29 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
>>> index c59c3cb002b..c6e86282b66 100644
>>> --- a/tests/qtest/arm-cpu-features.c
>>> +++ b/tests/qtest/arm-cpu-features.c
>>> @@ -20,7 +20,7 @@
>>> */
>>> #define SVE_MAX_VQ 16
>>>
>>> -#define MACHINE "-machine virt,gic-version=max -accel tcg "
>>> +#define MACHINE_TCG "-machine virt,gic-version=max -accel tcg "
>>> #define MACHINE_KVM "-machine virt,gic-version=max -accel kvm -accel tcg "
>>> #define QUERY_HEAD "{ 'execute': 'query-cpu-model-expansion', " \
>>> " 'arguments': { 'type': 'full', "
>>> @@ -41,6 +41,16 @@ static bool kvm_enabled(QTestState *qts)
>>> return enabled;
>>> }
>>>
>>> +static bool tcg_enabled(QTestState *qts)
>>> +{
>>> +/* TODO: Implement QMP query-accel? */
>>> +#ifdef CONFIG_TCG
>>> +return true;
>>> +#else
>>> +return false;
>>> +#endif /* CONFIG_TCG */
>>
>>
>> I would not use the same name as the existing tcg_enabled(), which has
>> different semantics, even in test code;
>>
>> what you mean here is tcg_available() right?
>
> No, I meant static tcg_enabled as the kvm_enabled() earlier method:
Aha, so it's the other way around, we are actually testing if the TCG
accelerator is currently selected in QEMU,
and the patch implements it using CONFIG_TCG as a placeholder for it, since we
do not have query-accel yet, got it.
>
> static bool kvm_enabled(QTestState *qts)
> {
> QDict *resp, *qdict;
> bool enabled;
>
> resp = qtest_qmp(qts, "{ 'execute': 'query-kvm' }");
> g_assert(qdict_haskey(resp, "return"));
> qdict = qdict_get_qdict(resp, "return");
> g_assert(qdict_haskey(qdict, "enabled"));
> enabled = qdict_get_bool(qdict, "enabled");
> qobject_unref(resp);
>
> return enabled;
> }
>
> This should be moved to something generic to QTest IMO,
> and we need some runtime qtest_is_accelerator_enabled().
>
Agreed,
thanks,
Claudio
Re: [RFC PATCH 9/9] tests/qtest/arm-cpu-features: Restrict TCG-only tests
On 2/5/21 4:20 PM, Claudio Fontana wrote:
> On 2/5/21 3:43 PM, Philippe Mathieu-Daudé wrote:
>> Some tests explicitly request the TCG accelerator. As these
>> tests will obviously fails if TCG is not present, disable
>> them in such case.
>>
>> Signed-off-by: Philippe Mathieu-Daudé
>> ---
>> Cc: Roman Bolshakov
>> Cc: Claudio Fontana
>>
>> RFC because of the TODO.
>>
>> Roman posted a series to have a QMP command to query enabled
>> accelerators.
>> ---
>> tests/qtest/arm-cpu-features.c | 33 +
>> 1 file changed, 29 insertions(+), 4 deletions(-)
>>
>> diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
>> index c59c3cb002b..c6e86282b66 100644
>> --- a/tests/qtest/arm-cpu-features.c
>> +++ b/tests/qtest/arm-cpu-features.c
>> @@ -20,7 +20,7 @@
>> */
>> #define SVE_MAX_VQ 16
>>
>> -#define MACHINE "-machine virt,gic-version=max -accel tcg "
>> +#define MACHINE_TCG "-machine virt,gic-version=max -accel tcg "
>> #define MACHINE_KVM "-machine virt,gic-version=max -accel kvm -accel tcg "
>> #define QUERY_HEAD "{ 'execute': 'query-cpu-model-expansion', " \
>> " 'arguments': { 'type': 'full', "
>> @@ -41,6 +41,16 @@ static bool kvm_enabled(QTestState *qts)
>> return enabled;
>> }
>>
>> +static bool tcg_enabled(QTestState *qts)
>> +{
>> +/* TODO: Implement QMP query-accel? */
>> +#ifdef CONFIG_TCG
>> +return true;
>> +#else
>> +return false;
>> +#endif /* CONFIG_TCG */
>
>
> I would not use the same name as the existing tcg_enabled(), which has
> different semantics, even in test code;
>
> what you mean here is tcg_available() right?
No, I meant static tcg_enabled as the kvm_enabled() earlier method:
static bool kvm_enabled(QTestState *qts)
{
QDict *resp, *qdict;
bool enabled;
resp = qtest_qmp(qts, "{ 'execute': 'query-kvm' }");
g_assert(qdict_haskey(resp, "return"));
qdict = qdict_get_qdict(resp, "return");
g_assert(qdict_haskey(qdict, "enabled"));
enabled = qdict_get_bool(qdict, "enabled");
qobject_unref(resp);
return enabled;
}
This should be moved to something generic to QTest IMO,
and we need some runtime qtest_is_accelerator_enabled().
Re: [RFC PATCH 9/9] tests/qtest/arm-cpu-features: Restrict TCG-only tests
On Fri, Feb 05, 2021 at 03:43:45PM +0100, Philippe Mathieu-Daudé wrote:
> Some tests explicitly request the TCG accelerator. As these
> tests will obviously fails if TCG is not present, disable
> them in such case.
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> Cc: Roman Bolshakov
> Cc: Claudio Fontana
>
> RFC because of the TODO.
>
> Roman posted a series to have a QMP command to query enabled
> accelerators.
> ---
> tests/qtest/arm-cpu-features.c | 33 +
> 1 file changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
> index c59c3cb002b..c6e86282b66 100644
> --- a/tests/qtest/arm-cpu-features.c
> +++ b/tests/qtest/arm-cpu-features.c
> @@ -20,7 +20,7 @@
> */
> #define SVE_MAX_VQ 16
>
> -#define MACHINE "-machine virt,gic-version=max -accel tcg "
> +#define MACHINE_TCG "-machine virt,gic-version=max -accel tcg "
> #define MACHINE_KVM "-machine virt,gic-version=max -accel kvm -accel tcg "
Should probably also drop the TCG fallback from MACHINE_KVM when
TCG is not present and then find another way to confirm KVM is
present in the kvm tests prior to calling qtest_init().
> #define QUERY_HEAD "{ 'execute': 'query-cpu-model-expansion', " \
> " 'arguments': { 'type': 'full', "
> @@ -41,6 +41,16 @@ static bool kvm_enabled(QTestState *qts)
> return enabled;
> }
>
> +static bool tcg_enabled(QTestState *qts)
> +{
> +/* TODO: Implement QMP query-accel? */
> +#ifdef CONFIG_TCG
> +return true;
> +#else
> +return false;
> +#endif /* CONFIG_TCG */
> +}
> +
> static QDict *do_query_no_props(QTestState *qts, const char *cpu_type)
> {
> return qtest_qmp(qts, QUERY_HEAD "'model': { 'name': %s }"
> @@ -352,7 +362,12 @@ static void sve_tests_sve_max_vq_8(const void *data)
> {
> QTestState *qts;
>
> -qts = qtest_init(MACHINE "-cpu max,sve-max-vq=8");
> +qts = qtest_init(MACHINE_TCG "-cpu max,sve-max-vq=8");
Won't this fail when TCG isn't present? If so, then the test will
either have already aborted or at least qts can't be passed to
tcg_enabled().
> +
> +if (!tcg_enabled(qts)) {
> +qtest_quit(qts);
> +return;
> +}
>
> assert_sve_vls(qts, "max", BIT_ULL(8) - 1, NULL);
>
> @@ -387,7 +402,12 @@ static void sve_tests_sve_off(const void *data)
> {
> QTestState *qts;
>
> -qts = qtest_init(MACHINE "-cpu max,sve=off");
> +qts = qtest_init(MACHINE_TCG "-cpu max,sve=off");
> +
> +if (!tcg_enabled(qts)) {
> +qtest_quit(qts);
> +return;
> +}
>
> /* SVE is off, so the map should be empty. */
> assert_sve_vls(qts, "max", 0, NULL);
> @@ -443,7 +463,12 @@ static void test_query_cpu_model_expansion(const void
> *data)
> {
> QTestState *qts;
>
> -qts = qtest_init(MACHINE "-cpu max");
> +qts = qtest_init(MACHINE_TCG "-cpu max");
> +
> +if (!tcg_enabled(qts)) {
> +qtest_quit(qts);
> +return;
> +}
>
> /* Test common query-cpu-model-expansion input validation */
> assert_type_full(qts);
> --
> 2.26.2
>
Thanks,
drew
Re: [RFC PATCH 9/9] tests/qtest/arm-cpu-features: Restrict TCG-only tests
On 2/5/21 3:43 PM, Philippe Mathieu-Daudé wrote:
> Some tests explicitly request the TCG accelerator. As these
> tests will obviously fails if TCG is not present, disable
> them in such case.
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> Cc: Roman Bolshakov
> Cc: Claudio Fontana
>
> RFC because of the TODO.
>
> Roman posted a series to have a QMP command to query enabled
> accelerators.
> ---
> tests/qtest/arm-cpu-features.c | 33 +
> 1 file changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
> index c59c3cb002b..c6e86282b66 100644
> --- a/tests/qtest/arm-cpu-features.c
> +++ b/tests/qtest/arm-cpu-features.c
> @@ -20,7 +20,7 @@
> */
> #define SVE_MAX_VQ 16
>
> -#define MACHINE "-machine virt,gic-version=max -accel tcg "
> +#define MACHINE_TCG "-machine virt,gic-version=max -accel tcg "
> #define MACHINE_KVM "-machine virt,gic-version=max -accel kvm -accel tcg "
> #define QUERY_HEAD "{ 'execute': 'query-cpu-model-expansion', " \
> " 'arguments': { 'type': 'full', "
> @@ -41,6 +41,16 @@ static bool kvm_enabled(QTestState *qts)
> return enabled;
> }
>
> +static bool tcg_enabled(QTestState *qts)
> +{
> +/* TODO: Implement QMP query-accel? */
> +#ifdef CONFIG_TCG
> +return true;
> +#else
> +return false;
> +#endif /* CONFIG_TCG */
I would not use the same name as the existing tcg_enabled(), which has
different semantics, even in test code;
what you mean here is tcg_available() right?
> +}
> +
> static QDict *do_query_no_props(QTestState *qts, const char *cpu_type)
> {
> return qtest_qmp(qts, QUERY_HEAD "'model': { 'name': %s }"
> @@ -352,7 +362,12 @@ static void sve_tests_sve_max_vq_8(const void *data)
> {
> QTestState *qts;
>
> -qts = qtest_init(MACHINE "-cpu max,sve-max-vq=8");
> +qts = qtest_init(MACHINE_TCG "-cpu max,sve-max-vq=8");
> +
> +if (!tcg_enabled(qts)) {
> +qtest_quit(qts);
> +return;
> +}
>
> assert_sve_vls(qts, "max", BIT_ULL(8) - 1, NULL);
>
> @@ -387,7 +402,12 @@ static void sve_tests_sve_off(const void *data)
> {
> QTestState *qts;
>
> -qts = qtest_init(MACHINE "-cpu max,sve=off");
> +qts = qtest_init(MACHINE_TCG "-cpu max,sve=off");
> +
> +if (!tcg_enabled(qts)) {
> +qtest_quit(qts);
> +return;
> +}
>
> /* SVE is off, so the map should be empty. */
> assert_sve_vls(qts, "max", 0, NULL);
> @@ -443,7 +463,12 @@ static void test_query_cpu_model_expansion(const void
> *data)
> {
> QTestState *qts;
>
> -qts = qtest_init(MACHINE "-cpu max");
> +qts = qtest_init(MACHINE_TCG "-cpu max");
> +
> +if (!tcg_enabled(qts)) {
> +qtest_quit(qts);
> +return;
> +}
>
> /* Test common query-cpu-model-expansion input validation */
> assert_type_full(qts);
>
[RFC PATCH 9/9] tests/qtest/arm-cpu-features: Restrict TCG-only tests
Some tests explicitly request the TCG accelerator. As these
tests will obviously fails if TCG is not present, disable
them in such case.
Signed-off-by: Philippe Mathieu-Daudé
---
Cc: Roman Bolshakov
Cc: Claudio Fontana
RFC because of the TODO.
Roman posted a series to have a QMP command to query enabled
accelerators.
---
tests/qtest/arm-cpu-features.c | 33 +
1 file changed, 29 insertions(+), 4 deletions(-)
diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
index c59c3cb002b..c6e86282b66 100644
--- a/tests/qtest/arm-cpu-features.c
+++ b/tests/qtest/arm-cpu-features.c
@@ -20,7 +20,7 @@
*/
#define SVE_MAX_VQ 16
-#define MACHINE "-machine virt,gic-version=max -accel tcg "
+#define MACHINE_TCG "-machine virt,gic-version=max -accel tcg "
#define MACHINE_KVM "-machine virt,gic-version=max -accel kvm -accel tcg "
#define QUERY_HEAD "{ 'execute': 'query-cpu-model-expansion', " \
" 'arguments': { 'type': 'full', "
@@ -41,6 +41,16 @@ static bool kvm_enabled(QTestState *qts)
return enabled;
}
+static bool tcg_enabled(QTestState *qts)
+{
+/* TODO: Implement QMP query-accel? */
+#ifdef CONFIG_TCG
+return true;
+#else
+return false;
+#endif /* CONFIG_TCG */
+}
+
static QDict *do_query_no_props(QTestState *qts, const char *cpu_type)
{
return qtest_qmp(qts, QUERY_HEAD "'model': { 'name': %s }"
@@ -352,7 +362,12 @@ static void sve_tests_sve_max_vq_8(const void *data)
{
QTestState *qts;
-qts = qtest_init(MACHINE "-cpu max,sve-max-vq=8");
+qts = qtest_init(MACHINE_TCG "-cpu max,sve-max-vq=8");
+
+if (!tcg_enabled(qts)) {
+qtest_quit(qts);
+return;
+}
assert_sve_vls(qts, "max", BIT_ULL(8) - 1, NULL);
@@ -387,7 +402,12 @@ static void sve_tests_sve_off(const void *data)
{
QTestState *qts;
-qts = qtest_init(MACHINE "-cpu max,sve=off");
+qts = qtest_init(MACHINE_TCG "-cpu max,sve=off");
+
+if (!tcg_enabled(qts)) {
+qtest_quit(qts);
+return;
+}
/* SVE is off, so the map should be empty. */
assert_sve_vls(qts, "max", 0, NULL);
@@ -443,7 +463,12 @@ static void test_query_cpu_model_expansion(const void
*data)
{
QTestState *qts;
-qts = qtest_init(MACHINE "-cpu max");
+qts = qtest_init(MACHINE_TCG "-cpu max");
+
+if (!tcg_enabled(qts)) {
+qtest_quit(qts);
+return;
+}
/* Test common query-cpu-model-expansion input validation */
assert_type_full(qts);
--
2.26.2
