[Qemu-devel] sh4 linux user emulation on an x86_64 system

[Qemu-devel] sh4 linux user emulation on an x86_64 system

[michael]

Hi all,

I have some trouble using the qemu emulator on the x86_64 host system. 
The system emulate
an qemu-sh4 machine. I try to fix it without success:

if I run:

qemu-sh4-32 I don't have problems. I have the log and the program run ok.

If I recompile for the qemu-sh4-64 the system crash. I find the reason 
of the crash, but
the problem appears early in the generated code. The effect is the env 
variabile
env->pr is filled with an an invalid value
then the pr is taken by the delayed_pc and finally by the pc.

The %r14 register point to the CPUState structure

OUT: [size=242]
0x6018c9e0:  mov    0x28(%r14),%edi
0x6018c9e4:  mov    0x40(%r14),%esi
0x6018c9e8:  add    $0x60,%esi
0x6018c9eb:  mov    %rsi,%rdx
0x6018c9ee:  mov    %edx,0x44(%r14)
0x6018c9f2:  mov    %esi,0x40(%r14)
0x6018c9f6:  mov    %edi,0x8(%r14)
0x6018c9fa:  movslq (%rdx),%rdi
0x6018c9fd:  mov    0x44(%r14),%esi
0x6018ca01:  add    $0x4,%esi
0x6018ca04:  mov    %rsi,%rdx
0x6018ca07:  mov    %esi,0x44(%r14)
0x6018ca0b:  mov    %edi,0x114(%r14) <--- the %edi is invalid and here I 
set the env->pr

the pr is then copied to the delayed_pc and then in the pc.
variabile

Then the system crash with an access violation. I set a break condition 
and a watch point to
trace down the problem and try to compare the i386 code with the x86_64 
code. It appears that the
problem is before.
It is a problem if the regs_to_env and env_to_regs are not implemented 
during a longjmp and setjmp.

Regards Michael

Re: [Qemu-devel] sh4 linux user emulation on an x86_64 system

[Vince Weaver]

Hello

>On Wed, 15 Oct 2008, michael wrote:
> qemu-sh4-32 I don't have problems. I have the log and the program run ok.
>
> If I recompile for the qemu-sh4-64 the system crash. I find the reason of the

I have nothing useful to add, except that I have been trying to track down 
a very similar problem with qemu-cris.  It works fine on x86, but gets 
stuck in x86-64 due to the PC getting mis-set.  The problem seems possibly 
related to the exception handling code which uses longjmp.  Maybe the 
problems are related.

Vince

Re: [Qemu-devel] sh4 linux user emulation on an x86_64 system

[michael]

Vince Weaver wrote:
> Hello
>
>> On Wed, 15 Oct 2008, michael wrote:
>> qemu-sh4-32 I don't have problems. I have the log and the program run 
>> ok.
>>
>> If I recompile for the qemu-sh4-64 the system crash. I find the 
>> reason of the
>
> I have nothing useful to add, except that I have been trying to track 
> down a very similar problem with qemu-cris.  It works fine on x86, but 
> gets stuck in x86-64 due to the PC getting mis-set.  The problem seems 
> possibly related to the exception handling code which uses longjmp.  
> Maybe the problems are related.
>
> Vince
>
In the user-mode I see that the expection handling just set the 
expeciont reason to -1. I will try to
track down the problem. I don't find how the tcg avoid the use of low 
part of a register that is used
used for special purpose. The x86_64 has the notion only of the 64bit 
register but a 64bit register
can be used only in the low part. Another thing is the define:

#define CODE_GEN_ALIGN           16 /* must be >= of the size of a 
icache line */

In the x86_64 is not 64?

Regards Michael



>
>

[Qemu-devel] [PATCH] SH: Fix struct target_stat64 for 64bit host

[takasi-y]

Hi,

> I have some trouble using the qemu emulator on the x86_64 host system. 
> The system emulate
> an qemu-sh4 machine. I try to fix it without success:

I found stat64 has damaged return address on stack because of bloated
 struct stat64.

On sh4 target, struct target_stat64 has "unsigned int" as its member.
It may results unwanted struct layout on 64bit host. Switched to abi_ulong.
We also have to use packed attribute for sh4, because the struct has
unaligned 64bit member st_size.

 Signed-off-by: Takashi YOSHII <takasi-y@ops.dti.ne.jp>

---
 linux-user/syscall_defs.h |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 5a58010..01e48fd 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -1475,15 +1475,15 @@ struct target_stat {
 /* This matches struct stat64 in glibc2.1, hence the absolutely
  * insane amounts of padding around dev_t's.
  */
-struct target_stat64 {
+struct __attribute__((__packed__)) target_stat64 {
 	unsigned long long	st_dev;
 	unsigned char	__pad0[4];
 
 #define TARGET_STAT64_HAS_BROKEN_ST_INO	1
 	abi_ulong	__st_ino;
 
-	unsigned int	st_mode;
-	unsigned int	st_nlink;
+	abi_ulong	st_mode;
+	abi_ulong	st_nlink;
 
 	abi_ulong	st_uid;
 	abi_ulong	st_gid;
-- 
1.5.4.3

[Qemu-devel] Re: [PATCH] SH: Fix struct target_stat64 for 64bit host

[michael]

Hi,

takasi-y@ops.dti.ne.jp wrote:
> Hi,
>
>   
>> I have some trouble using the qemu emulator on the x86_64 host system. 
>> The system emulate
>> an qemu-sh4 machine. I try to fix it without success:
>>     
>
> I found stat64 has damaged return address on stack because of bloated
>  struct stat64.
>
> On sh4 target, struct target_stat64 has "unsigned int" as its member.
> It may results unwanted struct layout on 64bit host. Switched to abi_ulong.
> We also have to use packed attribute for sh4, because the struct has
> unaligned 64bit member st_size.
>
>  Signed-off-by: Takashi YOSHII <takasi-y@ops.dti.ne.jp>
>
> ---
>  linux-user/syscall_defs.h |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 5a58010..01e48fd 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -1475,15 +1475,15 @@ struct target_stat {
>  /* This matches struct stat64 in glibc2.1, hence the absolutely
>   * insane amounts of padding around dev_t's.
>   */
> -struct target_stat64 {
> +struct __attribute__((__packed__)) target_stat64 {
>  	unsigned long long	st_dev;
>  	unsigned char	__pad0[4];
>  
>  #define TARGET_STAT64_HAS_BROKEN_ST_INO	1
>  	abi_ulong	__st_ino;
>  
> -	unsigned int	st_mode;
> -	unsigned int	st_nlink;
> +	abi_ulong	st_mode;
> +	abi_ulong	st_nlink;
>  
>  	abi_ulong	st_uid;
>  	abi_ulong	st_gid;
>   
The patch fix the problem for me.

Regards Michael

Re: [Qemu-devel] [PATCH] SH: Fix struct target_stat64 for 64bit host

[michael]

takasi-y@ops.dti.ne.jp wrote:
> Hi,
>
>   
>> I have some trouble using the qemu emulator on the x86_64 host system. 
>> The system emulate
>> an qemu-sh4 machine. I try to fix it without success:
>>     
>
> I found stat64 has damaged return address on stack because of bloated
>  struct stat64.
>
> On sh4 target, struct target_stat64 has "unsigned int" as its member.
> It may results unwanted struct layout on 64bit host. Switched to abi_ulong.
> We also have to use packed attribute for sh4, because the struct has
> unaligned 64bit member st_size.
>
>  Signed-off-by: Takashi YOSHII <takasi-y@ops.dti.ne.jp>
>
> ---
>  linux-user/syscall_defs.h |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 5a58010..01e48fd 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -1475,15 +1475,15 @@ struct target_stat {
>  /* This matches struct stat64 in glibc2.1, hence the absolutely
>   * insane amounts of padding around dev_t's.
>   */
> -struct target_stat64 {
> +struct __attribute__((__packed__)) target_stat64 {
>  	unsigned long long	st_dev;
>   
Define it packets assecure me that each type occopies the exact dimension
>  	unsigned char	__pad0[4];
>  
>  #define TARGET_STAT64_HAS_BROKEN_ST_INO	1
>  	abi_ulong	__st_ino;
>  
> -	unsigned int	st_mode;
> -	unsigned int	st_nlink;
> +	abi_ulong	st_mode;
> +	abi_ulong	st_nlink;
>   
The size of an unsigned int is 4 byte and is ok? Why this change?
>  
>  	abi_ulong	st_uid;
>  	abi_ulong	st_gid;
>   

Regards Michael

Re: [Qemu-devel] [PATCH] SH: Fix struct target_stat64 for 64bit host

[michael]

[-- Attachment #1: Type: text/plain, Size: 1471 bytes --]

hi,

takasi-y@ops.dti.ne.jp wrote:
> Hi,
>
>   
>> I have some trouble using the qemu emulator on the x86_64 host system. 
>> The system emulate
>> an qemu-sh4 machine. I try to fix it without success:
>>     
>
> I found stat64 has damaged return address on stack because of bloated
>  struct stat64.
>
> On sh4 target, struct target_stat64 has "unsigned int" as its member.
> It may results unwanted struct layout on 64bit host. Switched to abi_ulong.
> We also have to use packed attribute for sh4, because the struct has
> unaligned 64bit member st_size.
>
>  Signed-off-by: Takashi YOSHII <takasi-y@ops.dti.ne.jp>
>
> ---
>  linux-user/syscall_defs.h |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 5a58010..01e48fd 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -1475,15 +1475,15 @@ struct target_stat {
>  /* This matches struct stat64 in glibc2.1, hence the absolutely
>   * insane amounts of padding around dev_t's.
>   */
> -struct target_stat64 {
> +struct __attribute__((__packed__)) target_stat64 {
>  	unsigned long long	st_dev;
>  	unsigned char	__pad0[4];
>  
>  #define TARGET_STAT64_HAS_BROKEN_ST_INO	1
>  	abi_ulong	__st_ino;
>  
> -	unsigned int	st_mode;
> -	unsigned int	st_nlink;
> +	abi_ulong	st_mode;
> +	abi_ulong	st_nlink;
>  
>  	abi_ulong	st_uid;
>  	abi_ulong	st_gid;
>   
What would you think about this one?



[-- Attachment #2: stat64_patch --]
[-- Type: text/plain, Size: 390 bytes --]

Index: linux-user/syscall_defs.h
===================================================================
--- linux-user/syscall_defs.h	(revision 5499)
+++ linux-user/syscall_defs.h	(working copy)
@@ -1506,7 +1506,7 @@
 	abi_ulong	target_st_ctime_nsec;
 
 	unsigned long long	st_ino;
-};
+}  __attribute__((packed));
 
 #elif defined(TARGET_I386) && !defined(TARGET_ABI32)
 struct target_stat {

[Qemu-devel] Re: [PATCH] SH: Fix struct target_stat64 for 64bit host

[takasi-y]

Hi,

> The size of an unsigned int is 4 byte and is ok? Why this change?
Oh, you are right. I seems to be confused during debugging.
So, one line modification adding "packed" attribute like your patch
is the answer. I confirmed the issue fixed with your patch.

Thank you, Michael.
/yoshii @ lunch break at office :)

[Qemu-devel] [PATCH] take2. SH: Fix struct target_stat64 for 64bit host

[takasi-y]

I send fixed one, because I found (perhaps) I should send fixed patch with
 signed-off line to be committed to the repository....

This one simply add packed attribute to the struct, as michael pointed out.

Signed-off-by: Takashi YOSHII <takasi-y@ops.dti.ne.jp>
---
 linux-user/syscall_defs.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 5a58010..8131905 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -1475,7 +1475,7 @@ struct target_stat {
 /* This matches struct stat64 in glibc2.1, hence the absolutely
  * insane amounts of padding around dev_t's.
  */
-struct target_stat64 {
+struct __attribute__((__packed__)) target_stat64 {
 	unsigned long long	st_dev;
 	unsigned char	__pad0[4];
 
-- 
1.5.4.3

Re: [Qemu-devel] [PATCH] take2. SH: Fix struct target_stat64 for 64bit host

[Aurelien Jarno]

On Mon, Oct 27, 2008 at 12:37:39AM +0900, takasi-y@ops.dti.ne.jp wrote:
> I send fixed one, because I found (perhaps) I should send fixed patch with
>  signed-off line to be committed to the repository....
> 
> This one simply add packed attribute to the struct, as michael pointed out.
> 
> Signed-off-by: Takashi YOSHII <takasi-y@ops.dti.ne.jp>

Thanks, applied.

> ---
>  linux-user/syscall_defs.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 5a58010..8131905 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -1475,7 +1475,7 @@ struct target_stat {
>  /* This matches struct stat64 in glibc2.1, hence the absolutely
>   * insane amounts of padding around dev_t's.
>   */
> -struct target_stat64 {
> +struct __attribute__((__packed__)) target_stat64 {
>  	unsigned long long	st_dev;
>  	unsigned char	__pad0[4];
>  
> -- 
> 1.5.4.3
> 
> 
> 
> 

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net