| Message ID | 1378485978-32108-1-git-send-email-sw@weilnetz.de |
|---|---|
| State | Superseded |
| Headers | show |
On 6 September 2013 17:46, Stefan Weil <sw@weilnetz.de> wrote: > This test case is fixed now: > sparc64-linux-user/qemu-sparc64 /usr/gnemul/qemu-sparc64/busybox ls -l block.c > > Signed-off-by: Stefan Weil <sw@weilnetz.de> > --- > linux-user/syscall.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > index ecead51..e498a92 100644 > --- a/linux-user/syscall.c > +++ b/linux-user/syscall.c > @@ -4764,7 +4764,7 @@ static inline abi_long host_to_target_stat64(void *cpu_env, > } else > #endif > { > -#if TARGET_ABI_BITS == 64 && !defined(TARGET_ALPHA) > +#if TARGET_ABI_BITS == 64 && !defined(TARGET_ALPHA) && !defined(TARGET_SPARC64) > struct target_stat *target_st; > #else > struct target_stat64 *target_st; So this condition is trying to identify the platforms which don't actually have a struct stat64 (which is some but not all of the 64 bit platforms). I think that rather than trying to enumerate those here it would be better if in the sections of syscall_defs.h which define target_stat/target_stat64 we had those sections which don't have a target_stat64 definition instead #define TARGET_NO_STRUCT_STAT64 and then used that here. -- PMM
Am 06.09.2013 19:16, schrieb Peter Maydell: > On 6 September 2013 17:46, Stefan Weil <sw@weilnetz.de> wrote: >> This test case is fixed now: >> sparc64-linux-user/qemu-sparc64 /usr/gnemul/qemu-sparc64/busybox ls -l block.c >> >> Signed-off-by: Stefan Weil <sw@weilnetz.de> >> --- >> linux-user/syscall.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/linux-user/syscall.c b/linux-user/syscall.c >> index ecead51..e498a92 100644 >> --- a/linux-user/syscall.c >> +++ b/linux-user/syscall.c >> @@ -4764,7 +4764,7 @@ static inline abi_long host_to_target_stat64(void *cpu_env, >> } else >> #endif >> { >> -#if TARGET_ABI_BITS == 64 && !defined(TARGET_ALPHA) >> +#if TARGET_ABI_BITS == 64 && !defined(TARGET_ALPHA) && !defined(TARGET_SPARC64) >> struct target_stat *target_st; >> #else >> struct target_stat64 *target_st; > So this condition is trying to identify the platforms > which don't actually have a struct stat64 (which is > some but not all of the 64 bit platforms). I think that > rather than trying to enumerate those here it would > be better if in the sections of syscall_defs.h which > define target_stat/target_stat64 we had those sections > which don't have a target_stat64 definition instead > #define TARGET_NO_STRUCT_STAT64 > and then used that here. > > -- PMM I even thought about a totally different solution: Could we write some sample code with all those structs and system calls, compile it once for each target, and get all information we need from the resulting binaries (using tools like pahole, for example)? For the moment, I think my patch is sufficient (it is also needed for QEMU 1.6!). Stefan
Am 06.09.2013 19:24, schrieb Stefan Weil: > Am 06.09.2013 19:16, schrieb Peter Maydell: >> On 6 September 2013 17:46, Stefan Weil <sw@weilnetz.de> wrote: >>> This test case is fixed now: >>> sparc64-linux-user/qemu-sparc64 /usr/gnemul/qemu-sparc64/busybox ls -l block.c >>> >>> Signed-off-by: Stefan Weil <sw@weilnetz.de> >>> --- >>> linux-user/syscall.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c >>> index ecead51..e498a92 100644 >>> --- a/linux-user/syscall.c >>> +++ b/linux-user/syscall.c >>> @@ -4764,7 +4764,7 @@ static inline abi_long host_to_target_stat64(void *cpu_env, >>> } else >>> #endif >>> { >>> -#if TARGET_ABI_BITS == 64 && !defined(TARGET_ALPHA) >>> +#if TARGET_ABI_BITS == 64 && !defined(TARGET_ALPHA) && !defined(TARGET_SPARC64) >>> struct target_stat *target_st; >>> #else >>> struct target_stat64 *target_st; >> So this condition is trying to identify the platforms >> which don't actually have a struct stat64 (which is >> some but not all of the 64 bit platforms). I think that >> rather than trying to enumerate those here it would >> be better if in the sections of syscall_defs.h which >> define target_stat/target_stat64 we had those sections >> which don't have a target_stat64 definition instead >> #define TARGET_NO_STRUCT_STAT64 >> and then used that here. >> >> -- PMM > I even thought about a totally different solution: > > Could we write some sample code with all those structs > and system calls, compile it once for each target, and > get all information we need from the resulting binaries > (using tools like pahole, for example)? > > For the moment, I think my patch is sufficient (it is also > needed for QEMU 1.6!). > > Stefan Ping? Are there any more opinions how qemu-sparc64 should be fixed? Should we choose Peter's approach (which is good, but with a higher risk than my patch)? Stefan
On Thu, Sep 19, 2013 at 07:31:51PM +0200, Stefan Weil wrote: > Ping? Are there any more opinions how qemu-sparc64 should be fixed? > Should we choose Peter's approach (which is good, but with a > higher risk than my patch)? I've included it now in my qeu, since it fixes qemu-sparc64 in my smoketest and causes seemingly no regressions on other platforms. Riku
On 23 September 2013 20:57, Riku Voipio <riku.voipio@iki.fi> wrote: > On Thu, Sep 19, 2013 at 07:31:51PM +0200, Stefan Weil wrote: >> Ping? Are there any more opinions how qemu-sparc64 should be fixed? >> Should we choose Peter's approach (which is good, but with a >> higher risk than my patch)? > > I've included it now in my qeu, since it fixes qemu-sparc64 in my > smoketest and causes seemingly no regressions on other platforms. Yeah, but I bet there are other platforms we're not getting this right on since the #ifdef here isn't the same as the #ifdef that decides whether we have a target_stat64 struct or not... -- PMM
On Mon, Sep 23, 2013 at 09:05:03PM +0900, Peter Maydell wrote: > On 23 September 2013 20:57, Riku Voipio <riku.voipio@iki.fi> wrote: > > On Thu, Sep 19, 2013 at 07:31:51PM +0200, Stefan Weil wrote: > >> Ping? Are there any more opinions how qemu-sparc64 should be fixed? > >> Should we choose Peter's approach (which is good, but with a > >> higher risk than my patch)? > > > > I've included it now in my qeu, since it fixes qemu-sparc64 in my > > smoketest and causes seemingly no regressions on other platforms. > Yeah, but I bet there are other platforms we're not getting this > right on since the #ifdef here isn't the same as the #ifdef that > decides whether we have a target_stat64 struct or not... Ok, I'll leave the patch on table then, and send this pullrequest without it. Stefan, can you base the fix on setting TARGET_NO_STRUCT_STAT64 when it is not available? Riku
diff --git a/linux-user/syscall.c b/linux-user/syscall.c index ecead51..e498a92 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -4764,7 +4764,7 @@ static inline abi_long host_to_target_stat64(void *cpu_env, } else #endif { -#if TARGET_ABI_BITS == 64 && !defined(TARGET_ALPHA) +#if TARGET_ABI_BITS == 64 && !defined(TARGET_ALPHA) && !defined(TARGET_SPARC64) struct target_stat *target_st; #else struct target_stat64 *target_st;
This test case is fixed now: sparc64-linux-user/qemu-sparc64 /usr/gnemul/qemu-sparc64/busybox ls -l block.c Signed-off-by: Stefan Weil <sw@weilnetz.de> --- linux-user/syscall.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)