* [PATCH v2 0/2] target/m68k: Reject immediate as destination @ 2023-03-09 20:16 Richard Henderson 2023-03-09 20:16 ` [PATCH v2 1/2] target/m68k: Reject immediate as destination in gen_ea_mode Richard Henderson 2023-03-09 20:16 ` [PATCH v2 2/2] linux-user/m68k: Handle EXCP_ADDRESS in cpu_loop Richard Henderson 0 siblings, 2 replies; 7+ messages in thread From: Richard Henderson @ 2023-03-09 20:16 UTC (permalink / raw) To: qemu-devel; +Cc: laurent The translate.c fix is extracted from a larger patch set. I then add a test case and fix cpu_loop to raise the proper signal. r~ Richard Henderson (2): target/m68k: Reject immediate as destination in gen_ea_mode linux-user/m68k: Handle EXCP_ADDRESS in cpu_loop linux-user/m68k/cpu_loop.c | 5 ++++- target/m68k/translate.c | 4 ++++ tests/tcg/m68k/excp-address.c | 32 ++++++++++++++++++++++++++++++++ tests/tcg/m68k/Makefile.target | 1 + 4 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 tests/tcg/m68k/excp-address.c -- 2.34.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] target/m68k: Reject immediate as destination in gen_ea_mode 2023-03-09 20:16 [PATCH v2 0/2] target/m68k: Reject immediate as destination Richard Henderson @ 2023-03-09 20:16 ` Richard Henderson 2023-03-10 10:03 ` Laurent Vivier 2023-03-15 16:41 ` Laurent Vivier 2023-03-09 20:16 ` [PATCH v2 2/2] linux-user/m68k: Handle EXCP_ADDRESS in cpu_loop Richard Henderson 1 sibling, 2 replies; 7+ messages in thread From: Richard Henderson @ 2023-03-09 20:16 UTC (permalink / raw) To: qemu-devel; +Cc: laurent In theory this should never happen, as all such instructions are illegal. This is checked in e.g. gen_lea_mode and gen_ea_mode_fp but not here. In case something higher up isn't checking modes properly, return NULL_QREG. This will result in an illegal instruction exception being raised. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/m68k/translate.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/target/m68k/translate.c b/target/m68k/translate.c index 422f4652f1..e16c608ef8 100644 --- a/target/m68k/translate.c +++ b/target/m68k/translate.c @@ -894,6 +894,10 @@ static TCGv gen_ea_mode(CPUM68KState *env, DisasContext *s, int mode, int reg0, case 3: /* pc index+displacement. */ goto do_indirect; case 4: /* Immediate. */ + /* Should never be used for an output or RMW input. */ + if (what == EA_STORE || addrp) { + return NULL_QREG; + } /* Sign extend values for consistency. */ switch (opsize) { case OS_BYTE: -- 2.34.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] target/m68k: Reject immediate as destination in gen_ea_mode 2023-03-09 20:16 ` [PATCH v2 1/2] target/m68k: Reject immediate as destination in gen_ea_mode Richard Henderson @ 2023-03-10 10:03 ` Laurent Vivier 2023-03-15 16:41 ` Laurent Vivier 1 sibling, 0 replies; 7+ messages in thread From: Laurent Vivier @ 2023-03-10 10:03 UTC (permalink / raw) To: Richard Henderson, qemu-devel Le 09/03/2023 à 21:16, Richard Henderson a écrit : > In theory this should never happen, as all such instructions > are illegal. This is checked in e.g. gen_lea_mode and > gen_ea_mode_fp but not here. In case something higher up > isn't checking modes properly, return NULL_QREG. This will > result in an illegal instruction exception being raised. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/m68k/translate.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/target/m68k/translate.c b/target/m68k/translate.c > index 422f4652f1..e16c608ef8 100644 > --- a/target/m68k/translate.c > +++ b/target/m68k/translate.c > @@ -894,6 +894,10 @@ static TCGv gen_ea_mode(CPUM68KState *env, DisasContext *s, int mode, int reg0, > case 3: /* pc index+displacement. */ > goto do_indirect; > case 4: /* Immediate. */ > + /* Should never be used for an output or RMW input. */ > + if (what == EA_STORE || addrp) { > + return NULL_QREG; > + } > /* Sign extend values for consistency. */ > switch (opsize) { > case OS_BYTE: Reviewed-by: Laurent Vivier <laurent@vivier.eu> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] target/m68k: Reject immediate as destination in gen_ea_mode 2023-03-09 20:16 ` [PATCH v2 1/2] target/m68k: Reject immediate as destination in gen_ea_mode Richard Henderson 2023-03-10 10:03 ` Laurent Vivier @ 2023-03-15 16:41 ` Laurent Vivier 1 sibling, 0 replies; 7+ messages in thread From: Laurent Vivier @ 2023-03-15 16:41 UTC (permalink / raw) To: Richard Henderson, qemu-devel Le 09/03/2023 à 21:16, Richard Henderson a écrit : > In theory this should never happen, as all such instructions > are illegal. This is checked in e.g. gen_lea_mode and > gen_ea_mode_fp but not here. In case something higher up > isn't checking modes properly, return NULL_QREG. This will > result in an illegal instruction exception being raised. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/m68k/translate.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/target/m68k/translate.c b/target/m68k/translate.c > index 422f4652f1..e16c608ef8 100644 > --- a/target/m68k/translate.c > +++ b/target/m68k/translate.c > @@ -894,6 +894,10 @@ static TCGv gen_ea_mode(CPUM68KState *env, DisasContext *s, int mode, int reg0, > case 3: /* pc index+displacement. */ > goto do_indirect; > case 4: /* Immediate. */ > + /* Should never be used for an output or RMW input. */ > + if (what == EA_STORE || addrp) { > + return NULL_QREG; > + } > /* Sign extend values for consistency. */ > switch (opsize) { > case OS_BYTE: Reviewed-by: Laurent Vivier <laurent@vivier.eu> ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] linux-user/m68k: Handle EXCP_ADDRESS in cpu_loop 2023-03-09 20:16 [PATCH v2 0/2] target/m68k: Reject immediate as destination Richard Henderson 2023-03-09 20:16 ` [PATCH v2 1/2] target/m68k: Reject immediate as destination in gen_ea_mode Richard Henderson @ 2023-03-09 20:16 ` Richard Henderson 2023-03-15 18:16 ` Laurent Vivier 1 sibling, 1 reply; 7+ messages in thread From: Richard Henderson @ 2023-03-09 20:16 UTC (permalink / raw) To: qemu-devel; +Cc: laurent This exception can be raised by illegal instructions. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- linux-user/m68k/cpu_loop.c | 5 ++++- tests/tcg/m68k/excp-address.c | 32 ++++++++++++++++++++++++++++++++ tests/tcg/m68k/Makefile.target | 1 + 3 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 tests/tcg/m68k/excp-address.c diff --git a/linux-user/m68k/cpu_loop.c b/linux-user/m68k/cpu_loop.c index caead1cb74..84b5d290c0 100644 --- a/linux-user/m68k/cpu_loop.c +++ b/linux-user/m68k/cpu_loop.c @@ -35,7 +35,10 @@ void cpu_loop(CPUM68KState *env) cpu_exec_end(cs); process_queued_cpu_work(cs); - switch(trapnr) { + switch (trapnr) { + case EXCP_ADDRESS: + force_sig_fault(TARGET_SIGBUS, TARGET_BUS_ADRALN, env->pc); + break; case EXCP_ILLEGAL: case EXCP_LINEA: case EXCP_LINEF: diff --git a/tests/tcg/m68k/excp-address.c b/tests/tcg/m68k/excp-address.c new file mode 100644 index 0000000000..1dbdddef58 --- /dev/null +++ b/tests/tcg/m68k/excp-address.c @@ -0,0 +1,32 @@ +/* + * Test m68k address exception + */ + +#define _GNU_SOURCE 1 +#include <signal.h> +#include <stdlib.h> + +static void sig_handler(int sig, siginfo_t *si, void *puc) +{ + exit(0); +} + +int main(int argc, char **argv) +{ + struct sigaction act = { + .sa_sigaction = sig_handler, + .sa_flags = SA_SIGINFO + }; + + sigaction(SIGBUS, &act, NULL); + + /* + * addl %d0,#0 -- with immediate as destination is illegal. + * Buggy qemu interpreted the insn as 5 words: 2 for immediate source + * and another 2 for immediate destination. Provide all that padding + * so that abort gets called. + */ + asm volatile(".word 0xd1bc,0,0,0,0"); + + abort(); +} diff --git a/tests/tcg/m68k/Makefile.target b/tests/tcg/m68k/Makefile.target index 1163c7ef03..d3b0bc48a3 100644 --- a/tests/tcg/m68k/Makefile.target +++ b/tests/tcg/m68k/Makefile.target @@ -4,6 +4,7 @@ # VPATH += $(SRC_PATH)/tests/tcg/m68k +TESTS += excp-address TESTS += trap # On m68k Linux supports 4k and 8k pages (but 8k is currently broken) -- 2.34.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] linux-user/m68k: Handle EXCP_ADDRESS in cpu_loop 2023-03-09 20:16 ` [PATCH v2 2/2] linux-user/m68k: Handle EXCP_ADDRESS in cpu_loop Richard Henderson @ 2023-03-15 18:16 ` Laurent Vivier 2023-03-16 14:23 ` Richard Henderson 0 siblings, 1 reply; 7+ messages in thread From: Laurent Vivier @ 2023-03-15 18:16 UTC (permalink / raw) To: Richard Henderson, qemu-devel Le 09/03/2023 à 21:16, Richard Henderson a écrit : > This exception can be raised by illegal instructions. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > linux-user/m68k/cpu_loop.c | 5 ++++- > tests/tcg/m68k/excp-address.c | 32 ++++++++++++++++++++++++++++++++ > tests/tcg/m68k/Makefile.target | 1 + > 3 files changed, 37 insertions(+), 1 deletion(-) > create mode 100644 tests/tcg/m68k/excp-address.c > > diff --git a/linux-user/m68k/cpu_loop.c b/linux-user/m68k/cpu_loop.c > index caead1cb74..84b5d290c0 100644 > --- a/linux-user/m68k/cpu_loop.c > +++ b/linux-user/m68k/cpu_loop.c > @@ -35,7 +35,10 @@ void cpu_loop(CPUM68KState *env) > cpu_exec_end(cs); > process_queued_cpu_work(cs); > > - switch(trapnr) { > + switch (trapnr) { > + case EXCP_ADDRESS: > + force_sig_fault(TARGET_SIGBUS, TARGET_BUS_ADRALN, env->pc); > + break; > case EXCP_ILLEGAL: > case EXCP_LINEA: > case EXCP_LINEF: > diff --git a/tests/tcg/m68k/excp-address.c b/tests/tcg/m68k/excp-address.c > new file mode 100644 > index 0000000000..1dbdddef58 > --- /dev/null > +++ b/tests/tcg/m68k/excp-address.c > @@ -0,0 +1,32 @@ > +/* > + * Test m68k address exception > + */ > + > +#define _GNU_SOURCE 1 > +#include <signal.h> > +#include <stdlib.h> > + > +static void sig_handler(int sig, siginfo_t *si, void *puc) > +{ > + exit(0); > +} > + > +int main(int argc, char **argv) > +{ > + struct sigaction act = { > + .sa_sigaction = sig_handler, > + .sa_flags = SA_SIGINFO > + }; > + > + sigaction(SIGBUS, &act, NULL); > + > + /* > + * addl %d0,#0 -- with immediate as destination is illegal. > + * Buggy qemu interpreted the insn as 5 words: 2 for immediate source > + * and another 2 for immediate destination. Provide all that padding > + * so that abort gets called. > + */ > + asm volatile(".word 0xd1bc,0,0,0,0"); > + > + abort(); > +} > diff --git a/tests/tcg/m68k/Makefile.target b/tests/tcg/m68k/Makefile.target > index 1163c7ef03..d3b0bc48a3 100644 > --- a/tests/tcg/m68k/Makefile.target > +++ b/tests/tcg/m68k/Makefile.target > @@ -4,6 +4,7 @@ > # > > VPATH += $(SRC_PATH)/tests/tcg/m68k > +TESTS += excp-address > TESTS += trap > > # On m68k Linux supports 4k and 8k pages (but 8k is currently broken) Reviewed-by: Laurent Vivier <laurent@vivier.eu> I've tested tests/tcg/m68k/excp-address.c on a real hardware (Q800), and the result differs from the one from QEMU: On Q800 (etch m68k, kernel 5.14.0): $ ./excp-address Illegal instruction $ strace ./excp-address ... rt_sigaction(SIGBUS, {0x80000478, [], SA_SIGINFO}, NULL, 8) = 0 --- SIGILL (Illegal instruction) @ 0 (0) --- +++ killed by SIGILL +++ With QEMU, we have: # QEMU_STRACE= ./excp-address ... 677354 rt_sigaction(SIGBUS,0x40800454,NULL) = 0 --- SIGBUS {si_signo=SIGBUS, si_code=1, si_addr=0x800004ce} --- 677354 exit_group(0) Thanks, Laurent ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] linux-user/m68k: Handle EXCP_ADDRESS in cpu_loop 2023-03-15 18:16 ` Laurent Vivier @ 2023-03-16 14:23 ` Richard Henderson 0 siblings, 0 replies; 7+ messages in thread From: Richard Henderson @ 2023-03-16 14:23 UTC (permalink / raw) To: Laurent Vivier, qemu-devel On 3/15/23 11:16, Laurent Vivier wrote: > I've tested tests/tcg/m68k/excp-address.c on a real hardware (Q800), and the result > differs from the one from QEMU: > > On Q800 (etch m68k, kernel 5.14.0): > > $ ./excp-address > Illegal instruction > $ strace ./excp-address > ... > rt_sigaction(SIGBUS, {0x80000478, [], SA_SIGINFO}, NULL, 8) = 0 > --- SIGILL (Illegal instruction) @ 0 (0) --- > +++ killed by SIGILL +++ Ok, that suggests that we need to do something different in the translator in patch 1. I'll give it some thought. r~ ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-03-16 14:24 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-03-09 20:16 [PATCH v2 0/2] target/m68k: Reject immediate as destination Richard Henderson 2023-03-09 20:16 ` [PATCH v2 1/2] target/m68k: Reject immediate as destination in gen_ea_mode Richard Henderson 2023-03-10 10:03 ` Laurent Vivier 2023-03-15 16:41 ` Laurent Vivier 2023-03-09 20:16 ` [PATCH v2 2/2] linux-user/m68k: Handle EXCP_ADDRESS in cpu_loop Richard Henderson 2023-03-15 18:16 ` Laurent Vivier 2023-03-16 14:23 ` Richard Henderson
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.