Re: [Qemu-devel] [PATCH] fcntl64 fix
On [Tue, 20.03.2007 21:47], Thiemo Seufer wrote:
Stuart Anderson wrote:
[snip]
--- linux-user/syscall_defs.h.orig 2007-02-23 15:44:47.0 -0500
+++ linux-user/syscall_defs.h 2007-02-23 15:44:26.0 -0500
@@ -1414,7 +1414,9 @@
struct target_eabi_flock64 {
short l_type;
short l_whence;
+#if HOST_LONG_BITS == 32
int __pad;
+#endif
Still, this part makes no sense to me since it is in a packed struct.
Can you explain why this works better for you?
Primarily, I also thought that problem is in padding, because, without the
patch F_GETLK, on 32-bit target recognises as F_GETLK64 on 64-bit host. It's
happen because on 64-bit host and 32-bit target F_GETLK == F_GETLK64 ==
TARGET_F_GETLK. So if you're using qemu-arm on 64-bit host and target eabi
program calls fcntl(fd,F_GETLK,...), target_eabi_flock64 will be used by
mistake. Disabling padding can helps in some trivial cases to pass
pseudo-correct args to fcntl.
Stuart, am I right?
--
Regards, Kirill A. Shutemov
+ Belarus, Minsk
+ Velesys LLC, http://www.velesys.com/
+ ALT Linux Team, http://www.altlinux.com/
signature.asc
Description: Digital signature
Re: [Qemu-devel] [PATCH] fcntl64 fix
On [Tue, 20.03.2007 19:05], Stuart Anderson wrote: Now that the dust has settled, I see where the change is probably a no-op anyway. A quick little test program indicates that on x86_64, l_start will have an offset of 8 wether the structure is packed or not, and wether the __pad member is present or not. The unsigned long long is always going to be aligned to a 8 byte boundary. It architecture specific I think. -- Regards, Kirill A. Shutemov + Belarus, Minsk + Velesys LLC, http://www.velesys.com/ + ALT Linux Team, http://www.altlinux.com/ signature.asc Description: Digital signature
Re: [Qemu-devel] [PATCH] fcntl64 fix
On [Tue, 20.03.2007 13:42], Kirill A. Shutemov wrote:
Yep. You're right. Fixed patch in the attachment.
Patch that fixes dummy bug in the attachment. Sorry.
--
Regards, Kirill A. Shutemov
+ Belarus, Minsk
+ Velesys LLC, http://www.velesys.com/
+ ALT Linux Team, http://www.altlinux.com/
diff --git a/qemu/linux-user/syscall.c b/qemu/linux-user/syscall.c
index 5ff364d..a23f3f7 100644
--- a/qemu/linux-user/syscall.c
+++ b/qemu/linux-user/syscall.c
@@ -3882,10 +3882,13 @@ long do_syscall(void *cpu_env, int num, long arg1, long
arg2, long arg3,
switch(arg2){
case TARGET_F_GETLK64:
cmd = F_GETLK64;
+break;
case TARGET_F_SETLK64:
cmd = F_SETLK64;
+break;
case TARGET_F_SETLKW64:
cmd = F_SETLK64;
+break;
default:
cmd = arg2;
}
signature.asc
Description: Digital signature
___
Qemu-devel mailing list
Qemu-devel@nongnu.org
http://lists.nongnu.org/mailman/listinfo/qemu-devel
Re: [Qemu-devel] [PATCH] fcntl64 fix
On Wed, 21 Mar 2007, Kirill A. Shutemov wrote: Primarily, I also thought that problem is in padding, because, without the patch F_GETLK, on 32-bit target recognises as F_GETLK64 on 64-bit host. It's happen because on 64-bit host and 32-bit target F_GETLK == F_GETLK64 == TARGET_F_GETLK. So if you're using qemu-arm on 64-bit host and a target eabi program calls fcntl(fd,F_GETLK,...), target_eabi_flock64 will be used by mistake. Disabling padding can helps in some trivial cases to pass pseudo-correct args to fcntl. I guess this part of patch wrong. Stuart, am I right? Yes, this is a good summary of the trap I initially fell into. Stuart Stuart R. Anderson [EMAIL PROTECTED] Network Software Engineering http://www.netsweng.com/ 1024D/37A79149: 0791 D3B8 9A4C 2CDC A31F BD03 0A62 E534 37A7 9149 ___ Qemu-devel mailing list Qemu-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/qemu-devel
Re: [Qemu-devel] [PATCH] fcntl64 fix
On Tue, 20 Mar 2007, Paul Brook wrote: Now that the dust has settled, I see where the change is probably a no-op anyway. A quick little test program indicates that on x86_64, l_start will have an offset of 8 wether the structure is packed or not, and wether the __pad member is present or not. The unsigned long long is always going to be aligned to a 8 byte boundary. The __pad member is essential. Your logic is wrong is two ways: a) The struct is packed. This overrides normal alignment and ensures the structure contains no padding. And in this case, it does remove some tail padding at the end of the structure. b) long long has whatever alignment the host feels like giving it. There's no guarantee it's going to be 8 byte aligned. No there isn't. This was just an observation of what occurs when building a simple test case on x86_64. Stuart Stuart R. Anderson [EMAIL PROTECTED] Network Software Engineering http://www.netsweng.com/ 1024D/37A79149: 0791 D3B8 9A4C 2CDC A31F BD03 0A62 E534 37A7 9149 ___ Qemu-devel mailing list Qemu-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/qemu-devel
Re: [Qemu-devel] [PATCH] fcntl64 fix
Yep. You're right. Fixed patch in the attachment.
On [Mon, 19.03.2007 17:12], Thiemo Seufer wrote:
Kirill A. Shutemov wrote:
TARGET_F_*64 should be used instead of F_*64, because on 64-bit host
systems F_GETLK == F_GETLK64(same for SETLK and SETLKW), so we cannot
determinate if it's a long lock or not on a target 32-bit system.
Patch in the attachment.
P.S. Please, review my privious patches, which I have added description
recently. Or should I repost it?
diff -uNr qemu-0.9.0.cvs20070304.orig/linux-user/syscall.c
qemu-0.9.0.cvs20070304/linux-user/syscall.c
--- qemu-0.9.0.cvs20070304.orig/linux-user/syscall.c2007-03-09
20:08:59 +0200
+++ qemu-0.9.0.cvs20070304/linux-user/syscall.c 2007-03-09 20:09:54
+0200
@@ -4063,7 +4063,7 @@
#endif
switch(arg2) {
-case F_GETLK64:
+case TARGET_F_GETLK64:
ret = get_errno(fcntl(arg1, arg2, fl));
This changes the bug from checking the wrong flag to (potentially)
passing down the wrong flag...
if (ret == 0) {
#ifdef TARGET_ARM
@@ -4089,8 +4089,8 @@
}
break;
-case F_SETLK64:
-case F_SETLKW64:
+case TARGET_F_SETLK64:
+case TARGET_F_SETLKW64:
#ifdef TARGET_ARM
if (((CPUARMState *)cpu_env)-eabi) {
lock_user_struct(target_efl, arg3, 1);
Likewise here. We should always check TARGET_* flags and pass down the
corresponding host flag.
diff -uNr qemu-0.9.0.cvs20070320.orig/linux-user/syscall.c
qemu-0.9.0.cvs20070320/linux-user/syscall.c
--- qemu-0.9.0.cvs20070320.orig/linux-user/syscall.c2007-03-20 13:26:04
+0200
+++ qemu-0.9.0.cvs20070320/linux-user/syscall.c 2007-03-20 13:34:09 +0200
@@ -4058,15 +4058,27 @@
#if TARGET_LONG_BITS == 32
case TARGET_NR_fcntl64:
{
+ int cmd;
struct flock64 fl;
struct target_flock64 *target_fl;
#ifdef TARGET_ARM
struct target_eabi_flock64 *target_efl;
#endif
+switch(arg2){
+case TARGET_F_GETLK64:
+cmd = F_GETLK64;
+case TARGET_F_SETLK64:
+cmd = F_SETLK64;
+case TARGET_F_SETLKW64:
+cmd = F_SETLK64;
+default:
+cmd = arg2;
+}
+
switch(arg2) {
-case F_GETLK64:
-ret = get_errno(fcntl(arg1, arg2, fl));
+case TARGET_F_GETLK64:
+ret = get_errno(fcntl(arg1, cmd, fl));
if (ret == 0) {
#ifdef TARGET_ARM
if (((CPUARMState *)cpu_env)-eabi) {
@@ -4091,8 +4103,8 @@
}
break;
-case F_SETLK64:
-case F_SETLKW64:
+case TARGET_F_SETLK64:
+case TARGET_F_SETLKW64:
#ifdef TARGET_ARM
if (((CPUARMState *)cpu_env)-eabi) {
lock_user_struct(target_efl, arg3, 1);
@@ -4113,10 +4125,10 @@
fl.l_pid = tswapl(target_fl-l_pid);
unlock_user_struct(target_fl, arg3, 0);
}
-ret = get_errno(fcntl(arg1, arg2, fl));
+ret = get_errno(fcntl(arg1, cmd, fl));
break;
default:
-ret = get_errno(do_fcntl(arg1, arg2, arg3));
+ret = get_errno(do_fcntl(arg1, cmd, arg3));
break;
}
break;
signature.asc
Description: Digital signature
___
Qemu-devel mailing list
Qemu-devel@nongnu.org
http://lists.nongnu.org/mailman/listinfo/qemu-devel
Re: [Qemu-devel] [PATCH] fcntl64 fix
On [Tue, 20.03.2007 09:55], Stuart Anderson wrote: On Tue, 20 Mar 2007, Kirill A. Shutemov wrote: Yep. You're right. Fixed patch in the attachment. Kiril, What 32 bit host and 64 bit host are you using? I'm working on arm on x86_64, and I'm starting to think that perhaps all of the different parts of the fix are needed to ensure it works correctly on all target/host combinations. I'm using arm(little-endian) on x86_64. -- Regards, Kirill A. Shutemov + Belarus, Minsk + Velesys LLC, http://www.velesys.com/ + ALT Linux Team, http://www.altlinux.com/ signature.asc Description: Digital signature ___ Qemu-devel mailing list Qemu-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/qemu-devel
Re: [Qemu-devel] [PATCH] fcntl64 fix
On Tue, 20 Mar 2007, Kirill A. Shutemov wrote: Kiril, What 32 bit host and 64 bit host are you using? I'm working on arm on x86_64, and I'm starting to think that perhaps all of the different parts of the fix are needed to ensure it works correctly on all target/host combinations. I'm using arm(little-endian) on x86_64. That blows that theory 8-). What are you using as a test app? I think that remapping the constants is needed, but I'm just curious how we seem to be coming up with different parts of the fix when we have the same target/host combination. Stuart Stuart R. Anderson [EMAIL PROTECTED] Network Software Engineering http://www.netsweng.com/ 1024D/37A79149: 0791 D3B8 9A4C 2CDC A31F BD03 0A62 E534 37A7 9149 ___ Qemu-devel mailing list Qemu-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/qemu-devel
Re: [Qemu-devel] [PATCH] fcntl64 fix
On [Tue, 20.03.2007 12:53], Stuart Anderson wrote: On Tue, 20 Mar 2007, Kirill A. Shutemov wrote: Kiril, What 32 bit host and 64 bit host are you using? I'm working on arm on x86_64, and I'm starting to think that perhaps all of the different parts of the fix are needed to ensure it works correctly on all target/host combinations. I'm using arm(little-endian) on x86_64. That blows that theory 8-). :) What are you using as a test app? I got error when runing Debian's apt-get and tried to fix it. I think that remapping the constants is needed, but I'm just curious how we seem to be coming up with different parts of the fix when we have the same target/host combination. I'm not sure that I understand you... -- Regards, Kirill A. Shutemov + Belarus, Minsk + Velesys LLC, http://www.velesys.com/ + ALT Linux Team, http://www.altlinux.com/ signature.asc Description: Digital signature ___ Qemu-devel mailing list Qemu-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/qemu-devel
Re: [Qemu-devel] [PATCH] fcntl64 fix
On Tue, 20 Mar 2007, Kirill A. Shutemov wrote: What are you using as a test app? I got error when runing Debian's apt-get and tried to fix it. OK, that's what got me started on this one, but I switched to using the ltp-kernel-test package for a more comprehensive set of tests once I got past that first eabi structure change. I think that remapping the constants is needed, but I'm just curious how we seem to be coming up with different parts of the fix when we have the same target/host combination. I'm not sure that I understand you... On the arm/x86_64 combination, I think the host target cmd values are the same, so the remapping is a noop. It may be needed for other combinations though. Some architectures have very different values for constants like this in their ABI. I was trying to understand how your fix made apt-get/dpkg happy, or if you were just using a different app that was hitting a different case for fcntl(). Stuart Stuart R. Anderson [EMAIL PROTECTED] Network Software Engineering http://www.netsweng.com/ 1024D/37A79149: 0791 D3B8 9A4C 2CDC A31F BD03 0A62 E534 37A7 9149 ___ Qemu-devel mailing list Qemu-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/qemu-devel
Re: [Qemu-devel] [PATCH] fcntl64 fix
On [Tue, 20.03.2007 14:03], Stuart Anderson wrote: On Tue, 20 Mar 2007, Kirill A. Shutemov wrote: What are you using as a test app? I got error when runing Debian's apt-get and tried to fix it. OK, that's what got me started on this one, but I switched to using the ltp-kernel-test package for a more comprehensive set of tests once I got past that first eabi structure change. I think that remapping the constants is needed, but I'm just curious how we seem to be coming up with different parts of the fix when we have the same target/host combination. I'm not sure that I understand you... On the arm/x86_64 combination, I think the host target cmd values are the same, so the remapping is a noop. No. Remap is needed: $ uname -m; echo -e '#include fcntl.h\nF_GETLK64' | cpp | tail -1 x86_64 5 $ uname -m; echo -e '#include fcntl.h\nF_GETLK64' | cpp | tail -1 armv5l 12 Same for F_SETLK64 and F_SETLKW64. It may be needed for other combinations though. Some architectures have very different values for constants like this in their ABI. I was trying to understand how your fix made apt-get/dpkg happy, or if you were just using a different app that was hitting a different case for fcntl(). Stuart Stuart R. Anderson [EMAIL PROTECTED] Network Software Engineering http://www.netsweng.com/ 1024D/37A79149: 0791 D3B8 9A4C 2CDC A31F BD03 0A62 E534 37A7 9149 ___ Qemu-devel mailing list Qemu-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/qemu-devel -- Regards, Kirill A. Shutemov + Belarus, Minsk + Velesys LLC, http://www.velesys.com/ + ALT Linux Team, http://www.altlinux.com/ signature.asc Description: Digital signature ___ Qemu-devel mailing list Qemu-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/qemu-devel
Re: [Qemu-devel] [PATCH] fcntl64 fix
On Tue, 20 Mar 2007, Kirill A. Shutemov wrote: No. Remap is needed: $ uname -m; echo -e '#include fcntl.h\nF_GETLK64' | cpp | tail -1 x86_64 5 $ uname -m; echo -e '#include fcntl.h\nF_GETLK64' | cpp | tail -1 armv5l 12 Same for F_SETLK64 and F_SETLKW64. You are right. I had previously printed out the non-64 versions, and they are the same, but we not using those. TARGET_F_GETLK 5 F_GETLK 5 TARGET_F_GETLK64 12 F_GETLK64 5 My confusion... Stuart Stuart R. Anderson [EMAIL PROTECTED] Network Software Engineering http://www.netsweng.com/ 1024D/37A79149: 0791 D3B8 9A4C 2CDC A31F BD03 0A62 E534 37A7 9149 ___ Qemu-devel mailing list Qemu-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/qemu-devel
Re: [Qemu-devel] [PATCH] fcntl64 fix
OK, I think I finally have it all sorted out. Sorry if I sounded dense
along the way.. there were multiple variable, which increases the number
of possible combinations quickly.
The patch from Kirill is needed, and makes things better. One thing I
notice with it is that we now handle TARGET_F_GETLK64 in two places,
first in the case for TARGET_NR_fcntl64 (around line 4300), and then
again in do_fcntl(), which is called in the default case of the first
location. Once difference between the two locations is wether or not
the case for EABI is handled.
In addition to Kirill's patch, my original patch for target_eabi_flock64
is still needed as well as an expanded version of the revised patch I
sent later that does target-host strcture mapping for the F_GETLK*
cases.
I have used the fcntl test sets out of the Linux Test Projects to
measure with an without the different parts of these patches. With
the entire set (Which is attached), 16 of the 18 test sets pass
completely, and a significant portion of test14 (one of the two that
don't pass completely) passes as well. The tests in test14 that fail
may be do to a problem with a syscall other than fcntl(), but I haven't
completely resulved it yet. Without my portion of the patch, the results
are much worse (maybe half-ish are passing).
There is something interesting about test18 (the other one that doesn't
pass). It intentionally passes in a bad value (-1) as the 3rd argument
to fcntl(). It is testing wether it will get EFAULT. With these fixes,
qemu will SEGV as it tries to convert the struct flock (or struct
flock64) from target-host, and encounters the bad address that was
passed in. The initial SEGV is caught, but the handler for it then
SEGVs again. Ideally, we could detect that we are inside an emulated
system call, and be able to just return the EFAULT.
I ran the LTP tests for both old ABI and EABI, and got the same results.
Attached is the combined patch for fcntl().
Stuart
Stuart R. Anderson [EMAIL PROTECTED]
Network Software Engineering http://www.netsweng.com/
1024D/37A79149: 0791 D3B8 9A4C 2CDC A31F
BD03 0A62 E534 37A7 9149--- linux-user/syscall_defs.h.orig 2007-02-23 15:44:47.0 -0500
+++ linux-user/syscall_defs.h 2007-02-23 15:44:26.0 -0500
@@ -1414,7 +1414,9 @@
struct target_eabi_flock64 {
short l_type;
short l_whence;
+#if HOST_LONG_BITS == 32
int __pad;
+#endif
unsigned long long l_start;
unsigned long long l_len;
int l_pid;
Index: linux-user/syscall.c
===
--- linux-user/syscall.c.orig 2007-03-20 16:19:11.0 -0400
+++ linux-user/syscall.c2007-03-20 17:04:40.0 -0400
@@ -2107,6 +2107,13 @@
switch(cmd) {
case TARGET_F_GETLK:
+lock_user_struct(target_fl, arg, 1);
+fl.l_type = tswap16(target_fl-l_type);
+fl.l_whence = tswap16(target_fl-l_whence);
+fl.l_start = tswapl(target_fl-l_start);
+fl.l_len = tswapl(target_fl-l_len);
+fl.l_pid = tswapl(target_fl-l_pid);
+unlock_user_struct(target_fl, arg, 0);
ret = fcntl(fd, cmd, fl);
if (ret == 0) {
lock_user_struct(target_fl, arg, 0);
@@ -2132,6 +2139,13 @@
break;
case TARGET_F_GETLK64:
+lock_user_struct(target_fl64, arg, 1);
+fl64.l_type = tswap16(target_fl64-l_type) 1;
+fl64.l_whence = tswap16(target_fl64-l_whence);
+fl64.l_start = tswapl(target_fl64-l_start);
+fl64.l_len = tswapl(target_fl64-l_len);
+fl64.l_pid = tswap16(target_fl64-l_pid);
+unlock_user_struct(target_fl64, arg, 0);
ret = fcntl(fd, cmd 1, fl64);
if (ret == 0) {
lock_user_struct(target_fl64, arg, 0);
@@ -4201,15 +4215,47 @@
#if TARGET_LONG_BITS == 32
case TARGET_NR_fcntl64:
{
+ int cmd;
struct flock64 fl;
struct target_flock64 *target_fl;
#ifdef TARGET_ARM
struct target_eabi_flock64 *target_efl;
#endif
+ switch(arg2){
+ case TARGET_F_GETLK64:
+ cmd = F_GETLK64;
+ case TARGET_F_SETLK64:
+ cmd = F_SETLK64;
+ case TARGET_F_SETLKW64:
+ cmd = F_SETLKW64;
+ default:
+ cmd = arg2;
+ }
+
switch(arg2) {
-case F_GETLK64:
-ret = get_errno(fcntl(arg1, arg2, fl));
+case TARGET_F_GETLK64:
+#ifdef TARGET_ARM
+if (((CPUARMState *)cpu_env)-eabi) {
+lock_user_struct(target_efl, arg3, 1);
+fl.l_type = tswap16(target_efl-l_type);
+fl.l_whence = tswap16(target_efl-l_whence);
+fl.l_start = tswap64(target_efl-l_start);
+fl.l_len = tswap64(target_efl-l_len);
+
Re: [Qemu-devel] [PATCH] fcntl64 fix
struct target_eabi_flock64 {
short l_type;
short l_whence;
+#if HOST_LONG_BITS == 32
int __pad;
+#endif
This change is definitely wrong. This is a target structure, and is packed, so
should be independent of the host.
Paul
___
Qemu-devel mailing list
Qemu-devel@nongnu.org
http://lists.nongnu.org/mailman/listinfo/qemu-devel
Re: [Qemu-devel] [PATCH] fcntl64 fix
Stuart Anderson wrote:
[snip]
--- linux-user/syscall_defs.h.orig2007-02-23 15:44:47.0 -0500
+++ linux-user/syscall_defs.h 2007-02-23 15:44:26.0 -0500
@@ -1414,7 +1414,9 @@
struct target_eabi_flock64 {
short l_type;
short l_whence;
+#if HOST_LONG_BITS == 32
int __pad;
+#endif
Still, this part makes no sense to me since it is in a packed struct.
Can you explain why this works better for you?
Thiemo
___
Qemu-devel mailing list
Qemu-devel@nongnu.org
http://lists.nongnu.org/mailman/listinfo/qemu-devel
Re: [Qemu-devel] [PATCH] fcntl64 fix
On Tue, 20 Mar 2007, Thiemo Seufer wrote: Still, this part makes no sense to me since it is in a packed struct. Can you explain why this works better for you? It worked better, in that it fixed a problem that let me continue on to fix other issues. After revisiting fcntl() and coming up with the more comprehensive patch, this change now seems to have no effect, so I must conceed that it is not needed, please drop that part of the patch. Now that the dust has settled, I see where the change is probably a no-op anyway. A quick little test program indicates that on x86_64, l_start will have an offset of 8 wether the structure is packed or not, and wether the __pad member is present or not. The unsigned long long is always going to be aligned to a 8 byte boundary. Stuart Stuart R. Anderson [EMAIL PROTECTED] Network Software Engineering http://www.netsweng.com/ 1024D/37A79149: 0791 D3B8 9A4C 2CDC A31F BD03 0A62 E534 37A7 9149 ___ Qemu-devel mailing list Qemu-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/qemu-devel
Re: [Qemu-devel] [PATCH] fcntl64 fix
On [Tue, 20.03.2007 21:47], Thiemo Seufer wrote:
Stuart Anderson wrote:
[snip]
--- linux-user/syscall_defs.h.orig 2007-02-23 15:44:47.0 -0500
+++ linux-user/syscall_defs.h 2007-02-23 15:44:26.0 -0500
@@ -1414,7 +1414,9 @@
struct target_eabi_flock64 {
short l_type;
short l_whence;
+#if HOST_LONG_BITS == 32
int __pad;
+#endif
Still, this part makes no sense to me since it is in a packed struct.
Can you explain why this works better for you?
Primarily, I also thought that problem is in padding, because, without the
patch F_GETLK, on 32-bit target recognises as F_GETLK64 on 64-bit host.
It's happen because on 64-bit host and 32-bit target F_GETLK == F_GETLK64 ==
TARGET_F_GETLK. So if you're using qemu-arm on 64-bit host and a target eabi
program calls fcntl(fd,F_GETLK,...), target_eabi_flock64 will be used by
mistake. Disabling padding can helps in some trivial cases to pass
pseudo-correct args to fcntl. I guess this part of patch wrong.
Stuart, am I right?
--
Regards, Kirill A. Shutemov
+ Belarus, Minsk
+ Velesys LLC, http://www.velesys.com/
+ ALT Linux Team, http://www.altlinux.com/
signature.asc
Description: Digital signature
___
Qemu-devel mailing list
Qemu-devel@nongnu.org
http://lists.nongnu.org/mailman/listinfo/qemu-devel
Re: [Qemu-devel] [PATCH] fcntl64 fix
On [Tue, 20.03.2007 19:05], Stuart Anderson wrote: Now that the dust has settled, I see where the change is probably a no-op anyway. A quick little test program indicates that on x86_64, l_start will have an offset of 8 wether the structure is packed or not, and wether the __pad member is present or not. The unsigned long long is always going to be aligned to a 8 byte boundary. Its' architecture specific I think. -- Regards, Kirill A. Shutemov + Belarus, Minsk + Velesys LLC, http://www.velesys.com/ + ALT Linux Team, http://www.altlinux.com/ signature.asc Description: Digital signature ___ Qemu-devel mailing list Qemu-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/qemu-devel
Re: [Qemu-devel] [PATCH] fcntl64 fix
Now that the dust has settled, I see where the change is probably a no-op anyway. A quick little test program indicates that on x86_64, l_start will have an offset of 8 wether the structure is packed or not, and wether the __pad member is present or not. The unsigned long long is always going to be aligned to a 8 byte boundary. The __pad member is essential. Your logic is wrong is two ways: a) The struct is packed. This overrides normal alignment and ensures the structure contains no padding. b) long long has whatever alignment the host feels like giving it. There's no guarantee it's going to be 8 byte aligned. Paul ___ Qemu-devel mailing list Qemu-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/qemu-devel
Re: [Qemu-devel] [PATCH] fcntl64 fix
Kirill A. Shutemov wrote:
TARGET_F_*64 should be used instead of F_*64, because on 64-bit host
systems F_GETLK == F_GETLK64(same for SETLK and SETLKW), so we cannot
determinate if it's a long lock or not on a target 32-bit system.
Patch in the attachment.
P.S. Please, review my privious patches, which I have added description
recently. Or should I repost it?
diff -uNr qemu-0.9.0.cvs20070304.orig/linux-user/syscall.c
qemu-0.9.0.cvs20070304/linux-user/syscall.c
--- qemu-0.9.0.cvs20070304.orig/linux-user/syscall.c 2007-03-09 20:08:59
+0200
+++ qemu-0.9.0.cvs20070304/linux-user/syscall.c 2007-03-09 20:09:54
+0200
@@ -4063,7 +4063,7 @@
#endif
switch(arg2) {
-case F_GETLK64:
+case TARGET_F_GETLK64:
ret = get_errno(fcntl(arg1, arg2, fl));
This changes the bug from checking the wrong flag to (potentially)
passing down the wrong flag...
if (ret == 0) {
#ifdef TARGET_ARM
@@ -4089,8 +4089,8 @@
}
break;
-case F_SETLK64:
-case F_SETLKW64:
+case TARGET_F_SETLK64:
+case TARGET_F_SETLKW64:
#ifdef TARGET_ARM
if (((CPUARMState *)cpu_env)-eabi) {
lock_user_struct(target_efl, arg3, 1);
Likewise here. We should always check TARGET_* flags and pass down the
corresponding host flag.
Thiemo
___
Qemu-devel mailing list
Qemu-devel@nongnu.org
http://lists.nongnu.org/mailman/listinfo/qemu-devel
Re: [Qemu-devel] [PATCH] fcntl64 fix
My initial fix was before I started using LTP, and just took care of a single case that was holding me up. Now I have run the fcntl tests in LTP on ARM (both oABI and EABI) and there are a lot of failures indicating that there is a lot more work to be done yet on fcntl(). I'll take a look into it, and probably resubmit a bigger patch later. Stuart Stuart R. Anderson [EMAIL PROTECTED] Network Software Engineering http://www.netsweng.com/ 1024D/37A79149: 0791 D3B8 9A4C 2CDC A31F BD03 0A62 E534 37A7 9149 ___ Qemu-devel mailing list Qemu-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/qemu-devel
Re: [Qemu-devel] [PATCH] fcntl64 fix
On Mon, 19 Mar 2007, Stuart Anderson wrote:
My initial fix was before I started using LTP, and just took care of a
single case that was holding me up. Now I have run the fcntl tests in
LTP on ARM (both oABI and EABI) and there are a lot of failures indicating
that there is a lot more work to be done yet on fcntl().
I'll take a look into it, and probably resubmit a bigger patch later.
One more small fix to repack a structure from taget - host before using
it clears up most of the fcntl() errors that showed up in LTP. This is
one of those that probably doesn't happen when runngin 32 on 32, but I'm
running 32 on 64.
Stuart
Stuart R. Anderson [EMAIL PROTECTED]
Network Software Engineering http://www.netsweng.com/
1024D/37A79149: 0791 D3B8 9A4C 2CDC A31F
BD03 0A62 E534 37A7 9149--- linux-user/syscall.c.orig 2007-03-20 01:25:39.0 -0400
+++ linux-user/syscall.c2007-03-20 02:32:39.0 -0400
@@ -2107,6 +2107,13 @@
switch(cmd) {
case TARGET_F_GETLK:
+lock_user_struct(target_fl, arg, 1);
+fl.l_type = tswap16(target_fl-l_type);
+fl.l_whence = tswap16(target_fl-l_whence);
+fl.l_start = tswapl(target_fl-l_start);
+fl.l_len = tswapl(target_fl-l_len);
+fl.l_pid = tswapl(target_fl-l_pid);
+unlock_user_struct(target_fl, arg, 0);
ret = fcntl(fd, cmd, fl);
if (ret == 0) {
lock_user_struct(target_fl, arg, 0);
___
Qemu-devel mailing list
Qemu-devel@nongnu.org
http://lists.nongnu.org/mailman/listinfo/qemu-devel
