Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
RFC: CONFIG_HVF is poisoned.

We could pass host config definitions to qtest using:

  diff --git a/meson.build b/meson.build
  @@ -2547,6 +2547,7 @@ foreach target : target_dirs

     accel_kconfig = []
     foreach sym: accelerators
  +    config_host_data.set(sym + '_QTEST', '')
       if sym == 'CONFIG_TCG' or target in accelerator_targets.get(sym, [])
         config_target += { sym: 'y' }
         config_all += { sym: 'y' }

Then test for CONFIG_HVF_QTEST ...
---
 tests/qtest/libqtest.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 6b2216cb20..31650bdc9f 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -901,6 +901,8 @@ bool qtest_has_accel(const char *accel_name)
                 }
             }
         }
+    } else if (g_str_equal(accel_name, "hvf")) {
+        return true; /* XXX CONFIG_HVF is poisoned... */
     } else {
         /* not implemented */
         g_assert_not_reached();
-- 
2.38.1

On 19/01/2023 11.05, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> RFC: CONFIG_HVF is poisoned.
> 
> We could pass host config definitions to qtest using:
> 
>    diff --git a/meson.build b/meson.build
>    @@ -2547,6 +2547,7 @@ foreach target : target_dirs
> 
>       accel_kconfig = []
>       foreach sym: accelerators
>    +    config_host_data.set(sym + '_QTEST', '')
>         if sym == 'CONFIG_TCG' or target in accelerator_targets.get(sym, [])
>           config_target += { sym: 'y' }
>           config_all += { sym: 'y' }
> 
> Then test for CONFIG_HVF_QTEST ...

I don't think that would really work well. The qtests are build once for all 
targets, and HVF is only available in the target that matches the host 
architecture. It's poisoned on purpose.

The TCG accelerator is special, since we have it in either none or in all 
targets, that's why we can use CONFIG_TCG there.

The kvm part is also rather a hack... we should maybe rather additionally 
use the "query-kvm" QAPI command to check whether it is really available...?

To fix this properly for HVF, I think you'd need a way to query the 
available accelerators via QMP, too... Hmmm, weren't there some patches for 
something like that in the past ... can't remember right now ...

  Thomas

On 19/01/2023 12.24, Thomas Huth wrote:
> On 19/01/2023 11.05, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> RFC: CONFIG_HVF is poisoned.
>>
>> We could pass host config definitions to qtest using:
>>
>>    diff --git a/meson.build b/meson.build
>>    @@ -2547,6 +2547,7 @@ foreach target : target_dirs
>>
>>       accel_kconfig = []
>>       foreach sym: accelerators
>>    +    config_host_data.set(sym + '_QTEST', '')
>>         if sym == 'CONFIG_TCG' or target in accelerator_targets.get(sym, [])
>>           config_target += { sym: 'y' }
>>           config_all += { sym: 'y' }
>>
>> Then test for CONFIG_HVF_QTEST ...
> 
> I don't think that would really work well. The qtests are build once for all 
> targets, and HVF is only available in the target that matches the host 
> architecture. It's poisoned on purpose.
> 
> The TCG accelerator is special, since we have it in either none or in all 
> targets, that's why we can use CONFIG_TCG there.
> 
> The kvm part is also rather a hack... we should maybe rather additionally 
> use the "query-kvm" QAPI command to check whether it is really available...?

Scratch that ... I forgot that you already have to run with "-accel kvm" to 
see whether the accelerator is working with "query-kvm" ... so that would 
not work here for probing whether "-accel kvm" should be used or not ;-)

  Thomas

On 19/1/23 12:24, Thomas Huth wrote:
> On 19/01/2023 11.05, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> RFC: CONFIG_HVF is poisoned.
>>
>> We could pass host config definitions to qtest using:
>>
>>    diff --git a/meson.build b/meson.build
>>    @@ -2547,6 +2547,7 @@ foreach target : target_dirs
>>
>>       accel_kconfig = []
>>       foreach sym: accelerators
>>    +    config_host_data.set(sym + '_QTEST', '')
>>         if sym == 'CONFIG_TCG' or target in 
>> accelerator_targets.get(sym, [])
>>           config_target += { sym: 'y' }
>>           config_all += { sym: 'y' }
>>
>> Then test for CONFIG_HVF_QTEST ...
> 
> I don't think that would really work well. The qtests are build once for 
> all targets, and HVF is only available in the target that matches the 
> host architecture. It's poisoned on purpose.
> 
> The TCG accelerator is special, since we have it in either none or in 
> all targets, that's why we can use CONFIG_TCG there.
> 
> The kvm part is also rather a hack... we should maybe rather 
> additionally use the "query-kvm" QAPI command to check whether it is 
> really available...?
> 
> To fix this properly for HVF, I think you'd need a way to query the 
> available accelerators via QMP, too... Hmmm, weren't there some patches 
> for something like that in the past ... can't remember right now ...

https://lore.kernel.org/qemu-devel/20210505125806.1263441-3-philmd@redhat.com/ 
:(

On 19/01/2023 12.30, Philippe Mathieu-Daudé wrote:
> On 19/1/23 12:24, Thomas Huth wrote:
>> On 19/01/2023 11.05, Philippe Mathieu-Daudé wrote:
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>> RFC: CONFIG_HVF is poisoned.
>>>
>>> We could pass host config definitions to qtest using:
>>>
>>>    diff --git a/meson.build b/meson.build
>>>    @@ -2547,6 +2547,7 @@ foreach target : target_dirs
>>>
>>>       accel_kconfig = []
>>>       foreach sym: accelerators
>>>    +    config_host_data.set(sym + '_QTEST', '')
>>>         if sym == 'CONFIG_TCG' or target in accelerator_targets.get(sym, [])
>>>           config_target += { sym: 'y' }
>>>           config_all += { sym: 'y' }
>>>
>>> Then test for CONFIG_HVF_QTEST ...
>>
>> I don't think that would really work well. The qtests are build once for 
>> all targets, and HVF is only available in the target that matches the host 
>> architecture. It's poisoned on purpose.
>>
>> The TCG accelerator is special, since we have it in either none or in all 
>> targets, that's why we can use CONFIG_TCG there.
>>
>> The kvm part is also rather a hack... we should maybe rather additionally 
>> use the "query-kvm" QAPI command to check whether it is really available...?
>>
>> To fix this properly for HVF, I think you'd need a way to query the 
>> available accelerators via QMP, too... Hmmm, weren't there some patches 
>> for something like that in the past ... can't remember right now ...
> 
> https://lore.kernel.org/qemu-devel/20210505125806.1263441-3-philmd@redhat.com/ 
> :(

Ah, right, and we ended up with the competing patch from Igor since we could 
not quite settle on the QAPI extensions?

Hmm, what happens if you execute "query-qmp-schema" on a HVF-enabled host 
these days? Is there a "hvf"-related entry somewhere in the response?

  Thomas

On 19/01/2023 13.05, Thomas Huth wrote:
> On 19/01/2023 12.30, Philippe Mathieu-Daudé wrote:
>> On 19/1/23 12:24, Thomas Huth wrote:
>>> On 19/01/2023 11.05, Philippe Mathieu-Daudé wrote:
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>> ---
>>>> RFC: CONFIG_HVF is poisoned.
>>>>
>>>> We could pass host config definitions to qtest using:
>>>>
>>>>    diff --git a/meson.build b/meson.build
>>>>    @@ -2547,6 +2547,7 @@ foreach target : target_dirs
>>>>
>>>>       accel_kconfig = []
>>>>       foreach sym: accelerators
>>>>    +    config_host_data.set(sym + '_QTEST', '')
>>>>         if sym == 'CONFIG_TCG' or target in accelerator_targets.get(sym, 
>>>> [])
>>>>           config_target += { sym: 'y' }
>>>>           config_all += { sym: 'y' }
>>>>
>>>> Then test for CONFIG_HVF_QTEST ...
>>>
>>> I don't think that would really work well. The qtests are build once for 
>>> all targets, and HVF is only available in the target that matches the 
>>> host architecture. It's poisoned on purpose.
>>>
>>> The TCG accelerator is special, since we have it in either none or in all 
>>> targets, that's why we can use CONFIG_TCG there.
>>>
>>> The kvm part is also rather a hack... we should maybe rather additionally 
>>> use the "query-kvm" QAPI command to check whether it is really available...?
>>>
>>> To fix this properly for HVF, I think you'd need a way to query the 
>>> available accelerators via QMP, too... Hmmm, weren't there some patches 
>>> for something like that in the past ... can't remember right now ...
>>
>> https://lore.kernel.org/qemu-devel/20210505125806.1263441-3-philmd@redhat.com/ 
>> :(
> 
> Ah, right, and we ended up with the competing patch from Igor since we could 
> not quite settle on the QAPI extensions?
> 
> Hmm, what happens if you execute "query-qmp-schema" on a HVF-enabled host 
> these days? Is there a "hvf"-related entry somewhere in the response?

Alternative idea: execute QEMU once with "-accel help" via 
g_spawn_command_line_sync() or g_spawn_sync() once and look for the 
accelerator in the standard output.

  Thomas

On 20/1/23 11:26, Thomas Huth wrote:
> On 19/01/2023 13.05, Thomas Huth wrote:
>> On 19/01/2023 12.30, Philippe Mathieu-Daudé wrote:
>>> On 19/1/23 12:24, Thomas Huth wrote:
>>>> On 19/01/2023 11.05, Philippe Mathieu-Daudé wrote:
>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>>> ---
>>>>> RFC: CONFIG_HVF is poisoned.
>>>>>
>>>>> We could pass host config definitions to qtest using:
>>>>>
>>>>>    diff --git a/meson.build b/meson.build
>>>>>    @@ -2547,6 +2547,7 @@ foreach target : target_dirs
>>>>>
>>>>>       accel_kconfig = []
>>>>>       foreach sym: accelerators
>>>>>    +    config_host_data.set(sym + '_QTEST', '')
>>>>>         if sym == 'CONFIG_TCG' or target in 
>>>>> accelerator_targets.get(sym, [])
>>>>>           config_target += { sym: 'y' }
>>>>>           config_all += { sym: 'y' }
>>>>>
>>>>> Then test for CONFIG_HVF_QTEST ...
>>>>
>>>> I don't think that would really work well. The qtests are build once 
>>>> for all targets, and HVF is only available in the target that 
>>>> matches the host architecture. It's poisoned on purpose.
>>>>
>>>> The TCG accelerator is special, since we have it in either none or 
>>>> in all targets, that's why we can use CONFIG_TCG there.
>>>>
>>>> The kvm part is also rather a hack... we should maybe rather 
>>>> additionally use the "query-kvm" QAPI command to check whether it is 
>>>> really available...?
>>>>
>>>> To fix this properly for HVF, I think you'd need a way to query the 
>>>> available accelerators via QMP, too... Hmmm, weren't there some 
>>>> patches for something like that in the past ... can't remember right 
>>>> now ...
>>>
>>> https://lore.kernel.org/qemu-devel/20210505125806.1263441-3-philmd@redhat.com/ :(
>>
>> Ah, right, and we ended up with the competing patch from Igor since we 
>> could not quite settle on the QAPI extensions?
>>
>> Hmm, what happens if you execute "query-qmp-schema" on a HVF-enabled 
>> host these days? Is there a "hvf"-related entry somewhere in the 
>> response?
> 
> Alternative idea: execute QEMU once with "-accel help" via 
> g_spawn_command_line_sync() or g_spawn_sync() once and look for the 
> accelerator in the standard output.

There is no stability guaranty with the help output.

QMP is a stable API, we should really rely on it here IMO.

On 20/01/2023 12.18, Philippe Mathieu-Daudé wrote:
> On 20/1/23 11:26, Thomas Huth wrote:
>> On 19/01/2023 13.05, Thomas Huth wrote:
>>> On 19/01/2023 12.30, Philippe Mathieu-Daudé wrote:
>>>> On 19/1/23 12:24, Thomas Huth wrote:
>>>>> On 19/01/2023 11.05, Philippe Mathieu-Daudé wrote:
>>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>>>> ---
>>>>>> RFC: CONFIG_HVF is poisoned.
>>>>>>
>>>>>> We could pass host config definitions to qtest using:
>>>>>>
>>>>>>    diff --git a/meson.build b/meson.build
>>>>>>    @@ -2547,6 +2547,7 @@ foreach target : target_dirs
>>>>>>
>>>>>>       accel_kconfig = []
>>>>>>       foreach sym: accelerators
>>>>>>    +    config_host_data.set(sym + '_QTEST', '')
>>>>>>         if sym == 'CONFIG_TCG' or target in 
>>>>>> accelerator_targets.get(sym, [])
>>>>>>           config_target += { sym: 'y' }
>>>>>>           config_all += { sym: 'y' }
>>>>>>
>>>>>> Then test for CONFIG_HVF_QTEST ...
>>>>>
>>>>> I don't think that would really work well. The qtests are build once 
>>>>> for all targets, and HVF is only available in the target that matches 
>>>>> the host architecture. It's poisoned on purpose.
>>>>>
>>>>> The TCG accelerator is special, since we have it in either none or in 
>>>>> all targets, that's why we can use CONFIG_TCG there.
>>>>>
>>>>> The kvm part is also rather a hack... we should maybe rather 
>>>>> additionally use the "query-kvm" QAPI command to check whether it is 
>>>>> really available...?
>>>>>
>>>>> To fix this properly for HVF, I think you'd need a way to query the 
>>>>> available accelerators via QMP, too... Hmmm, weren't there some patches 
>>>>> for something like that in the past ... can't remember right now ...
>>>>
>>>> https://lore.kernel.org/qemu-devel/20210505125806.1263441-3-philmd@redhat.com/ 
>>>> :(
>>>
>>> Ah, right, and we ended up with the competing patch from Igor since we 
>>> could not quite settle on the QAPI extensions?
>>>
>>> Hmm, what happens if you execute "query-qmp-schema" on a HVF-enabled host 
>>> these days? Is there a "hvf"-related entry somewhere in the response?
>>
>> Alternative idea: execute QEMU once with "-accel help" via 
>> g_spawn_command_line_sync() or g_spawn_sync() once and look for the 
>> accelerator in the standard output.
> 
> There is no stability guaranty with the help output.

Sure, it's rather meant as a temporary solution until a proper QMP method is 
in place. We also have the advantage here that it is internal to the QEMU 
repository only - it's not like libvirt or some other upper layers are 
trying to parse the output here.

> QMP is a stable API, we should really rely on it here IMO.

I agree, QMP should certainly be the final goal.

  Thomas