From: "Philippe Mathieu-Daudé" <philmd@linaro.org>
To: "Richard Henderson" <richard.henderson@linaro.org>,
qemu-devel@nongnu.org, "Alex Bennée" <alex.bennee@linaro.org>,
qemu-ppc <qemu-ppc@nongnu.org>
Cc: "Anton Johansson" <anjo@rev.ng>,
"Pierrick Bouvier" <pierrick.bouvier@linaro.org>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Mark Cave-Ayland" <mark.cave-ayland@ilande.co.uk>,
"Cédric Le Goater" <clg@kaod.org>
Subject: Re: [PATCH-for-10.1 4/4] tcg: Convert TARGET_SUPPORTS_MTTCG to TCGCPUOps::mttcg_supported field
Date: Sun, 23 Mar 2025 22:24:31 +0100 [thread overview]
Message-ID: <5bd8498c-b5ff-4fb1-94d7-a2efa2f20fd9@linaro.org> (raw)
In-Reply-To: <8cc6f6a4-b868-4dc8-bc14-25b438ad62a5@linaro.org>
On 23/3/25 20:07, Richard Henderson wrote:
> On 3/21/25 08:59, Philippe Mathieu-Daudé wrote:
>> Instead of having a compile-time TARGET_SUPPORTS_MTTCG definition,
>> have each target set the 'mttcg_supported' field in the TCGCPUOps
>> structure.
>>
>> Since so far we only emulate one target architecture at a time,
>> tcg_init_machine() gets whether MTTCG is supported via the
>> &first_cpu global.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> docs/devel/multi-thread-tcg.rst | 2 +-
>> configs/targets/aarch64-softmmu.mak | 1 -
>> configs/targets/alpha-softmmu.mak | 1 -
>> configs/targets/arm-softmmu.mak | 1 -
>> configs/targets/hppa-softmmu.mak | 1 -
>> configs/targets/i386-softmmu.mak | 1 -
>> configs/targets/loongarch64-softmmu.mak | 1 -
>> configs/targets/microblaze-softmmu.mak | 1 -
>> configs/targets/microblazeel-softmmu.mak | 1 -
>> configs/targets/mips-softmmu.mak | 1 -
>> configs/targets/mipsel-softmmu.mak | 1 -
>> configs/targets/or1k-softmmu.mak | 1 -
>> configs/targets/ppc64-softmmu.mak | 1 -
>> configs/targets/riscv32-softmmu.mak | 1 -
>> configs/targets/riscv64-softmmu.mak | 1 -
>> configs/targets/s390x-softmmu.mak | 1 -
>> configs/targets/sparc-softmmu.mak | 1 -
>> configs/targets/sparc64-softmmu.mak | 1 -
>> configs/targets/x86_64-softmmu.mak | 1 -
>> configs/targets/xtensa-softmmu.mak | 1 -
>> configs/targets/xtensaeb-softmmu.mak | 1 -
>> include/accel/tcg/cpu-ops.h | 8 ++++++++
>> include/exec/poison.h | 1 -
>> accel/tcg/tcg-all.c | 7 ++-----
>> target/alpha/cpu.c | 1 +
>> target/arm/cpu.c | 1 +
>> target/arm/tcg/cpu-v7m.c | 1 +
>> target/avr/cpu.c | 1 +
>> target/hexagon/cpu.c | 1 +
>> target/hppa/cpu.c | 1 +
>> target/i386/tcg/tcg-cpu.c | 1 +
>> target/loongarch/cpu.c | 1 +
>> target/m68k/cpu.c | 1 +
>> target/microblaze/cpu.c | 1 +
>> target/mips/cpu.c | 1 +
>> target/openrisc/cpu.c | 1 +
>> target/ppc/cpu_init.c | 1 +
>> target/riscv/tcg/tcg-cpu.c | 1 +
>> target/rx/cpu.c | 1 +
>> target/s390x/cpu.c | 1 +
>> target/sh4/cpu.c | 1 +
>> target/sparc/cpu.c | 1 +
>> target/tricore/cpu.c | 1 +
>> target/xtensa/cpu.c | 1 +
>> 44 files changed, 31 insertions(+), 27 deletions(-)
>
> As far as this goes,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
>> @@ -91,11 +92,7 @@ static int tcg_init_machine(MachineState *ms)
>> #else
>> unsigned max_cpus = ms->smp.max_cpus;
>> #endif
>> -#ifdef TARGET_SUPPORTS_MTTCG
>> - bool mttcg_supported = true;
>> -#else
>> - bool mttcg_supported = false;
>> -#endif
>> + bool mttcg_supported = first_cpu->cc->tcg_ops->mttcg_supported; /
>> * FIXME avoid first_cpu */
>
> Intersection of CPU_FOREACH?
Short term I used CPU_RESOLVING_TYPE:
CPUClass *cc = CPU_CLASS(object_class_by_name(CPU_RESOLVING_TYPE));
mttcg_supported = cc->tcg_ops->mttcg_supported;
I'll move to some helper.
Long term we need the intersection. I'll add a comment for now.
>> diff --git a/target/avr/cpu.c b/target/avr/cpu.c
>> index 24e52e28f44..9bc921c1cb2 100644
>> --- a/target/avr/cpu.c
>> +++ b/target/avr/cpu.c
>> @@ -216,6 +216,7 @@ static const TCGCPUOps avr_tcg_ops = {
>> .cpu_exec_halt = avr_cpu_has_work,
>> .tlb_fill = avr_cpu_tlb_fill,
>> .do_interrupt = avr_cpu_do_interrupt,
>> + .mttcg_supported = false,
>> .guest_default_memory_order = 0,
>> };
>
> This may well be the only true not supported.
> I'm not sure what needs to happen to make that true.
I'd postpone that for later ;)
>> diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
>> index 34734b0edb0..7bcf297998f 100644
>> --- a/target/hexagon/cpu.c
>> +++ b/target/hexagon/cpu.c
>> @@ -324,6 +324,7 @@ static const TCGCPUOps hexagon_tcg_ops = {
>> .translate_code = hexagon_translate_code,
>> .synchronize_from_tb = hexagon_cpu_synchronize_from_tb,
>> .restore_state_to_opc = hexagon_restore_state_to_opc,
>> + .mttcg_supported = false,
>> /* MTTCG not yet supported: require strict ordering */
>> .guest_default_memory_order = TCG_MO_ALL,
>
> We now have strict ordering, right there with TCG_MO_ALL.
> Likewise for m68k, sh4, tricore.
> I think we simply forgot to update.
Doh... I'll clean in a preliminary patch, thanks.
>> diff --git a/target/mips/cpu.c b/target/mips/cpu.c
>> index 5ddc9bbb829..eee604e9254 100644
>> --- a/target/mips/cpu.c
>> +++ b/target/mips/cpu.c
>> @@ -554,6 +554,7 @@ static const TCGCPUOps mips_tcg_ops = {
>> .synchronize_from_tb = mips_cpu_synchronize_from_tb,
>> .restore_state_to_opc = mips_restore_state_to_opc,
>> + .mttcg_supported = TARGET_LONG_BITS == 32,
>> .guest_default_memory_order = 0,
>
> What is missing from mip64?
See:
commit a092a95547710bcbb8c168e5ec03f55fb4ab1ad7
Author: Alex Bennée <alex.bennee@linaro.org>
Date: Mon Mar 23 16:15:09 2020 +0000
configure: disable MTTCG for MIPS guests
While debugging check-acceptance failures I found an instability in
the mips64el test case. Briefly the test case:
retry.py -n 100 -c -- ./mips64el-softmmu/qemu-system-mips64el \
-display none -vga none -serial mon:stdio \
-machine malta -kernel ./vmlinux-4.7.0-rc1.I6400 \
-cpu I6400 -smp 8 -vga std \
-append "printk.time=0 clocksource=GIC console=tty0 " \
"console=ttyS0 panic=-1" \
--no-reboot
Reports about a 9% failure rate:
Results summary:
0: 91 times (91.00%), avg time 5.547 (0.45 varience/0.67 deviation)
-6: 9 times (9.00%), avg time 3.394 (0.02 varience/0.13 deviation)
Ran command 100 times, 91 passes
When re-run with "--accel tcg,thread=single" the instability goes
away.
Results summary:
0: 100 times (100.00%), avg time 17.318 (249.76 varience/15.80
deviation)
Ran command 100 times, 100 passes
Which seems to indicate there is some aspect of the MIPS MTTCG fixes
that has been missed. Ideally we would fix that but I'm afraid I
don't have time to investigate and am not super familiar with the
architecture anyway. In lieu of someone tracking down the failure
lets disable it for now.
>
>> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
>> index 28fbbb8d3c1..ed79cc1a6b7 100644
>> --- a/target/ppc/cpu_init.c
>> +++ b/target/ppc/cpu_init.c
>> @@ -7490,6 +7490,7 @@ static const TCGCPUOps ppc_tcg_ops = {
>> .translate_code = ppc_translate_code,
>> .restore_state_to_opc = ppc_restore_state_to_opc,
>> + .mttcg_supported = TARGET_LONG_BITS == 64,
>> .guest_default_memory_order = 0,
>
> Similarly. I'd be surprised if ppc32 can't use mttcg, really.
Per Cédric on IRC our ppc32 implementations are single core,
so never tested for mttcg.
Regards,
Phil.
next prev parent reply other threads:[~2025-03-23 21:25 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-21 15:59 [PATCH-for-10.1 0/4] tcg: Convert TARGET_SUPPORTS_MTTCG to TCGCPUOps::mttcg_supported field Philippe Mathieu-Daudé
2025-03-21 15:59 ` [PATCH-for-10.1 1/4] target/riscv: Restrict RV128 MTTCG check on system emulation Philippe Mathieu-Daudé
2025-03-21 17:32 ` Pierrick Bouvier
2025-03-23 18:08 ` Richard Henderson
2025-04-02 14:25 ` Philippe Mathieu-Daudé
2025-04-02 14:40 ` Philippe Mathieu-Daudé
2025-04-04 3:22 ` Alistair Francis
2025-03-21 15:59 ` [PATCH-for-10.1 2/4] tcg: Move qemu_tcg_mttcg_enabled() to 'system/tcg.h' Philippe Mathieu-Daudé
2025-03-21 17:33 ` Pierrick Bouvier
2025-03-23 18:36 ` Richard Henderson
2025-03-21 15:59 ` [PATCH-for-10.1 3/4] tcg: Convert TCGState::mttcg_enabled to TriState Philippe Mathieu-Daudé
2025-03-21 17:01 ` Anton Johansson via
2025-03-21 17:45 ` Philippe Mathieu-Daudé
2025-03-21 17:36 ` Pierrick Bouvier
2025-03-21 18:26 ` Philippe Mathieu-Daudé
2025-03-21 18:33 ` Pierrick Bouvier
2025-03-23 18:43 ` Richard Henderson
2025-04-02 21:12 ` Philippe Mathieu-Daudé
2025-03-21 15:59 ` [PATCH-for-10.1 4/4] tcg: Convert TARGET_SUPPORTS_MTTCG to TCGCPUOps::mttcg_supported field Philippe Mathieu-Daudé
2025-03-21 16:41 ` Anton Johansson via
2025-03-21 17:43 ` Pierrick Bouvier
2025-03-23 19:07 ` Richard Henderson
2025-03-23 21:24 ` Philippe Mathieu-Daudé [this message]
2025-03-23 22:13 ` BALATON Zoltan
2025-03-23 22:17 ` Richard Henderson
2025-03-23 22:49 ` BALATON Zoltan
2025-03-24 16:39 ` Philippe Mathieu-Daudé
2025-03-23 23:15 ` Philippe Mathieu-Daudé
2025-03-21 16:00 ` [PATCH-for-10.1 0/4] " Philippe Mathieu-Daudé
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5bd8498c-b5ff-4fb1-94d7-a2efa2f20fd9@linaro.org \
--to=philmd@linaro.org \
--cc=alex.bennee@linaro.org \
--cc=anjo@rev.ng \
--cc=clg@kaod.org \
--cc=mark.cave-ayland@ilande.co.uk \
--cc=pbonzini@redhat.com \
--cc=pierrick.bouvier@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
--cc=richard.henderson@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).