* [PULL 0/3] SIGSEGV fixes
@ 2021-07-21 21:19 Taylor Simpson
2021-07-21 21:19 ` [PULL 1/3] Hexagon (target/hexagon) remove put_user_*/get_user_* Taylor Simpson
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Taylor Simpson @ 2021-07-21 21:19 UTC (permalink / raw)
To: qemu-devel; +Cc: ale, peter.maydell, bcain, richard.henderson, tsimpson, philmd
The following changes since commit 7457b407edd6e8555e4b46488aab2f13959fccf8:
Merge remote-tracking branch 'remotes/thuth-gitlab/tags/pull-request-2021-07-19' into staging (2021-07-19 11:34:08 +0100)
are available in the git repository at:
https://github.com/quic/qemu tags/pull-hex-20210721
for you to fetch changes up to 953ea3e4b426ee0c8349343c53e3358cfec720f2:
linux-test (tests/tcg/multiarch/linux-test.c) add check (2021-07-21 15:54:28 -0500)
----------------------------------------------------------------
The Hexagon target was silently failing the SIGSEGV test because
the signal handler was not called.
Patch 1/3 fixes the Hexagon target
Patch 2/3 drops include qemu.h from target/hexagon/op_helper.c
Patch 3/3 adds a check that the signal handler is called
----------------------------------------------------------------
Peter Maydell (1):
target/hexagon: Drop include of qemu.h
Taylor Simpson (2):
Hexagon (target/hexagon) remove put_user_*/get_user_*
linux-test (tests/tcg/multiarch/linux-test.c) add check
target/hexagon/op_helper.c | 42 +++++++++++++++++++---------------------
tests/tcg/multiarch/linux-test.c | 8 ++++++++
2 files changed, 28 insertions(+), 22 deletions(-)
^ permalink raw reply [flat|nested] 8+ messages in thread* [PULL 1/3] Hexagon (target/hexagon) remove put_user_*/get_user_* 2021-07-21 21:19 [PULL 0/3] SIGSEGV fixes Taylor Simpson @ 2021-07-21 21:19 ` Taylor Simpson 2021-07-21 21:19 ` [PULL 2/3] target/hexagon: Drop include of qemu.h Taylor Simpson ` (2 subsequent siblings) 3 siblings, 0 replies; 8+ messages in thread From: Taylor Simpson @ 2021-07-21 21:19 UTC (permalink / raw) To: qemu-devel; +Cc: ale, peter.maydell, bcain, richard.henderson, tsimpson, philmd Replace put_user_* with cpu_st*_data_ra Replace get_user_* with cpu_ld*_data_ra Suggested-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Taylor Simpson <tsimpson@quicinc.com> Message-Id: <1626384156-6248-2-git-send-email-tsimpson@quicinc.com> --- target/hexagon/op_helper.c | 39 ++++++++++++++++++--------------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/target/hexagon/op_helper.c b/target/hexagon/op_helper.c index 4595559..a959dba 100644 --- a/target/hexagon/op_helper.c +++ b/target/hexagon/op_helper.c @@ -17,6 +17,7 @@ #include "qemu/osdep.h" #include "qemu.h" +#include "exec/cpu_ldst.h" #include "exec/helper-proto.h" #include "fpu/softfloat.h" #include "cpu.h" @@ -140,22 +141,22 @@ void HELPER(debug_check_store_width)(CPUHexagonState *env, int slot, int check) void HELPER(commit_store)(CPUHexagonState *env, int slot_num) { - switch (env->mem_log_stores[slot_num].width) { + uintptr_t ra = GETPC(); + uint8_t width = env->mem_log_stores[slot_num].width; + target_ulong va = env->mem_log_stores[slot_num].va; + + switch (width) { case 1: - put_user_u8(env->mem_log_stores[slot_num].data32, - env->mem_log_stores[slot_num].va); + cpu_stb_data_ra(env, va, env->mem_log_stores[slot_num].data32, ra); break; case 2: - put_user_u16(env->mem_log_stores[slot_num].data32, - env->mem_log_stores[slot_num].va); + cpu_stw_data_ra(env, va, env->mem_log_stores[slot_num].data32, ra); break; case 4: - put_user_u32(env->mem_log_stores[slot_num].data32, - env->mem_log_stores[slot_num].va); + cpu_stl_data_ra(env, va, env->mem_log_stores[slot_num].data32, ra); break; case 8: - put_user_u64(env->mem_log_stores[slot_num].data64, - env->mem_log_stores[slot_num].va); + cpu_stq_data_ra(env, va, env->mem_log_stores[slot_num].data64, ra); break; default: g_assert_not_reached(); @@ -393,37 +394,33 @@ static void check_noshuf(CPUHexagonState *env, uint32_t slot) static uint8_t mem_load1(CPUHexagonState *env, uint32_t slot, target_ulong vaddr) { - uint8_t retval; + uintptr_t ra = GETPC(); check_noshuf(env, slot); - get_user_u8(retval, vaddr); - return retval; + return cpu_ldub_data_ra(env, vaddr, ra); } static uint16_t mem_load2(CPUHexagonState *env, uint32_t slot, target_ulong vaddr) { - uint16_t retval; + uintptr_t ra = GETPC(); check_noshuf(env, slot); - get_user_u16(retval, vaddr); - return retval; + return cpu_lduw_data_ra(env, vaddr, ra); } static uint32_t mem_load4(CPUHexagonState *env, uint32_t slot, target_ulong vaddr) { - uint32_t retval; + uintptr_t ra = GETPC(); check_noshuf(env, slot); - get_user_u32(retval, vaddr); - return retval; + return cpu_ldl_data_ra(env, vaddr, ra); } static uint64_t mem_load8(CPUHexagonState *env, uint32_t slot, target_ulong vaddr) { - uint64_t retval; + uintptr_t ra = GETPC(); check_noshuf(env, slot); - get_user_u64(retval, vaddr); - return retval; + return cpu_ldq_data_ra(env, vaddr, ra); } /* Floating point */ -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PULL 2/3] target/hexagon: Drop include of qemu.h 2021-07-21 21:19 [PULL 0/3] SIGSEGV fixes Taylor Simpson 2021-07-21 21:19 ` [PULL 1/3] Hexagon (target/hexagon) remove put_user_*/get_user_* Taylor Simpson @ 2021-07-21 21:19 ` Taylor Simpson 2021-07-21 21:19 ` [PULL 3/3] linux-test (tests/tcg/multiarch/linux-test.c) add check Taylor Simpson 2021-07-22 17:31 ` [PULL 0/3] SIGSEGV fixes Peter Maydell 3 siblings, 0 replies; 8+ messages in thread From: Taylor Simpson @ 2021-07-21 21:19 UTC (permalink / raw) To: qemu-devel; +Cc: ale, peter.maydell, bcain, richard.henderson, tsimpson, philmd From: Peter Maydell <peter.maydell@linaro.org> The qemu.h file is a CONFIG_USER_ONLY header; it doesn't appear on the include path for softmmu builds. Currently we include it unconditionally in target/hexagon/op_helper.c. We used to need it for the put_user_*() and get_user_*() functions, but now that we have removed the uses of those from op_helper.c, the only reason it's still there is that we're implicitly relying on it pulling in some other headers. Explicitly include the headers we need for other functions, and drop the include of qemu.h. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-Id: <20210717103017.20491-1-peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Taylor Simpson <tsimpson@quicinc.com> Signed-off-by: Taylor Simpson <tsimpson@quicinc.com> --- target/hexagon/op_helper.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/target/hexagon/op_helper.c b/target/hexagon/op_helper.c index a959dba..61d5cde 100644 --- a/target/hexagon/op_helper.c +++ b/target/hexagon/op_helper.c @@ -16,7 +16,8 @@ */ #include "qemu/osdep.h" -#include "qemu.h" +#include "qemu/log.h" +#include "exec/exec-all.h" #include "exec/cpu_ldst.h" #include "exec/helper-proto.h" #include "fpu/softfloat.h" -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PULL 3/3] linux-test (tests/tcg/multiarch/linux-test.c) add check 2021-07-21 21:19 [PULL 0/3] SIGSEGV fixes Taylor Simpson 2021-07-21 21:19 ` [PULL 1/3] Hexagon (target/hexagon) remove put_user_*/get_user_* Taylor Simpson 2021-07-21 21:19 ` [PULL 2/3] target/hexagon: Drop include of qemu.h Taylor Simpson @ 2021-07-21 21:19 ` Taylor Simpson 2021-07-22 17:31 ` [PULL 0/3] SIGSEGV fixes Peter Maydell 3 siblings, 0 replies; 8+ messages in thread From: Taylor Simpson @ 2021-07-21 21:19 UTC (permalink / raw) To: qemu-devel; +Cc: ale, peter.maydell, bcain, richard.henderson, tsimpson, philmd Add a check that the SIGSEGV handler is called Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Taylor Simpson <tsimpson@quicinc.com> Message-Id: <1626384156-6248-3-git-send-email-tsimpson@quicinc.com> --- tests/tcg/multiarch/linux-test.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/tcg/multiarch/linux-test.c b/tests/tcg/multiarch/linux-test.c index c8c6aed..e2d88f8 100644 --- a/tests/tcg/multiarch/linux-test.c +++ b/tests/tcg/multiarch/linux-test.c @@ -439,10 +439,14 @@ static void sig_alarm(int sig) alarm_count++; } +/* Count the number of times handler is called */ +static int sig_segv_called; + static void sig_segv(int sig, siginfo_t *info, void *puc) { if (sig != SIGSEGV) error("signal"); + sig_segv_called++; longjmp(jmp_env, 1); } @@ -492,6 +496,10 @@ static void test_signal(void) *(volatile uint8_t *)0 = 0; } + if (sig_segv_called == 0) { + error("SIGSEGV handler not called"); + } + act.sa_handler = SIG_DFL; sigemptyset(&act.sa_mask); act.sa_flags = 0; -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PULL 0/3] SIGSEGV fixes 2021-07-21 21:19 [PULL 0/3] SIGSEGV fixes Taylor Simpson ` (2 preceding siblings ...) 2021-07-21 21:19 ` [PULL 3/3] linux-test (tests/tcg/multiarch/linux-test.c) add check Taylor Simpson @ 2021-07-22 17:31 ` Peter Maydell 2021-07-23 18:48 ` Taylor Simpson 3 siblings, 1 reply; 8+ messages in thread From: Peter Maydell @ 2021-07-22 17:31 UTC (permalink / raw) To: Taylor Simpson Cc: Alessandro Di Federico, Philippe Mathieu-Daudé, Richard Henderson, QEMU Developers, Brian Cain On Wed, 21 Jul 2021 at 22:19, Taylor Simpson <tsimpson@quicinc.com> wrote: > > The following changes since commit 7457b407edd6e8555e4b46488aab2f13959fccf8: > > Merge remote-tracking branch 'remotes/thuth-gitlab/tags/pull-request-2021-07-19' into staging (2021-07-19 11:34:08 +0100) > > are available in the git repository at: > > https://github.com/quic/qemu tags/pull-hex-20210721 > > for you to fetch changes up to 953ea3e4b426ee0c8349343c53e3358cfec720f2: > > linux-test (tests/tcg/multiarch/linux-test.c) add check (2021-07-21 15:54:28 -0500) > > ---------------------------------------------------------------- > The Hexagon target was silently failing the SIGSEGV test because > the signal handler was not called. > > Patch 1/3 fixes the Hexagon target > Patch 2/3 drops include qemu.h from target/hexagon/op_helper.c > Patch 3/3 adds a check that the signal handler is called > > ---------------------------------------------------------------- Hi; the check added in patch 2 seems to fire about 50% of the time for qemu-riscv64, causing 'make check-tcg' to fail. $ ./qemu-riscv64 ./tests/tcg/riscv64-linux-user/linux-test $ ./qemu-riscv64 ./tests/tcg/riscv64-linux-user/linux-test $ ./qemu-riscv64 ./tests/tcg/riscv64-linux-user/linux-test $ ./qemu-riscv64 ./tests/tcg/riscv64-linux-user/linux-test /mnt/nvmedisk/linaro/qemu-for-merges/tests/tcg/multiarch/linux-test.c:500: SIGSEGV handler not called $ ./qemu-riscv64 ./tests/tcg/riscv64-linux-user/linux-test $ ./qemu-riscv64 ./tests/tcg/riscv64-linux-user/linux-test /mnt/nvmedisk/linaro/qemu-for-merges/tests/tcg/multiarch/linux-test.c:500: SIGSEGV handler not called $ ./qemu-riscv64 ./tests/tcg/riscv64-linux-user/linux-test /mnt/nvmedisk/linaro/qemu-for-merges/tests/tcg/multiarch/linux-test.c:500: SIGSEGV handler not called thanks -- PMM ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PULL 0/3] SIGSEGV fixes 2021-07-22 17:31 ` [PULL 0/3] SIGSEGV fixes Peter Maydell @ 2021-07-23 18:48 ` Taylor Simpson 2021-07-23 19:10 ` Richard Henderson 0 siblings, 1 reply; 8+ messages in thread From: Taylor Simpson @ 2021-07-23 18:48 UTC (permalink / raw) To: Peter Maydell Cc: Alessandro Di Federico, Brian Cain, alex.bennee, bin.meng, Richard Henderson, QEMU Developers, laurent, palmer, alistair.francis, Philippe Mathieu-Daudé I've added the riscv maintainers and Laurent and Alex to the CC list. Please advise on how to proceed. Is this a known issue with riscv? Should I try to debug the riscv target or remove the change to linux-test.c from the pull request? Thanks, Taylor > -----Original Message----- > From: Peter Maydell <peter.maydell@linaro.org> > Sent: Thursday, July 22, 2021 12:31 PM > To: Taylor Simpson <tsimpson@quicinc.com> > Cc: QEMU Developers <qemu-devel@nongnu.org>; Richard Henderson > <richard.henderson@linaro.org>; Philippe Mathieu-Daudé > <philmd@redhat.com>; Alessandro Di Federico <ale@rev.ng>; Brian Cain > <bcain@quicinc.com> > Subject: Re: [PULL 0/3] SIGSEGV fixes > > On Wed, 21 Jul 2021 at 22:19, Taylor Simpson <tsimpson@quicinc.com> > wrote: > > > > The following changes since commit > 7457b407edd6e8555e4b46488aab2f13959fccf8: > > > > Merge remote-tracking branch > > 'remotes/thuth-gitlab/tags/pull-request-2021-07-19' into staging > > (2021-07-19 11:34:08 +0100) > > > > are available in the git repository at: > > > > https://github.com/quic/qemu tags/pull-hex-20210721 > > > > for you to fetch changes up to > 953ea3e4b426ee0c8349343c53e3358cfec720f2: > > > > linux-test (tests/tcg/multiarch/linux-test.c) add check (2021-07-21 > > 15:54:28 -0500) > > > > ---------------------------------------------------------------- > > The Hexagon target was silently failing the SIGSEGV test because the > > signal handler was not called. > > > > Patch 1/3 fixes the Hexagon target > > Patch 2/3 drops include qemu.h from target/hexagon/op_helper.c Patch > > 3/3 adds a check that the signal handler is called > > > > ---------------------------------------------------------------- > > Hi; the check added in patch 2 seems to fire about 50% of the time for qemu- > riscv64, causing 'make check-tcg' to fail. > > $ ./qemu-riscv64 ./tests/tcg/riscv64-linux-user/linux-test > $ ./qemu-riscv64 ./tests/tcg/riscv64-linux-user/linux-test > $ ./qemu-riscv64 ./tests/tcg/riscv64-linux-user/linux-test > $ ./qemu-riscv64 ./tests/tcg/riscv64-linux-user/linux-test > /mnt/nvmedisk/linaro/qemu-for-merges/tests/tcg/multiarch/linux- > test.c:500: > SIGSEGV handler not called > $ ./qemu-riscv64 ./tests/tcg/riscv64-linux-user/linux-test > $ ./qemu-riscv64 ./tests/tcg/riscv64-linux-user/linux-test > /mnt/nvmedisk/linaro/qemu-for-merges/tests/tcg/multiarch/linux- > test.c:500: > SIGSEGV handler not called > $ ./qemu-riscv64 ./tests/tcg/riscv64-linux-user/linux-test > /mnt/nvmedisk/linaro/qemu-for-merges/tests/tcg/multiarch/linux- > test.c:500: > SIGSEGV handler not called ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PULL 0/3] SIGSEGV fixes 2021-07-23 18:48 ` Taylor Simpson @ 2021-07-23 19:10 ` Richard Henderson 2021-07-23 19:56 ` Philippe Mathieu-Daudé 0 siblings, 1 reply; 8+ messages in thread From: Richard Henderson @ 2021-07-23 19:10 UTC (permalink / raw) To: Taylor Simpson, Peter Maydell Cc: Alessandro Di Federico, Brian Cain, alex.bennee, bin.meng, QEMU Developers, laurent, alistair.francis, palmer, Philippe Mathieu-Daudé On 7/23/21 8:48 AM, Taylor Simpson wrote: > I've added the riscv maintainers and Laurent and Alex to the CC list. > > Please advise on how to proceed. Is this a known issue with riscv? Should I try to debug the riscv target or remove the change to linux-test.c from the pull request? Remove the linux-test.c from the pull request. Someone will need to debug riscv, but I don't think you want that to block the bug fix for hexagon. r~ PS: Please use some subject-line indicator for what component you're patching. "SIGSEGV fixes" is impossibly generic. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PULL 0/3] SIGSEGV fixes 2021-07-23 19:10 ` Richard Henderson @ 2021-07-23 19:56 ` Philippe Mathieu-Daudé 0 siblings, 0 replies; 8+ messages in thread From: Philippe Mathieu-Daudé @ 2021-07-23 19:56 UTC (permalink / raw) To: Richard Henderson, Taylor Simpson, Peter Maydell Cc: Alessandro Di Federico, Brian Cain, bin.meng, QEMU Developers, laurent, alistair.francis, palmer, alex.bennee On 7/23/21 9:10 PM, Richard Henderson wrote: > On 7/23/21 8:48 AM, Taylor Simpson wrote: >> I've added the riscv maintainers and Laurent and Alex to the CC list. >> >> Please advise on how to proceed. Is this a known issue with riscv? >> Should I try to debug the riscv target or remove the change to >> linux-test.c from the pull request? > > Remove the linux-test.c from the pull request. > > Someone will need to debug riscv, but I don't think you want that to > block the bug fix for hexagon. > > > r~ > > > PS: Please use some subject-line indicator for what component you're > patching. "SIGSEGV fixes" is impossibly generic. Although more generic is possible: "fixes" ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-07-23 19:57 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-07-21 21:19 [PULL 0/3] SIGSEGV fixes Taylor Simpson 2021-07-21 21:19 ` [PULL 1/3] Hexagon (target/hexagon) remove put_user_*/get_user_* Taylor Simpson 2021-07-21 21:19 ` [PULL 2/3] target/hexagon: Drop include of qemu.h Taylor Simpson 2021-07-21 21:19 ` [PULL 3/3] linux-test (tests/tcg/multiarch/linux-test.c) add check Taylor Simpson 2021-07-22 17:31 ` [PULL 0/3] SIGSEGV fixes Peter Maydell 2021-07-23 18:48 ` Taylor Simpson 2021-07-23 19:10 ` Richard Henderson 2021-07-23 19:56 ` Philippe Mathieu-Daudé
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.