Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge dev to qemu-cheri #259

Open
wants to merge 13 commits into
base: qemu-cheri
Choose a base branch
from
Open

Merge dev to qemu-cheri #259

wants to merge 13 commits into from

Conversation

jrtc27
Copy link
Member

@jrtc27 jrtc27 commented Jul 17, 2024

No description provided.

arichardson and others added 13 commits June 18, 2024 14:17
It was logging the next address rather than the current one so was
always off by 4.
This does not include a full audit of all writes (helpers that
directly modify registers are not included), but at least handles
the common case where store_reg() is called.
This bumps it to 23.11 and will automatically pick up 24.05 soon.
Compiler optimizations can cache TLS values across coroutine yield
points, resulting in stale values from the previous thread when a
coroutine is re-entered by a new thread.

Serge Guelton developed an __attribute__((noinline)) wrapper and tested
it with clang and gcc. I formatted his idea according to QEMU's coding
style and wrote documentation.

The compiler can still optimize based on analyzing noinline code, so an
asm volatile barrier with an output constraint is required to prevent
unwanted optimizations.

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1952483
Suggested-by: Serge Guelton <[email protected]>
Signed-off-by: Stefan Hajnoczi <[email protected]>
Message-Id: <[email protected]>
Signed-off-by: Kevin Wolf <[email protected]>
(cherry picked from commit 7d29c341c9d402cf0bcb3a3b76fce0c09dd24e94)
RCU may be used from coroutines. Standard __thread variables cannot be
used by coroutines. Use the coroutine TLS macros instead.

Signed-off-by: Stefan Hajnoczi <[email protected]>
Message-Id: <[email protected]>
Signed-off-by: Kevin Wolf <[email protected]>
(cherry picked from commit 17c78154b0ba2237c37f3e4a95140b754cb6ac8b)
qemu_mutex_iothread_locked() may be used from coroutines. Standard
__thread variables cannot be used by coroutines. Use the coroutine TLS
macros instead.

Signed-off-by: Stefan Hajnoczi <[email protected]>
Message-Id: <[email protected]>
Signed-off-by: Kevin Wolf <[email protected]>
(cherry picked from commit d5d2b15ecf62c662985983ca065ddeeec48fd248)
Thread-Local Storage variables cannot be used directly from coroutine
code because the compiler may optimize TLS variable accesses across
qemu_coroutine_yield() calls. When the coroutine is re-entered from
another thread the TLS variables from the old thread must no longer be
used.

Use QEMU_DEFINE_STATIC_CO_TLS() for the current and leader variables.

Signed-off-by: Stefan Hajnoczi <[email protected]>
Message-Id: <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Signed-off-by: Kevin Wolf <[email protected]>
(cherry picked from commit 34145a307d849d0b6734d0222a7aa0bb9eef7407)
Thread-Local Storage variables cannot be used directly from coroutine
code because the compiler may optimize TLS variable accesses across
qemu_coroutine_yield() calls. When the coroutine is re-entered from
another thread the TLS variables from the old thread must no longer be
used.

Use QEMU_DEFINE_STATIC_CO_TLS() for the current and leader variables.
The alloc_pool QSLIST needs a typedef so the return value of
get_ptr_alloc_pool() can be stored in a local variable.

One example of why this code is necessary: a coroutine that yields
before calling qemu_coroutine_create() to create another coroutine is
affected by the TLS issue.

Signed-off-by: Stefan Hajnoczi <[email protected]>
Message-Id: <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Signed-off-by: Kevin Wolf <[email protected]>
(cherry picked from commit ac387a08a9c9f6b36757da912f0339c25f421f90)
Thread-Local Storage variables cannot be used directly from coroutine
code because the compiler may optimize TLS variable accesses across
qemu_coroutine_yield() calls. When the coroutine is re-entered from
another thread the TLS variables from the old thread must no longer be
used.

Use QEMU_DEFINE_STATIC_CO_TLS() for the current and leader variables.

I think coroutine-win32.c could get away with __thread because the
variables are only used in situations where either the stale value is
correct (current) or outside coroutine context (loading leader when
current is NULL). Due to the difficulty of being sure that this is
really safe in all scenarios it seems worth converting it anyway.

Signed-off-by: Stefan Hajnoczi <[email protected]>
Message-Id: <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Signed-off-by: Kevin Wolf <[email protected]>
(cherry picked from commit c1fe694357a328c807ae3cc6961c19e923448fcc)
Upstream's code is env->pc -= insn_len, like the AArch32 case, which is
doing a binary subtraction on the promoted (to target_ulong, i.e.
64-bit) insn_len. However, by using a unary minus downstream, we instead
negate the 32-bit value prior to promoting (again to target_ulong, the
type for the callee's argument) and thus, given insn_len is unsigned,
that means we zero-extend rather than sign-extend. This has the result
of causing PC to be 2^32 more than it should be.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants