[PULL 22/30] gdbstub: only compile gdbstub twice for whole build

Alex Bennée posted 30 patches 2 years, 11 months ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Warner Losh <imp@bsdimp.com>, Kyle Evans <kevans@freebsd.org>, "Alex Bennée" <alex.bennee@linaro.org>, Thomas Huth <thuth@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Yanan Wang <wangyanan55@huawei.com>, Daniel Henrique Barboza <danielhb413@gmail.com>, "Cédric Le Goater" <clg@kaod.org>, David Gibson <david@gibson.dropbear.id.au>, Greg Kurz <groug@kaod.org>, Laurent Vivier <laurent@vivier.eu>, Alexandre Iooss <erdnaxe@crans.org>, Mahmoud Mandour <ma.mandourr@gmail.com>, John Snow <jsnow@redhat.com>, Cleber Rosa <crosa@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Michael Rolnik <mrolnik@gmail.com>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Taylor Simpson <tsimpson@quicinc.com>, Sunil Muthuswamy <sunilmut@microsoft.com>, Song Gao <gaosong@loongson.cn>, Xiaojuan Yang <yangxiaojuan@loongson.cn>, Aurelien Jarno <aurelien@aurel32.net>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>, Chris Wulff <crwulff@gmail.com>, Marek Vasut <marex@denx.de>, Stafford Horne <shorne@gmail.com>, Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bin.meng@windriver.com>, Weiwei Li <liweiwei@iscas.ac.cn>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Yoshinori Sato <ysato@users.sourceforge.jp>, David Hildenbrand <david@redhat.com>, Ilya Leoshkevich <iii@linux.ibm.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Artyom Tarasenko <atar4qemu@gmail.com>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Max Filippov <jcmvbkbc@gmail.com>
[PULL 22/30] gdbstub: only compile gdbstub twice for whole build
Posted by Alex Bennée 2 years, 11 months ago
Now we have removed any target specific bits from the core gdbstub
code we only need to build it twice. We have to jump a few meson hoops
to manually define the CONFIG_USER_ONLY symbol but it seems to work.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20230302190846.2593720-23-alex.bennee@linaro.org>
Message-Id: <20230303025805.625589-23-richard.henderson@linaro.org>

diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index e264ed04e7..d9e9bf9294 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -39,9 +39,7 @@
 
 #include "sysemu/hw_accel.h"
 #include "sysemu/runstate.h"
-#include "exec/exec-all.h"
 #include "exec/replay-core.h"
-#include "exec/tb-flush.h"
 #include "exec/hwaddr.h"
 
 #include "internals.h"
@@ -1612,7 +1610,7 @@ static const GdbCmdParseEntry gdb_gen_query_table[] = {
         .cmd_startswith = 1,
         .schema = "s:l,l0"
     },
-#if defined(CONFIG_USER_ONLY) && defined(CONFIG_LINUX_USER)
+#if defined(CONFIG_USER_ONLY) && defined(CONFIG_LINUX)
     {
         .handler = gdb_handle_query_xfer_auxv,
         .cmd = "Xfer:auxv:read::",
diff --git a/gdbstub/meson.build b/gdbstub/meson.build
index c876222b9c..d679c7ab86 100644
--- a/gdbstub/meson.build
+++ b/gdbstub/meson.build
@@ -4,13 +4,35 @@
 # types such as hwaddr.
 #
 
-specific_ss.add(files('gdbstub.c'))
+# We need to build the core gdb code via a library to be able to tweak
+# cflags so:
+
+gdb_user_ss = ss.source_set()
+gdb_softmmu_ss = ss.source_set()
+
+# We build two versions of gdbstub, one for each mode
+gdb_user_ss.add(files('gdbstub.c', 'user.c'))
+gdb_softmmu_ss.add(files('gdbstub.c', 'softmmu.c'))
+
+gdb_user_ss = gdb_user_ss.apply(config_host, strict: false)
+gdb_softmmu_ss = gdb_softmmu_ss.apply(config_host, strict: false)
+
+libgdb_user = static_library('gdb_user',
+                             gdb_user_ss.sources() + genh,
+                             name_suffix: 'fa',
+                             c_args: '-DCONFIG_USER_ONLY')
+
+libgdb_softmmu = static_library('gdb_softmmu',
+                                gdb_softmmu_ss.sources() + genh,
+                                name_suffix: 'fa')
+
+gdb_user = declare_dependency(link_whole: libgdb_user)
+user_ss.add(gdb_user)
+gdb_softmmu = declare_dependency(link_whole: libgdb_softmmu)
+softmmu_ss.add(gdb_softmmu)
 
 # These have to built to the target ABI
 specific_ss.add(files('syscalls.c'))
 
-softmmu_ss.add(files('softmmu.c'))
-user_ss.add(files('user.c'))
-
 # The user-target is specialised by the guest
 specific_ss.add(when: 'CONFIG_USER_ONLY', if_true: files('user-target.c'))
-- 
2.39.2


Re: [PULL 22/30] gdbstub: only compile gdbstub twice for whole build
Posted by Philippe Mathieu-Daudé 2 years, 10 months ago
Hi Alex, Paolo,

On 7/3/23 22:21, Alex Bennée wrote:
> Now we have removed any target specific bits from the core gdbstub
> code we only need to build it twice. We have to jump a few meson hoops
> to manually define the CONFIG_USER_ONLY symbol but it seems to work.
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20230302190846.2593720-23-alex.bennee@linaro.org>
> Message-Id: <20230303025805.625589-23-richard.henderson@linaro.org>
> 
> diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
> index e264ed04e7..d9e9bf9294 100644
> --- a/gdbstub/gdbstub.c
> +++ b/gdbstub/gdbstub.c
> @@ -39,9 +39,7 @@
>   
>   #include "sysemu/hw_accel.h"
>   #include "sysemu/runstate.h"
> -#include "exec/exec-all.h"
>   #include "exec/replay-core.h"
> -#include "exec/tb-flush.h"
>   #include "exec/hwaddr.h"
>   
>   #include "internals.h"
> @@ -1612,7 +1610,7 @@ static const GdbCmdParseEntry gdb_gen_query_table[] = {
>           .cmd_startswith = 1,
>           .schema = "s:l,l0"
>       },
> -#if defined(CONFIG_USER_ONLY) && defined(CONFIG_LINUX_USER)
> +#if defined(CONFIG_USER_ONLY) && defined(CONFIG_LINUX)
>       {
>           .handler = gdb_handle_query_xfer_auxv,
>           .cmd = "Xfer:auxv:read::",
> diff --git a/gdbstub/meson.build b/gdbstub/meson.build
> index c876222b9c..d679c7ab86 100644
> --- a/gdbstub/meson.build
> +++ b/gdbstub/meson.build
> @@ -4,13 +4,35 @@
>   # types such as hwaddr.
>   #
>   
> -specific_ss.add(files('gdbstub.c'))
> +# We need to build the core gdb code via a library to be able to tweak
> +# cflags so:
> +
> +gdb_user_ss = ss.source_set()
> +gdb_softmmu_ss = ss.source_set()
> +
> +# We build two versions of gdbstub, one for each mode
> +gdb_user_ss.add(files('gdbstub.c', 'user.c'))
> +gdb_softmmu_ss.add(files('gdbstub.c', 'softmmu.c'))
> +
> +gdb_user_ss = gdb_user_ss.apply(config_host, strict: false)
> +gdb_softmmu_ss = gdb_softmmu_ss.apply(config_host, strict: false)
> +
> +libgdb_user = static_library('gdb_user',
> +                             gdb_user_ss.sources() + genh,
> +                             name_suffix: 'fa',
> +                             c_args: '-DCONFIG_USER_ONLY')

FYI building configured as '--disable-user --disable-tcg' I still see:

[13/810] Compiling C object gdbstub/libgdb_user.fa.p/gdbstub.c.o

> +libgdb_softmmu = static_library('gdb_softmmu',
> +                                gdb_softmmu_ss.sources() + genh,
> +                                name_suffix: 'fa')
> +
> +gdb_user = declare_dependency(link_whole: libgdb_user)
> +user_ss.add(gdb_user)

Later we have:

common_ss.add_all(when: 'CONFIG_USER_ONLY', if_true: user_ss)

Also:

config_all += {
   'CONFIG_SOFTMMU': have_system,
   'CONFIG_USER_ONLY': have_user,
   'CONFIG_ALL': true,
}

Why is libgdb_user.fa built while using --disable-user
(have_user=false)?

> +gdb_softmmu = declare_dependency(link_whole: libgdb_softmmu)
> +softmmu_ss.add(gdb_softmmu)
>   
>   # These have to built to the target ABI
>   specific_ss.add(files('syscalls.c'))
>   
> -softmmu_ss.add(files('softmmu.c'))
> -user_ss.add(files('user.c'))
> -
>   # The user-target is specialised by the guest
>   specific_ss.add(when: 'CONFIG_USER_ONLY', if_true: files('user-target.c'))


Re: [PULL 22/30] gdbstub: only compile gdbstub twice for whole build
Posted by Philippe Mathieu-Daudé 2 years, 10 months ago
On 23/3/23 11:05, Philippe Mathieu-Daudé wrote:
> Hi Alex, Paolo,
> 
> On 7/3/23 22:21, Alex Bennée wrote:
>> Now we have removed any target specific bits from the core gdbstub
>> code we only need to build it twice. We have to jump a few meson hoops
>> to manually define the CONFIG_USER_ONLY symbol but it seems to work.
>>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Message-Id: <20230302190846.2593720-23-alex.bennee@linaro.org>
>> Message-Id: <20230303025805.625589-23-richard.henderson@linaro.org>
>>
>> diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
>> index e264ed04e7..d9e9bf9294 100644
>> --- a/gdbstub/gdbstub.c
>> +++ b/gdbstub/gdbstub.c
>> @@ -39,9 +39,7 @@
>>   #include "sysemu/hw_accel.h"
>>   #include "sysemu/runstate.h"
>> -#include "exec/exec-all.h"
>>   #include "exec/replay-core.h"
>> -#include "exec/tb-flush.h"
>>   #include "exec/hwaddr.h"
>>   #include "internals.h"
>> @@ -1612,7 +1610,7 @@ static const GdbCmdParseEntry 
>> gdb_gen_query_table[] = {
>>           .cmd_startswith = 1,
>>           .schema = "s:l,l0"
>>       },
>> -#if defined(CONFIG_USER_ONLY) && defined(CONFIG_LINUX_USER)
>> +#if defined(CONFIG_USER_ONLY) && defined(CONFIG_LINUX)
>>       {
>>           .handler = gdb_handle_query_xfer_auxv,
>>           .cmd = "Xfer:auxv:read::",
>> diff --git a/gdbstub/meson.build b/gdbstub/meson.build
>> index c876222b9c..d679c7ab86 100644
>> --- a/gdbstub/meson.build
>> +++ b/gdbstub/meson.build
>> @@ -4,13 +4,35 @@
>>   # types such as hwaddr.
>>   #
>> -specific_ss.add(files('gdbstub.c'))
>> +# We need to build the core gdb code via a library to be able to tweak
>> +# cflags so:
>> +
>> +gdb_user_ss = ss.source_set()
>> +gdb_softmmu_ss = ss.source_set()
>> +
>> +# We build two versions of gdbstub, one for each mode
>> +gdb_user_ss.add(files('gdbstub.c', 'user.c'))
>> +gdb_softmmu_ss.add(files('gdbstub.c', 'softmmu.c'))
>> +
>> +gdb_user_ss = gdb_user_ss.apply(config_host, strict: false)
>> +gdb_softmmu_ss = gdb_softmmu_ss.apply(config_host, strict: false)
>> +
>> +libgdb_user = static_library('gdb_user',
>> +                             gdb_user_ss.sources() + genh,
>> +                             name_suffix: 'fa',
>> +                             c_args: '-DCONFIG_USER_ONLY')
> 
> FYI building configured as '--disable-user --disable-tcg' I still see:
> 
> [13/810] Compiling C object gdbstub/libgdb_user.fa.p/gdbstub.c.o
> 
>> +libgdb_softmmu = static_library('gdb_softmmu',
>> +                                gdb_softmmu_ss.sources() + genh,
>> +                                name_suffix: 'fa')
>> +
>> +gdb_user = declare_dependency(link_whole: libgdb_user)
>> +user_ss.add(gdb_user)
> 
> Later we have:
> 
> common_ss.add_all(when: 'CONFIG_USER_ONLY', if_true: user_ss)
> 
> Also:
> 
> config_all += {
>    'CONFIG_SOFTMMU': have_system,
>    'CONFIG_USER_ONLY': have_user,
>    'CONFIG_ALL': true,
> }
> 
> Why is libgdb_user.fa built while using --disable-user
> (have_user=false)?

Not sure if this is the best way to do this, but this fixed it:

-- >8 --
diff --git a/gdbstub/meson.build b/gdbstub/meson.build
--- a/gdbstub/meson.build
+++ b/gdbstub/meson.build
@@ -20,11 +20,13 @@ gdb_softmmu_ss = gdb_softmmu_ss.apply(config_host, 
strict: false)
  libgdb_user = static_library('gdb_user',
                               gdb_user_ss.sources() + genh,
                               name_suffix: 'fa',
-                             c_args: '-DCONFIG_USER_ONLY')
+                             c_args: '-DCONFIG_USER_ONLY',
+                             build_by_default: have_user)

  libgdb_softmmu = static_library('gdb_softmmu',
                                  gdb_softmmu_ss.sources() + genh,
-                                name_suffix: 'fa')
+                                name_suffix: 'fa',
+                                build_by_default: have_system)

  gdb_user = declare_dependency(link_whole: libgdb_user)
  user_ss.add(gdb_user)
---

>> +gdb_softmmu = declare_dependency(link_whole: libgdb_softmmu)
>> +softmmu_ss.add(gdb_softmmu)
>>   # These have to built to the target ABI
>>   specific_ss.add(files('syscalls.c'))
>> -softmmu_ss.add(files('softmmu.c'))
>> -user_ss.add(files('user.c'))
>> -
>>   # The user-target is specialised by the guest
>>   specific_ss.add(when: 'CONFIG_USER_ONLY', if_true: 
>> files('user-target.c'))
>