[PATCH 00/22] Maintainer updates for 8.2 (gdbstub, tests, plugins) pre-PR

[PATCH 00/22] Maintainer updates for 8.2 (gdbstub, tests, plugins) pre-PR

[Alex Bennée]

I ran into various issues getting the register API ready in time so
those have been dropped although I've included a bunch of the
pre-requisites. There has been more tweaking of the registers test
which keeps finding kinks in our gdbstub report. I've also rolled up
the acked gitdm changes. There has been a bit of juggling to the
configure and meson bits for the Windows plugin support. Finally I
noticed nios2 signal handling is broken again so I've disabled testing
again and we shouldn't re-enable it until we can defend in CI (if we
ever want to, the architecture is currently orphaned).

I'll be rolling the PR tomorrow for soft-freeze so this is your last
chance to complain ;-)

The following still need review:

  Revert "tests/tcg/nios2: Re-enable linux-user tests"
  configure: tell meson and contrib_plugins about DLLTOOL
  tests/avocado: update the tcg_plugins test
  tests/tcg: add an explicit gdbstub register tester
  target/arm: hide aliased MIDR from gdbstub
  target/arm: hide all versions of DBGD[RS]AR from gdbstub
  target/arm: hide the 32bit version of PAR from gdbstub

Akihiko Odaki (5):
  default-configs: Add TARGET_XML_FILES definition
  gdbstub: Add num_regs member to GDBFeature
  gdbstub: Introduce gdb_find_static_feature()
  gdbstub: Introduce GDBFeatureBuilder
  cpu: Call plugin hooks only when ready

Alex Bennée (12):
  gdb-xml: fix duplicate register in arm-neon.xml
  target/arm: hide the 32bit version of PAR from gdbstub
  target/arm: hide all versions of DBGD[RS]AR from gdbstub
  target/arm: hide aliased MIDR from gdbstub
  tests/tcg: add an explicit gdbstub register tester
  tests/avocado: update the tcg_plugins test
  configure: tell meson and contrib_plugins about DLLTOOL
  contrib/gitdm: Add Rivos Inc to the domain map
  contrib/gitdm: map HiSilicon to Huawei
  contrib/gitdm: add Daynix to domain-map
  mailmap: fixup some more corrupted author fields
  Revert "tests/tcg/nios2: Re-enable linux-user tests"

Greg Manning (4):
  plugins: add dllexport and dllimport to api funcs
  plugins: make test/example plugins work on windows
  plugins: disable lockstep plugin on windows
  plugins: allow plugins to be enabled on windows

luzhipeng (1):
  contrib/gitdm: add domain-map for Cestc

 configure                                     |  13 +-
 configs/targets/loongarch64-linux-user.mak    |   1 +
 meson.build                                   |   5 +
 include/exec/gdbstub.h                        |  59 ++++++
 include/qemu/qemu-plugin.h                    |  50 ++++-
 contrib/plugins/win32_linker.c                |  34 +++
 cpu-target.c                                  |  11 -
 gdbstub/gdbstub.c                             |  78 +++++++
 hw/core/cpu-common.c                          |  10 +
 target/arm/debug_helper.c                     |   8 +-
 target/arm/helper.c                           |   4 +-
 .mailmap                                      |   2 +
 contrib/gitdm/domain-map                      |   4 +
 contrib/plugins/Makefile                      |  26 ++-
 gdb-xml/arm-neon.xml                          |   2 +-
 plugins/meson.build                           |  19 ++
 scripts/feature_to_c.py                       |  46 +++-
 tests/avocado/tcg_plugins.py                  |  28 ++-
 tests/plugin/meson.build                      |  14 +-
 tests/tcg/multiarch/Makefile.target           |  11 +-
 tests/tcg/multiarch/gdbstub/registers.py      | 196 ++++++++++++++++++
 .../multiarch/system/Makefile.softmmu-target  |  13 +-
 tests/tcg/nios2/Makefile.target               |  11 +
 23 files changed, 601 insertions(+), 44 deletions(-)
 create mode 100644 contrib/plugins/win32_linker.c
 create mode 100644 tests/tcg/multiarch/gdbstub/registers.py
 create mode 100644 tests/tcg/nios2/Makefile.target

-- 
2.39.2

[PATCH 01/22] default-configs: Add TARGET_XML_FILES definition

[Alex Bennée]

From: Akihiko Odaki <akihiko.odaki@daynix.com>

loongarch64-linux-user has references to XML files so include them.

Fixes: d32688ecdb ("default-configs: Add loongarch linux-user support")
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Message-Id: <20231030054834.39145-6-akihiko.odaki@daynix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20231103195956.1998255-2-alex.bennee@linaro.org>
[AJB: remove base32 from list]
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 configs/targets/loongarch64-linux-user.mak | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/targets/loongarch64-linux-user.mak b/configs/targets/loongarch64-linux-user.mak
index 7d1b964020..d878e5a113 100644
--- a/configs/targets/loongarch64-linux-user.mak
+++ b/configs/targets/loongarch64-linux-user.mak
@@ -1,3 +1,4 @@
 # Default configuration for loongarch64-linux-user
 TARGET_ARCH=loongarch64
 TARGET_BASE_ARCH=loongarch
+TARGET_XML_FILES=gdb-xml/loongarch-base64.xml gdb-xml/loongarch-fpu.xml
-- 
2.39.2

Re: [PATCH 01/22] default-configs: Add TARGET_XML_FILES definition

[Richard Henderson]

On 11/6/23 10:50, Alex Bennée wrote:
> From: Akihiko Odaki <akihiko.odaki@daynix.com>
> 
> loongarch64-linux-user has references to XML files so include them.
> 
> Fixes: d32688ecdb ("default-configs: Add loongarch linux-user support")
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> Message-Id: <20231030054834.39145-6-akihiko.odaki@daynix.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Message-Id: <20231103195956.1998255-2-alex.bennee@linaro.org>
> [AJB: remove base32 from list]
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   configs/targets/loongarch64-linux-user.mak | 1 +
>   1 file changed, 1 insertion(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

> 
> diff --git a/configs/targets/loongarch64-linux-user.mak b/configs/targets/loongarch64-linux-user.mak
> index 7d1b964020..d878e5a113 100644
> --- a/configs/targets/loongarch64-linux-user.mak
> +++ b/configs/targets/loongarch64-linux-user.mak
> @@ -1,3 +1,4 @@
>   # Default configuration for loongarch64-linux-user
>   TARGET_ARCH=loongarch64
>   TARGET_BASE_ARCH=loongarch
> +TARGET_XML_FILES=gdb-xml/loongarch-base64.xml gdb-xml/loongarch-fpu.xml

[PATCH 02/22] gdb-xml: fix duplicate register in arm-neon.xml

[Alex Bennée]

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20231103195956.1998255-3-alex.bennee@linaro.org>
---
 gdb-xml/arm-neon.xml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb-xml/arm-neon.xml b/gdb-xml/arm-neon.xml
index 9dce0a996f..d61f6b8549 100644
--- a/gdb-xml/arm-neon.xml
+++ b/gdb-xml/arm-neon.xml
@@ -76,7 +76,7 @@
   <reg name="q8" bitsize="128" type="neon_q"/>
   <reg name="q9" bitsize="128" type="neon_q"/>
   <reg name="q10" bitsize="128" type="neon_q"/>
-  <reg name="q10" bitsize="128" type="neon_q"/>
+  <reg name="q11" bitsize="128" type="neon_q"/>
   <reg name="q12" bitsize="128" type="neon_q"/>
   <reg name="q13" bitsize="128" type="neon_q"/>
   <reg name="q14" bitsize="128" type="neon_q"/>
-- 
2.39.2

Re: [PATCH 02/22] gdb-xml: fix duplicate register in arm-neon.xml

[Philippe Mathieu-Daudé]

On 6/11/23 19:50, Alex Bennée wrote:
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20231103195956.1998255-3-alex.bennee@linaro.org>
> ---
>   gdb-xml/arm-neon.xml | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Cc: qemu-stable@nongnu.org
Fixes: 56aebc8916 ("Add GDB XML register description support")
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 03/22] target/arm: hide the 32bit version of PAR from gdbstub

[Alex Bennée]

This is a slightly hacky way to avoid duplicate PAR's in the system
register XML we send to gdb which causes an alias. However the other
alternative would be to post process ARMCPRegInfo once all registers
have been defined looking for textual duplicates. And that seems like
overkill.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20231103195956.1998255-4-alex.bennee@linaro.org>
---
 target/arm/helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 5dc0d20a84..104f9378b4 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -3727,7 +3727,7 @@ static const ARMCPRegInfo vapa_cp_reginfo[] = {
       .access = PL1_RW, .resetvalue = 0,
       .bank_fieldoffsets = { offsetoflow32(CPUARMState, cp15.par_s),
                              offsetoflow32(CPUARMState, cp15.par_ns) },
-      .writefn = par_write },
+      .writefn = par_write, .type = ARM_CP_NO_GDB },
 #ifndef CONFIG_USER_ONLY
     /* This underdecoding is safe because the reginfo is NO_RAW. */
     { .name = "ATS", .cp = 15, .crn = 7, .crm = 8, .opc1 = 0, .opc2 = CP_ANY,
-- 
2.39.2

Re: [PATCH 03/22] target/arm: hide the 32bit version of PAR from gdbstub

[Richard Henderson]

On 11/6/23 10:50, Alex Bennée wrote:
> This is a slightly hacky way to avoid duplicate PAR's in the system
> register XML we send to gdb which causes an alias. However the other
> alternative would be to post process ARMCPRegInfo once all registers
> have been defined looking for textual duplicates. And that seems like
> overkill.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20231103195956.1998255-4-alex.bennee@linaro.org>
> ---
>   target/arm/helper.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 5dc0d20a84..104f9378b4 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -3727,7 +3727,7 @@ static const ARMCPRegInfo vapa_cp_reginfo[] = {
>         .access = PL1_RW, .resetvalue = 0,
>         .bank_fieldoffsets = { offsetoflow32(CPUARMState, cp15.par_s),
>                                offsetoflow32(CPUARMState, cp15.par_ns) },
> -      .writefn = par_write },
> +      .writefn = par_write, .type = ARM_CP_NO_GDB },
>   #ifndef CONFIG_USER_ONLY
>       /* This underdecoding is safe because the reginfo is NO_RAW. */
>       { .name = "ATS", .cp = 15, .crn = 7, .crm = 8, .opc1 = 0, .opc2 = CP_ANY,

If the implementation includes LPAE, this is an alias of the full 64-bit register (the 
"other PAR", and so type should contain ARM_CP_ALIAS.

If the implementation does not include LPAE, this should not be ARM_CP_NO_GDB because 
there is no 64-bit alternate.


r~

Re: [PATCH 03/22] target/arm: hide the 32bit version of PAR from gdbstub

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> On 11/6/23 10:50, Alex Bennée wrote:
>> This is a slightly hacky way to avoid duplicate PAR's in the system
>> register XML we send to gdb which causes an alias. However the other
>> alternative would be to post process ARMCPRegInfo once all registers
>> have been defined looking for textual duplicates. And that seems like
>> overkill.
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Message-Id: <20231103195956.1998255-4-alex.bennee@linaro.org>
>> ---
>>   target/arm/helper.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>> index 5dc0d20a84..104f9378b4 100644
>> --- a/target/arm/helper.c
>> +++ b/target/arm/helper.c
>> @@ -3727,7 +3727,7 @@ static const ARMCPRegInfo vapa_cp_reginfo[] = {
>>         .access = PL1_RW, .resetvalue = 0,
>>         .bank_fieldoffsets = { offsetoflow32(CPUARMState, cp15.par_s),
>>                                offsetoflow32(CPUARMState, cp15.par_ns) },
>> -      .writefn = par_write },
>> +      .writefn = par_write, .type = ARM_CP_NO_GDB },
>>   #ifndef CONFIG_USER_ONLY
>>       /* This underdecoding is safe because the reginfo is NO_RAW. */
>>       { .name = "ATS", .cp = 15, .crn = 7, .crm = 8, .opc1 = 0, .opc2 = CP_ANY,
>
> If the implementation includes LPAE, this is an alias of the full
> 64-bit register (the "other PAR", and so type should contain
> ARM_CP_ALIAS.
>
> If the implementation does not include LPAE, this should not be
> ARM_CP_NO_GDB because there is no 64-bit alternate.

I did discuss with Peter on IRC but its a lot of hoop jumping for very
little benefit. But I guess I can add some logic to do that.

  [17:58:51] <pm215> no, those are different registers. the PAR in lpae_cp_reginfo is ARM_CP_64BIT, and the one in vapa_cp_reginfo is not
  [17:59:51] > ah yes slightly different opcs
  [17:59:54] >     { .name = "PAR", .cp = 15, .crm = 7, .opc1 = 0,
  [18:00:07] > vs
  [18:00:09] >     { .name = "PAR", .cp = 15, .crn = 7, .crm = 4, .opc1 = 0, .opc2 = 0,
  [18:00:18] <pm215> the important bit is the ARM_CP_64BIT
  [18:00:20] > pm215 should they have differnt names then?
  [18:00:39] <pm215> historically we have said that the .name field is purely for debug purposes and not worried too much about overlap
  [18:01:27] <pm215> architecturally these are considered two different access methods for getting at the same PAR register
  [18:01:54] <pm215> (unlike the AArch64 PAR_EL1, which is considered a separate register whose bits are 'architecturally mapped' to the AArch32 PAR)
  [18:02:51] > PAR_A32 and PAR_A64?
  [18:03:19] <pm215> no
  [18:03:20] > afterall these are extra registers we report to gdb so we can control the naming
  [18:03:24] <pm215> the AArch64 register is PAR_EL1
  [18:03:43] > or so PAR and PAR_EL1
  [18:03:58] <pm215> the AArch32 register is PAR, and there are two different ways to read it, with the 32-bit MRC/MCR insns, or with the 64-bit MCRR/MRRC insns
  [18:04:23] > oh we define PAR_EL1 as well
  [18:04:26] >     { .name = "PAR_EL1", .state = ARM_CP_STATE_AA64,
  [18:04:26] >       .type = ARM_CP_ALIAS,
  [18:04:26] >       .opc0 = 3, .opc1 = 0, .crn = 7, .crm = 4, .opc2 = 0,
  [18:04:29] <pm215> yep
  [18:05:08] > PAR, PAR_A64 and PAR_El1?
  [18:05:20] <pm215> PAR_A64 implies AArch64, which it is not
  [18:05:34] > PAR_64?
  [18:05:49] <pm215> what are you trying to achieve here? that every cpreg has a distinct name string?
  [18:06:22] > pm215 yes - as we expose them to gdb and hence are over writing one definition with the other
  [18:06:40] <pm215> the correct way to expose them to the user would be to only present one PAR, which is 64 bits
  [18:06:56] <pm215> i.e. if the 64-bit version is present, ignore the 32-bit version
  [18:07:09] * stsquad2 has also fixed a duplicate q10 in arm-neon.xml
  [18:07:39] <pm215> duplicate q10> I wonder if that's in gdb's copy too?
  [18:09:14] > this is qemu-arm so
  [18:09:36] <pm215> yes, qemu-arm will have both the 32-bit and 64-bit PAR
  [18:09:40] <pm215> (depending on cpu type)
  [18:10:20] > *sigh*
  [18:10:33] > I suspect this will be messy to fix... 
  [18:11:02] <pm215> the gdb exposure of sysregs was always a bit of a "this basically works" hack
  [18:11:36] <pm215> ideally for an AArch64 vcpu we would expose them with the correct-for-AArch64 names, but we don't in many cases where the regdef is shared with AArch32
  [18:12:24] > so I could split vapa_cp_reginfo into two parts and do a check on if the 64 bit feature exits before adding it?
  [18:13:00] <pm215> that won't really help you, because on a CPU with FEAT_LPAE we need both regdefs
  [18:13:42] * stsquad2 does not want to add a .dont_expose_to_gdb field to ARMCPRegInfo
  [18:14:21] <pm215> stsquad2: we already have ARM_CP_NO_GDB :-)
  [18:17:19] > pm215 we have logic that applies         r2->type |= ARM_CP_ALIAS | ARM_CP_NO_GDB;
  [18:19:40] <pm215> stsquad2: yeah, that's for the CP_ANY wildcarding. But there's no inherent reason you couldn't mark a regdef with ARM_CP_NO_GDB by hand
  [18:19:51] > pm215 looking at the commit that added it: "This bit could be enabled manually for any register we want to remove from the
  [18:19:51] > dynamic XML description." 
  [18:20:32] <pm215> (ARM_CP_NO_RAW registers also don't get exposed to gdb)
  [18:20:49] > pm215 so I'll just add it to the 32 bit register and wait for the 1 user in the future to complain they can't see it out of the 100s of sysregs we expose
  [18:21:28] <pm215> that breaks reading PAR from gdbstub for any pre-v7VE CPU
  [18:22:05] > pm215 we can argue the case on the review ;-)
  [18:22:33] > in the current case its failing a linux-user test which is explicitly -cpu max anyway
  [18:30:52] * stsquad2 looks at DBGDSAR next


>
>
> r~

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

[PATCH 04/22] target/arm: hide all versions of DBGD[RS]AR from gdbstub

[Alex Bennée]

This avoids two duplicates being presented to gdbstub. As the
registers are RAZ anyway it is unlikely their value would be of use to
someone using gdbstub anyway.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20231103195956.1998255-5-alex.bennee@linaro.org>
---
 target/arm/debug_helper.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
index 79a3659c0c..dc783adba5 100644
--- a/target/arm/debug_helper.c
+++ b/target/arm/debug_helper.c
@@ -937,14 +937,14 @@ static const ARMCPRegInfo debug_cp_reginfo[] = {
      */
     { .name = "DBGDRAR", .cp = 14, .crn = 1, .crm = 0, .opc1 = 0, .opc2 = 0,
       .access = PL0_R, .accessfn = access_tdra,
-      .type = ARM_CP_CONST, .resetvalue = 0 },
+      .type = ARM_CP_CONST | ARM_CP_NO_GDB, .resetvalue = 0 },
     { .name = "MDRAR_EL1", .state = ARM_CP_STATE_AA64,
       .opc0 = 2, .opc1 = 0, .crn = 1, .crm = 0, .opc2 = 0,
       .access = PL1_R, .accessfn = access_tdra,
       .type = ARM_CP_CONST, .resetvalue = 0 },
     { .name = "DBGDSAR", .cp = 14, .crn = 2, .crm = 0, .opc1 = 0, .opc2 = 0,
       .access = PL0_R, .accessfn = access_tdra,
-      .type = ARM_CP_CONST, .resetvalue = 0 },
+      .type = ARM_CP_CONST | ARM_CP_NO_GDB, .resetvalue = 0 },
     /* Monitor debug system control register; the 32-bit alias is DBGDSCRext. */
     { .name = "MDSCR_EL1", .state = ARM_CP_STATE_BOTH,
       .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = 2, .opc2 = 2,
@@ -1065,9 +1065,9 @@ static const ARMCPRegInfo debug_cp_reginfo[] = {
 static const ARMCPRegInfo debug_lpae_cp_reginfo[] = {
     /* 64 bit access versions of the (dummy) debug registers */
     { .name = "DBGDRAR", .cp = 14, .crm = 1, .opc1 = 0,
-      .access = PL0_R, .type = ARM_CP_CONST | ARM_CP_64BIT, .resetvalue = 0 },
+      .access = PL0_R, .type = ARM_CP_CONST | ARM_CP_64BIT | ARM_CP_NO_GDB, .resetvalue = 0 },
     { .name = "DBGDSAR", .cp = 14, .crm = 2, .opc1 = 0,
-      .access = PL0_R, .type = ARM_CP_CONST | ARM_CP_64BIT, .resetvalue = 0 },
+      .access = PL0_R, .type = ARM_CP_CONST | ARM_CP_64BIT | ARM_CP_NO_GDB, .resetvalue = 0 },
 };
 
 static void dbgwvr_write(CPUARMState *env, const ARMCPRegInfo *ri,
-- 
2.39.2

Re: [PATCH 04/22] target/arm: hide all versions of DBGD[RS]AR from gdbstub

[Richard Henderson]

On 11/6/23 10:50, Alex Bennée wrote:
> This avoids two duplicates being presented to gdbstub. As the
> registers are RAZ anyway it is unlikely their value would be of use to
> someone using gdbstub anyway.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20231103195956.1998255-5-alex.bennee@linaro.org>
> ---
>   target/arm/debug_helper.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)

Acked-by: Richard Henderson <richard.henderson@linaro.org>


r~

> 
> diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
> index 79a3659c0c..dc783adba5 100644
> --- a/target/arm/debug_helper.c
> +++ b/target/arm/debug_helper.c
> @@ -937,14 +937,14 @@ static const ARMCPRegInfo debug_cp_reginfo[] = {
>        */
>       { .name = "DBGDRAR", .cp = 14, .crn = 1, .crm = 0, .opc1 = 0, .opc2 = 0,
>         .access = PL0_R, .accessfn = access_tdra,
> -      .type = ARM_CP_CONST, .resetvalue = 0 },
> +      .type = ARM_CP_CONST | ARM_CP_NO_GDB, .resetvalue = 0 },
>       { .name = "MDRAR_EL1", .state = ARM_CP_STATE_AA64,
>         .opc0 = 2, .opc1 = 0, .crn = 1, .crm = 0, .opc2 = 0,
>         .access = PL1_R, .accessfn = access_tdra,
>         .type = ARM_CP_CONST, .resetvalue = 0 },
>       { .name = "DBGDSAR", .cp = 14, .crn = 2, .crm = 0, .opc1 = 0, .opc2 = 0,
>         .access = PL0_R, .accessfn = access_tdra,
> -      .type = ARM_CP_CONST, .resetvalue = 0 },
> +      .type = ARM_CP_CONST | ARM_CP_NO_GDB, .resetvalue = 0 },
>       /* Monitor debug system control register; the 32-bit alias is DBGDSCRext. */
>       { .name = "MDSCR_EL1", .state = ARM_CP_STATE_BOTH,
>         .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = 2, .opc2 = 2,
> @@ -1065,9 +1065,9 @@ static const ARMCPRegInfo debug_cp_reginfo[] = {
>   static const ARMCPRegInfo debug_lpae_cp_reginfo[] = {
>       /* 64 bit access versions of the (dummy) debug registers */
>       { .name = "DBGDRAR", .cp = 14, .crm = 1, .opc1 = 0,
> -      .access = PL0_R, .type = ARM_CP_CONST | ARM_CP_64BIT, .resetvalue = 0 },
> +      .access = PL0_R, .type = ARM_CP_CONST | ARM_CP_64BIT | ARM_CP_NO_GDB, .resetvalue = 0 },
>       { .name = "DBGDSAR", .cp = 14, .crm = 2, .opc1 = 0,
> -      .access = PL0_R, .type = ARM_CP_CONST | ARM_CP_64BIT, .resetvalue = 0 },
> +      .access = PL0_R, .type = ARM_CP_CONST | ARM_CP_64BIT | ARM_CP_NO_GDB, .resetvalue = 0 },
>   };
>   
>   static void dbgwvr_write(CPUARMState *env, const ARMCPRegInfo *ri,

[PATCH 05/22] target/arm: hide aliased MIDR from gdbstub

[Alex Bennée]

This is just a constant alias register with the same value as the
"other" MIDR so it serves no purpose being presented to gdbstub.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20231103195956.1998255-6-alex.bennee@linaro.org>
---
 target/arm/helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 104f9378b4..a681bcba62 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -8993,7 +8993,7 @@ void register_cp_regs_for_features(ARMCPU *cpu)
               .type = ARM_CP_CONST, .resetvalue = cpu->revidr },
         };
         ARMCPRegInfo id_v8_midr_alias_cp_reginfo = {
-            .name = "MIDR", .type = ARM_CP_ALIAS | ARM_CP_CONST,
+            .name = "MIDR", .type = ARM_CP_ALIAS | ARM_CP_CONST | ARM_CP_NO_GDB,
             .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 4,
             .access = PL1_R, .resetvalue = cpu->midr
         };
-- 
2.39.2

Re: [PATCH 05/22] target/arm: hide aliased MIDR from gdbstub

[Peter Maydell]

On Mon, 6 Nov 2023 at 18:51, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> This is just a constant alias register with the same value as the
> "other" MIDR so it serves no purpose being presented to gdbstub.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20231103195956.1998255-6-alex.bennee@linaro.org>
> ---
>  target/arm/helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 104f9378b4..a681bcba62 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -8993,7 +8993,7 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>                .type = ARM_CP_CONST, .resetvalue = cpu->revidr },
>          };
>          ARMCPRegInfo id_v8_midr_alias_cp_reginfo = {
> -            .name = "MIDR", .type = ARM_CP_ALIAS | ARM_CP_CONST,
> +            .name = "MIDR", .type = ARM_CP_ALIAS | ARM_CP_CONST | ARM_CP_NO_GDB,
>              .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 4,
>              .access = PL1_R, .resetvalue = cpu->midr
>          };

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

It's a bit weird to hide this one but not the alias in
id_v8_midr_cp_reginfo[], though.

If we want to expose all these things to gdb I wonder
if we should have the ARMCPRegInfo give both the AArch64
and the AArch32 names for the registers, so that the
user sees the register names they expect, rather than
having their names be a random mix of the two.

thanks
-- PMM

[PATCH 06/22] tests/tcg: add an explicit gdbstub register tester

[Alex Bennée]

We already do a couple of "info registers" for specific tests but this
is a more comprehensive multiarch test. It also has some output
helpful for debugging the gdbstub by showing which XML features are
advertised and what the underlying register numbers are.

My initial motivation was to see if there are any duplicate register
names exposed via the gdbstub while I was reviewing the proposed
register interface for TCG plugins.

Mismatches between the xml and remote-desc are reported for debugging
but do not fail the test.

Cc: Akihiko Odaki <akihiko.odaki@daynix.com>
Cc: Luis Machado <luis.machado@linaro.org>
Message-Id: <20231012170426.1335442-1-alex.bennee@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20231103195956.1998255-7-alex.bennee@linaro.org>

---
v3
  - don't __str__ values when we don't need to
  - handle _exit/_Exit disparity on Microblaze
  - move seen test outside try/except
  - report failed register reads
---
 tests/tcg/multiarch/Makefile.target           |  11 +-
 tests/tcg/multiarch/gdbstub/registers.py      | 196 ++++++++++++++++++
 .../multiarch/system/Makefile.softmmu-target  |  13 +-
 3 files changed, 218 insertions(+), 2 deletions(-)
 create mode 100644 tests/tcg/multiarch/gdbstub/registers.py

diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target
index f3bfaf1a22..d31ba8d6ae 100644
--- a/tests/tcg/multiarch/Makefile.target
+++ b/tests/tcg/multiarch/Makefile.target
@@ -93,12 +93,21 @@ run-gdbstub-thread-breakpoint: testthread
 		--qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
 		--bin $< --test $(MULTIARCH_SRC)/gdbstub/test-thread-breakpoint.py, \
 	hitting a breakpoint on non-main thread)
+
+run-gdbstub-registers: sha512
+	$(call run-test, $@, $(GDB_SCRIPT) \
+		--gdb $(GDB) \
+		--qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
+		--bin $< --test $(MULTIARCH_SRC)/gdbstub/registers.py, \
+	checking register enumeration)
+
 else
 run-gdbstub-%:
 	$(call skip-test, "gdbstub test $*", "need working gdb with $(patsubst -%,,$(TARGET_NAME)) support")
 endif
 EXTRA_RUNS += run-gdbstub-sha1 run-gdbstub-qxfer-auxv-read \
-	      run-gdbstub-proc-mappings run-gdbstub-thread-breakpoint
+	      run-gdbstub-proc-mappings run-gdbstub-thread-breakpoint \
+	      run-gdbstub-registers
 
 # ARM Compatible Semi Hosting Tests
 #
diff --git a/tests/tcg/multiarch/gdbstub/registers.py b/tests/tcg/multiarch/gdbstub/registers.py
new file mode 100644
index 0000000000..4ae63f1cf0
--- /dev/null
+++ b/tests/tcg/multiarch/gdbstub/registers.py
@@ -0,0 +1,196 @@
+# Exercise the register functionality by exhaustively iterating
+# through all supported registers on the system.
+#
+# This is launched via tests/guest-debug/run-test.py but you can also
+# call it directly if using it for debugging/introspection:
+#
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+import gdb
+import sys
+import xml.etree.ElementTree as ET
+
+initial_vlen = 0
+failcount = 0
+
+def report(cond, msg):
+    "Report success/fail of test."
+    if cond:
+        print("PASS: %s" % (msg))
+    else:
+        print("FAIL: %s" % (msg))
+        global failcount
+        failcount += 1
+
+
+def fetch_xml_regmap():
+    """
+    Iterate through the XML descriptions and validate.
+
+    We check for any duplicate registers and report them. Return a
+    reg_map hash containing the names, regnums and initial values of
+    all registers.
+    """
+
+    # First check the XML descriptions we have sent. Most arches
+    # support XML but a few of the ancient ones don't in which case we
+    # need to gracefully fail.
+
+    try:
+        xml = gdb.execute("maint print xml-tdesc", False, True)
+    except (gdb.error):
+        print("SKIP: target does not support XML")
+        return None
+
+    total_regs = 0
+    reg_map = {}
+    frame = gdb.selected_frame()
+
+    tree = ET.fromstring(xml)
+    for f in tree.findall("feature"):
+        name = f.attrib["name"]
+        regs = f.findall("reg")
+
+        total = len(regs)
+        total_regs += total
+        base = int(regs[0].attrib["regnum"])
+        top = int(regs[-1].attrib["regnum"])
+
+        print(f"feature: {name} has {total} registers from {base} to {top}")
+
+        for r in regs:
+            name = r.attrib["name"]
+            regnum = int(r.attrib["regnum"])
+            try:
+                value = frame.read_register(name)
+            except ValueError:
+                report(False, f"failed to read reg: {name}")
+
+            entry = { "name": name, "initial": value, "regnum": regnum }
+
+            if name in reg_map:
+                report(False, f"duplicate register {entry} vs {reg_map[name]}")
+                continue
+
+            reg_map[name] = entry
+
+    # Validate we match
+    report(total_regs == len(reg_map.keys()),
+           f"counted all {total_regs} registers in XML")
+
+    return reg_map
+
+def crosscheck_remote_xml(reg_map):
+    """
+    Cross-check the list of remote-registers with the XML info.
+    """
+
+    remote = gdb.execute("maint print remote-registers", False, True)
+    r_regs = remote.split("\n")
+
+    total_regs = len(reg_map.keys())
+    total_r_regs = 0
+
+    for r in r_regs:
+        fields = r.split()
+        # Some of the registers reported here are "pseudo" registers that
+        # gdb invents based on actual registers so we need to filter them
+        # out.
+        if len(fields) == 8:
+            r_name = fields[0]
+            r_regnum = int(fields[6])
+
+            # check in the XML
+            try:
+                x_reg = reg_map[r_name]
+            except KeyError:
+                report(False, f"{r_name} not in XML description")
+                continue
+
+            x_reg["seen"] = True
+            x_regnum = x_reg["regnum"]
+            if r_regnum != x_regnum:
+                report(False, f"{r_name} {r_regnum} == {x_regnum} (xml)")
+            else:
+                total_r_regs += 1
+
+    # Just print a mismatch in totals as gdb will filter out 64 bit
+    # registers on a 32 bit machine. Also print what is missing to
+    # help with debug.
+    if total_regs != total_r_regs:
+        print(f"xml-tdesc ({total_regs}) and remote-registers ({total_r_regs}) do not agree")
+
+        for x_key in reg_map.keys():
+            x_reg = reg_map[x_key]
+            if "seen" not in x_reg:
+                print(f"{x_reg} wasn't seen in remote-registers")
+
+def complete_and_diff(reg_map):
+    """
+    Let the program run to (almost) completion and then iterate
+    through all the registers we know about and report which ones have
+    changed.
+    """
+    # Let the program get to the end and we can check what changed
+    b = gdb.Breakpoint("_exit")
+    if b.pending: # workaround Microblaze weirdness
+        b.delete()
+        gdb.Breakpoint("_Exit")
+
+    gdb.execute("continue")
+
+    frame = gdb.selected_frame()
+    changed = 0
+
+    for e in reg_map.values():
+        name = e["name"]
+        old_val = e["initial"]
+
+        try:
+            new_val = frame.read_register(name)
+        except:
+            report(False, f"failed to read {name} at end of run")
+            continue
+
+        if new_val != old_val:
+            print(f"{name} changes from {old_val} to {new_val}")
+            changed += 1
+
+    # as long as something changed we can be confident its working
+    report(changed > 0, f"{changed} registers were changed")
+
+
+def run_test():
+    "Run through the tests"
+
+    reg_map = fetch_xml_regmap()
+
+    if reg_map is not None:
+        crosscheck_remote_xml(reg_map)
+        complete_and_diff(reg_map)
+
+
+#
+# This runs as the script it sourced (via -x, via run-test.py)
+#
+try:
+    inferior = gdb.selected_inferior()
+    arch = inferior.architecture()
+    print("ATTACHED: %s" % arch.name())
+except (gdb.error, AttributeError):
+    print("SKIPPING (not connected)", file=sys.stderr)
+    exit(0)
+
+if gdb.parse_and_eval('$pc') == 0:
+    print("SKIP: PC not set")
+    exit(0)
+
+try:
+    run_test()
+except (gdb.error):
+    print ("GDB Exception: %s" % (sys.exc_info()[0]))
+    failcount += 1
+    pass
+
+print("All tests complete: %d failures" % failcount)
+exit(failcount)
diff --git a/tests/tcg/multiarch/system/Makefile.softmmu-target b/tests/tcg/multiarch/system/Makefile.softmmu-target
index dee4f58dea..32dc0f9830 100644
--- a/tests/tcg/multiarch/system/Makefile.softmmu-target
+++ b/tests/tcg/multiarch/system/Makefile.softmmu-target
@@ -48,9 +48,20 @@ run-gdbstub-untimely-packet: hello
 	$(call quiet-command, \
 		(! grep -Fq 'Packet instead of Ack, ignoring it' untimely-packet.gdb.err), \
 		"GREP", file untimely-packet.gdb.err)
+
+run-gdbstub-registers: memory
+	$(call run-test, $@, $(GDB_SCRIPT) \
+		--gdb $(GDB) \
+		--qemu $(QEMU) \
+		--output $<.registers.gdb.out \
+		--qargs \
+		"-monitor none -display none -chardev file$(COMMA)path=$<.out$(COMMA)id=output $(QEMU_OPTS)" \
+		--bin $< --test $(MULTIARCH_SRC)/gdbstub/registers.py, \
+	softmmu gdbstub support)
 else
 run-gdbstub-%:
 	$(call skip-test, "gdbstub test $*", "need working gdb with $(patsubst -%,,$(TARGET_NAME)) support")
 endif
 
-MULTIARCH_RUNS += run-gdbstub-memory run-gdbstub-interrupt run-gdbstub-untimely-packet
+MULTIARCH_RUNS += run-gdbstub-memory run-gdbstub-interrupt \
+	run-gdbstub-untimely-packet run-gdbstub-registers
-- 
2.39.2

[PATCH 07/22] tests/avocado: update the tcg_plugins test

[Alex Bennée]

There are a number of things that are broken on the test currently so
lets fix that up:

  - replace retired Debian kernel for tuxrun_baseline one
  - remove "detected repeat instructions test" since ea185a55
  - log total counted instructions/memory accesses

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20231103195956.1998255-8-alex.bennee@linaro.org>
---
 tests/avocado/tcg_plugins.py | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/tests/avocado/tcg_plugins.py b/tests/avocado/tcg_plugins.py
index 642d2e49e3..15fd87b2c1 100644
--- a/tests/avocado/tcg_plugins.py
+++ b/tests/avocado/tcg_plugins.py
@@ -54,13 +54,11 @@ def run_vm(self, kernel_path, kernel_command_line,
 class PluginKernelNormal(PluginKernelBase):
 
     def _grab_aarch64_kernel(self):
-        kernel_url = ('http://security.debian.org/'
-                      'debian-security/pool/updates/main/l/linux-signed-arm64/'
-                      'linux-image-4.19.0-12-arm64_4.19.152-1_arm64.deb')
-        kernel_sha1 = '2036c2792f80ac9c4ccaae742b2e0a28385b6010'
-        kernel_deb = self.fetch_asset(kernel_url, asset_hash=kernel_sha1)
-        kernel_path = self.extract_from_deb(kernel_deb,
-                                            "/boot/vmlinuz-4.19.0-12-arm64")
+        kernel_url = ('https://storage.tuxboot.com/20230331/arm64/Image')
+        kernel_sha256 = 'ce95a7101a5fecebe0fe630deee6bd97b32ba41bc8754090e9ad8961ea8674c7'
+        kernel_path = self.fetch_asset(kernel_url,
+                                       asset_hash=kernel_sha256,
+                                       algorithm = "sha256")
         return kernel_path
 
     def test_aarch64_virt_insn(self):
@@ -88,6 +86,10 @@ def test_aarch64_virt_insn(self):
             m = re.search(br"insns: (?P<count>\d+)", s)
             if "count" not in m.groupdict():
                 self.fail("Failed to find instruction count")
+            else:
+                count = int(m.group("count"))
+                self.log.info(f"Counted: {count} instructions")
+
 
     def test_aarch64_virt_insn_icount(self):
         """
@@ -111,9 +113,13 @@ def test_aarch64_virt_insn_icount(self):
 
         with plugin_log as lf, \
              mmap.mmap(lf.fileno(), 0, access=mmap.ACCESS_READ) as s:
-            m = re.search(br"detected repeat execution @ (?P<addr>0x[0-9A-Fa-f]+)", s)
-            if m is not None and "addr" in m.groupdict():
-                self.fail("detected repeated instructions")
+
+            m = re.search(br"insns: (?P<count>\d+)", s)
+            if "count" not in m.groupdict():
+                self.fail("Failed to find instruction count")
+            else:
+                count = int(m.group("count"))
+                self.log.info(f"Counted: {count} instructions")
 
     def test_aarch64_virt_mem_icount(self):
         """
@@ -145,3 +151,5 @@ def test_aarch64_virt_mem_icount(self):
                 callback = int(m[1])
                 if inline != callback:
                     self.fail("mismatched access counts")
+                else:
+                    self.log.info(f"Counted {inline} memory accesses")
-- 
2.39.2

Re: [PATCH 07/22] tests/avocado: update the tcg_plugins test

[Richard Henderson]

On 11/6/23 10:50, Alex Bennée wrote:
> There are a number of things that are broken on the test currently so
> lets fix that up:
> 
>    - replace retired Debian kernel for tuxrun_baseline one
>    - remove "detected repeat instructions test" since ea185a55
>    - log total counted instructions/memory accesses
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20231103195956.1998255-8-alex.bennee@linaro.org>
> ---
>   tests/avocado/tcg_plugins.py | 28 ++++++++++++++++++----------
>   1 file changed, 18 insertions(+), 10 deletions(-)

Acked-by: Richard Henderson <richard.henderson@linaro.org>


r~

[PATCH 08/22] gdbstub: Add num_regs member to GDBFeature

[Alex Bennée]

From: Akihiko Odaki <akihiko.odaki@daynix.com>

Currently the number of registers exposed to GDB is written as magic
numbers in code. Derive the number of registers GDB actually see from
XML files to replace the magic numbers in code later.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20231025093128.33116-2-akihiko.odaki@daynix.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20231103195956.1998255-9-alex.bennee@linaro.org>
---
 include/exec/gdbstub.h  |  1 +
 scripts/feature_to_c.py | 46 +++++++++++++++++++++++++++++++++++++++--
 2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 1a01c35f8e..a43aa34dad 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -13,6 +13,7 @@
 typedef struct GDBFeature {
     const char *xmlname;
     const char *xml;
+    int num_regs;
 } GDBFeature;
 
 
diff --git a/scripts/feature_to_c.py b/scripts/feature_to_c.py
index bcbcb83beb..e04d6b2df7 100644
--- a/scripts/feature_to_c.py
+++ b/scripts/feature_to_c.py
@@ -1,7 +1,7 @@
 #!/usr/bin/env python3
 # SPDX-License-Identifier: GPL-2.0-or-later
 
-import os, sys
+import os, sys, xml.etree.ElementTree
 
 def writeliteral(indent, bytes):
     sys.stdout.write(' ' * indent)
@@ -39,10 +39,52 @@ def writeliteral(indent, bytes):
     with open(input, 'rb') as file:
         read = file.read()
 
+    parser = xml.etree.ElementTree.XMLPullParser(['start', 'end'])
+    parser.feed(read)
+    events = parser.read_events()
+    event, element = next(events)
+    if event != 'start':
+        sys.stderr.write(f'unexpected event: {event}\n')
+        exit(1)
+    if element.tag != 'feature':
+        sys.stderr.write(f'unexpected start tag: {element.tag}\n')
+        exit(1)
+
+    regnum = 0
+    regnums = []
+    tags = ['feature']
+    for event, element in events:
+        if event == 'end':
+            if element.tag != tags[len(tags) - 1]:
+                sys.stderr.write(f'unexpected end tag: {element.tag}\n')
+                exit(1)
+
+            tags.pop()
+            if element.tag == 'feature':
+                break
+        elif event == 'start':
+            if len(tags) < 2 and element.tag == 'reg':
+                if 'regnum' in element.attrib:
+                    regnum = int(element.attrib['regnum'])
+
+                regnums.append(regnum)
+                regnum += 1
+
+            tags.append(element.tag)
+        else:
+            raise Exception(f'unexpected event: {event}\n')
+
+    if len(tags):
+        sys.stderr.write('unterminated feature tag\n')
+        exit(1)
+
+    base_reg = min(regnums)
+    num_regs = max(regnums) - base_reg + 1 if len(regnums) else 0
+
     sys.stdout.write('    {\n')
     writeliteral(8, bytes(os.path.basename(input), 'utf-8'))
     sys.stdout.write(',\n')
     writeliteral(8, read)
-    sys.stdout.write('\n    },\n')
+    sys.stdout.write(f',\n        {num_regs},\n    }},\n')
 
 sys.stdout.write('    { NULL }\n};\n')
-- 
2.39.2

Re: [PATCH 08/22] gdbstub: Add num_regs member to GDBFeature

[Philippe Mathieu-Daudé]

Hi Alex,

On 6/11/23 19:50, Alex Bennée wrote:
> From: Akihiko Odaki <akihiko.odaki@daynix.com>
> 
> Currently the number of registers exposed to GDB is written as magic
> numbers in code. Derive the number of registers GDB actually see from
> XML files to replace the magic numbers in code later.
> 
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20231025093128.33116-2-akihiko.odaki@daynix.com>

Something in your workflow is odd, you should keep this Message-Id,

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20231103195956.1998255-9-alex.bennee@linaro.org>

and not propagate this one, IMO.

> ---
>   include/exec/gdbstub.h  |  1 +
>   scripts/feature_to_c.py | 46 +++++++++++++++++++++++++++++++++++++++--
>   2 files changed, 45 insertions(+), 2 deletions(-)

Re: [PATCH 08/22] gdbstub: Add num_regs member to GDBFeature

[Alex Bennée]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Hi Alex,
>
> On 6/11/23 19:50, Alex Bennée wrote:
>> From: Akihiko Odaki <akihiko.odaki@daynix.com>
>> Currently the number of registers exposed to GDB is written as magic
>> numbers in code. Derive the number of registers GDB actually see from
>> XML files to replace the magic numbers in code later.
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>> Message-Id: <20231025093128.33116-2-akihiko.odaki@daynix.com>
>
> Something in your workflow is odd, you should keep this Message-Id,
>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Message-Id: <20231103195956.1998255-9-alex.bennee@linaro.org>
>
> and not propagate this one, IMO.

Why not - it tracks all the review stuff. I explicitly keep on
Message-Id per domain so we see the original posting and the last time
it was posted (which you can then follow the chain of reviews from
there).

If we want to have an explicit policy on which Message-Id's to keep then
we should document it.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 08/22] gdbstub: Add num_regs member to GDBFeature

[Philippe Mathieu-Daudé]

On 7/11/23 11:24, Alex Bennée wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
>> Hi Alex,
>>
>> On 6/11/23 19:50, Alex Bennée wrote:
>>> From: Akihiko Odaki <akihiko.odaki@daynix.com>
>>> Currently the number of registers exposed to GDB is written as magic
>>> numbers in code. Derive the number of registers GDB actually see from
>>> XML files to replace the magic numbers in code later.
>>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>>> Message-Id: <20231025093128.33116-2-akihiko.odaki@daynix.com>
>>
>> Something in your workflow is odd, you should keep this Message-Id,
>>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> Message-Id: <20231103195956.1998255-9-alex.bennee@linaro.org>
>>
>> and not propagate this one, IMO.
> 
> Why not - it tracks all the review stuff. I explicitly keep on
> Message-Id per domain so we see the original posting and the last time
> it was posted (which you can then follow the chain of reviews from
> there).
> 
> If we want to have an explicit policy on which Message-Id's to keep then
> we should document it.

Fair enough :)

[PATCH 09/22] gdbstub: Introduce gdb_find_static_feature()

[Alex Bennée]

From: Akihiko Odaki <akihiko.odaki@daynix.com>

This function is useful to determine the number of registers exposed to
GDB from the XML name.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20231025093128.33116-3-akihiko.odaki@daynix.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20231103195956.1998255-10-alex.bennee@linaro.org>
---
 include/exec/gdbstub.h |  8 ++++++++
 gdbstub/gdbstub.c      | 13 +++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index a43aa34dad..7fe00506c7 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -44,6 +44,14 @@ void gdb_register_coprocessor(CPUState *cpu,
  */
 int gdbserver_start(const char *port_or_device);
 
+/**
+ * gdb_find_static_feature() - Find a static feature.
+ * @xmlname: The name of the XML.
+ *
+ * Return: The static feature.
+ */
+const GDBFeature *gdb_find_static_feature(const char *xmlname);
+
 void gdb_set_stop_cpu(CPUState *cpu);
 
 /* in gdbstub-xml.c, generated by scripts/feature_to_c.py */
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 29540a0284..ae24c4848f 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -422,6 +422,19 @@ static const char *get_feature_xml(const char *p, const char **newp,
     return NULL;
 }
 
+const GDBFeature *gdb_find_static_feature(const char *xmlname)
+{
+    const GDBFeature *feature;
+
+    for (feature = gdb_static_features; feature->xmlname; feature++) {
+        if (!strcmp(feature->xmlname, xmlname)) {
+            return feature;
+        }
+    }
+
+    g_assert_not_reached();
+}
+
 static int gdb_read_register(CPUState *cpu, GByteArray *buf, int reg)
 {
     CPUClass *cc = CPU_GET_CLASS(cpu);
-- 
2.39.2

[PATCH 10/22] gdbstub: Introduce GDBFeatureBuilder

[Alex Bennée]

From: Akihiko Odaki <akihiko.odaki@daynix.com>

GDBFeatureBuilder unifies the logic to generate dynamic GDBFeature.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20231025093128.33116-4-akihiko.odaki@daynix.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20231103195956.1998255-11-alex.bennee@linaro.org>
---
 include/exec/gdbstub.h | 50 ++++++++++++++++++++++++++++++++
 gdbstub/gdbstub.c      | 65 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 115 insertions(+)

diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 7fe00506c7..d8a3c56fa2 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -16,6 +16,12 @@ typedef struct GDBFeature {
     int num_regs;
 } GDBFeature;
 
+typedef struct GDBFeatureBuilder {
+    GDBFeature *feature;
+    GPtrArray *xml;
+    int base_reg;
+} GDBFeatureBuilder;
+
 
 /* Get or set a register.  Returns the size of the register.  */
 typedef int (*gdb_get_reg_cb)(CPUArchState *env, GByteArray *buf, int reg);
@@ -44,6 +50,50 @@ void gdb_register_coprocessor(CPUState *cpu,
  */
 int gdbserver_start(const char *port_or_device);
 
+/**
+ * gdb_feature_builder_init() - Initialize GDBFeatureBuilder.
+ * @builder: The builder to be initialized.
+ * @feature: The feature to be filled.
+ * @name: The name of the feature.
+ * @xmlname: The name of the XML.
+ * @base_reg: The base number of the register ID.
+ */
+void gdb_feature_builder_init(GDBFeatureBuilder *builder, GDBFeature *feature,
+                              const char *name, const char *xmlname,
+                              int base_reg);
+
+/**
+ * gdb_feature_builder_append_tag() - Append a tag.
+ * @builder: The builder.
+ * @format: The format of the tag.
+ * @...: The values to be formatted.
+ */
+void G_GNUC_PRINTF(2, 3)
+gdb_feature_builder_append_tag(const GDBFeatureBuilder *builder,
+                               const char *format, ...);
+
+/**
+ * gdb_feature_builder_append_reg() - Append a register.
+ * @builder: The builder.
+ * @name: The register's name; it must be unique within a CPU.
+ * @bitsize: The register's size, in bits.
+ * @regnum: The offset of the register's number in the feature.
+ * @type: The type of the register.
+ * @group: The register group to which this register belongs; it can be NULL.
+ */
+void gdb_feature_builder_append_reg(const GDBFeatureBuilder *builder,
+                                    const char *name,
+                                    int bitsize,
+                                    int regnum,
+                                    const char *type,
+                                    const char *group);
+
+/**
+ * gdb_feature_builder_end() - End building GDBFeature.
+ * @builder: The builder.
+ */
+void gdb_feature_builder_end(const GDBFeatureBuilder *builder);
+
 /**
  * gdb_find_static_feature() - Find a static feature.
  * @xmlname: The name of the XML.
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index ae24c4848f..ebb912da1b 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -422,6 +422,71 @@ static const char *get_feature_xml(const char *p, const char **newp,
     return NULL;
 }
 
+void gdb_feature_builder_init(GDBFeatureBuilder *builder, GDBFeature *feature,
+                              const char *name, const char *xmlname,
+                              int base_reg)
+{
+    char *header = g_markup_printf_escaped(
+        "<?xml version=\"1.0\"?>"
+        "<!DOCTYPE feature SYSTEM \"gdb-target.dtd\">"
+        "<feature name=\"%s\">",
+        name);
+
+    builder->feature = feature;
+    builder->xml = g_ptr_array_new();
+    g_ptr_array_add(builder->xml, header);
+    builder->base_reg = base_reg;
+    feature->xmlname = xmlname;
+    feature->num_regs = 0;
+}
+
+void gdb_feature_builder_append_tag(const GDBFeatureBuilder *builder,
+                                    const char *format, ...)
+{
+    va_list ap;
+    va_start(ap, format);
+    g_ptr_array_add(builder->xml, g_markup_vprintf_escaped(format, ap));
+    va_end(ap);
+}
+
+void gdb_feature_builder_append_reg(const GDBFeatureBuilder *builder,
+                                    const char *name,
+                                    int bitsize,
+                                    int regnum,
+                                    const char *type,
+                                    const char *group)
+{
+    if (builder->feature->num_regs < regnum) {
+        builder->feature->num_regs = regnum;
+    }
+
+    if (group) {
+        gdb_feature_builder_append_tag(
+            builder,
+            "<reg name=\"%s\" bitsize=\"%d\" regnum=\"%d\" type=\"%s\" group=\"%s\"/>",
+            name, bitsize, builder->base_reg + regnum, type, group);
+    } else {
+        gdb_feature_builder_append_tag(
+            builder,
+            "<reg name=\"%s\" bitsize=\"%d\" regnum=\"%d\" type=\"%s\"/>",
+            name, bitsize, builder->base_reg + regnum, type);
+    }
+}
+
+void gdb_feature_builder_end(const GDBFeatureBuilder *builder)
+{
+    g_ptr_array_add(builder->xml, (void *)"</feature>");
+    g_ptr_array_add(builder->xml, NULL);
+
+    builder->feature->xml = g_strjoinv(NULL, (void *)builder->xml->pdata);
+
+    for (guint i = 0; i < builder->xml->len - 2; i++) {
+        g_free(g_ptr_array_index(builder->xml, i));
+    }
+
+    g_ptr_array_free(builder->xml, TRUE);
+}
+
 const GDBFeature *gdb_find_static_feature(const char *xmlname)
 {
     const GDBFeature *feature;
-- 
2.39.2

[PATCH 11/22] cpu: Call plugin hooks only when ready

[Alex Bennée]

From: Akihiko Odaki <akihiko.odaki@daynix.com>

The initialization and exit hooks will not affect the state of vCPU
outside TCG context, but they may depend on the state of vCPU.
Therefore, it's better to call plugin hooks after the vCPU state is
fully initialized and before it gets uninitialized.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20231025093128.33116-16-akihiko.odaki@daynix.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20231103195956.1998255-23-alex.bennee@linaro.org>
---
 cpu-target.c         | 11 -----------
 hw/core/cpu-common.c | 10 ++++++++++
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/cpu-target.c b/cpu-target.c
index 79363ae370..00cd7f4d69 100644
--- a/cpu-target.c
+++ b/cpu-target.c
@@ -42,7 +42,6 @@
 #include "hw/core/accel-cpu.h"
 #include "trace/trace-root.h"
 #include "qemu/accel.h"
-#include "qemu/plugin.h"
 
 uintptr_t qemu_host_page_size;
 intptr_t qemu_host_page_mask;
@@ -143,11 +142,6 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp)
     /* Wait until cpu initialization complete before exposing cpu. */
     cpu_list_add(cpu);
 
-    /* Plugin initialization must wait until cpu_index assigned. */
-    if (tcg_enabled()) {
-        qemu_plugin_vcpu_init_hook(cpu);
-    }
-
 #ifdef CONFIG_USER_ONLY
     assert(qdev_get_vmsd(DEVICE(cpu)) == NULL ||
            qdev_get_vmsd(DEVICE(cpu))->unmigratable);
@@ -174,11 +168,6 @@ void cpu_exec_unrealizefn(CPUState *cpu)
     }
 #endif
 
-    /* Call the plugin hook before clearing cpu->cpu_index in cpu_list_remove */
-    if (tcg_enabled()) {
-        qemu_plugin_vcpu_exit_hook(cpu);
-    }
-
     cpu_list_remove(cpu);
     /*
      * Now that the vCPU has been removed from the RCU list, we can call
diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
index bab8942c30..0acfed4c0f 100644
--- a/hw/core/cpu-common.c
+++ b/hw/core/cpu-common.c
@@ -209,6 +209,11 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp)
         cpu_resume(cpu);
     }
 
+    /* Plugin initialization must wait until the cpu is fully realized. */
+    if (tcg_enabled()) {
+        qemu_plugin_vcpu_init_hook(cpu);
+    }
+
     /* NOTE: latest generic point where the cpu is fully realized */
 }
 
@@ -216,6 +221,11 @@ static void cpu_common_unrealizefn(DeviceState *dev)
 {
     CPUState *cpu = CPU(dev);
 
+    /* Call the plugin hook before clearing the cpu is fully unrealized */
+    if (tcg_enabled()) {
+        qemu_plugin_vcpu_exit_hook(cpu);
+    }
+
     /* NOTE: latest generic point before the cpu is fully unrealized */
     cpu_exec_unrealizefn(cpu);
 }
-- 
2.39.2

[PATCH 12/22] configure: tell meson and contrib_plugins about DLLTOOL

[Alex Bennée]

To cleanly handle cross-building we need to export the details of
dlltool into meson's list of cross binaries and into the
contrib/plugins/ make configuration.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Greg Manning <gmanning@rapitasystems.com>
---
 configure | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/configure b/configure
index f1456f6123..cd6c521bd8 100755
--- a/configure
+++ b/configure
@@ -309,6 +309,7 @@ fi
 ar="${AR-${cross_prefix}ar}"
 as="${AS-${cross_prefix}as}"
 ccas="${CCAS-$cc}"
+dlltool="${DLLTOOL-${cross_prefix}dlltool}"
 objcopy="${OBJCOPY-${cross_prefix}objcopy}"
 ld="${LD-${cross_prefix}ld}"
 ranlib="${RANLIB-${cross_prefix}ranlib}"
@@ -1659,6 +1660,9 @@ echo "SRC_PATH=$source_path/contrib/plugins" >> contrib/plugins/$config_host_mak
 echo "PKG_CONFIG=${pkg_config}" >> contrib/plugins/$config_host_mak
 echo "CC=$cc $CPU_CFLAGS" >> contrib/plugins/$config_host_mak
 echo "CFLAGS=${CFLAGS-$default_cflags} $EXTRA_CFLAGS" >> contrib/plugins/$config_host_mak
+if test "$targetos" = windows; then
+  echo "DLLTOOL=$dlltool" >> contrib/plugins/$config_host_mak
+fi
 if test "$targetos" = darwin; then
   echo "CONFIG_DARWIN=y" >> contrib/plugins/$config_host_mak
 fi
@@ -1764,6 +1768,7 @@ if test "$skip_meson" = no; then
   test -n "$cxx" && echo "cpp = [$(meson_quote $cxx $CPU_CFLAGS)]" >> $cross
   test -n "$objcc" && echo "objc = [$(meson_quote $objcc $CPU_CFLAGS)]" >> $cross
   echo "ar = [$(meson_quote $ar)]" >> $cross
+  echo "dlltool = [$(meson_quote $dlltool)]" >> $cross
   echo "nm = [$(meson_quote $nm)]" >> $cross
   echo "pkgconfig = [$(meson_quote $pkg_config)]" >> $cross
   echo "pkg-config = [$(meson_quote $pkg_config)]" >> $cross
@@ -1869,6 +1874,7 @@ preserve_env CC
 preserve_env CFLAGS
 preserve_env CXX
 preserve_env CXXFLAGS
+preserve_env DLLTOOL
 preserve_env LD
 preserve_env LDFLAGS
 preserve_env LD_LIBRARY_PATH
-- 
2.39.2

Re: [PATCH 12/22] configure: tell meson and contrib_plugins about DLLTOOL

[Paolo Bonzini]

On 11/6/23 19:51, Alex Bennée wrote:
> To cleanly handle cross-building we need to export the details of
> dlltool into meson's list of cross binaries and into the
> contrib/plugins/ make configuration.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Greg Manning <gmanning@rapitasystems.com>

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

> ---
>   configure | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/configure b/configure
> index f1456f6123..cd6c521bd8 100755
> --- a/configure
> +++ b/configure
> @@ -309,6 +309,7 @@ fi
>   ar="${AR-${cross_prefix}ar}"
>   as="${AS-${cross_prefix}as}"
>   ccas="${CCAS-$cc}"
> +dlltool="${DLLTOOL-${cross_prefix}dlltool}"
>   objcopy="${OBJCOPY-${cross_prefix}objcopy}"
>   ld="${LD-${cross_prefix}ld}"
>   ranlib="${RANLIB-${cross_prefix}ranlib}"
> @@ -1659,6 +1660,9 @@ echo "SRC_PATH=$source_path/contrib/plugins" >> contrib/plugins/$config_host_mak
>   echo "PKG_CONFIG=${pkg_config}" >> contrib/plugins/$config_host_mak
>   echo "CC=$cc $CPU_CFLAGS" >> contrib/plugins/$config_host_mak
>   echo "CFLAGS=${CFLAGS-$default_cflags} $EXTRA_CFLAGS" >> contrib/plugins/$config_host_mak
> +if test "$targetos" = windows; then
> +  echo "DLLTOOL=$dlltool" >> contrib/plugins/$config_host_mak
> +fi
>   if test "$targetos" = darwin; then
>     echo "CONFIG_DARWIN=y" >> contrib/plugins/$config_host_mak
>   fi
> @@ -1764,6 +1768,7 @@ if test "$skip_meson" = no; then
>     test -n "$cxx" && echo "cpp = [$(meson_quote $cxx $CPU_CFLAGS)]" >> $cross
>     test -n "$objcc" && echo "objc = [$(meson_quote $objcc $CPU_CFLAGS)]" >> $cross
>     echo "ar = [$(meson_quote $ar)]" >> $cross
> +  echo "dlltool = [$(meson_quote $dlltool)]" >> $cross
>     echo "nm = [$(meson_quote $nm)]" >> $cross
>     echo "pkgconfig = [$(meson_quote $pkg_config)]" >> $cross
>     echo "pkg-config = [$(meson_quote $pkg_config)]" >> $cross
> @@ -1869,6 +1874,7 @@ preserve_env CC
>   preserve_env CFLAGS
>   preserve_env CXX
>   preserve_env CXXFLAGS
> +preserve_env DLLTOOL
>   preserve_env LD
>   preserve_env LDFLAGS
>   preserve_env LD_LIBRARY_PATH

Re: [PATCH 12/22] configure: tell meson and contrib_plugins about DLLTOOL

[Philippe Mathieu-Daudé]

On 6/11/23 19:51, Alex Bennée wrote:
> To cleanly handle cross-building we need to export the details of
> dlltool into meson's list of cross binaries and into the
> contrib/plugins/ make configuration.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Greg Manning <gmanning@rapitasystems.com>
> ---
>   configure | 6 ++++++
>   1 file changed, 6 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 13/22] plugins: add dllexport and dllimport to api funcs

[Alex Bennée]

From: Greg Manning <gmanning@rapitasystems.com>

In qemu-plugin.h, mark all API functions as __declspec(dllexport) when
compiling the executables, and as __declspec(dllimport) when being used
to compile plugins against.

Signed-off-by: Greg Manning <gmanning@rapitasystems.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20231102172053.17692-2-gmanning@rapitasystems.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20231103195956.1998255-27-alex.bennee@linaro.org>
---
 include/qemu/qemu-plugin.h | 50 +++++++++++++++++++++++++++++++++++---
 1 file changed, 47 insertions(+), 3 deletions(-)

diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 50a9957279..4daab6efd2 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -22,15 +22,18 @@
  *   https://gcc.gnu.org/wiki/Visibility
  */
 #if defined _WIN32 || defined __CYGWIN__
-  #ifdef BUILDING_DLL
-    #define QEMU_PLUGIN_EXPORT __declspec(dllexport)
-  #else
+  #ifdef CONFIG_PLUGIN
     #define QEMU_PLUGIN_EXPORT __declspec(dllimport)
+    #define QEMU_PLUGIN_API __declspec(dllexport)
+  #else
+    #define QEMU_PLUGIN_EXPORT __declspec(dllexport)
+    #define QEMU_PLUGIN_API __declspec(dllimport)
   #endif
   #define QEMU_PLUGIN_LOCAL
 #else
   #define QEMU_PLUGIN_EXPORT __attribute__((visibility("default")))
   #define QEMU_PLUGIN_LOCAL  __attribute__((visibility("hidden")))
+  #define QEMU_PLUGIN_API
 #endif
 
 /**
@@ -147,6 +150,7 @@ typedef void (*qemu_plugin_vcpu_udata_cb_t)(unsigned int vcpu_index,
  *
  * Note: Calling this function from qemu_plugin_install() is a bug.
  */
+QEMU_PLUGIN_API
 void qemu_plugin_uninstall(qemu_plugin_id_t id, qemu_plugin_simple_cb_t cb);
 
 /**
@@ -160,6 +164,7 @@ void qemu_plugin_uninstall(qemu_plugin_id_t id, qemu_plugin_simple_cb_t cb);
  * Plugins are reset asynchronously, and therefore the given plugin receives
  * callbacks until @cb is called.
  */
+QEMU_PLUGIN_API
 void qemu_plugin_reset(qemu_plugin_id_t id, qemu_plugin_simple_cb_t cb);
 
 /**
@@ -171,6 +176,7 @@ void qemu_plugin_reset(qemu_plugin_id_t id, qemu_plugin_simple_cb_t cb);
  *
  * See also: qemu_plugin_register_vcpu_exit_cb()
  */
+QEMU_PLUGIN_API
 void qemu_plugin_register_vcpu_init_cb(qemu_plugin_id_t id,
                                        qemu_plugin_vcpu_simple_cb_t cb);
 
@@ -183,6 +189,7 @@ void qemu_plugin_register_vcpu_init_cb(qemu_plugin_id_t id,
  *
  * See also: qemu_plugin_register_vcpu_init_cb()
  */
+QEMU_PLUGIN_API
 void qemu_plugin_register_vcpu_exit_cb(qemu_plugin_id_t id,
                                        qemu_plugin_vcpu_simple_cb_t cb);
 
@@ -193,6 +200,7 @@ void qemu_plugin_register_vcpu_exit_cb(qemu_plugin_id_t id,
  *
  * The @cb function is called every time a vCPU idles.
  */
+QEMU_PLUGIN_API
 void qemu_plugin_register_vcpu_idle_cb(qemu_plugin_id_t id,
                                        qemu_plugin_vcpu_simple_cb_t cb);
 
@@ -203,6 +211,7 @@ void qemu_plugin_register_vcpu_idle_cb(qemu_plugin_id_t id,
  *
  * The @cb function is called every time a vCPU resumes execution.
  */
+QEMU_PLUGIN_API
 void qemu_plugin_register_vcpu_resume_cb(qemu_plugin_id_t id,
                                          qemu_plugin_vcpu_simple_cb_t cb);
 
@@ -253,6 +262,7 @@ typedef void (*qemu_plugin_vcpu_tb_trans_cb_t)(qemu_plugin_id_t id,
  * callbacks to be triggered when the block or individual instruction
  * executes.
  */
+QEMU_PLUGIN_API
 void qemu_plugin_register_vcpu_tb_trans_cb(qemu_plugin_id_t id,
                                            qemu_plugin_vcpu_tb_trans_cb_t cb);
 
@@ -265,6 +275,7 @@ void qemu_plugin_register_vcpu_tb_trans_cb(qemu_plugin_id_t id,
  *
  * The @cb function is called every time a translated unit executes.
  */
+QEMU_PLUGIN_API
 void qemu_plugin_register_vcpu_tb_exec_cb(struct qemu_plugin_tb *tb,
                                           qemu_plugin_vcpu_udata_cb_t cb,
                                           enum qemu_plugin_cb_flags flags,
@@ -296,6 +307,7 @@ enum qemu_plugin_op {
  * Note: ops are not atomic so in multi-threaded/multi-smp situations
  * you will get inexact results.
  */
+QEMU_PLUGIN_API
 void qemu_plugin_register_vcpu_tb_exec_inline(struct qemu_plugin_tb *tb,
                                               enum qemu_plugin_op op,
                                               void *ptr, uint64_t imm);
@@ -309,6 +321,7 @@ void qemu_plugin_register_vcpu_tb_exec_inline(struct qemu_plugin_tb *tb,
  *
  * The @cb function is called every time an instruction is executed
  */
+QEMU_PLUGIN_API
 void qemu_plugin_register_vcpu_insn_exec_cb(struct qemu_plugin_insn *insn,
                                             qemu_plugin_vcpu_udata_cb_t cb,
                                             enum qemu_plugin_cb_flags flags,
@@ -324,6 +337,7 @@ void qemu_plugin_register_vcpu_insn_exec_cb(struct qemu_plugin_insn *insn,
  * Insert an inline op to every time an instruction executes. Useful
  * if you just want to increment a single counter somewhere in memory.
  */
+QEMU_PLUGIN_API
 void qemu_plugin_register_vcpu_insn_exec_inline(struct qemu_plugin_insn *insn,
                                                 enum qemu_plugin_op op,
                                                 void *ptr, uint64_t imm);
@@ -334,6 +348,7 @@ void qemu_plugin_register_vcpu_insn_exec_inline(struct qemu_plugin_insn *insn,
  *
  * Returns: number of instructions in this block
  */
+QEMU_PLUGIN_API
 size_t qemu_plugin_tb_n_insns(const struct qemu_plugin_tb *tb);
 
 /**
@@ -342,6 +357,7 @@ size_t qemu_plugin_tb_n_insns(const struct qemu_plugin_tb *tb);
  *
  * Returns: virtual address of block start
  */
+QEMU_PLUGIN_API
 uint64_t qemu_plugin_tb_vaddr(const struct qemu_plugin_tb *tb);
 
 /**
@@ -355,6 +371,7 @@ uint64_t qemu_plugin_tb_vaddr(const struct qemu_plugin_tb *tb);
  *
  * Returns: opaque handle to instruction
  */
+QEMU_PLUGIN_API
 struct qemu_plugin_insn *
 qemu_plugin_tb_get_insn(const struct qemu_plugin_tb *tb, size_t idx);
 
@@ -368,6 +385,7 @@ qemu_plugin_tb_get_insn(const struct qemu_plugin_tb *tb, size_t idx);
  * Returns: pointer to a stream of bytes containing the value of this
  * instructions opcode.
  */
+QEMU_PLUGIN_API
 const void *qemu_plugin_insn_data(const struct qemu_plugin_insn *insn);
 
 /**
@@ -376,6 +394,7 @@ const void *qemu_plugin_insn_data(const struct qemu_plugin_insn *insn);
  *
  * Returns: size of instruction in bytes
  */
+QEMU_PLUGIN_API
 size_t qemu_plugin_insn_size(const struct qemu_plugin_insn *insn);
 
 /**
@@ -384,6 +403,7 @@ size_t qemu_plugin_insn_size(const struct qemu_plugin_insn *insn);
  *
  * Returns: virtual address of instruction
  */
+QEMU_PLUGIN_API
 uint64_t qemu_plugin_insn_vaddr(const struct qemu_plugin_insn *insn);
 
 /**
@@ -392,6 +412,7 @@ uint64_t qemu_plugin_insn_vaddr(const struct qemu_plugin_insn *insn);
  *
  * Returns: hardware (physical) target address of instruction
  */
+QEMU_PLUGIN_API
 void *qemu_plugin_insn_haddr(const struct qemu_plugin_insn *insn);
 
 /**
@@ -410,6 +431,7 @@ struct qemu_plugin_hwaddr;
  *
  * Returns: size of access in ^2 (0=byte, 1=16bit, 2=32bit etc...)
  */
+QEMU_PLUGIN_API
 unsigned int qemu_plugin_mem_size_shift(qemu_plugin_meminfo_t info);
 /**
  * qemu_plugin_mem_is_sign_extended() - was the access sign extended
@@ -417,6 +439,7 @@ unsigned int qemu_plugin_mem_size_shift(qemu_plugin_meminfo_t info);
  *
  * Returns: true if it was, otherwise false
  */
+QEMU_PLUGIN_API
 bool qemu_plugin_mem_is_sign_extended(qemu_plugin_meminfo_t info);
 /**
  * qemu_plugin_mem_is_big_endian() - was the access big endian
@@ -424,6 +447,7 @@ bool qemu_plugin_mem_is_sign_extended(qemu_plugin_meminfo_t info);
  *
  * Returns: true if it was, otherwise false
  */
+QEMU_PLUGIN_API
 bool qemu_plugin_mem_is_big_endian(qemu_plugin_meminfo_t info);
 /**
  * qemu_plugin_mem_is_store() - was the access a store
@@ -431,6 +455,7 @@ bool qemu_plugin_mem_is_big_endian(qemu_plugin_meminfo_t info);
  *
  * Returns: true if it was, otherwise false
  */
+QEMU_PLUGIN_API
 bool qemu_plugin_mem_is_store(qemu_plugin_meminfo_t info);
 
 /**
@@ -446,6 +471,7 @@ bool qemu_plugin_mem_is_store(qemu_plugin_meminfo_t info);
  * information about the handle should be recovered before the
  * callback returns.
  */
+QEMU_PLUGIN_API
 struct qemu_plugin_hwaddr *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
                                                   uint64_t vaddr);
 
@@ -462,6 +488,7 @@ struct qemu_plugin_hwaddr *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
  * Returns true if the handle's memory operation is to memory-mapped IO, or
  * false if it is to RAM
  */
+QEMU_PLUGIN_API
 bool qemu_plugin_hwaddr_is_io(const struct qemu_plugin_hwaddr *haddr);
 
 /**
@@ -473,12 +500,14 @@ bool qemu_plugin_hwaddr_is_io(const struct qemu_plugin_hwaddr *haddr);
  * Note that the returned physical address may not be unique if you are dealing
  * with multiple address spaces.
  */
+QEMU_PLUGIN_API
 uint64_t qemu_plugin_hwaddr_phys_addr(const struct qemu_plugin_hwaddr *haddr);
 
 /*
  * Returns a string representing the device. The string is valid for
  * the lifetime of the plugin.
  */
+QEMU_PLUGIN_API
 const char *qemu_plugin_hwaddr_device_name(const struct qemu_plugin_hwaddr *h);
 
 /**
@@ -513,6 +542,7 @@ typedef void (*qemu_plugin_vcpu_mem_cb_t) (unsigned int vcpu_index,
  * callback so the plugin is responsible for ensuring it doesn't get
  * confused by making appropriate use of locking if required.
  */
+QEMU_PLUGIN_API
 void qemu_plugin_register_vcpu_mem_cb(struct qemu_plugin_insn *insn,
                                       qemu_plugin_vcpu_mem_cb_t cb,
                                       enum qemu_plugin_cb_flags flags,
@@ -531,6 +561,7 @@ void qemu_plugin_register_vcpu_mem_cb(struct qemu_plugin_insn *insn,
  * instruction. This provides for a lightweight but not thread-safe
  * way of counting the number of operations done.
  */
+QEMU_PLUGIN_API
 void qemu_plugin_register_vcpu_mem_inline(struct qemu_plugin_insn *insn,
                                           enum qemu_plugin_mem_rw rw,
                                           enum qemu_plugin_op op, void *ptr,
@@ -544,6 +575,7 @@ typedef void
                                  uint64_t a3, uint64_t a4, uint64_t a5,
                                  uint64_t a6, uint64_t a7, uint64_t a8);
 
+QEMU_PLUGIN_API
 void qemu_plugin_register_vcpu_syscall_cb(qemu_plugin_id_t id,
                                           qemu_plugin_vcpu_syscall_cb_t cb);
 
@@ -551,6 +583,7 @@ typedef void
 (*qemu_plugin_vcpu_syscall_ret_cb_t)(qemu_plugin_id_t id, unsigned int vcpu_idx,
                                      int64_t num, int64_t ret);
 
+QEMU_PLUGIN_API
 void
 qemu_plugin_register_vcpu_syscall_ret_cb(qemu_plugin_id_t id,
                                          qemu_plugin_vcpu_syscall_ret_cb_t cb);
@@ -563,6 +596,7 @@ qemu_plugin_register_vcpu_syscall_ret_cb(qemu_plugin_id_t id,
  * Returns an allocated string containing the disassembly
  */
 
+QEMU_PLUGIN_API
 char *qemu_plugin_insn_disas(const struct qemu_plugin_insn *insn);
 
 /**
@@ -572,6 +606,7 @@ char *qemu_plugin_insn_disas(const struct qemu_plugin_insn *insn);
  * Return a static string referring to the symbol. This is dependent
  * on the binary QEMU is running having provided a symbol table.
  */
+QEMU_PLUGIN_API
 const char *qemu_plugin_insn_symbol(const struct qemu_plugin_insn *insn);
 
 /**
@@ -583,9 +618,11 @@ const char *qemu_plugin_insn_symbol(const struct qemu_plugin_insn *insn);
  *
  * See also: qemu_plugin_register_vcpu_init_cb()
  */
+QEMU_PLUGIN_API
 void qemu_plugin_vcpu_for_each(qemu_plugin_id_t id,
                                qemu_plugin_vcpu_simple_cb_t cb);
 
+QEMU_PLUGIN_API
 void qemu_plugin_register_flush_cb(qemu_plugin_id_t id,
                                    qemu_plugin_simple_cb_t cb);
 
@@ -602,6 +639,7 @@ void qemu_plugin_register_flush_cb(qemu_plugin_id_t id,
  * In user-mode it is possible a few un-instrumented instructions from
  * child threads may run before the host kernel reaps the threads.
  */
+QEMU_PLUGIN_API
 void qemu_plugin_register_atexit_cb(qemu_plugin_id_t id,
                                     qemu_plugin_udata_cb_t cb, void *userdata);
 
@@ -615,6 +653,7 @@ int qemu_plugin_n_max_vcpus(void);
  * qemu_plugin_outs() - output string via QEMU's logging system
  * @string: a string
  */
+QEMU_PLUGIN_API
 void qemu_plugin_outs(const char *string);
 
 /**
@@ -628,6 +667,7 @@ void qemu_plugin_outs(const char *string);
  * returns true if the combination @name=@val parses correctly to a boolean
  * argument, and false otherwise
  */
+QEMU_PLUGIN_API
 bool qemu_plugin_bool_parse(const char *name, const char *val, bool *ret);
 
 /**
@@ -638,6 +678,7 @@ bool qemu_plugin_bool_parse(const char *name, const char *val, bool *ret);
  * return NULL. The user should g_free() the string once no longer
  * needed.
  */
+QEMU_PLUGIN_API
 const char *qemu_plugin_path_to_binary(void);
 
 /**
@@ -646,6 +687,7 @@ const char *qemu_plugin_path_to_binary(void);
  * Returns the nominal start address of the main text segment in
  * user-mode. Currently returns 0 for system emulation.
  */
+QEMU_PLUGIN_API
 uint64_t qemu_plugin_start_code(void);
 
 /**
@@ -654,6 +696,7 @@ uint64_t qemu_plugin_start_code(void);
  * Returns the nominal end address of the main text segment in
  * user-mode. Currently returns 0 for system emulation.
  */
+QEMU_PLUGIN_API
 uint64_t qemu_plugin_end_code(void);
 
 /**
@@ -662,6 +705,7 @@ uint64_t qemu_plugin_end_code(void);
  * Returns the nominal entry address of the main text segment in
  * user-mode. Currently returns 0 for system emulation.
  */
+QEMU_PLUGIN_API
 uint64_t qemu_plugin_entry_code(void);
 
 #endif /* QEMU_QEMU_PLUGIN_H */
-- 
2.39.2

Re: [PATCH 13/22] plugins: add dllexport and dllimport to api funcs

[Paolo Bonzini]

On 11/6/23 19:51, Alex Bennée wrote:
> From: Greg Manning <gmanning@rapitasystems.com>
> 
> In qemu-plugin.h, mark all API functions as __declspec(dllexport) when
> compiling the executables, and as __declspec(dllimport) when being used
> to compile plugins against.
> 
> Signed-off-by: Greg Manning <gmanning@rapitasystems.com>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20231102172053.17692-2-gmanning@rapitasystems.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

> ---
>   include/qemu/qemu-plugin.h | 50 +++++++++++++++++++++++++++++++++++---
>   1 file changed, 47 insertions(+), 3 deletions(-)
> 
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index 50a9957279..4daab6efd2 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -22,15 +22,18 @@
>    *   https://gcc.gnu.org/wiki/Visibility
>    */
>   #if defined _WIN32 || defined __CYGWIN__
> -  #ifdef BUILDING_DLL
> -    #define QEMU_PLUGIN_EXPORT __declspec(dllexport)
> -  #else
> +  #ifdef CONFIG_PLUGIN
>       #define QEMU_PLUGIN_EXPORT __declspec(dllimport)
> +    #define QEMU_PLUGIN_API __declspec(dllexport)
> +  #else
> +    #define QEMU_PLUGIN_EXPORT __declspec(dllexport)
> +    #define QEMU_PLUGIN_API __declspec(dllimport)
>     #endif
>     #define QEMU_PLUGIN_LOCAL
>   #else
>     #define QEMU_PLUGIN_EXPORT __attribute__((visibility("default")))
>     #define QEMU_PLUGIN_LOCAL  __attribute__((visibility("hidden")))
> +  #define QEMU_PLUGIN_API
>   #endif
>   
>   /**
> @@ -147,6 +150,7 @@ typedef void (*qemu_plugin_vcpu_udata_cb_t)(unsigned int vcpu_index,
>    *
>    * Note: Calling this function from qemu_plugin_install() is a bug.
>    */
> +QEMU_PLUGIN_API
>   void qemu_plugin_uninstall(qemu_plugin_id_t id, qemu_plugin_simple_cb_t cb);
>   
>   /**
> @@ -160,6 +164,7 @@ void qemu_plugin_uninstall(qemu_plugin_id_t id, qemu_plugin_simple_cb_t cb);
>    * Plugins are reset asynchronously, and therefore the given plugin receives
>    * callbacks until @cb is called.
>    */
> +QEMU_PLUGIN_API
>   void qemu_plugin_reset(qemu_plugin_id_t id, qemu_plugin_simple_cb_t cb);
>   
>   /**
> @@ -171,6 +176,7 @@ void qemu_plugin_reset(qemu_plugin_id_t id, qemu_plugin_simple_cb_t cb);
>    *
>    * See also: qemu_plugin_register_vcpu_exit_cb()
>    */
> +QEMU_PLUGIN_API
>   void qemu_plugin_register_vcpu_init_cb(qemu_plugin_id_t id,
>                                          qemu_plugin_vcpu_simple_cb_t cb);
>   
> @@ -183,6 +189,7 @@ void qemu_plugin_register_vcpu_init_cb(qemu_plugin_id_t id,
>    *
>    * See also: qemu_plugin_register_vcpu_init_cb()
>    */
> +QEMU_PLUGIN_API
>   void qemu_plugin_register_vcpu_exit_cb(qemu_plugin_id_t id,
>                                          qemu_plugin_vcpu_simple_cb_t cb);
>   
> @@ -193,6 +200,7 @@ void qemu_plugin_register_vcpu_exit_cb(qemu_plugin_id_t id,
>    *
>    * The @cb function is called every time a vCPU idles.
>    */
> +QEMU_PLUGIN_API
>   void qemu_plugin_register_vcpu_idle_cb(qemu_plugin_id_t id,
>                                          qemu_plugin_vcpu_simple_cb_t cb);
>   
> @@ -203,6 +211,7 @@ void qemu_plugin_register_vcpu_idle_cb(qemu_plugin_id_t id,
>    *
>    * The @cb function is called every time a vCPU resumes execution.
>    */
> +QEMU_PLUGIN_API
>   void qemu_plugin_register_vcpu_resume_cb(qemu_plugin_id_t id,
>                                            qemu_plugin_vcpu_simple_cb_t cb);
>   
> @@ -253,6 +262,7 @@ typedef void (*qemu_plugin_vcpu_tb_trans_cb_t)(qemu_plugin_id_t id,
>    * callbacks to be triggered when the block or individual instruction
>    * executes.
>    */
> +QEMU_PLUGIN_API
>   void qemu_plugin_register_vcpu_tb_trans_cb(qemu_plugin_id_t id,
>                                              qemu_plugin_vcpu_tb_trans_cb_t cb);
>   
> @@ -265,6 +275,7 @@ void qemu_plugin_register_vcpu_tb_trans_cb(qemu_plugin_id_t id,
>    *
>    * The @cb function is called every time a translated unit executes.
>    */
> +QEMU_PLUGIN_API
>   void qemu_plugin_register_vcpu_tb_exec_cb(struct qemu_plugin_tb *tb,
>                                             qemu_plugin_vcpu_udata_cb_t cb,
>                                             enum qemu_plugin_cb_flags flags,
> @@ -296,6 +307,7 @@ enum qemu_plugin_op {
>    * Note: ops are not atomic so in multi-threaded/multi-smp situations
>    * you will get inexact results.
>    */
> +QEMU_PLUGIN_API
>   void qemu_plugin_register_vcpu_tb_exec_inline(struct qemu_plugin_tb *tb,
>                                                 enum qemu_plugin_op op,
>                                                 void *ptr, uint64_t imm);
> @@ -309,6 +321,7 @@ void qemu_plugin_register_vcpu_tb_exec_inline(struct qemu_plugin_tb *tb,
>    *
>    * The @cb function is called every time an instruction is executed
>    */
> +QEMU_PLUGIN_API
>   void qemu_plugin_register_vcpu_insn_exec_cb(struct qemu_plugin_insn *insn,
>                                               qemu_plugin_vcpu_udata_cb_t cb,
>                                               enum qemu_plugin_cb_flags flags,
> @@ -324,6 +337,7 @@ void qemu_plugin_register_vcpu_insn_exec_cb(struct qemu_plugin_insn *insn,
>    * Insert an inline op to every time an instruction executes. Useful
>    * if you just want to increment a single counter somewhere in memory.
>    */
> +QEMU_PLUGIN_API
>   void qemu_plugin_register_vcpu_insn_exec_inline(struct qemu_plugin_insn *insn,
>                                                   enum qemu_plugin_op op,
>                                                   void *ptr, uint64_t imm);
> @@ -334,6 +348,7 @@ void qemu_plugin_register_vcpu_insn_exec_inline(struct qemu_plugin_insn *insn,
>    *
>    * Returns: number of instructions in this block
>    */
> +QEMU_PLUGIN_API
>   size_t qemu_plugin_tb_n_insns(const struct qemu_plugin_tb *tb);
>   
>   /**
> @@ -342,6 +357,7 @@ size_t qemu_plugin_tb_n_insns(const struct qemu_plugin_tb *tb);
>    *
>    * Returns: virtual address of block start
>    */
> +QEMU_PLUGIN_API
>   uint64_t qemu_plugin_tb_vaddr(const struct qemu_plugin_tb *tb);
>   
>   /**
> @@ -355,6 +371,7 @@ uint64_t qemu_plugin_tb_vaddr(const struct qemu_plugin_tb *tb);
>    *
>    * Returns: opaque handle to instruction
>    */
> +QEMU_PLUGIN_API
>   struct qemu_plugin_insn *
>   qemu_plugin_tb_get_insn(const struct qemu_plugin_tb *tb, size_t idx);
>   
> @@ -368,6 +385,7 @@ qemu_plugin_tb_get_insn(const struct qemu_plugin_tb *tb, size_t idx);
>    * Returns: pointer to a stream of bytes containing the value of this
>    * instructions opcode.
>    */
> +QEMU_PLUGIN_API
>   const void *qemu_plugin_insn_data(const struct qemu_plugin_insn *insn);
>   
>   /**
> @@ -376,6 +394,7 @@ const void *qemu_plugin_insn_data(const struct qemu_plugin_insn *insn);
>    *
>    * Returns: size of instruction in bytes
>    */
> +QEMU_PLUGIN_API
>   size_t qemu_plugin_insn_size(const struct qemu_plugin_insn *insn);
>   
>   /**
> @@ -384,6 +403,7 @@ size_t qemu_plugin_insn_size(const struct qemu_plugin_insn *insn);
>    *
>    * Returns: virtual address of instruction
>    */
> +QEMU_PLUGIN_API
>   uint64_t qemu_plugin_insn_vaddr(const struct qemu_plugin_insn *insn);
>   
>   /**
> @@ -392,6 +412,7 @@ uint64_t qemu_plugin_insn_vaddr(const struct qemu_plugin_insn *insn);
>    *
>    * Returns: hardware (physical) target address of instruction
>    */
> +QEMU_PLUGIN_API
>   void *qemu_plugin_insn_haddr(const struct qemu_plugin_insn *insn);
>   
>   /**
> @@ -410,6 +431,7 @@ struct qemu_plugin_hwaddr;
>    *
>    * Returns: size of access in ^2 (0=byte, 1=16bit, 2=32bit etc...)
>    */
> +QEMU_PLUGIN_API
>   unsigned int qemu_plugin_mem_size_shift(qemu_plugin_meminfo_t info);
>   /**
>    * qemu_plugin_mem_is_sign_extended() - was the access sign extended
> @@ -417,6 +439,7 @@ unsigned int qemu_plugin_mem_size_shift(qemu_plugin_meminfo_t info);
>    *
>    * Returns: true if it was, otherwise false
>    */
> +QEMU_PLUGIN_API
>   bool qemu_plugin_mem_is_sign_extended(qemu_plugin_meminfo_t info);
>   /**
>    * qemu_plugin_mem_is_big_endian() - was the access big endian
> @@ -424,6 +447,7 @@ bool qemu_plugin_mem_is_sign_extended(qemu_plugin_meminfo_t info);
>    *
>    * Returns: true if it was, otherwise false
>    */
> +QEMU_PLUGIN_API
>   bool qemu_plugin_mem_is_big_endian(qemu_plugin_meminfo_t info);
>   /**
>    * qemu_plugin_mem_is_store() - was the access a store
> @@ -431,6 +455,7 @@ bool qemu_plugin_mem_is_big_endian(qemu_plugin_meminfo_t info);
>    *
>    * Returns: true if it was, otherwise false
>    */
> +QEMU_PLUGIN_API
>   bool qemu_plugin_mem_is_store(qemu_plugin_meminfo_t info);
>   
>   /**
> @@ -446,6 +471,7 @@ bool qemu_plugin_mem_is_store(qemu_plugin_meminfo_t info);
>    * information about the handle should be recovered before the
>    * callback returns.
>    */
> +QEMU_PLUGIN_API
>   struct qemu_plugin_hwaddr *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
>                                                     uint64_t vaddr);
>   
> @@ -462,6 +488,7 @@ struct qemu_plugin_hwaddr *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
>    * Returns true if the handle's memory operation is to memory-mapped IO, or
>    * false if it is to RAM
>    */
> +QEMU_PLUGIN_API
>   bool qemu_plugin_hwaddr_is_io(const struct qemu_plugin_hwaddr *haddr);
>   
>   /**
> @@ -473,12 +500,14 @@ bool qemu_plugin_hwaddr_is_io(const struct qemu_plugin_hwaddr *haddr);
>    * Note that the returned physical address may not be unique if you are dealing
>    * with multiple address spaces.
>    */
> +QEMU_PLUGIN_API
>   uint64_t qemu_plugin_hwaddr_phys_addr(const struct qemu_plugin_hwaddr *haddr);
>   
>   /*
>    * Returns a string representing the device. The string is valid for
>    * the lifetime of the plugin.
>    */
> +QEMU_PLUGIN_API
>   const char *qemu_plugin_hwaddr_device_name(const struct qemu_plugin_hwaddr *h);
>   
>   /**
> @@ -513,6 +542,7 @@ typedef void (*qemu_plugin_vcpu_mem_cb_t) (unsigned int vcpu_index,
>    * callback so the plugin is responsible for ensuring it doesn't get
>    * confused by making appropriate use of locking if required.
>    */
> +QEMU_PLUGIN_API
>   void qemu_plugin_register_vcpu_mem_cb(struct qemu_plugin_insn *insn,
>                                         qemu_plugin_vcpu_mem_cb_t cb,
>                                         enum qemu_plugin_cb_flags flags,
> @@ -531,6 +561,7 @@ void qemu_plugin_register_vcpu_mem_cb(struct qemu_plugin_insn *insn,
>    * instruction. This provides for a lightweight but not thread-safe
>    * way of counting the number of operations done.
>    */
> +QEMU_PLUGIN_API
>   void qemu_plugin_register_vcpu_mem_inline(struct qemu_plugin_insn *insn,
>                                             enum qemu_plugin_mem_rw rw,
>                                             enum qemu_plugin_op op, void *ptr,
> @@ -544,6 +575,7 @@ typedef void
>                                    uint64_t a3, uint64_t a4, uint64_t a5,
>                                    uint64_t a6, uint64_t a7, uint64_t a8);
>   
> +QEMU_PLUGIN_API
>   void qemu_plugin_register_vcpu_syscall_cb(qemu_plugin_id_t id,
>                                             qemu_plugin_vcpu_syscall_cb_t cb);
>   
> @@ -551,6 +583,7 @@ typedef void
>   (*qemu_plugin_vcpu_syscall_ret_cb_t)(qemu_plugin_id_t id, unsigned int vcpu_idx,
>                                        int64_t num, int64_t ret);
>   
> +QEMU_PLUGIN_API
>   void
>   qemu_plugin_register_vcpu_syscall_ret_cb(qemu_plugin_id_t id,
>                                            qemu_plugin_vcpu_syscall_ret_cb_t cb);
> @@ -563,6 +596,7 @@ qemu_plugin_register_vcpu_syscall_ret_cb(qemu_plugin_id_t id,
>    * Returns an allocated string containing the disassembly
>    */
>   
> +QEMU_PLUGIN_API
>   char *qemu_plugin_insn_disas(const struct qemu_plugin_insn *insn);
>   
>   /**
> @@ -572,6 +606,7 @@ char *qemu_plugin_insn_disas(const struct qemu_plugin_insn *insn);
>    * Return a static string referring to the symbol. This is dependent
>    * on the binary QEMU is running having provided a symbol table.
>    */
> +QEMU_PLUGIN_API
>   const char *qemu_plugin_insn_symbol(const struct qemu_plugin_insn *insn);
>   
>   /**
> @@ -583,9 +618,11 @@ const char *qemu_plugin_insn_symbol(const struct qemu_plugin_insn *insn);
>    *
>    * See also: qemu_plugin_register_vcpu_init_cb()
>    */
> +QEMU_PLUGIN_API
>   void qemu_plugin_vcpu_for_each(qemu_plugin_id_t id,
>                                  qemu_plugin_vcpu_simple_cb_t cb);
>   
> +QEMU_PLUGIN_API
>   void qemu_plugin_register_flush_cb(qemu_plugin_id_t id,
>                                      qemu_plugin_simple_cb_t cb);
>   
> @@ -602,6 +639,7 @@ void qemu_plugin_register_flush_cb(qemu_plugin_id_t id,
>    * In user-mode it is possible a few un-instrumented instructions from
>    * child threads may run before the host kernel reaps the threads.
>    */
> +QEMU_PLUGIN_API
>   void qemu_plugin_register_atexit_cb(qemu_plugin_id_t id,
>                                       qemu_plugin_udata_cb_t cb, void *userdata);
>   
> @@ -615,6 +653,7 @@ int qemu_plugin_n_max_vcpus(void);
>    * qemu_plugin_outs() - output string via QEMU's logging system
>    * @string: a string
>    */
> +QEMU_PLUGIN_API
>   void qemu_plugin_outs(const char *string);
>   
>   /**
> @@ -628,6 +667,7 @@ void qemu_plugin_outs(const char *string);
>    * returns true if the combination @name=@val parses correctly to a boolean
>    * argument, and false otherwise
>    */
> +QEMU_PLUGIN_API
>   bool qemu_plugin_bool_parse(const char *name, const char *val, bool *ret);
>   
>   /**
> @@ -638,6 +678,7 @@ bool qemu_plugin_bool_parse(const char *name, const char *val, bool *ret);
>    * return NULL. The user should g_free() the string once no longer
>    * needed.
>    */
> +QEMU_PLUGIN_API
>   const char *qemu_plugin_path_to_binary(void);
>   
>   /**
> @@ -646,6 +687,7 @@ const char *qemu_plugin_path_to_binary(void);
>    * Returns the nominal start address of the main text segment in
>    * user-mode. Currently returns 0 for system emulation.
>    */
> +QEMU_PLUGIN_API
>   uint64_t qemu_plugin_start_code(void);
>   
>   /**
> @@ -654,6 +696,7 @@ uint64_t qemu_plugin_start_code(void);
>    * Returns the nominal end address of the main text segment in
>    * user-mode. Currently returns 0 for system emulation.
>    */
> +QEMU_PLUGIN_API
>   uint64_t qemu_plugin_end_code(void);
>   
>   /**
> @@ -662,6 +705,7 @@ uint64_t qemu_plugin_end_code(void);
>    * Returns the nominal entry address of the main text segment in
>    * user-mode. Currently returns 0 for system emulation.
>    */
> +QEMU_PLUGIN_API
>   uint64_t qemu_plugin_entry_code(void);
>   
>   #endif /* QEMU_QEMU_PLUGIN_H */

Re: [PATCH 13/22] plugins: add dllexport and dllimport to api funcs

[Philippe Mathieu-Daudé]

On 6/11/23 19:51, Alex Bennée wrote:
> From: Greg Manning <gmanning@rapitasystems.com>
> 
> In qemu-plugin.h, mark all API functions as __declspec(dllexport) when
> compiling the executables, and as __declspec(dllimport) when being used
> to compile plugins against.
> 
> Signed-off-by: Greg Manning <gmanning@rapitasystems.com>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20231102172053.17692-2-gmanning@rapitasystems.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20231103195956.1998255-27-alex.bennee@linaro.org>
> ---
>   include/qemu/qemu-plugin.h | 50 +++++++++++++++++++++++++++++++++++---
>   1 file changed, 47 insertions(+), 3 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 14/22] plugins: make test/example plugins work on windows

[Alex Bennée]

From: Greg Manning <gmanning@rapitasystems.com>

Generate a qemu_plugin_api.lib delay import lib on windows, for
windows qemu plugins to link against.

Implement an example dll load fail hook to link up the API functions
correctly when a plugin is loaded on windows.

Update the build scripts for the test and example plugins to use these
things.

Signed-off-by: Greg Manning <gmanning@rapitasystems.com>
Acked-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20231102172053.17692-3-gmanning@rapitasystems.com>
[AJB: use find_program for dlltool]
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20231103195956.1998255-28-alex.bennee@linaro.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
---
 configure                      |  3 +++
 contrib/plugins/win32_linker.c | 34 ++++++++++++++++++++++++++++++++++
 contrib/plugins/Makefile       | 20 ++++++++++++++++----
 plugins/meson.build            | 19 +++++++++++++++++++
 tests/plugin/meson.build       | 14 +++++++++++---
 5 files changed, 83 insertions(+), 7 deletions(-)
 create mode 100644 contrib/plugins/win32_linker.c

diff --git a/configure b/configure
index cd6c521bd8..e50ec99fe2 100755
--- a/configure
+++ b/configure
@@ -1666,6 +1666,9 @@ fi
 if test "$targetos" = darwin; then
   echo "CONFIG_DARWIN=y" >> contrib/plugins/$config_host_mak
 fi
+if test "$targetos" = windows; then
+  echo "CONFIG_WIN32=y" >> contrib/plugins/$config_host_mak
+fi
 
 # tests/tcg configuration
 (config_host_mak=tests/tcg/config-host.mak
diff --git a/contrib/plugins/win32_linker.c b/contrib/plugins/win32_linker.c
new file mode 100644
index 0000000000..50797d616e
--- /dev/null
+++ b/contrib/plugins/win32_linker.c
@@ -0,0 +1,34 @@
+/*
+ * Copyright (C) 2023, Greg Manning <gmanning@rapitasystems.com>
+ *
+ * This hook, __pfnDliFailureHook2, is documented in the microsoft documentation here:
+ * https://learn.microsoft.com/en-us/cpp/build/reference/error-handling-and-notification
+ * It gets called when a delay-loaded DLL encounters various errors.
+ * We handle the specific case of a DLL looking for a "qemu.exe",
+ * and give it the running executable (regardless of what it is named).
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ */
+
+#include <Windows.h>
+#include <delayimp.h>
+
+FARPROC WINAPI dll_failure_hook(unsigned dliNotify, PDelayLoadInfo pdli);
+
+
+PfnDliHook __pfnDliFailureHook2 = dll_failure_hook;
+
+FARPROC WINAPI dll_failure_hook(unsigned dliNotify, PDelayLoadInfo pdli) {
+    if (dliNotify == dliFailLoadLib) {
+        /* If the failing request was for qemu.exe, ... */
+        if (strcmp(pdli->szDll, "qemu.exe") == 0) {
+            /* Then pass back a pointer to the top level module. */
+            HMODULE top = GetModuleHandle(NULL);
+            return (FARPROC) top;
+        }
+    }
+    /* Otherwise we can't do anything special. */
+    return 0;
+}
+
diff --git a/contrib/plugins/Makefile b/contrib/plugins/Makefile
index 8ba78c7a32..751fa38619 100644
--- a/contrib/plugins/Makefile
+++ b/contrib/plugins/Makefile
@@ -22,7 +22,14 @@ NAMES += hwprofile
 NAMES += cache
 NAMES += drcov
 
-SONAMES := $(addsuffix .so,$(addprefix lib,$(NAMES)))
+ifeq ($(CONFIG_WIN32),y)
+SO_SUFFIX := .dll
+LDLIBS += $(shell $(PKG_CONFIG) --libs glib-2.0)
+else
+SO_SUFFIX := .so
+endif
+
+SONAMES := $(addsuffix $(SO_SUFFIX),$(addprefix lib,$(NAMES)))
 
 # The main QEMU uses Glib extensively so it's perfectly fine to use it
 # in plugins (which many example do).
@@ -35,15 +42,20 @@ all: $(SONAMES)
 %.o: %.c
 	$(CC) $(CFLAGS) $(PLUGIN_CFLAGS) -c -o $@ $<
 
-lib%.so: %.o
-ifeq ($(CONFIG_DARWIN),y)
+ifeq ($(CONFIG_WIN32),y)
+lib%$(SO_SUFFIX): %.o win32_linker.o ../../plugins/qemu_plugin_api.lib
+	$(CC) -shared -o $@ $^ $(LDLIBS)
+else ifeq ($(CONFIG_DARWIN),y)
+lib%$(SO_SUFFIX): %.o
 	$(CC) -bundle -Wl,-undefined,dynamic_lookup -o $@ $^ $(LDLIBS)
 else
+lib%$(SO_SUFFIX): %.o
 	$(CC) -shared -o $@ $^ $(LDLIBS)
 endif
 
+
 clean:
-	rm -f *.o *.so *.d
+	rm -f *.o *$(SO_SUFFIX) *.d
 	rm -Rf .libs
 
 .PHONY: all clean
diff --git a/plugins/meson.build b/plugins/meson.build
index 71ed996ed3..40d24529c0 100644
--- a/plugins/meson.build
+++ b/plugins/meson.build
@@ -14,6 +14,25 @@ if not enable_modules
 endif
 
 if get_option('plugins')
+  if targetos == 'windows'
+    dlltool = find_program('dlltool', required: true)
+
+    # Generate a .lib file for plugins to link against.
+    # First, create a .def file listing all the symbols a plugin should expect to have
+    # available in qemu
+    win32_plugin_def = configure_file(
+      input: files('qemu-plugins.symbols'),
+      output: 'qemu_plugin_api.def',
+      capture: true,
+      command: ['sed', '-e', '0,/^/s//EXPORTS/; s/[{};]//g', '@INPUT@'])
+    # then use dlltool to assemble a delaylib.
+    win32_qemu_plugin_api_lib = configure_file(
+      input: win32_plugin_def,
+      output: 'qemu_plugin_api.lib',
+      command: [dlltool, '--input-def', '@INPUT@',
+                '--output-delaylib', '@OUTPUT@', '--dllname', 'qemu.exe']
+    )
+  endif
   specific_ss.add(files(
     'loader.c',
     'core.c',
diff --git a/tests/plugin/meson.build b/tests/plugin/meson.build
index 322cafcdf6..528bb9d86c 100644
--- a/tests/plugin/meson.build
+++ b/tests/plugin/meson.build
@@ -1,9 +1,17 @@
 t = []
 if get_option('plugins')
   foreach i : ['bb', 'empty', 'insn', 'mem', 'syscall']
-    t += shared_module(i, files(i + '.c'),
-                       include_directories: '../../include/qemu',
-                       dependencies: glib)
+    if targetos == 'windows'
+      t += shared_module(i, files(i + '.c') + '../../contrib/plugins/win32_linker.c',
+                        include_directories: '../../include/qemu',
+                        objects: [win32_qemu_plugin_api_lib],
+                        dependencies: glib)
+
+    else
+      t += shared_module(i, files(i + '.c'),
+                        include_directories: '../../include/qemu',
+                        dependencies: glib)
+    endif
   endforeach
 endif
 if t.length() > 0
-- 
2.39.2

Re: [PATCH 14/22] plugins: make test/example plugins work on windows

[Paolo Bonzini]

One important remark below that Greg can answer; the others are nits.

On 11/6/23 19:51, Alex Bennée wrote:
> diff --git a/contrib/plugins/win32_linker.c b/contrib/plugins/win32_linker.c
> new file mode 100644
> index 0000000000..50797d616e
> --- /dev/null
> +++ b/contrib/plugins/win32_linker.c
> @@ -0,0 +1,34 @@
> +/*
> + * Copyright (C) 2023, Greg Manning <gmanning@rapitasystems.com>
> + *
> + * This hook, __pfnDliFailureHook2, is documented in the microsoft documentation here:
> + * https://learn.microsoft.com/en-us/cpp/build/reference/error-handling-and-notification
> + * It gets called when a delay-loaded DLL encounters various errors.
> + * We handle the specific case of a DLL looking for a "qemu.exe",
> + * and give it the running executable (regardless of what it is named).
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + */
> +
> +#include <Windows.h>

Just a nit, but we generally use lowercase "windows.h".

> +#include <delayimp.h>
> +
> +FARPROC WINAPI dll_failure_hook(unsigned dliNotify, PDelayLoadInfo pdli);
> +
> +
> +PfnDliHook __pfnDliFailureHook2 = dll_failure_hook;
> +
> +FARPROC WINAPI dll_failure_hook(unsigned dliNotify, PDelayLoadInfo pdli) {
> +    if (dliNotify == dliFailLoadLib) {

I think this could also use the notification hook and 
dliNotePreLoadLibrary.  That's a little more tidy but it's okay either way.

A bit more important: would it make sense to include the hook *in the 
QEMU executable itself*, rather than in the DLL?  If it works, it would 
be much preferrable.  You still would have to add the .lib file to the 
compilation, but win32_linker.c could simply be placed in os-win32.c 
with fewer changes to meson.build and the makefiles.

> +    if targetos == 'windows'
> +      t += shared_module(i, files(i + '.c') + '../../contrib/plugins/win32_linker.c',
> +                        include_directories: '../../include/qemu',
> +                        objects: [win32_qemu_plugin_api_lib],
> +                        dependencies: glib)
> +
> +    else
> +      t += shared_module(i, files(i + '.c'),
> +                        include_directories: '../../include/qemu',
> +                        dependencies: glib)
> +    endif

If the win32_linker.c file can be removed (by moving the hook into the 
emulator), I'd rather have this where win32_qemu_plugin_api_lib is created:

if targetos == 'windows'
   ...
else
   win32_qemu_plugin_api_lib = []
endif

and then here you can just use "objects: [win32_qemu_plugin_api_lib]" 
unconditionally, saving an "if" and some duplication.

Paolo

>     endforeach
>   endif
>   if t.length() > 0

Re: [PATCH 14/22] plugins: make test/example plugins work on windows

[Greg Manning]

From: Paolo Bonzini <pbonzini@redhat.com>

One important remark below that Greg can answer; the others are nits.
> ...
> I think this could also use the notification hook and
dliNotePreLoadLibrary.  That's a little more tidy but it's okay either way.

I don't really mind. I had in mind that there might someday be a
single executable and when that happens the hook would silently
get out of the way.

On the other hand, doing it this way means if the user /happens/
to have a qemu.exe in an unfortunate place then things will fail
with very unhelpful error messages, because the linker would
sucessfully load the qemu.exe, then (presumably) fail when
looking up symbols.

> A bit more important: would it make sense to include the hook *in the
> QEMU executable itself*, rather than in the DLL?  If it works, it would
> be much preferrable.  You still would have to add the .lib file to the
> compilation, but win32_linker.c could simply be placed in os-win32.c
> with fewer changes to meson.build and the makefiles.

My initial trials of this didn't work. But having read the docs again, I'm
going to have another go at it now...

Greg.
--

Follow Rapita Systems on LinkedIn<https://www.linkedin.com/company/rapita-systems?utm_source=rs_email_sig>

Re: [PATCH 14/22] plugins: make test/example plugins work on windows

[Greg Manning]

> > A bit more important: would it make sense to include the hook *in the
> > QEMU executable itself*, rather than in the DLL?  If it works, it would
> > be much preferrable.  You still would have to add the .lib file to the
> > compilation, but win32_linker.c could simply be placed in os-win32.c
> > with fewer changes to meson.build and the makefiles.

> My initial trials of this didn't work. But having read the docs again, I'm
> going to have another go at it now...

Here is a diagram of what's going on.

  /---dynamic load------v
qemu               libplugin.dll
  ^---delay load--------/

First, qemu dynamically loads the plugin, which does no linking. It finds
the function qemu_plugin_install, and invokes it. As soon as libplugin.dll
calls any qemu_plugin_* function, the delay loader steps in and searches for
qemu.exe. It fails to find it, and the delay failure helper steps in and
returns the right reference to the top level executable. Everything gets
linked OK.

Windows will look for __pfnDliFailureHook2 in the module doing the delay
loading, which in this case is the plugin DLL. So, the __pfnDliFailureHook2
function pointer needs to be in the plugin DLL. We could have qemu set the
value of that pointer before it calls install, but I can't find the way to
expose a function pointer in such a way that `g_module_symbol` can find it.
Possibly we could pass in a pointer to it in qemu_plugin_install or a new
qemu_plugin_set_linker_fn, but that is getting back to the sort of
shenanigans we're trying to avoid, and which previous attempts at windows
plugins were based on.

so: maybe, but I'm not sure it would be any tidier.

Greg.
--

Follow Rapita Systems on LinkedIn<https://www.linkedin.com/company/rapita-systems?utm_source=rs_email_sig>

Re: [PATCH 14/22] plugins: make test/example plugins work on windows

[Paolo Bonzini]

On Tue, Nov 7, 2023 at 1:43 PM Greg Manning <gmanning@rapitasystems.com> wrote:
> Windows will look for __pfnDliFailureHook2 in the module doing the delay
> loading, which in this case is the plugin DLL.

Fair enough, this is what was not clear from the Microsoft docs.

Paolo

[PATCH 15/22] plugins: disable lockstep plugin on windows

[Alex Bennée]

From: Greg Manning <gmanning@rapitasystems.com>

The lockstep plugin uses unix sockets and would require a different
communication mechanism to work on Windows.

Signed-off-by: Greg Manning <gmanning@rapitasystems.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20231102172053.17692-4-gmanning@rapitasystems.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20231103195956.1998255-29-alex.bennee@linaro.org>
---
 contrib/plugins/Makefile | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/contrib/plugins/Makefile b/contrib/plugins/Makefile
index 751fa38619..1783750cf6 100644
--- a/contrib/plugins/Makefile
+++ b/contrib/plugins/Makefile
@@ -17,7 +17,13 @@ NAMES += execlog
 NAMES += hotblocks
 NAMES += hotpages
 NAMES += howvec
+
+# The lockstep example communicates using unix sockets,
+# and can't be easily made to work on windows.
+ifneq ($(CONFIG_WIN32),y)
 NAMES += lockstep
+endif
+
 NAMES += hwprofile
 NAMES += cache
 NAMES += drcov
-- 
2.39.2

Re: [PATCH 15/22] plugins: disable lockstep plugin on windows

[Philippe Mathieu-Daudé]

On 6/11/23 19:51, Alex Bennée wrote:
> From: Greg Manning <gmanning@rapitasystems.com>
> 
> The lockstep plugin uses unix sockets and would require a different
> communication mechanism to work on Windows.
> 
> Signed-off-by: Greg Manning <gmanning@rapitasystems.com>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20231102172053.17692-4-gmanning@rapitasystems.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20231103195956.1998255-29-alex.bennee@linaro.org>
> ---
>   contrib/plugins/Makefile | 6 ++++++
>   1 file changed, 6 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 16/22] plugins: allow plugins to be enabled on windows

[Alex Bennée]

From: Greg Manning <gmanning@rapitasystems.com>

allow plugins to be enabled in the configure script on windows. Also,
add the qemu_plugin_api.lib to the installer.

Signed-off-by: Greg Manning <gmanning@rapitasystems.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20231102172053.17692-5-gmanning@rapitasystems.com>
[AJB: add check for dlltool to configure]
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20231103195956.1998255-30-alex.bennee@linaro.org>
---
 configure   | 4 ++--
 meson.build | 5 +++++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index e50ec99fe2..abcb199aa8 100755
--- a/configure
+++ b/configure
@@ -1011,9 +1011,9 @@ if test "$targetos" = "bogus"; then
 fi
 
 # test for any invalid configuration combinations
-if test "$targetos" = "windows"; then
+if test "$targetos" = "windows" && ! has "$dlltool"; then
   if test "$plugins" = "yes"; then
-    error_exit "TCG plugins not currently supported on Windows platforms"
+    error_exit "TCG plugins requires dlltool to build on Windows platforms"
   fi
   plugins="no"
 fi
diff --git a/meson.build b/meson.build
index dcef8b1e79..b855224acc 100644
--- a/meson.build
+++ b/meson.build
@@ -3904,6 +3904,11 @@ endforeach
 
 if get_option('plugins')
   install_headers('include/qemu/qemu-plugin.h')
+  if targetos == 'windows'
+    # On windows, we want to deliver the qemu_plugin_api.lib file in the qemu installer,
+    # so that plugin authors can compile against it.
+    install_data(win32_qemu_plugin_api_lib, install_dir: 'lib')
+  endif
 endif
 
 subdir('qga')
-- 
2.39.2

Re: [PATCH 16/22] plugins: allow plugins to be enabled on windows

[Philippe Mathieu-Daudé]

On 6/11/23 19:51, Alex Bennée wrote:
> From: Greg Manning <gmanning@rapitasystems.com>
> 
> allow plugins to be enabled in the configure script on windows. Also,
> add the qemu_plugin_api.lib to the installer.
> 
> Signed-off-by: Greg Manning <gmanning@rapitasystems.com>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20231102172053.17692-5-gmanning@rapitasystems.com>
> [AJB: add check for dlltool to configure]
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20231103195956.1998255-30-alex.bennee@linaro.org>
> ---
>   configure   | 4 ++--
>   meson.build | 5 +++++
>   2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/configure b/configure
> index e50ec99fe2..abcb199aa8 100755
> --- a/configure
> +++ b/configure
> @@ -1011,9 +1011,9 @@ if test "$targetos" = "bogus"; then
>   fi
>   
>   # test for any invalid configuration combinations
> -if test "$targetos" = "windows"; then
> +if test "$targetos" = "windows" && ! has "$dlltool"; then
>     if test "$plugins" = "yes"; then
> -    error_exit "TCG plugins not currently supported on Windows platforms"
> +    error_exit "TCG plugins requires dlltool to build on Windows platforms"
>     fi
>     plugins="no"
>   fi
> diff --git a/meson.build b/meson.build
> index dcef8b1e79..b855224acc 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -3904,6 +3904,11 @@ endforeach
>   
>   if get_option('plugins')
>     install_headers('include/qemu/qemu-plugin.h')
> +  if targetos == 'windows'
> +    # On windows, we want to deliver the qemu_plugin_api.lib file in the qemu installer,
> +    # so that plugin authors can compile against it.
> +    install_data(win32_qemu_plugin_api_lib, install_dir: 'lib')
> +  endif
>   endif
>   
>   subdir('qga')

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 17/22] contrib/gitdm: Add Rivos Inc to the domain map

[Alex Bennée]

Whatever they are up to a number of people for the company are
contributing to QEMU so lets group them together.

Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>
Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20231013154424.1536392-2-alex.bennee@linaro.org>
---
 contrib/gitdm/domain-map | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/gitdm/domain-map b/contrib/gitdm/domain-map
index 3e31a06245..e676da8d47 100644
--- a/contrib/gitdm/domain-map
+++ b/contrib/gitdm/domain-map
@@ -38,6 +38,7 @@ proxmox.com     Proxmox
 quicinc.com     Qualcomm Innovation Center
 redhat.com      Red Hat
 rev.ng          rev.ng Labs
+rivosinc.com    Rivos Inc
 rt-rk.com       RT-RK
 samsung.com     Samsung
 siemens.com     Siemens
-- 
2.39.2

[PATCH 18/22] contrib/gitdm: add domain-map for Cestc

[Alex Bennée]

From: luzhipeng <luzhipeng@cestc.cn>

Signed-off-by: luzhipeng <luzhipeng@cestc.cn>
Message-Id: <20230628072236.1925-1-luzhipeng@cestc.cn>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20231013154424.1536392-6-alex.bennee@linaro.org>
---
 contrib/gitdm/domain-map | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/gitdm/domain-map b/contrib/gitdm/domain-map
index e676da8d47..38945cddf0 100644
--- a/contrib/gitdm/domain-map
+++ b/contrib/gitdm/domain-map
@@ -12,6 +12,7 @@ amd.com         AMD
 aspeedtech.com  ASPEED Technology Inc.
 baidu.com       Baidu
 bytedance.com   ByteDance
+cestc.cn        Cestc
 cmss.chinamobile.com China Mobile
 citrix.com      Citrix
 crudebyte.com   Crudebyte
-- 
2.39.2

[PATCH 19/22] contrib/gitdm: map HiSilicon to Huawei

[Alex Bennée]

HiSilicon is a wholly owned subsidiary of Huawei so map the domain to
the same company to avoid splitting the contributions.

Reviewed-by: Yicong Yang <yangyicong@hisilicon.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20231013154424.1536392-7-alex.bennee@linaro.org>
---
 contrib/gitdm/domain-map | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/gitdm/domain-map b/contrib/gitdm/domain-map
index 38945cddf0..42571fc1c4 100644
--- a/contrib/gitdm/domain-map
+++ b/contrib/gitdm/domain-map
@@ -22,6 +22,7 @@ fb.com          Facebook
 fujitsu.com     Fujitsu
 google.com      Google
 greensocs.com   GreenSocs
+hisilicon.com   Huawei
 huawei.com      Huawei
 ibm.com         IBM
 igalia.com      Igalia
-- 
2.39.2

[PATCH 20/22] contrib/gitdm: add Daynix to domain-map

[Alex Bennée]

Daynix describes itself as a cloud technology company so I assume
employee contributions should count as such.

Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20231013154424.1536392-8-alex.bennee@linaro.org>
---
 contrib/gitdm/domain-map | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/gitdm/domain-map b/contrib/gitdm/domain-map
index 42571fc1c4..bf1dce03fd 100644
--- a/contrib/gitdm/domain-map
+++ b/contrib/gitdm/domain-map
@@ -17,6 +17,7 @@ cmss.chinamobile.com China Mobile
 citrix.com      Citrix
 crudebyte.com   Crudebyte
 chinatelecom.cn China Telecom
+daynix.com      Daynix
 eldorado.org.br Instituto de Pesquisas Eldorado
 fb.com          Facebook
 fujitsu.com     Fujitsu
-- 
2.39.2

[PATCH 21/22] mailmap: fixup some more corrupted author fields

[Alex Bennée]

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20231013154424.1536392-9-alex.bennee@linaro.org>
Cc: Timothée Cocault <timothee.cocault@gmail.com>
Cc: fanwenjie <fanwj@mail.ustc.edu.cn>
---
 .mailmap | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/.mailmap b/.mailmap
index 94f19a0ac9..e12e19f691 100644
--- a/.mailmap
+++ b/.mailmap
@@ -30,10 +30,12 @@ malc <av1474@comtv.ru> malc <malc@c046a42c-6fe2-441c-8c8c-71466251a162>
 # Corrupted Author fields
 Aaron Larson <alarson@ddci.com> alarson@ddci.com
 Andreas Färber <andreas.faerber@web.de> Andreas Färber <andreas.faerber>
+fanwenjie <fanwj@mail.ustc.edu.cn> fanwj@mail.ustc.edu.cn <fanwj@mail.ustc.edu.cn>
 Jason Wang <jasowang@redhat.com> Jason Wang <jasowang>
 Marek Dolata <mkdolata@us.ibm.com> mkdolata@us.ibm.com <mkdolata@us.ibm.com>
 Michael Ellerman <mpe@ellerman.id.au> michael@ozlabs.org <michael@ozlabs.org>
 Nick Hudson <hnick@vmware.com> hnick@vmware.com <hnick@vmware.com>
+Timothée Cocault <timothee.cocault@gmail.com> timothee.cocault@gmail.com <timothee.cocault@gmail.com>
 
 # There is also a:
 #    (no author) <(no author)@c046a42c-6fe2-441c-8c8c-71466251a162>
-- 
2.39.2

[PATCH 22/22] Revert "tests/tcg/nios2: Re-enable linux-user tests"

[Alex Bennée]

nios2 signal tests are broken again:

  retry.py -n 10 -c -- ./qemu-nios2 ./tests/tcg/nios2-linux-user/signals
  Results summary:
  0: 8 times (80.00%), avg time 2.254 (0.00 varience/0.00 deviation)
  -11: 2 times (20.00%), avg time 0.253 (0.00 varience/0.00 deviation)
  Ran command 10 times, 8 passes

This wasn't picked up by CI as we don't have a docker container that
can build QEMU with the nios2 compiler. I don't have to bisect the
breakage and the target is orphaned anyway so take the easy route and
revert it.

This reverts commit 20e7524ff9f0cab4c9a0306014d6f3d7b467ae1e.

Cc: Chris Wulff <crwulff@gmail.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/tcg/nios2/Makefile.target | 11 +++++++++++
 1 file changed, 11 insertions(+)
 create mode 100644 tests/tcg/nios2/Makefile.target

diff --git a/tests/tcg/nios2/Makefile.target b/tests/tcg/nios2/Makefile.target
new file mode 100644
index 0000000000..b38e2352b7
--- /dev/null
+++ b/tests/tcg/nios2/Makefile.target
@@ -0,0 +1,11 @@
+# nios2 specific test tweaks
+
+# Currently nios2 signal handling is broken
+run-signals: signals
+	$(call skip-test, $<, "BROKEN")
+run-plugin-signals-with-%:
+	$(call skip-test, $<, "BROKEN")
+run-linux-test: linux-test
+	$(call skip-test, $<, "BROKEN")
+run-plugin-linux-test-with-%:
+	$(call skip-test, $<, "BROKEN")
-- 
2.39.2

Re: [PATCH 22/22] Revert "tests/tcg/nios2: Re-enable linux-user tests"

[Richard Henderson]

On 11/6/23 10:51, Alex Bennée wrote:
> nios2 signal tests are broken again:
> 
>    retry.py -n 10 -c -- ./qemu-nios2 ./tests/tcg/nios2-linux-user/signals
>    Results summary:
>    0: 8 times (80.00%), avg time 2.254 (0.00 varience/0.00 deviation)
>    -11: 2 times (20.00%), avg time 0.253 (0.00 varience/0.00 deviation)
>    Ran command 10 times, 8 passes
> 
> This wasn't picked up by CI as we don't have a docker container that
> can build QEMU with the nios2 compiler. I don't have to bisect the
> breakage and the target is orphaned anyway so take the easy route and
> revert it.
> 
> This reverts commit 20e7524ff9f0cab4c9a0306014d6f3d7b467ae1e.
> 
> Cc: Chris Wulff <crwulff@gmail.com>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

Ug.  It's definitely time to deprecate prior to removal.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~