[Qemu-devel] [PATCH 0/3] linux-user: Fix miscelaneous Mips-specific issues

[Qemu-devel] [PATCH 0/3] linux-user: Fix miscelaneous Mips-specific issues

[Aleksandar Markovic]

From: Aleksandar Markovic <aleksandar.markovic@imgtec.com>

This series fixes several wrong definitions of preprocessor constants and
structures in Qemu user mode.

All patches are completely isolated Mips issues.

The serias fixes certain number of LTP test failures, if executed in
Qemu user mode for Mips platform.

Aleksandar Markovic (3):
  linux-user: Fix preprocessor constant TARGET_F_GETOWN for Mips
  linux-user: Fix structure target_flock definition for Mips
  linux-user: Fix structure target_semid64_ds definition for Mips

 linux-user/mips/target_structs.h |   19 +++++++++++++++++++
 linux-user/syscall_defs.h        |    8 +++++++-
 2 files changed, 26 insertions(+), 1 deletion(-)

-- 
1.7.9.5

[Qemu-devel] [PATCH 1/3] linux-user: Fix preprocessor constant TARGET_F_GETOWN for Mips

[Aleksandar Markovic]

From: Aleksandar Markovic <aleksandar.markovic@imgtec.com>

For some reason, Qemu's TARGET_F_GETOWN constant for Mips does not
match the correct value of correspondant F_GETOWN. This patch fixes
this problem.

For reference, see Mips' F_GETOWN definition in Linux kernel at
arch/mips/include/uapi/asm/fcntl.h#L44.

This patch also fixes some fcntl()-related LTP tests for Qemu
user mode for Mips.

Signed-off-by: Miodrag Dinic <miodrag.dinic@imgtec.com>
---
 linux-user/syscall_defs.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index afd9191..0fe0735 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -2182,7 +2182,7 @@ struct target_statfs64 {
 #define TARGET_F_SETLK         6
 #define TARGET_F_SETLKW        7
 #define TARGET_F_SETOWN        24       /*  for sockets. */
-#define TARGET_F_GETOWN        25       /*  for sockets. */
+#define TARGET_F_GETOWN        23       /*  for sockets. */
 #else
 #define TARGET_F_GETLK         5
 #define TARGET_F_SETLK         6
-- 
1.7.9.5

[Qemu-devel] [PATCH 2/3] linux-user: Fix structure target_flock definition for Mips

[Aleksandar Markovic]

From: Aleksandar Markovic <aleksandar.markovic@imgtec.com>

Structure flock is defined for Mips in a way defferent than any other
platform. For reference, see Linux kernel files:

arch/mips/include/uapi/asm/fcntl.h#L63 (for Mips)
include/uapi/asm-generic/fcntl.h#L195 (for all other platforms)

This patch fix this problem, by amending structure target_flock,
for Mips only.

Besides, this patch fixes LTP tests fcntl11, fcntl17, fcntl19, fcntl20,
and fcntl21, which are currently failing if executed in Qemu user mode
for Mips platforms.

Signed-off-by: Aleksandar MArkovic <aleksandar.markovic@imgtec.com>
---
 linux-user/syscall_defs.h |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 0fe0735..4afc504 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -2351,7 +2351,13 @@ struct target_flock {
     short l_whence;
     abi_long l_start;
     abi_long l_len;
+#if defined(TARGET_MIPS)
+    target_long l_sysid;
+#endif
     int l_pid;
+#if defined(TARGET_MIPS)
+    target_long pad[4];
+#endif
 };
 
 struct target_flock64 {
-- 
1.7.9.5

[Qemu-devel] [PATCH 3/3] linux-user: Fix structure target_semid64_ds definition for Mips

[Aleksandar Markovic]

From: Aleksandar Markovic <aleksandar.markovic@imgtec.com>

This patch corrects target_semid64_ds structure definition for Mips.

See, for example definition of semid64_ds for Mips in Linux kernel:
arch/mips/include/uapi/asm/sembuf.h#L13.

This patch will also fix certain semaphore-related LTP tests for Mips,
if they are executed in Qemu user mode for any Mips platform.

Signed-off-by: Miodrag Dinic <miodrag.dinic@imgtec.com>
---
 linux-user/mips/target_structs.h |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/linux-user/mips/target_structs.h b/linux-user/mips/target_structs.h
index fbd9955..6875506 100644
--- a/linux-user/mips/target_structs.h
+++ b/linux-user/mips/target_structs.h
@@ -45,4 +45,23 @@ struct target_shmid_ds {
     abi_ulong __unused2;
 };
 
+#define TARGET_SEMID64_DS
+/*
+ * http://lxr.free-electrons.com/source/arch/mips/include/uapi/asm/sembuf.h#L13
+ *
+ * The semid64_ds structure for the MIPS architecture.
+ * Note extra padding because this structure is passed back and forth
+ * between kernel and user space.
+ *
+ * MIPS uses the same structure layout for both 32bit and 64bit variants.
+ */
+struct target_semid64_ds {
+    struct target_ipc_perm sem_perm;
+    abi_ulong sem_otime;
+    abi_ulong sem_ctime;
+    abi_ulong sem_nsems;
+    abi_ulong __unused3;
+    abi_ulong __unused4;
+};
+
 #endif
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 3/3] linux-user: Fix structure target_semid64_ds definition for Mips

[Peter Maydell]

On 29 August 2016 at 17:07, Aleksandar Markovic
<aleksandar.markovic@rt-rk.com> wrote:
> From: Aleksandar Markovic <aleksandar.markovic@imgtec.com>
>
> This patch corrects target_semid64_ds structure definition for Mips.
>
> See, for example definition of semid64_ds for Mips in Linux kernel:
> arch/mips/include/uapi/asm/sembuf.h#L13.
>
> This patch will also fix certain semaphore-related LTP tests for Mips,
> if they are executed in Qemu user mode for any Mips platform.
>
> Signed-off-by: Miodrag Dinic <miodrag.dinic@imgtec.com>
> ---
>  linux-user/mips/target_structs.h |   19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> diff --git a/linux-user/mips/target_structs.h b/linux-user/mips/target_structs.h
> index fbd9955..6875506 100644
> --- a/linux-user/mips/target_structs.h
> +++ b/linux-user/mips/target_structs.h
> @@ -45,4 +45,23 @@ struct target_shmid_ds {
>      abi_ulong __unused2;
>  };
>
> +#define TARGET_SEMID64_DS
> +/*
> + * http://lxr.free-electrons.com/source/arch/mips/include/uapi/asm/sembuf.h#L13
> + *
> + * The semid64_ds structure for the MIPS architecture.
> + * Note extra padding because this structure is passed back and forth
> + * between kernel and user space.
> + *
> + * MIPS uses the same structure layout for both 32bit and 64bit variants.
> + */

Isn't it specifically *not* using the same structure layout for
32 and 64 bit? The struct below uses abi_ulong, which has a
different size for the two cases, so the structure layout will
differ. The 'asm generic' version of the struct (which QEMU defines
in syscall.c) is the one which is the same for both 32 and 64 bit
because it adds the explicit padding for the TARGET_ABI_BITS=32 case.

> +struct target_semid64_ds {
> +    struct target_ipc_perm sem_perm;
> +    abi_ulong sem_otime;
> +    abi_ulong sem_ctime;
> +    abi_ulong sem_nsems;
> +    abi_ulong __unused3;
> +    abi_ulong __unused4;
> +};

This does seem to be what the kernel does, though, so
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
if you clarify the comment.

thanks
-- PMM

Re: [Qemu-devel] ?==?utf-8?q? ?==?utf-8?q? [PATCH 3/3]?==?utf-8?q? linux-user: Fix structure target_semid64_ds definition for Mips

[Aleksandar Markovic]

-------- Original Message --------
Subject: Re: [Qemu-devel] [PATCH 3/3] linux-user: Fix structure target_semid64_ds definition for Mips
Date: Monday, August 29, 2016 23:41 CEST
From: Peter Maydell <peter.maydell@linaro.org>
To: Aleksandar Markovic <aleksandar.markovic@rt-rk.com>
CC: Riku Voipio <riku.voipio@iki.fi>, QEMU Developers <qemu-devel@nongnu.org>, Petar Jovanovic <petar.jovanovic@imgtec.com>, Aleksandar Markovic <aleksandar.markovic@imgtec.com>, aleksandar.rikalo@imgtec.com, Miodrag Dinic <miodrag.dinic@imgtec.com>, Leon Alrae <leon.alrae@imgtec.com>, Aurelien Jarno <aurelien@aurel32.net>
References: <1472504853-42497-1-git-send-email-aleksandar.markovic@rt-rk.com> <1472504853-42497-4-git-send-email-aleksandar.markovic@rt-rk.com>


 On 29 August 2016 at 17:07, Aleksandar Markovic
<aleksandar.markovic@rt-rk.com> wrote:
> From: Aleksandar Markovic <aleksandar.markovic@imgtec.com>
>
> This patch corrects target_semid64_ds structure definition for Mips.
>
> See, for example definition of semid64_ds for Mips in Linux kernel:
> arch/mips/include/uapi/asm/sembuf.h#L13.
>
> This patch will also fix certain semaphore-related LTP tests for Mips,
> if they are executed in Qemu user mode for any Mips platform.
>
> Signed-off-by: Miodrag Dinic <miodrag.dinic@imgtec.com>
> ---
> linux-user/mips/target_structs.h | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/linux-user/mips/target_structs.h b/linux-user/mips/target_structs.h
> index fbd9955..6875506 100644
> --- a/linux-user/mips/target_structs.h
> +++ b/linux-user/mips/target_structs.h
> @@ -45,4 +45,23 @@ struct target_shmid_ds {
> abi_ulong __unused2;
> };
>
> +#define TARGET_SEMID64_DS
> +/*
> + * http://lxr.free-electrons.com/source/arch/mips/include/uapi/asm/sembuf.h#L13
> + *
> + * The semid64_ds structure for the MIPS architecture.
> + * Note extra padding because this structure is passed back and forth
> + * between kernel and user space.
> + *
> + * MIPS uses the same structure layout for both 32bit and 64bit variants.
> + */

Isn't it specifically *not* using the same structure layout for
32 and 64 bit? The struct below uses abi_ulong, which has a
different size for the two cases, so the structure layout will
differ. The 'asm generic' version of the struct (which QEMU defines
in syscall.c) is the one which is the same for both 32 and 64 bit
because it adds the explicit padding for the TARGET_ABI_BITS=32 case.

> +struct target_semid64_ds {
> + struct target_ipc_perm sem_perm;
> + abi_ulong sem_otime;
> + abi_ulong sem_ctime;
> + abi_ulong sem_nsems;
> + abi_ulong __unused3;
> + abi_ulong __unused4;
> +};

This does seem to be what the kernel does, though, so
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
if you clarify the comment.

thanks
-- PMMYes, good point - in v2, I am going to remove that sentence altogether from the comment. I plan to remove also the link from the comment, since it is uncommon to insert links in the comments, and the kernel source location for Mips' semid64_ds is mentioned in the commit message anyway.

Thanks!