| Message ID | 20230503072331.1747057-84-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tcg: Build once for system, once for user | expand |
On 03/05/2023 09.23, Richard Henderson wrote: > If CONFIG_USER_ONLY is ok generically, so is CONFIG_SOFTMMU, > because they are exactly opposite. I thought there was a difference ... at least in the past? But looking at meson.build they seem to be handled quite equally now: common_ss.add_all(when: 'CONFIG_SOFTMMU', if_true: [softmmu_ss]) common_ss.add_all(when: 'CONFIG_USER_ONLY', if_true: user_ss) Paolo, do you remember whether there was a difference in the past? Anyway, as far as I can see, it should be fine now, so: Reviewed-by: Thomas Huth <thuth@redhat.com> > 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(-) > > 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/ .*//' \
On 5/8/23 16:27, Thomas Huth wrote: > On 03/05/2023 09.23, Richard Henderson wrote: >> If CONFIG_USER_ONLY is ok generically, so is CONFIG_SOFTMMU, >> because they are exactly opposite. > > I thought there was a difference ... at least in the past? > But looking at meson.build they seem to be handled quite equally now: > > common_ss.add_all(when: 'CONFIG_SOFTMMU', if_true: [softmmu_ss]) > common_ss.add_all(when: 'CONFIG_USER_ONLY', if_true: user_ss) > > Paolo, do you remember whether there was a difference in the past? No, I don't think so. Really _none_ of them are okay in general, but now that we have softmmu_ss/user_ss there is a usecase for using them in "generic" sourcesets. So perhaps we could have something like /* One of these is always defined in files that can use them. */ #if !defined CONFIG_SOFTMMU && !defined CONFIG_USER_ONLY #pragma GCC poison CONFIG_SOFTMMU #pragma GCC poison CONFIG_USER_ONLY #endif Paolo
On 08/05/2023 17.14, Paolo Bonzini wrote: > On 5/8/23 16:27, Thomas Huth wrote: >> On 03/05/2023 09.23, Richard Henderson wrote: >>> If CONFIG_USER_ONLY is ok generically, so is CONFIG_SOFTMMU, >>> because they are exactly opposite. >> >> I thought there was a difference ... at least in the past? >> But looking at meson.build they seem to be handled quite equally now: >> >> common_ss.add_all(when: 'CONFIG_SOFTMMU', if_true: [softmmu_ss]) >> common_ss.add_all(when: 'CONFIG_USER_ONLY', if_true: user_ss) >> >> Paolo, do you remember whether there was a difference in the past? > > No, I don't think so. Really _none_ of them are okay in general, but now > that we have softmmu_ss/user_ss there is a usecase for using them in > "generic" sourcesets. So perhaps we could have something like > > /* One of these is always defined in files that can use them. */ > #if !defined CONFIG_SOFTMMU && !defined CONFIG_USER_ONLY > #pragma GCC poison CONFIG_SOFTMMU > #pragma GCC poison CONFIG_USER_ONLY > #endif That's the thing that I had in mind: https://lists.gnu.org/archive/html/qemu-devel/2021-03/msg05269.html ... so instead of removing the poison from CONFIG_SOFTMMU, we should likely rather try to get CONFIG_USER_ONLY poisoned, too. Thomas
On 08/05/2023 16.27, Thomas Huth wrote: > On 03/05/2023 09.23, Richard Henderson wrote: >> If CONFIG_USER_ONLY is ok generically, so is CONFIG_SOFTMMU, >> because they are exactly opposite. > > I thought there was a difference ... at least in the past? > But looking at meson.build they seem to be handled quite equally now: > > common_ss.add_all(when: 'CONFIG_SOFTMMU', if_true: [softmmu_ss]) > common_ss.add_all(when: 'CONFIG_USER_ONLY', if_true: user_ss) > > Paolo, do you remember whether there was a difference in the past? > > Anyway, as far as I can see, it should be fine now, so: > > Reviewed-by: Thomas Huth <thuth@redhat.com> For the records, I withdraw my Reviewed-by here (since we should rather do it the other way round and poison CONFIG_USER_ONLY instead): Nacked-by: Thomas Huth <thuth@redhat.com>
On 5/8/23 16:19, Thomas Huth wrote: > On 08/05/2023 17.14, Paolo Bonzini wrote: >> On 5/8/23 16:27, Thomas Huth wrote: >>> On 03/05/2023 09.23, Richard Henderson wrote: >>>> If CONFIG_USER_ONLY is ok generically, so is CONFIG_SOFTMMU, >>>> because they are exactly opposite. >>> >>> I thought there was a difference ... at least in the past? >>> But looking at meson.build they seem to be handled quite equally now: >>> >>> common_ss.add_all(when: 'CONFIG_SOFTMMU', if_true: [softmmu_ss]) >>> common_ss.add_all(when: 'CONFIG_USER_ONLY', if_true: user_ss) >>> >>> Paolo, do you remember whether there was a difference in the past? >> >> No, I don't think so. Really _none_ of them are okay in general, but now that we have >> softmmu_ss/user_ss there is a usecase for using them in "generic" sourcesets. So >> perhaps we could have something like >> >> /* One of these is always defined in files that can use them. */ >> #if !defined CONFIG_SOFTMMU && !defined CONFIG_USER_ONLY >> #pragma GCC poison CONFIG_SOFTMMU >> #pragma GCC poison CONFIG_USER_ONLY >> #endif > > That's the thing that I had in mind: > > https://lists.gnu.org/archive/html/qemu-devel/2021-03/msg05269.html > > ... so instead of removing the poison from CONFIG_SOFTMMU, we should likely rather try to > get CONFIG_USER_ONLY poisoned, too. A worthy goal, but a large job, just looking at "exec/cpu-common.h". I will defer that to another patch set, and continue with non-poisoning of CONFIG_SOFTMMU for now. r~
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/ .*//' \
If CONFIG_USER_ONLY is ok generically, so is CONFIG_SOFTMMU, because they are exactly opposite. 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(-)