From: "Nicholas Piggin" <npiggin@gmail.com>
To: "Alex Bennée" <alex.bennee@linaro.org>
Cc: "Richard Henderson" <richard.henderson@linaro.org>,
"Ivan Warren" <ivan@vmfacility.fr>, <qemu-devel@nongnu.org>
Subject: Re: [PULL 01/39] accel/tcg: mttcg remove false-negative halted assertion
Date: Mon, 18 Sep 2023 20:53:23 +1000 [thread overview]
Message-ID: <CVLZJR0IZQHP.2SLPV8WML9QJ0@wheely> (raw)
In-Reply-To: <87v8c7x668.fsf@linaro.org>
On Mon Sep 18, 2023 at 5:59 PM AEST, Alex Bennée wrote:
>
> "Nicholas Piggin" <npiggin@gmail.com> writes:
>
> > On Sat Sep 16, 2023 at 1:29 PM AEST, Richard Henderson wrote:
> >> From: Nicholas Piggin <npiggin@gmail.com>
> >>
> >> mttcg asserts that an execution ending with EXCP_HALTED must have
> >> cpu->halted. However between the event or instruction that sets
> >> cpu->halted and requests exit and the assertion here, an
> >> asynchronous event could clear cpu->halted.
> >>
> >> This leads to crashes running AIX on ppc/pseries because it uses
> >> H_CEDE/H_PROD hcalls, where H_CEDE sets self->halted = 1 and
> >> H_PROD sets other cpu->halted = 0 and kicks it.
> >>
> >> H_PROD could be turned into an interrupt to wake, but several other
> >> places in ppc, sparc, and semihosting follow what looks like a similar
> >> pattern setting halted = 0 directly. So remove this assertion.
> >>
> >> Reported-by: Ivan Warren <ivan@vmfacility.fr>
> >> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> >> Message-Id: <20230829010658.8252-1-npiggin@gmail.com>
> >> [rth: Keep the case label and adjust the comment.]
> >
> > Hey Richard,
> >
> > Thanks for picking this up.
> >
> > I think EXCP_HLT and EXCP_HALTED are effectively the same, so they could
> > be merged after this.
> >
> > I couldn't quite decipher the intended difference between them, HLT is
> > "hlt instruction reached", but it does tend to go into a mode where it
> > is halted waiting for external event. Is there some useful difference in
> > semantics we should retain (and at least try to find a way to assert)?
>
> I always thought HALTED was where the system was halted (e.g. during a
> shutdown) but I agree its less than clear.
Maybe that was so. I didn't manage to track down the original intention
of them, but now they are not different, HALTED does just wait for event
too. EXCP_HALTED did previously require the operation set ->halted = 1
before calling (the assert only breaks due to concurrent wakeup clearing
it). But some ops that use EXCP_HLT also set ->halted.
So nowadays halted == 1 means to check ->cpu_has_work() before running
the CPU again (and otherwise wait on io event as you say). And
EXCP_HLT/HALTED are both just ways to return from the cpu exec loop.
One thing I'm not sure of is why you would set EXCP_HLT without setting
halted. In some cases it could be a bug (e.g., avr helper_sleep()), but
there are a few ops that use it after a CPU reset or shutdown which
might be valid. Could call those ones something like EXCP_RESET or
EXCP_REEXEC.
Thanks,
Nick
next prev parent reply other threads:[~2023-09-18 10:54 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-16 3:29 [PULL 00/39] tcg patch queue Richard Henderson
2023-09-16 3:29 ` [PULL 01/39] accel/tcg: mttcg remove false-negative halted assertion Richard Henderson
2023-09-18 6:44 ` Nicholas Piggin
2023-09-18 7:59 ` Alex Bennée
2023-09-18 10:53 ` Nicholas Piggin [this message]
2023-09-18 12:19 ` Alex Bennée
2023-09-16 3:29 ` [PULL 02/39] accel/tcg: Fix the comment for CPUTLBEntryFull Richard Henderson
2023-09-16 3:29 ` [PULL 03/39] util: Delete checks for old host definitions Richard Henderson
2023-09-16 3:29 ` [PULL 04/39] softmmu: " Richard Henderson
2023-09-16 3:29 ` [PULL 05/39] thunk: " Richard Henderson
2023-09-16 3:29 ` [PULL 06/39] tcg/loongarch64: Import LSX instructions Richard Henderson
2023-09-16 3:29 ` [PULL 07/39] tcg/loongarch64: Lower basic tcg vec ops to LSX Richard Henderson
2023-09-16 3:29 ` [PULL 08/39] tcg: pass vece to tcg_target_const_match() Richard Henderson
2023-09-16 3:29 ` [PULL 09/39] tcg/loongarch64: Lower cmp_vec to vseq/vsle/vslt Richard Henderson
2023-09-16 3:29 ` [PULL 10/39] tcg/loongarch64: Lower add/sub_vec to vadd/vsub Richard Henderson
2023-09-16 3:29 ` [PULL 11/39] tcg/loongarch64: Lower vector bitwise operations Richard Henderson
2023-09-16 3:29 ` [PULL 12/39] tcg/loongarch64: Lower neg_vec to vneg Richard Henderson
2023-09-16 3:29 ` [PULL 13/39] tcg/loongarch64: Lower mul_vec to vmul Richard Henderson
2023-09-16 3:29 ` [PULL 14/39] tcg/loongarch64: Lower vector min max ops Richard Henderson
2023-09-16 3:29 ` [PULL 15/39] tcg/loongarch64: Lower vector saturated ops Richard Henderson
2023-09-16 3:29 ` [PULL 16/39] tcg/loongarch64: Lower vector shift vector ops Richard Henderson
2023-09-16 3:29 ` [PULL 17/39] tcg/loongarch64: Lower bitsel_vec to vbitsel Richard Henderson
2023-09-16 3:29 ` [PULL 18/39] tcg/loongarch64: Lower vector shift integer ops Richard Henderson
2023-09-16 3:29 ` [PULL 19/39] tcg/loongarch64: Lower rotv_vec ops to LSX Richard Henderson
2023-09-16 3:29 ` [PULL 20/39] tcg/loongarch64: Lower rotli_vec to vrotri Richard Henderson
2023-09-16 3:29 ` [PULL 21/39] tcg/loongarch64: Implement 128-bit load & store Richard Henderson
2023-09-16 3:29 ` [PULL 22/39] tcg: Add gvec compare with immediate and scalar operand Richard Henderson
2023-09-16 3:29 ` [PULL 23/39] target/arm: Use tcg_gen_gvec_cmpi for compare vs 0 Richard Henderson
2023-09-16 3:29 ` [PULL 24/39] accel/tcg: Simplify tlb_plugin_lookup Richard Henderson
2023-09-16 3:29 ` [PULL 25/39] accel/tcg: Split out io_prepare and io_failed Richard Henderson
2023-09-16 3:29 ` [PULL 26/39] accel/tcg: Use CPUTLBEntryFull.phys_addr in io_failed Richard Henderson
2023-09-16 3:29 ` [PULL 27/39] plugin: Simplify struct qemu_plugin_hwaddr Richard Henderson
2023-09-16 3:30 ` [PULL 28/39] accel/tcg: Merge cpu_transaction_failed into io_failed Richard Henderson
2023-09-16 3:30 ` [PULL 29/39] accel/tcg: Replace direct use of io_readx/io_writex in do_{ld, st}_1 Richard Henderson
2023-09-16 3:30 ` [PULL 30/39] accel/tcg: Merge io_readx into do_ld_mmio_beN Richard Henderson
2023-09-16 3:30 ` [PULL 31/39] accel/tcg: Merge io_writex into do_st_mmio_leN Richard Henderson
2023-09-16 3:30 ` [PULL 32/39] accel/tcg: Introduce do_ld16_mmio_beN Richard Henderson
2023-09-16 3:30 ` [PULL 33/39] accel/tcg: Introduce do_st16_mmio_leN Richard Henderson
2023-09-16 3:30 ` [PULL 34/39] fpu: Add conversions between bfloat16 and [u]int8 Richard Henderson
2023-09-16 3:30 ` [PULL 35/39] fpu: Handle m68k extended precision denormals properly Richard Henderson
2023-09-16 3:30 ` [PULL 36/39] tcg: Add tcg_out_tb_start backend hook Richard Henderson
2023-09-16 3:30 ` [PULL 37/39] util/cpuinfo-aarch64: Add CPUINFO_BTI Richard Henderson
2023-09-16 3:30 ` [PULL 38/39] tcg/aarch64: Emit BTI insns at jump landing pads Richard Henderson
2023-09-16 3:30 ` [PULL 39/39] tcg: Map code_gen_buffer with PROT_BTI Richard Henderson
2023-09-16 4:07 ` [PULL 00/39] tcg patch queue Richard Henderson
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=CVLZJR0IZQHP.2SLPV8WML9QJ0@wheely \
--to=npiggin@gmail.com \
--cc=alex.bennee@linaro.org \
--cc=ivan@vmfacility.fr \
--cc=qemu-devel@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).