Re: [Qemu-devel] [RFC PATCH] exec-all: Translate TCI return addresses backwards too

[Richard Henderson <rth@twiddle.net>]

On 08/15/2015 11:21 PM, Peter Crosthwaite wrote:
> I'm trying to debug why TCI doesn't work for the Microblaze example at
> http://wiki.qemu.org/Testing. My debug led me to the return addresses
> for exceptions being too far forward and this adjustment looked
> related so I gave this change a shot, and it works!
> 
> I did some further-afield image testing, and it turns out that this
> patch fixes TCI for at least several arches. Here are my test results:
> 
> ARCH        TCI-WITH-PATCH      TCI-WITHOUT-PATCH       HOST-TCG
> microblaze  Y                   N (blank)               Y
> cris        Y                   N (K panic)             Y
> arm         Y                   N (halfway)             Y
> aarch64     N (abort)           N (abort)               Y
> SH4         Y                   N (K panic)             Y
> 
> The patch gets you to a linux login prompt on MB,CRIS,ARM and SH4
> whereas before the patch various crashes occured.
> 
> AArch64 has what appears to be a separate issue with TCI. I am use this
> for testing:
> 
> http://www.bennee.com/~alex/blog/2014/05/09/running-linux-in-qemus-aarch64-system-emulation-mode/
> 
> I don't understand this TCI code fully yet, so I doubt my change is
> correct, but RFCing incase someone has some theories to help me debug,
> or justify the change.
> 
> My debug strategy is to run QEMU both with and without TCI and pass:
> 
> -d op,exec,in_asm,mmu
> 
> then filter logs with:
> 
> s/\(exit_tb\).*$/\1/
> s/^\(Trace \)[0-9a-fx]*/\1/
> 
> The two logs then give you a nice diff between to TCI and host-TCG
> run. The first diff I saw was a bad exception return address in the TCI
> case:
> 
>  IN: PC=20
>  rmsr=4300 resr=412 rear=c7fffffc debug=0 imm=fffffffc iflags=2100 fsr=0
>  btaken=1 btarget=c0291d28 mode=kernel(saved=kernel) eip=512 ie=0
>  r00=00000000 r01=c026ff90 r02=c026de90 r03=deadbeef
>  r04=c7ffe38c r05=00001c74 r06=97ffe38c r07=00001c74
>  r08=00000000 r09=91fe0000 r10=00000010 r11=c1c8bd94
>  r12=c1c8bdc0 r13=c027f680 r14=00000000 r15=c0291d20
> -r16=00000000 r17=c0291d3c r18=07ffe38c r19=00000000
> +r16=00000000 r17=c0291d44 r18=07ffe38c r19=00000000
>  r20=00000000 r21=00000000 r22=00001c70 r23=c026ffac
>  r24=00000000 r25=00000000 r26=00000000 r27=00000000
>  r28=00000000 r29=01000000 r30=00000380 r31=c02722f8
> 
> In Microblaze, PC=20 is the MMU fault exception vector and R17 is
> the exception return address. The faulting instruction is at
> 0xc0291d2c as shown by the host-TCG run (-) but TCI has it at
> 0xc0291d44 (+). It is a store:
> 
>  0xc0291d38:  andi  r22, r22, -4
>  0xc0291d3c:  sw    r3, r4, r22
>  0xc0291d40:  imm   -15928
>  0xc0291d44:  lwi   r8, r0, -10452
> 
> Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> ---
>  include/exec/exec-all.h | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index a6fce04..31c2405 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -308,11 +308,7 @@ extern uintptr_t tci_tb_ptr;
>     to indicate the compressed mode; subtracting two works around that.  It
>     is also the case that there are no host isas that contain a call insn
>     smaller than 4 bytes, so we don't worry about special-casing this.  */
> -#if defined(CONFIG_TCG_INTERPRETER)
> -# define GETPC_ADJ   0
> -#else
>  # define GETPC_ADJ   2
> -#endif
>  
>  #define GETPC()  (GETRA() - GETPC_ADJ)
>  
> 

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~