diff mbox series

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

Message ID 20250403235821.9909-28-philmd@linaro.org
State New
Headers show
Series single-binary: Make hw/arm/ common | expand

Commit Message

Philippe Mathieu-Daudé April 3, 2025, 11:58 p.m. UTC 
Currently hvf_enabled() is restricted to target-specific code.
By defining CONFIG_HVF_IS_POSSIBLE we allow its use anywhere.

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

Comments

Pierrick Bouvier April 4, 2025, 6:23 p.m. UTC | #1 
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.

> 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)
Philippe Mathieu-Daudé April 4, 2025, 9:56 p.m. UTC | #2 
+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?

> 
>> 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)
>
Pierrick Bouvier April 5, 2025, 1:05 a.m. UTC | #3 
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)
>>
>
diff mbox series

Patch

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)