meson, NEED_CPU_H, CONFIG_TCG and tests/

meson, NEED_CPU_H, CONFIG_TCG and tests/

[Claudio Fontana]

Hi Paolo,

currently we have use of CONFIG_TCG in tests/,

but is that variable available at all in there?

I have to adapt some qemu/tests/qtest/* to work also without tcg for ARM,

but I think I am not seeing CONFIG_TCG filtering through, and I wonder whether all the checks in there are actually "wrong".

Looking at meson.build it would seem to me that only stuff in target/ would be able to see CONFIG_TCG,
as a result of

foreach target : target_dirs
  config_target = config_target_mak[target]
  target_name = config_target['TARGET_NAME']
  arch = config_target['TARGET_BASE_ARCH']
  arch_srcs = [config_target_h[target]]
  arch_deps = []
  c_args = ['-DNEED_CPU_H',
            '-DCONFIG_TARGET="@0@-config-target.h"'.format(target),
            '-DCONFIG_DEVICES="@0@-config-devices.h"'.format(target)]

But how could tests see this?

Thanks,

Claudio

-- 
Claudio Fontana
Engineering Manager Virtualization, SUSE Labs Core

SUSE Software Solutions Italy Srl

Re: meson, NEED_CPU_H, CONFIG_TCG and tests/

[Claudio Fontana]

On 2/19/21 2:04 PM, Claudio Fontana wrote:
> Hi Paolo,
> 
> currently we have use of CONFIG_TCG in tests/,
> 
> but is that variable available at all in there?
> 
> I have to adapt some qemu/tests/qtest/* to work also without tcg for ARM,
> 
> but I think I am not seeing CONFIG_TCG filtering through, and I wonder whether all the checks in there are actually "wrong".
> 
> Looking at meson.build it would seem to me that only stuff in target/ would be able to see CONFIG_TCG,
> as a result of
> 
> foreach target : target_dirs
>   config_target = config_target_mak[target]
>   target_name = config_target['TARGET_NAME']
>   arch = config_target['TARGET_BASE_ARCH']
>   arch_srcs = [config_target_h[target]]
>   arch_deps = []
>   c_args = ['-DNEED_CPU_H',
>             '-DCONFIG_TARGET="@0@-config-target.h"'.format(target),
>             '-DCONFIG_DEVICES="@0@-config-devices.h"'.format(target)]
> 
> But how could tests see this?
> 
> Thanks,
> 
> Claudio
> 

By the way, I think Alex is working around this in the TCG tests in the tests/tcg/Makefile.target:

all:
-include ../../../config-host.mak
-include ../config-$(TARGET).mak

..


In my view we need some solution for at least:

tests/qtest/qmp-cmd-test.c
tests/qtest/boot-serial-test.c
tests/qtest/bios-tables-test.c

which need to behave differently according to whether TCG is available or not (--disable-tcg):

For the specific case at hand that I encountered, reopening the issue, 

I need to make tests/qtest/boot-serial-test.c pass different cpu options for TCG and KVM,
ie "cortex-a57" for TCG, and "host" for KVM.

Wdyt?

Ciao,

Claudio

Re: meson, NEED_CPU_H, CONFIG_TCG and tests/

[Philippe Mathieu-Daudé]

On 2/19/21 2:04 PM, Claudio Fontana wrote:
> Hi Paolo,
> 
> currently we have use of CONFIG_TCG in tests/,
> 
> but is that variable available at all in there?
> 
> I have to adapt some qemu/tests/qtest/* to work also without tcg for ARM,
> 
> but I think I am not seeing CONFIG_TCG filtering through, and I wonder whether all the checks in there are actually "wrong".

Checking for ./configure definitions in tests is probably wrong,
it should be done via binary introspection (ask the binary if it
has the feature enabled).

Re: meson, NEED_CPU_H, CONFIG_TCG and tests/

[Claudio Fontana]

On 2/19/21 2:14 PM, Philippe Mathieu-Daudé wrote:
> On 2/19/21 2:04 PM, Claudio Fontana wrote:
>> Hi Paolo,
>>
>> currently we have use of CONFIG_TCG in tests/,
>>
>> but is that variable available at all in there?
>>
>> I have to adapt some qemu/tests/qtest/* to work also without tcg for ARM,
>>
>> but I think I am not seeing CONFIG_TCG filtering through, and I wonder whether all the checks in there are actually "wrong".
> 
> Checking for ./configure definitions in tests is probably wrong,
> it should be done via binary introspection (ask the binary if it
> has the feature enabled).
> 

How are we going to launch with the correct qemu options in qtest/bios-tables-test.c and qtest/boot-serial-tests.c ?

I mean I agree with your sentiment, but it does not solve the problem..

Ciao,

Claudio

Re: meson, NEED_CPU_H, CONFIG_TCG and tests/

[Thomas Huth]

On 19/02/2021 14.04, Claudio Fontana wrote:
> Hi Paolo,
> 
> currently we have use of CONFIG_TCG in tests/,
> 
> but is that variable available at all in there?
> 
> I have to adapt some qemu/tests/qtest/* to work also without tcg for ARM,
> 
> but I think I am not seeing CONFIG_TCG filtering through, and I wonder whether all the checks in there are actually "wrong".
> 
> Looking at meson.build it would seem to me that only stuff in target/ would be able to see CONFIG_TCG,
> as a result of
> 
> foreach target : target_dirs
>    config_target = config_target_mak[target]
>    target_name = config_target['TARGET_NAME']
>    arch = config_target['TARGET_BASE_ARCH']
>    arch_srcs = [config_target_h[target]]
>    arch_deps = []
>    c_args = ['-DNEED_CPU_H',
>              '-DCONFIG_TARGET="@0@-config-target.h"'.format(target),
>              '-DCONFIG_DEVICES="@0@-config-devices.h"'.format(target)]
> 
> But how could tests see this?

The test are generic, not target-specific code, so they can not see things 
like NEED_CPU_H, of course.

But CONFIG_TCG seems to be defined in config-host.h, so you should be able 
to use that define, shouldn't you?

  Thomas

Re: meson, NEED_CPU_H, CONFIG_TCG and tests/

[Philippe Mathieu-Daudé]

On 2/19/21 2:17 PM, Claudio Fontana wrote:
> On 2/19/21 2:14 PM, Philippe Mathieu-Daudé wrote:
>> On 2/19/21 2:04 PM, Claudio Fontana wrote:
>>> Hi Paolo,
>>>
>>> currently we have use of CONFIG_TCG in tests/,
>>>
>>> but is that variable available at all in there?
>>>
>>> I have to adapt some qemu/tests/qtest/* to work also without tcg for ARM,
>>>
>>> but I think I am not seeing CONFIG_TCG filtering through, and I wonder whether all the checks in there are actually "wrong".
>>
>> Checking for ./configure definitions in tests is probably wrong,
>> it should be done via binary introspection (ask the binary if it
>> has the feature enabled).
>>
> 
> How are we going to launch with the correct qemu options in qtest/bios-tables-test.c and qtest/boot-serial-tests.c ?

Isn't it what was recently discussed here?
https://www.mail-archive.com/qemu-devel@nongnu.org/msg779881.html
https://www.mail-archive.com/qemu-devel@nongnu.org/msg779868.html

> 
> I mean I agree with your sentiment, but it does not solve the problem..
> 
> Ciao,
> 
> Claudio
> 

Re: meson, NEED_CPU_H, CONFIG_TCG and tests/

[Claudio Fontana]

On 2/19/21 2:48 PM, Philippe Mathieu-Daudé wrote:
> On 2/19/21 2:17 PM, Claudio Fontana wrote:
>> On 2/19/21 2:14 PM, Philippe Mathieu-Daudé wrote:
>>> On 2/19/21 2:04 PM, Claudio Fontana wrote:
>>>> Hi Paolo,
>>>>
>>>> currently we have use of CONFIG_TCG in tests/,
>>>>
>>>> but is that variable available at all in there?
>>>>
>>>> I have to adapt some qemu/tests/qtest/* to work also without tcg for ARM,
>>>>
>>>> but I think I am not seeing CONFIG_TCG filtering through, and I wonder whether all the checks in there are actually "wrong".
>>>
>>> Checking for ./configure definitions in tests is probably wrong,
>>> it should be done via binary introspection (ask the binary if it
>>> has the feature enabled).
>>>
>>
>> How are we going to launch with the correct qemu options in qtest/bios-tables-test.c and qtest/boot-serial-tests.c ?
> 
> Isn't it what was recently discussed here?
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg779881.html
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg779868.html


Thanks for the reminder, I need to reconnect the two things in my multitasking mind now :-)

The cpu max thing at least allows me to continue on,

Thanks!

CLaudio


> 
>>
>> I mean I agree with your sentiment, but it does not solve the problem..
>>
>> Ciao,
>>
>> Claudio
>>
> 

Re: meson, NEED_CPU_H, CONFIG_TCG and tests/

[Paolo Bonzini]

On 19/02/21 14:30, Thomas Huth wrote:
>> But how could tests see this?
> 
> The test are generic, not target-specific code, so they can not see 
> things like NEED_CPU_H, of course.
> 
> But CONFIG_TCG seems to be defined in config-host.h, so you should be 
> able to use that define, shouldn't you?

Yes, CONFIG_TCG is defined in config_hsot just for that reason:

tcg_arch = config_host['ARCH']
if not get_option('tcg').disabled()
   ...
   accelerators += 'CONFIG_TCG'
   config_host += { 'CONFIG_TCG': 'y' }
endif

It's ugly, but I kept it that way when moving accelerator configuration 
to Meson.

Paolo

Re: meson, NEED_CPU_H, CONFIG_TCG and tests/

[Claudio Fontana]

On 2/19/21 5:04 PM, Paolo Bonzini wrote:
> On 19/02/21 14:30, Thomas Huth wrote:
>>> But how could tests see this?
>>
>> The test are generic, not target-specific code, so they can not see 
>> things like NEED_CPU_H, of course.
>>
>> But CONFIG_TCG seems to be defined in config-host.h, so you should be 
>> able to use that define, shouldn't you?
> 
> Yes, CONFIG_TCG is defined in config_hsot just for that reason:
> 
> tcg_arch = config_host['ARCH']
> if not get_option('tcg').disabled()
>    ...
>    accelerators += 'CONFIG_TCG'
>    config_host += { 'CONFIG_TCG': 'y' }
> endif
> 
> It's ugly, but I kept it that way when moving accelerator configuration 
> to Meson.
> 
> Paolo
> 

understood, thanks a lot for this important piece of the puzzle!

Ciao,

Claudio