diff mbox series

[RFC,PATCH-for-10.1,28/39] exec: Do not poison hardware accelerators

Message ID 20250403235821.9909-29-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 
Hardware accelerators depends on the host, not the guest.

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/exec/poison.h         | 4 ----
 scripts/make-config-poison.sh | 4 ++++
 2 files changed, 4 insertions(+), 4 deletions(-)

Comments

Pierrick Bouvier April 4, 2025, 6:25 p.m. UTC | #1 
On 4/3/25 16:58, Philippe Mathieu-Daudé wrote:
> Hardware accelerators depends on the host, not the guest.
> 

While this is true, no we can't unpoison those define.
They are applied per target, and not in config-host. So unpoisoining 
them opens the possibility to miss something.

I would stick to clean up those ifdef instead, by exposing interface to 
all code, and link appropriate stubs depending on the variant built.

> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/exec/poison.h         | 4 ----
>   scripts/make-config-p	oison.sh | 4 ++++
>   2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/include/exec/poison.h b/include/exec/poison.h
> index 00aedc41d82..0574d683f6a 100644
> --- a/include/exec/poison.h
> +++ b/include/exec/poison.h
> @@ -62,11 +62,7 @@
>   #pragma GCC poison CONFIG_SPARC_DIS
>   #pragma GCC poison CONFIG_XTENSA_DIS
>   
> -#pragma GCC poison CONFIG_HVF
>   #pragma GCC poison CONFIG_LINUX_USER
> -#pragma GCC poison CONFIG_KVM
> -#pragma GCC poison CONFIG_WHPX
> -#pragma GCC poison CONFIG_XEN
>   
>   #ifndef COMPILING_SYSTEM_VS_USER
>   #pragma GCC poison CONFIG_USER_ONLY
> diff --git a/scripts/make-config-poison.sh b/scripts/make-config-poison.sh
> index 2b36907e239..09d2384745f 100755
> --- a/scripts/make-config-poison.sh
> +++ b/scripts/make-config-poison.sh
> @@ -8,6 +8,10 @@ fi
>   # but filter out several which are handled manually.
>   exec sed -n \
>     -e' /CONFIG_TCG/d' \
> +  -e' /CONFIG_HVF/d' \
> +  -e' /CONFIG_KVM/d' \
> +  -e' /CONFIG_WHPX/d' \
> +  -e' /CONFIG_XEN/d' \
>     -e '/CONFIG_USER_ONLY/d' \
>     -e '/CONFIG_SOFTMMU/d' \
>     -e '/^#define / {' \
Philippe Mathieu-Daudé April 4, 2025, 10 p.m. UTC | #2 
+Paolo

On 4/4/25 20:25, Pierrick Bouvier wrote:
> On 4/3/25 16:58, Philippe Mathieu-Daudé wrote:
>> Hardware accelerators depends on the host, not the guest.
>>
> 
> While this is true, no we can't unpoison those define.
> They are applied per target, and not in config-host. So unpoisoining 
> them opens the possibility to miss something.
> 
> I would stick to clean up those ifdef instead, by exposing interface to 
> all code, and link appropriate stubs depending on the variant built.

OK, so IIUC this is the same reasoning than previous patch comment,
always declare kvm_enabled() so code elision isn't possible anymore,
then remove all CONFIG_KVM #ifdef'ry?

> 
>> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   include/exec/poison.h         | 4 ----
>>   scripts/make-config-p    oison.sh | 4 ++++
>>   2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/exec/poison.h b/include/exec/poison.h
>> index 00aedc41d82..0574d683f6a 100644
>> --- a/include/exec/poison.h
>> +++ b/include/exec/poison.h
>> @@ -62,11 +62,7 @@
>>   #pragma GCC poison CONFIG_SPARC_DIS
>>   #pragma GCC poison CONFIG_XTENSA_DIS
>> -#pragma GCC poison CONFIG_HVF
>>   #pragma GCC poison CONFIG_LINUX_USER
>> -#pragma GCC poison CONFIG_KVM
>> -#pragma GCC poison CONFIG_WHPX
>> -#pragma GCC poison CONFIG_XEN
>>   #ifndef COMPILING_SYSTEM_VS_USER
>>   #pragma GCC poison CONFIG_USER_ONLY
>> diff --git a/scripts/make-config-poison.sh b/scripts/make-config- 
>> poison.sh
>> index 2b36907e239..09d2384745f 100755
>> --- a/scripts/make-config-poison.sh
>> +++ b/scripts/make-config-poison.sh
>> @@ -8,6 +8,10 @@ fi
>>   # but filter out several which are handled manually.
>>   exec sed -n \
>>     -e' /CONFIG_TCG/d' \
>> +  -e' /CONFIG_HVF/d' \
>> +  -e' /CONFIG_KVM/d' \
>> +  -e' /CONFIG_WHPX/d' \
>> +  -e' /CONFIG_XEN/d' \
>>     -e '/CONFIG_USER_ONLY/d' \
>>     -e '/CONFIG_SOFTMMU/d' \
>>     -e '/^#define / {' \
>
Pierrick Bouvier April 5, 2025, 1:07 a.m. UTC | #3 
On 4/4/25 15:00, Philippe Mathieu-Daudé wrote:
> +Paolo
> 
> On 4/4/25 20:25, Pierrick Bouvier wrote:
>> On 4/3/25 16:58, Philippe Mathieu-Daudé wrote:
>>> Hardware accelerators depends on the host, not the guest.
>>>
>>
>> While this is true, no we can't unpoison those define.
>> They are applied per target, and not in config-host. So unpoisoining
>> them opens the possibility to miss something.
>>
>> I would stick to clean up those ifdef instead, by exposing interface to
>> all code, and link appropriate stubs depending on the variant built.
> 
> OK, so IIUC this is the same reasoning than previous patch comment,
> always declare kvm_enabled() so code elision isn't possible anymore,
> then remove all CONFIG_KVM #ifdef'ry?
> 

Yes, right. And this CONFIG_{accel} will be used only in build system to 
select the host implementation or the stubbed one.

>>
>>> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>    include/exec/poison.h         | 4 ----
>>>    scripts/make-config-p    oison.sh | 4 ++++
>>>    2 files changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/include/exec/poison.h b/include/exec/poison.h
>>> index 00aedc41d82..0574d683f6a 100644
>>> --- a/include/exec/poison.h
>>> +++ b/include/exec/poison.h
>>> @@ -62,11 +62,7 @@
>>>    #pragma GCC poison CONFIG_SPARC_DIS
>>>    #pragma GCC poison CONFIG_XTENSA_DIS
>>> -#pragma GCC poison CONFIG_HVF
>>>    #pragma GCC poison CONFIG_LINUX_USER
>>> -#pragma GCC poison CONFIG_KVM
>>> -#pragma GCC poison CONFIG_WHPX
>>> -#pragma GCC poison CONFIG_XEN
>>>    #ifndef COMPILING_SYSTEM_VS_USER
>>>    #pragma GCC poison CONFIG_USER_ONLY
>>> diff --git a/scripts/make-config-poison.sh b/scripts/make-config-
>>> poison.sh
>>> index 2b36907e239..09d2384745f 100755
>>> --- a/scripts/make-config-poison.sh
>>> +++ b/scripts/make-config-poison.sh
>>> @@ -8,6 +8,10 @@ fi
>>>    # but filter out several which are handled manually.
>>>    exec sed -n \
>>>      -e' /CONFIG_TCG/d' \
>>> +  -e' /CONFIG_HVF/d' \
>>> +  -e' /CONFIG_KVM/d' \
>>> +  -e' /CONFIG_WHPX/d' \
>>> +  -e' /CONFIG_XEN/d' \
>>>      -e '/CONFIG_USER_ONLY/d' \
>>>      -e '/CONFIG_SOFTMMU/d' \
>>>      -e '/^#define / {' \
>>
>
diff mbox series

Patch

diff --git a/include/exec/poison.h b/include/exec/poison.h
index 00aedc41d82..0574d683f6a 100644
--- a/include/exec/poison.h
+++ b/include/exec/poison.h
@@ -62,11 +62,7 @@ 
 #pragma GCC poison CONFIG_SPARC_DIS
 #pragma GCC poison CONFIG_XTENSA_DIS
 
-#pragma GCC poison CONFIG_HVF
 #pragma GCC poison CONFIG_LINUX_USER
-#pragma GCC poison CONFIG_KVM
-#pragma GCC poison CONFIG_WHPX
-#pragma GCC poison CONFIG_XEN
 
 #ifndef COMPILING_SYSTEM_VS_USER
 #pragma GCC poison CONFIG_USER_ONLY
diff --git a/scripts/make-config-poison.sh b/scripts/make-config-poison.sh
index 2b36907e239..09d2384745f 100755
--- a/scripts/make-config-poison.sh
+++ b/scripts/make-config-poison.sh
@@ -8,6 +8,10 @@  fi
 # but filter out several which are handled manually.
 exec sed -n \
   -e' /CONFIG_TCG/d' \
+  -e' /CONFIG_HVF/d' \
+  -e' /CONFIG_KVM/d' \
+  -e' /CONFIG_WHPX/d' \
+  -e' /CONFIG_XEN/d' \
   -e '/CONFIG_USER_ONLY/d' \
   -e '/CONFIG_SOFTMMU/d' \
   -e '/^#define / {' \