* Re: [PATCH v2] target/m68k: implement rtr instruction
2021-03-07 18:22 [PATCH v2] target/m68k: implement rtr instruction Laurent Vivier
@ 2021-03-07 18:29 ` BALATON Zoltan
2021-03-07 18:45 ` Richard Henderson
2021-03-07 19:18 ` Mark Cave-Ayland
2 siblings, 0 replies; 6+ messages in thread
From: BALATON Zoltan @ 2021-03-07 18:29 UTC (permalink / raw)
To: Laurent Vivier; +Cc: Mark Cave-Ayland, qemu-devel
On Sun, 7 Mar 2021, Laurent Vivier wrote:
> This is needed to boot MacOS ROM.
>
> Pull the condition code and the program counter from the stack.
>
> Operation:
>
> (SP) -> CCR
> SP + 2 -> SP
> (SP) -> PC
> SP + 4 - > SP
Doesn't look like the commit message was fixed, there's still a space
there.
Regards,
BALATON Zoltan
> This operation is not privileged.
>
> Reported-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>
> Notes:
> v2: update comment
> SP + 4 -> SP
>
> target/m68k/translate.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
> index ac936ebe8f14..e6942955713c 100644
> --- a/target/m68k/translate.c
> +++ b/target/m68k/translate.c
> @@ -2969,6 +2969,20 @@ DISAS_INSN(rtd)
> gen_jmp(s, tmp);
> }
>
> +DISAS_INSN(rtr)
> +{
> + TCGv tmp;
> + TCGv ccr;
> +
> + ccr = gen_load(s, OS_WORD, QREG_SP, 0, IS_USER(s));
> + gen_set_sr(s, ccr, true);
> + tcg_temp_free(ccr);
> + tcg_gen_addi_i32(QREG_SP, QREG_SP, 2);
> + tmp = gen_load(s, OS_LONG, QREG_SP, 0, IS_USER(s));
> + tcg_gen_addi_i32(QREG_SP, QREG_SP, 4);
> + gen_jmp(s, tmp);
> +}
> +
> DISAS_INSN(rts)
> {
> TCGv tmp;
> @@ -6015,6 +6029,7 @@ void register_m68k_insns (CPUM68KState *env)
> BASE(nop, 4e71, ffff);
> INSN(rtd, 4e74, ffff, RTD);
> BASE(rts, 4e75, ffff);
> + INSN(rtr, 4e77, ffff, M68000);
> BASE(jump, 4e80, ffc0);
> BASE(jump, 4ec0, ffc0);
> INSN(addsubq, 5000, f080, M68000);
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] target/m68k: implement rtr instruction
2021-03-07 18:22 [PATCH v2] target/m68k: implement rtr instruction Laurent Vivier
2021-03-07 18:29 ` BALATON Zoltan
@ 2021-03-07 18:45 ` Richard Henderson
2021-03-07 19:08 ` Laurent Vivier
2021-03-07 19:18 ` Mark Cave-Ayland
2 siblings, 1 reply; 6+ messages in thread
From: Richard Henderson @ 2021-03-07 18:45 UTC (permalink / raw)
To: Laurent Vivier, qemu-devel; +Cc: Mark Cave-Ayland
On 3/7/21 10:22 AM, Laurent Vivier wrote:
> + ccr = gen_load(s, OS_WORD, QREG_SP, 0, IS_USER(s));
> + gen_set_sr(s, ccr, true);
> + tcg_temp_free(ccr);
> + tcg_gen_addi_i32(QREG_SP, QREG_SP, 2);
> + tmp = gen_load(s, OS_LONG, QREG_SP, 0, IS_USER(s));
> + tcg_gen_addi_i32(QREG_SP, QREG_SP, 4);
> + gen_jmp(s, tmp);
Both loads should happen (and thus exceptions recognized) before any other
state writeback.
r~
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] target/m68k: implement rtr instruction
2021-03-07 18:45 ` Richard Henderson
@ 2021-03-07 19:08 ` Laurent Vivier
2021-03-08 0:37 ` Richard Henderson
0 siblings, 1 reply; 6+ messages in thread
From: Laurent Vivier @ 2021-03-07 19:08 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: Mark Cave-Ayland
Le 07/03/2021 à 19:45, Richard Henderson a écrit :
> On 3/7/21 10:22 AM, Laurent Vivier wrote:
>> + ccr = gen_load(s, OS_WORD, QREG_SP, 0, IS_USER(s));
>> + gen_set_sr(s, ccr, true);
>> + tcg_temp_free(ccr);
>> + tcg_gen_addi_i32(QREG_SP, QREG_SP, 2);
>> + tmp = gen_load(s, OS_LONG, QREG_SP, 0, IS_USER(s));
>> + tcg_gen_addi_i32(QREG_SP, QREG_SP, 4);
>> + gen_jmp(s, tmp);
>
> Both loads should happen (and thus exceptions recognized) before any other state writeback.
Something like that?
ccr = gen_load(s, OS_WORD, QREG_SP, 0, IS_USER(s));
tcg_gen_addi_i32(QREG_SP, QREG_SP, 2);
tmp = gen_load(s, OS_LONG, QREG_SP, 0, IS_USER(s));
tcg_gen_addi_i32(QREG_SP, QREG_SP, 4);
gen_set_sr(s, ccr, true);
tcg_temp_free(ccr);
gen_jmp(s, tmp);
Thanks,
Laurent
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] target/m68k: implement rtr instruction
2021-03-07 19:08 ` Laurent Vivier
@ 2021-03-08 0:37 ` Richard Henderson
0 siblings, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2021-03-08 0:37 UTC (permalink / raw)
To: Laurent Vivier, qemu-devel; +Cc: Mark Cave-Ayland
On 3/7/21 11:08 AM, Laurent Vivier wrote:
>> Both loads should happen (and thus exceptions recognized) before any other state writeback.
>
> Something like that?
>
> ccr = gen_load(s, OS_WORD, QREG_SP, 0, IS_USER(s));
> tcg_gen_addi_i32(QREG_SP, QREG_SP, 2);
> tmp = gen_load(s, OS_LONG, QREG_SP, 0, IS_USER(s));
> tcg_gen_addi_i32(QREG_SP, QREG_SP, 4);
Can't change SP before final load either.
addr = tcg_temp_new_i32();
tcg_gen_addi_i32(addr, QREG_SP, 2);
tmp = gen_load(s, OS_LONG, addr, 0, IS_USER(s));
tcg_gen_addi_i32(QREG_SP, addr, 4);
tcg_temp_free_i32(addr);
r~
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] target/m68k: implement rtr instruction
2021-03-07 18:22 [PATCH v2] target/m68k: implement rtr instruction Laurent Vivier
2021-03-07 18:29 ` BALATON Zoltan
2021-03-07 18:45 ` Richard Henderson
@ 2021-03-07 19:18 ` Mark Cave-Ayland
2 siblings, 0 replies; 6+ messages in thread
From: Mark Cave-Ayland @ 2021-03-07 19:18 UTC (permalink / raw)
To: Laurent Vivier, qemu-devel
On 07/03/2021 18:22, Laurent Vivier wrote:
> This is needed to boot MacOS ROM.
>
> Pull the condition code and the program counter from the stack.
>
> Operation:
>
> (SP) -> CCR
> SP + 2 -> SP
> (SP) -> PC
> SP + 4 - > SP
>
> This operation is not privileged.
>
> Reported-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>
> Notes:
> v2: update comment
> SP + 4 -> SP
>
> target/m68k/translate.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
> index ac936ebe8f14..e6942955713c 100644
> --- a/target/m68k/translate.c
> +++ b/target/m68k/translate.c
> @@ -2969,6 +2969,20 @@ DISAS_INSN(rtd)
> gen_jmp(s, tmp);
> }
>
> +DISAS_INSN(rtr)
> +{
> + TCGv tmp;
> + TCGv ccr;
> +
> + ccr = gen_load(s, OS_WORD, QREG_SP, 0, IS_USER(s));
> + gen_set_sr(s, ccr, true);
> + tcg_temp_free(ccr);
> + tcg_gen_addi_i32(QREG_SP, QREG_SP, 2);
> + tmp = gen_load(s, OS_LONG, QREG_SP, 0, IS_USER(s));
> + tcg_gen_addi_i32(QREG_SP, QREG_SP, 4);
> + gen_jmp(s, tmp);
> +}
> +
> DISAS_INSN(rts)
> {
> TCGv tmp;
> @@ -6015,6 +6029,7 @@ void register_m68k_insns (CPUM68KState *env)
> BASE(nop, 4e71, ffff);
> INSN(rtd, 4e74, ffff, RTD);
> BASE(rts, 4e75, ffff);
> + INSN(rtr, 4e77, ffff, M68000);
> BASE(jump, 4e80, ffc0);
> BASE(jump, 4ec0, ffc0);
> INSN(addsubq, 5000, f080, M68000);
Solves the issue for me, so:
Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
I also have a couple of other target/m68k patches from my experimental branch which
are probably worth adding to an m68k PR. Let me go dig them out...
ATB,
Mark.
^ permalink raw reply [flat|nested] 6+ messages in thread