Re: [RFC PATCH-for-10.1 27/39] system/hvf: Expose hvf_enabled() to common code

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 4/4/25 14:56, Philippe Mathieu-Daudé wrote:
> +Paolo
> 
> On 4/4/25 20:23, Pierrick Bouvier wrote:
>> On 4/3/25 16:58, Philippe Mathieu-Daudé wrote:
>>> Currently hvf_enabled() is restricted to target-specific code.
>>> By defining CONFIG_HVF_IS_POSSIBLE we allow its use anywhere.
>>>
>>
>> Instead, we can simply make hvf_enabled present for common and target
>> specific code, and link correct implementation, based on what we build.
>>
>> I don't think *_IS_POSSIBLE was a good idea to start with.
> 
> IIUC the point of *_IS_POSSIBLE is to elide code, which we
> aren't interested anymore for host-related configs, so we
> can also remove CONFIG_KVM_IS_POSSIBLE and CONFIG_XEN_IS_POSSIBLE.
> Is that correct?
> 

Ideally, yes, we should not have any CONFIG_{accel}, of any 
CONFIG_{accel}_IS_POSSIBLE in the code.
It should only be used in build system to include selectively 
implementations based on host supporting it or not.

It's not needed to clean that right now, but if we can avoid to 
reproduce this for HVF, it's better.

>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>    MAINTAINERS             |  1 +
>>>    include/system/hvf.h    | 14 +++++++++-----
>>>    accel/stubs/hvf-stub.c  | 12 ++++++++++++
>>>    accel/stubs/meson.build |  1 +
>>>    4 files changed, 23 insertions(+), 5 deletions(-)
>>>    create mode 100644 accel/stubs/hvf-stub.c
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index c7083ab1d93..00ef33be0e2 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -525,6 +525,7 @@ R: Phil Dennis-Jordan <phil@philjordan.eu>
>>>    W: https://wiki.qemu.org/Features/HVF
>>>    S: Maintained
>>>    F: accel/hvf/
>>> +F: accel/stubs/hvf-stub.c
>>>    F: include/system/hvf.h
>>>    F: include/system/hvf_int.h
>>> diff --git a/include/system/hvf.h b/include/system/hvf.h
>>> index d50049e1a1a..7b45a2e1988 100644
>>> --- a/include/system/hvf.h
>>> +++ b/include/system/hvf.h
>>> @@ -19,15 +19,19 @@
>>>    #include "qom/object.h"
>>>    #ifdef COMPILING_PER_TARGET
>>> +# ifdef CONFIG_HVF
>>> +#  define CONFIG_HVF_IS_POSSIBLE
>>> +# endif /* !CONFIG_HVF */
>>> +#else
>>> +# define CONFIG_HVF_IS_POSSIBLE
>>> +#endif /* COMPILING_PER_TARGET */
>>> -#ifdef CONFIG_HVF
>>> +#ifdef CONFIG_HVF_IS_POSSIBLE
>>>    extern bool hvf_allowed;
>>>    #define hvf_enabled() (hvf_allowed)
>>> -#else /* !CONFIG_HVF */
>>> +#else /* !CONFIG_HVF_IS_POSSIBLE */
>>>    #define hvf_enabled() 0
>>> -#endif /* !CONFIG_HVF */
>>> -
>>> -#endif /* COMPILING_PER_TARGET */
>>> +#endif /* !CONFIG_HVF_IS_POSSIBLE */
>>>    #define TYPE_HVF_ACCEL ACCEL_CLASS_NAME("hvf")
>>> diff --git a/accel/stubs/hvf-stub.c b/accel/stubs/hvf-stub.c
>>> new file mode 100644
>>> index 00000000000..42eadc5ca92
>>> --- /dev/null
>>> +++ b/accel/stubs/hvf-stub.c
>>> @@ -0,0 +1,12 @@
>>> +/*
>>> + * HVF stubs for QEMU
>>> + *
>>> + *  Copyright (c) Linaro
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>> + */
>>> +
>>> +#include "qemu/osdep.h"
>>> +#include "system/hvf.h"
>>> +
>>> +bool hvf_allowed;
>>> diff --git a/accel/stubs/meson.build b/accel/stubs/meson.build
>>> index 91a2d219258..8ca1a4529e2 100644
>>> --- a/accel/stubs/meson.build
>>> +++ b/accel/stubs/meson.build
>>> @@ -2,5 +2,6 @@ system_stubs_ss = ss.source_set()
>>>    system_stubs_ss.add(when: 'CONFIG_XEN', if_false: files('xen-stub.c'))
>>>    system_stubs_ss.add(when: 'CONFIG_KVM', if_false: files('kvm-stub.c'))
>>>    system_stubs_ss.add(when: 'CONFIG_TCG', if_false: files('tcg-stub.c'))
>>> +system_stubs_ss.add(when: 'CONFIG_HVF', if_false: files('hvf-stub.c'))
>>>    specific_ss.add_all(when: ['CONFIG_SYSTEM_ONLY'], if_true:
>>> system_stubs_ss)
>>
>