diff mbox series

[PULL,49/52] exec/poison: Do not poison CONFIG_SOFTMMU

Message ID 20230605201548.1596865-50-richard.henderson@linaro.org
State Accepted
Commit d31b84041d4353ef310ffde23c87b78c2aa32ead
Headers show
Series [PULL,01/52] tcg/ppc: Remove TARGET_LONG_BITS, TCG_TYPE_TL | expand

Commit Message

Richard Henderson June 5, 2023, 8:15 p.m. UTC
If CONFIG_USER_ONLY is ok generically, so is CONFIG_SOFTMMU,
because they are exactly opposite.

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

Comments

Peter Maydell June 20, 2023, 6:01 p.m. UTC | #1
On Mon, 5 Jun 2023 at 21:23, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> If CONFIG_USER_ONLY is ok generically, so is CONFIG_SOFTMMU,
> because they are exactly opposite.

This isn't quite right. CONFIG_USER_ONLY is theoretically
something we should poison, because it's unsafe in the general
case to use it in compiled-once source files. But in practice
we make quite a lot of use of it in "we know this specific
use of it is OK" situations, like ifdeffing out function
prototypes. So we'd like to poison it, but we can't poison
it without a huge amoun of refactoring which isn't really
worth the effort.

So it's not a good model for "therefore it's OK not to poison
CONFIG_SOFTMMU" -- we should leave that poisoned if we can,
so we don't introduce either new buggy uses of CONFIG_SOFTMMU,
or new "we know this is safe" uses of it which will make
it difficult to put it back into the poison-list later...

thanks
-- PMM
Richard Henderson June 21, 2023, 5:12 a.m. UTC | #2
On 6/20/23 20:01, Peter Maydell wrote:
> On Mon, 5 Jun 2023 at 21:23, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> If CONFIG_USER_ONLY is ok generically, so is CONFIG_SOFTMMU,
>> because they are exactly opposite.
> 
> This isn't quite right. CONFIG_USER_ONLY is theoretically
> something we should poison, because it's unsafe in the general
> case to use it in compiled-once source files. But in practice
> we make quite a lot of use of it in "we know this specific
> use of it is OK" situations, like ifdeffing out function
> prototypes. So we'd like to poison it, but we can't poison
> it without a huge amoun of refactoring which isn't really
> worth the effort.

Yes, a similar amount of refactoring would have been required within tcg/ to retain the 
poison of CONFIG_SOFTMMU.

> So it's not a good model for "therefore it's OK not to poison
> CONFIG_SOFTMMU" -- we should leave that poisoned if we can,
> so we don't introduce either new buggy uses of CONFIG_SOFTMMU,
> or new "we know this is safe" uses of it which will make
> it difficult to put it back into the poison-list later...

My plan is to remove it as a define entirely.  But not this cycle.


r~
diff mbox series

Patch

diff --git a/include/exec/poison.h b/include/exec/poison.h
index 256736e11a..e94ee8dfef 100644
--- a/include/exec/poison.h
+++ b/include/exec/poison.h
@@ -85,7 +85,6 @@ 
 #pragma GCC poison CONFIG_HVF
 #pragma GCC poison CONFIG_LINUX_USER
 #pragma GCC poison CONFIG_KVM
-#pragma GCC poison CONFIG_SOFTMMU
 #pragma GCC poison CONFIG_WHPX
 #pragma GCC poison CONFIG_XEN
 
diff --git a/scripts/make-config-poison.sh b/scripts/make-config-poison.sh
index 1892854261..2b36907e23 100755
--- a/scripts/make-config-poison.sh
+++ b/scripts/make-config-poison.sh
@@ -4,11 +4,12 @@  if test $# = 0; then
   exit 0
 fi
 
-# Create list of config switches that should be poisoned in common code...
-# but filter out CONFIG_TCG and CONFIG_USER_ONLY which are special.
+# Create list of config switches that should be poisoned in common code,
+# but filter out several which are handled manually.
 exec sed -n \
   -e' /CONFIG_TCG/d' \
   -e '/CONFIG_USER_ONLY/d' \
+  -e '/CONFIG_SOFTMMU/d' \
   -e '/^#define / {' \
   -e    's///' \
   -e    's/ .*//' \