All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] linux-user: improve target_to_host_sock_type conversion for MIPS
@ 2013-03-18 22:47 Petar Jovanovic
  2013-03-24 10:27 ` Aurelien Jarno
  0 siblings, 1 reply; 3+ messages in thread
From: Petar Jovanovic @ 2013-03-18 22:47 UTC (permalink / raw)
  To: qemu-devel; +Cc: riku.voipio, petar.jovanovic, aurelien

From: Petar Jovanovic <petar.jovanovic@imgtec.com>

Previous implementation has failed to take into account different value of
SOCK_NONBLOCK on target (MIPS) and host, and existence of SOCK_CLOEXEC.
The same conversion has to be applied both for do_socket and do_socketpair,
so the code has been isolated in a static inline function.
It is defined for MIPS target only.

enum sock_type in linux-user/socket.h has been extended to include
TARGET_SOCK_CLOEXEC and TARGET_SOCK_NONBLOCK, similar to definition in libc.
The patch also includes necessary code style changes (tab to spaces) in the
header file in the MIPS #ifdef block touched by the change.

Signed-off-by: Petar Jovanovic <petar.jovanovic@imgtec.com>
---
 linux-user/socket.h  |  172 ++++++++++++++++++++++++++------------------------
 linux-user/syscall.c |   45 ++++++++-----
 2 files changed, 119 insertions(+), 98 deletions(-)

diff --git a/linux-user/socket.h b/linux-user/socket.h
index 339cae5..e4dfe56 100644
--- a/linux-user/socket.h
+++ b/linux-user/socket.h
@@ -1,91 +1,101 @@
 
 #if defined(TARGET_MIPS)
-	// MIPS special values for constants
-
-	/*
-	 * For setsockopt(2)
-	 *
-	 * This defines are ABI conformant as far as Linux supports these ...
-	 */
-	#define TARGET_SOL_SOCKET      0xffff
-
-	#define TARGET_SO_DEBUG        0x0001  /* Record debugging information.  */
-	#define TARGET_SO_REUSEADDR    0x0004  /* Allow reuse of local addresses.  */
-	#define TARGET_SO_KEEPALIVE    0x0008  /* Keep connections alive and send
-					  SIGPIPE when they die.  */
-	#define TARGET_SO_DONTROUTE    0x0010  /* Don't do local routing.  */
-	#define TARGET_SO_BROADCAST    0x0020  /* Allow transmission of
-					  broadcast messages.  */
-	#define TARGET_SO_LINGER       0x0080  /* Block on close of a reliable
-					  socket to transmit pending data.  */
-	#define TARGET_SO_OOBINLINE 0x0100     /* Receive out-of-band data in-band.  */
-	#if 0
-	To add: #define TARGET_SO_REUSEPORT 0x0200     /* Allow local address and port reuse.  */
-	#endif
-
-	#define TARGET_SO_TYPE         0x1008  /* Compatible name for SO_STYLE.  */
-	#define TARGET_SO_STYLE        SO_TYPE /* Synonym */
-	#define TARGET_SO_ERROR        0x1007  /* get error status and clear */
-	#define TARGET_SO_SNDBUF       0x1001  /* Send buffer size. */
-	#define TARGET_SO_RCVBUF       0x1002  /* Receive buffer. */
-	#define TARGET_SO_SNDLOWAT     0x1003  /* send low-water mark */
-	#define TARGET_SO_RCVLOWAT     0x1004  /* receive low-water mark */
-	#define TARGET_SO_SNDTIMEO     0x1005  /* send timeout */
-	#define TARGET_SO_RCVTIMEO     0x1006  /* receive timeout */
-	#define TARGET_SO_ACCEPTCONN   0x1009
-
-	/* linux-specific, might as well be the same as on i386 */
-	#define TARGET_SO_NO_CHECK     11
-	#define TARGET_SO_PRIORITY     12
-	#define TARGET_SO_BSDCOMPAT    14
+    /* MIPS special values for constants */
+
+    /*
+     * For setsockopt(2)
+     *
+     * This defines are ABI conformant as far as Linux supports these ...
+     */
+    #define TARGET_SOL_SOCKET      0xffff
+
+    #define TARGET_SO_DEBUG        0x0001  /* Record debugging information. */
+    #define TARGET_SO_REUSEADDR    0x0004  /* Allow reuse of local addresses. */
+    #define TARGET_SO_KEEPALIVE    0x0008  /* Keep connections alive and send
+                                              SIGPIPE when they die. */
+    #define TARGET_SO_DONTROUTE    0x0010  /* Don't do local routing. */
+    #define TARGET_SO_BROADCAST    0x0020  /* Allow transmission of
+                                              broadcast messages. */
+    #define TARGET_SO_LINGER       0x0080  /* Block on close of a reliable
+                                            * socket to transmit pending data.
+                                            */
+    #define TARGET_SO_OOBINLINE 0x0100     /* Receive out-of-band data in-band.
+                                            */
+    #if 0
+    /* To add: Allow local address and port reuse. */
+    #define TARGET_SO_REUSEPORT 0x0200
+    #endif
+
+    #define TARGET_SO_TYPE         0x1008  /* Compatible name for SO_STYLE. */
+    #define TARGET_SO_STYLE        SO_TYPE /* Synonym */
+    #define TARGET_SO_ERROR        0x1007  /* get error status and clear */
+    #define TARGET_SO_SNDBUF       0x1001  /* Send buffer size. */
+    #define TARGET_SO_RCVBUF       0x1002  /* Receive buffer. */
+    #define TARGET_SO_SNDLOWAT     0x1003  /* send low-water mark */
+    #define TARGET_SO_RCVLOWAT     0x1004  /* receive low-water mark */
+    #define TARGET_SO_SNDTIMEO     0x1005  /* send timeout */
+    #define TARGET_SO_RCVTIMEO     0x1006  /* receive timeout */
+    #define TARGET_SO_ACCEPTCONN   0x1009
 
-	#define TARGET_SO_PASSCRED     17
-	#define TARGET_SO_PEERCRED     18
+    /* linux-specific, might as well be the same as on i386 */
+    #define TARGET_SO_NO_CHECK     11
+    #define TARGET_SO_PRIORITY     12
+    #define TARGET_SO_BSDCOMPAT    14
 
-	/* Security levels - as per NRL IPv6 - don't actually do anything */
-	#define TARGET_SO_SECURITY_AUTHENTICATION              22
-	#define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT        23
-	#define TARGET_SO_SECURITY_ENCRYPTION_NETWORK          24
+    #define TARGET_SO_PASSCRED     17
+    #define TARGET_SO_PEERCRED     18
+
+    /* Security levels - as per NRL IPv6 - don't actually do anything */
+    #define TARGET_SO_SECURITY_AUTHENTICATION              22
+    #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT        23
+    #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK          24
 
-	#define TARGET_SO_BINDTODEVICE         25
+    #define TARGET_SO_BINDTODEVICE         25
 
-	/* Socket filtering */
-	#define TARGET_SO_ATTACH_FILTER        26
-	#define TARGET_SO_DETACH_FILTER        27
+    /* Socket filtering */
+    #define TARGET_SO_ATTACH_FILTER        26
+    #define TARGET_SO_DETACH_FILTER        27
 
-	#define TARGET_SO_PEERNAME             28
-	#define TARGET_SO_TIMESTAMP            29
-	#define SCM_TIMESTAMP          SO_TIMESTAMP
-
-	#define TARGET_SO_PEERSEC              30
-	#define TARGET_SO_SNDBUFFORCE          31
-	#define TARGET_SO_RCVBUFFORCE          33
-
-	/** sock_type - Socket types
-	 *
-	 * Please notice that for binary compat reasons MIPS has to
-	 * override the enum sock_type in include/linux/net.h, so
-	 * we define ARCH_HAS_SOCKET_TYPES here.
-	 *
-	 * @SOCK_DGRAM - datagram (conn.less) socket
-	 * @SOCK_STREAM - stream (connection) socket
-	 * @SOCK_RAW - raw socket
-	 * @SOCK_RDM - reliably-delivered message
-	 * @SOCK_SEQPACKET - sequential packet socket
-	 * @SOCK_PACKET - linux specific way of getting packets at the dev level.
-	 *               For writing rarp and other similar things on the user level.
-	 */
-	enum sock_type {
-	       TARGET_SOCK_DGRAM       = 1,
-	       TARGET_SOCK_STREAM      = 2,
-	       TARGET_SOCK_RAW = 3,
-	       TARGET_SOCK_RDM = 4,
-	       TARGET_SOCK_SEQPACKET   = 5,
-	       TARGET_SOCK_DCCP        = 6,
-	       TARGET_SOCK_PACKET      = 10,
-	};
-
-	#define TARGET_SOCK_MAX (SOCK_PACKET + 1)
+    #define TARGET_SO_PEERNAME             28
+    #define TARGET_SO_TIMESTAMP            29
+    #define SCM_TIMESTAMP          SO_TIMESTAMP
+
+    #define TARGET_SO_PEERSEC              30
+    #define TARGET_SO_SNDBUFFORCE          31
+    #define TARGET_SO_RCVBUFFORCE          33
+
+    /** sock_type - Socket types
+     *
+     * Please notice that for binary compat reasons MIPS has to
+     * override the enum sock_type in include/linux/net.h, so
+     * we define ARCH_HAS_SOCKET_TYPES here.
+     *
+     * @SOCK_DGRAM - datagram (conn.less) socket
+     * @SOCK_STREAM - stream (connection) socket
+     * @SOCK_RAW - raw socket
+     * @SOCK_RDM - reliably-delivered message
+     * @SOCK_SEQPACKET - sequential packet socket
+     * @SOCK_DCCP - Datagram Congestion Control Protocol socket
+     * @SOCK_PACKET - linux specific way of getting packets at the dev level.
+     *                For writing rarp and other similar things on the user
+     *                level.
+     * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
+     * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
+     */
+    enum sock_type {
+           TARGET_SOCK_DGRAM       = 1,
+           TARGET_SOCK_STREAM      = 2,
+           TARGET_SOCK_RAW         = 3,
+           TARGET_SOCK_RDM         = 4,
+           TARGET_SOCK_SEQPACKET   = 5,
+           TARGET_SOCK_DCCP        = 6,
+           TARGET_SOCK_PACKET      = 10,
+           TARGET_SOCK_CLOEXEC     = 02000000,
+           TARGET_SOCK_NONBLOCK    = 0200,
+    };
+
+    #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
+    #define TARGET_SOCK_TYPE_MASK    0xf  /* Covers up to TARGET_SOCK_MAX-1. */
 
 #elif defined(TARGET_ALPHA)
 
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index ee82a2d..0a9e6db 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1859,30 +1859,38 @@ static void unlock_iovec(struct iovec *vec, abi_ulong target_addr,
     free(vec);
 }
 
-/* do_socket() Must return target values and target errnos. */
-static abi_long do_socket(int domain, int type, int protocol)
-{
 #if defined(TARGET_MIPS)
-    switch(type) {
+static inline void target_to_host_sock_type(int *type)
+{
+    int host_type = 0;
+    int target_type = *type;
+
+    switch (target_type & TARGET_SOCK_TYPE_MASK) {
     case TARGET_SOCK_DGRAM:
-        type = SOCK_DGRAM;
+        host_type = SOCK_DGRAM;
         break;
     case TARGET_SOCK_STREAM:
-        type = SOCK_STREAM;
+        host_type = SOCK_STREAM;
         break;
-    case TARGET_SOCK_RAW:
-        type = SOCK_RAW;
-        break;
-    case TARGET_SOCK_RDM:
-        type = SOCK_RDM;
-        break;
-    case TARGET_SOCK_SEQPACKET:
-        type = SOCK_SEQPACKET;
-        break;
-    case TARGET_SOCK_PACKET:
-        type = SOCK_PACKET;
+    default:
+        host_type = target_type & TARGET_SOCK_TYPE_MASK;
         break;
     }
+    if (target_type & TARGET_SOCK_CLOEXEC) {
+        host_type |= SOCK_CLOEXEC;
+    }
+    if (target_type & TARGET_SOCK_NONBLOCK) {
+        host_type |= SOCK_NONBLOCK;
+    }
+    *type = host_type;
+}
+#endif
+
+/* do_socket() Must return target values and target errnos. */
+static abi_long do_socket(int domain, int type, int protocol)
+{
+#if defined(TARGET_MIPS)
+    target_to_host_sock_type(&type);
 #endif
     if (domain == PF_NETLINK)
         return -EAFNOSUPPORT; /* do not NETLINK socket connections possible */
@@ -2116,6 +2124,9 @@ static abi_long do_socketpair(int domain, int type, int protocol,
     int tab[2];
     abi_long ret;
 
+#if defined(TARGET_MIPS)
+    target_to_host_sock_type(&type);
+#endif
     ret = get_errno(socketpair(domain, type, protocol, tab));
     if (!is_error(ret)) {
         if (put_user_s32(tab[0], target_tab_addr)
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH] linux-user: improve target_to_host_sock_type conversion for MIPS
  2013-03-18 22:47 [Qemu-devel] [PATCH] linux-user: improve target_to_host_sock_type conversion for MIPS Petar Jovanovic
@ 2013-03-24 10:27 ` Aurelien Jarno
  2013-04-01 15:23   ` Petar Jovanovic
  0 siblings, 1 reply; 3+ messages in thread
From: Aurelien Jarno @ 2013-03-24 10:27 UTC (permalink / raw)
  To: Petar Jovanovic
  Cc: Blue Swirl, riku.voipio, Richard Henderson, qemu-devel, petar.jovanovic

I am reviewing that for the MIPS part, and I am not really used to the
linux-user code, so a review from someone familiar with it would be
appreciated.

I added Richard Henderson and Blue Swirl in Cc: as alpha and sparc are 
also affected by this issue.

On Mon, Mar 18, 2013 at 11:47:05PM +0100, Petar Jovanovic wrote:
> From: Petar Jovanovic <petar.jovanovic@imgtec.com>
> 
> Previous implementation has failed to take into account different value of
> SOCK_NONBLOCK on target (MIPS) and host, and existence of SOCK_CLOEXEC.
> The same conversion has to be applied both for do_socket and do_socketpair,
> so the code has been isolated in a static inline function.
> It is defined for MIPS target only.
> 
> enum sock_type in linux-user/socket.h has been extended to include
> TARGET_SOCK_CLOEXEC and TARGET_SOCK_NONBLOCK, similar to definition in libc.
> The patch also includes necessary code style changes (tab to spaces) in the
> header file in the MIPS #ifdef block touched by the change.
> 
> Signed-off-by: Petar Jovanovic <petar.jovanovic@imgtec.com>
> ---
>  linux-user/socket.h  |  172 ++++++++++++++++++++++++++------------------------
>  linux-user/syscall.c |   45 ++++++++-----
>  2 files changed, 119 insertions(+), 98 deletions(-)
> 
> diff --git a/linux-user/socket.h b/linux-user/socket.h
> index 339cae5..e4dfe56 100644
> --- a/linux-user/socket.h
> +++ b/linux-user/socket.h
> @@ -1,91 +1,101 @@
>  
>  #if defined(TARGET_MIPS)
> -	// MIPS special values for constants
> -
> -	/*
> -	 * For setsockopt(2)
> -	 *
> -	 * This defines are ABI conformant as far as Linux supports these ...
> -	 */
> -	#define TARGET_SOL_SOCKET      0xffff
> -
> -	#define TARGET_SO_DEBUG        0x0001  /* Record debugging information.  */
> -	#define TARGET_SO_REUSEADDR    0x0004  /* Allow reuse of local addresses.  */
> -	#define TARGET_SO_KEEPALIVE    0x0008  /* Keep connections alive and send
> -					  SIGPIPE when they die.  */
> -	#define TARGET_SO_DONTROUTE    0x0010  /* Don't do local routing.  */
> -	#define TARGET_SO_BROADCAST    0x0020  /* Allow transmission of
> -					  broadcast messages.  */
> -	#define TARGET_SO_LINGER       0x0080  /* Block on close of a reliable
> -					  socket to transmit pending data.  */
> -	#define TARGET_SO_OOBINLINE 0x0100     /* Receive out-of-band data in-band.  */
> -	#if 0
> -	To add: #define TARGET_SO_REUSEPORT 0x0200     /* Allow local address and port reuse.  */
> -	#endif
> -
> -	#define TARGET_SO_TYPE         0x1008  /* Compatible name for SO_STYLE.  */
> -	#define TARGET_SO_STYLE        SO_TYPE /* Synonym */
> -	#define TARGET_SO_ERROR        0x1007  /* get error status and clear */
> -	#define TARGET_SO_SNDBUF       0x1001  /* Send buffer size. */
> -	#define TARGET_SO_RCVBUF       0x1002  /* Receive buffer. */
> -	#define TARGET_SO_SNDLOWAT     0x1003  /* send low-water mark */
> -	#define TARGET_SO_RCVLOWAT     0x1004  /* receive low-water mark */
> -	#define TARGET_SO_SNDTIMEO     0x1005  /* send timeout */
> -	#define TARGET_SO_RCVTIMEO     0x1006  /* receive timeout */
> -	#define TARGET_SO_ACCEPTCONN   0x1009
> -
> -	/* linux-specific, might as well be the same as on i386 */
> -	#define TARGET_SO_NO_CHECK     11
> -	#define TARGET_SO_PRIORITY     12
> -	#define TARGET_SO_BSDCOMPAT    14
> +    /* MIPS special values for constants */
> +
> +    /*
> +     * For setsockopt(2)
> +     *
> +     * This defines are ABI conformant as far as Linux supports these ...
> +     */
> +    #define TARGET_SOL_SOCKET      0xffff
> +
> +    #define TARGET_SO_DEBUG        0x0001  /* Record debugging information. */
> +    #define TARGET_SO_REUSEADDR    0x0004  /* Allow reuse of local addresses. */
> +    #define TARGET_SO_KEEPALIVE    0x0008  /* Keep connections alive and send
> +                                              SIGPIPE when they die. */
> +    #define TARGET_SO_DONTROUTE    0x0010  /* Don't do local routing. */
> +    #define TARGET_SO_BROADCAST    0x0020  /* Allow transmission of
> +                                              broadcast messages. */
> +    #define TARGET_SO_LINGER       0x0080  /* Block on close of a reliable
> +                                            * socket to transmit pending data.
> +                                            */
> +    #define TARGET_SO_OOBINLINE 0x0100     /* Receive out-of-band data in-band.
> +                                            */
> +    #if 0
> +    /* To add: Allow local address and port reuse. */
> +    #define TARGET_SO_REUSEPORT 0x0200
> +    #endif
> +
> +    #define TARGET_SO_TYPE         0x1008  /* Compatible name for SO_STYLE. */
> +    #define TARGET_SO_STYLE        SO_TYPE /* Synonym */
> +    #define TARGET_SO_ERROR        0x1007  /* get error status and clear */
> +    #define TARGET_SO_SNDBUF       0x1001  /* Send buffer size. */
> +    #define TARGET_SO_RCVBUF       0x1002  /* Receive buffer. */
> +    #define TARGET_SO_SNDLOWAT     0x1003  /* send low-water mark */
> +    #define TARGET_SO_RCVLOWAT     0x1004  /* receive low-water mark */
> +    #define TARGET_SO_SNDTIMEO     0x1005  /* send timeout */
> +    #define TARGET_SO_RCVTIMEO     0x1006  /* receive timeout */
> +    #define TARGET_SO_ACCEPTCONN   0x1009
>  
> -	#define TARGET_SO_PASSCRED     17
> -	#define TARGET_SO_PEERCRED     18
> +    /* linux-specific, might as well be the same as on i386 */
> +    #define TARGET_SO_NO_CHECK     11
> +    #define TARGET_SO_PRIORITY     12
> +    #define TARGET_SO_BSDCOMPAT    14
>  
> -	/* Security levels - as per NRL IPv6 - don't actually do anything */
> -	#define TARGET_SO_SECURITY_AUTHENTICATION              22
> -	#define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT        23
> -	#define TARGET_SO_SECURITY_ENCRYPTION_NETWORK          24
> +    #define TARGET_SO_PASSCRED     17
> +    #define TARGET_SO_PEERCRED     18
> +
> +    /* Security levels - as per NRL IPv6 - don't actually do anything */
> +    #define TARGET_SO_SECURITY_AUTHENTICATION              22
> +    #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT        23
> +    #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK          24
>  
> -	#define TARGET_SO_BINDTODEVICE         25
> +    #define TARGET_SO_BINDTODEVICE         25
>  
> -	/* Socket filtering */
> -	#define TARGET_SO_ATTACH_FILTER        26
> -	#define TARGET_SO_DETACH_FILTER        27
> +    /* Socket filtering */
> +    #define TARGET_SO_ATTACH_FILTER        26
> +    #define TARGET_SO_DETACH_FILTER        27
>  
> -	#define TARGET_SO_PEERNAME             28
> -	#define TARGET_SO_TIMESTAMP            29
> -	#define SCM_TIMESTAMP          SO_TIMESTAMP
> -
> -	#define TARGET_SO_PEERSEC              30
> -	#define TARGET_SO_SNDBUFFORCE          31
> -	#define TARGET_SO_RCVBUFFORCE          33
> -
> -	/** sock_type - Socket types
> -	 *
> -	 * Please notice that for binary compat reasons MIPS has to
> -	 * override the enum sock_type in include/linux/net.h, so
> -	 * we define ARCH_HAS_SOCKET_TYPES here.
> -	 *
> -	 * @SOCK_DGRAM - datagram (conn.less) socket
> -	 * @SOCK_STREAM - stream (connection) socket
> -	 * @SOCK_RAW - raw socket
> -	 * @SOCK_RDM - reliably-delivered message
> -	 * @SOCK_SEQPACKET - sequential packet socket
> -	 * @SOCK_PACKET - linux specific way of getting packets at the dev level.
> -	 *               For writing rarp and other similar things on the user level.
> -	 */
> -	enum sock_type {
> -	       TARGET_SOCK_DGRAM       = 1,
> -	       TARGET_SOCK_STREAM      = 2,
> -	       TARGET_SOCK_RAW = 3,
> -	       TARGET_SOCK_RDM = 4,
> -	       TARGET_SOCK_SEQPACKET   = 5,
> -	       TARGET_SOCK_DCCP        = 6,
> -	       TARGET_SOCK_PACKET      = 10,
> -	};
> -
> -	#define TARGET_SOCK_MAX (SOCK_PACKET + 1)
> +    #define TARGET_SO_PEERNAME             28
> +    #define TARGET_SO_TIMESTAMP            29
> +    #define SCM_TIMESTAMP          SO_TIMESTAMP
> +
> +    #define TARGET_SO_PEERSEC              30
> +    #define TARGET_SO_SNDBUFFORCE          31
> +    #define TARGET_SO_RCVBUFFORCE          33
> +
> +    /** sock_type - Socket types
> +     *
> +     * Please notice that for binary compat reasons MIPS has to
> +     * override the enum sock_type in include/linux/net.h, so
> +     * we define ARCH_HAS_SOCKET_TYPES here.
> +     *

Not related to your changes, but note that ARCH_HAS_SOCKET_TYPES is not
defined anywhere contrary to what is said in the comment. It might be a
good idea to revive it, as at least alpha and sparc also have the issue.

> +     * @SOCK_DGRAM - datagram (conn.less) socket
> +     * @SOCK_STREAM - stream (connection) socket
> +     * @SOCK_RAW - raw socket
> +     * @SOCK_RDM - reliably-delivered message
> +     * @SOCK_SEQPACKET - sequential packet socket
> +     * @SOCK_DCCP - Datagram Congestion Control Protocol socket
> +     * @SOCK_PACKET - linux specific way of getting packets at the dev level.
> +     *                For writing rarp and other similar things on the user
> +     *                level.
> +     * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
> +     * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
> +     */
> +    enum sock_type {
> +           TARGET_SOCK_DGRAM       = 1,
> +           TARGET_SOCK_STREAM      = 2,
> +           TARGET_SOCK_RAW         = 3,
> +           TARGET_SOCK_RDM         = 4,
> +           TARGET_SOCK_SEQPACKET   = 5,
> +           TARGET_SOCK_DCCP        = 6,
> +           TARGET_SOCK_PACKET      = 10,
> +           TARGET_SOCK_CLOEXEC     = 02000000,
> +           TARGET_SOCK_NONBLOCK    = 0200,
> +    };
> +
> +    #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
> +    #define TARGET_SOCK_TYPE_MASK    0xf  /* Covers up to TARGET_SOCK_MAX-1. */
>  
>  #elif defined(TARGET_ALPHA)
>  
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index ee82a2d..0a9e6db 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1859,30 +1859,38 @@ static void unlock_iovec(struct iovec *vec, abi_ulong target_addr,
>      free(vec);
>  }
>  
> -/* do_socket() Must return target values and target errnos. */
> -static abi_long do_socket(int domain, int type, int protocol)
> -{
>  #if defined(TARGET_MIPS)
> -    switch(type) {
> +static inline void target_to_host_sock_type(int *type)
> +{
> +    int host_type = 0;
> +    int target_type = *type;
> +
> +    switch (target_type & TARGET_SOCK_TYPE_MASK) {
>      case TARGET_SOCK_DGRAM:
> -        type = SOCK_DGRAM;
> +        host_type = SOCK_DGRAM;
>          break;
>      case TARGET_SOCK_STREAM:
> -        type = SOCK_STREAM;
> +        host_type = SOCK_STREAM;
>          break;
> -    case TARGET_SOCK_RAW:
> -        type = SOCK_RAW;
> -        break;
> -    case TARGET_SOCK_RDM:
> -        type = SOCK_RDM;
> -        break;
> -    case TARGET_SOCK_SEQPACKET:
> -        type = SOCK_SEQPACKET;
> -        break;
> -    case TARGET_SOCK_PACKET:
> -        type = SOCK_PACKET;
> +    default:
> +        host_type = target_type & TARGET_SOCK_TYPE_MASK;
>          break;
>      }

I am not sure we want to really copy the type without more checking than
the mask: a new value still within the mask limit could be added
differently depending on the architecture.

> +    if (target_type & TARGET_SOCK_CLOEXEC) {
> +        host_type |= SOCK_CLOEXEC;
> +    }
> +    if (target_type & TARGET_SOCK_NONBLOCK) {
> +        host_type |= SOCK_NONBLOCK;
> +    }
> +    *type = host_type;

Also I think the values should be checked in all cases returning -EINVAL
if not supported. On other architecture where no translation is done,
this is done by the host kernel. With the code above, unsupported
arguments are simply ignored.

> +}
> +#endif
> +
> +/* do_socket() Must return target values and target errnos. */
> +static abi_long do_socket(int domain, int type, int protocol)
> +{
> +#if defined(TARGET_MIPS)
> +    target_to_host_sock_type(&type);
>  #endif

As said above this could probably be conditional on
ARCH_HAS_SOCKET_TYPES to make it more generic.

>      if (domain == PF_NETLINK)
>          return -EAFNOSUPPORT; /* do not NETLINK socket connections possible */
> @@ -2116,6 +2124,9 @@ static abi_long do_socketpair(int domain, int type, int protocol,
>      int tab[2];
>      abi_long ret;
>  
> +#if defined(TARGET_MIPS)
> +    target_to_host_sock_type(&type);
> +#endif
>      ret = get_errno(socketpair(domain, type, protocol, tab));
>      if (!is_error(ret)) {
>          if (put_user_s32(tab[0], target_tab_addr)
> -- 
> 1.7.9.5
> 
> 

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH] linux-user: improve target_to_host_sock_type conversion for MIPS
  2013-03-24 10:27 ` Aurelien Jarno
@ 2013-04-01 15:23   ` Petar Jovanovic
  0 siblings, 0 replies; 3+ messages in thread
From: Petar Jovanovic @ 2013-04-01 15:23 UTC (permalink / raw)
  To: Aurelien Jarno, Petar Jovanovic
  Cc: Blue Swirl, riku.voipio, qemu-devel, Richard Henderson


________________________________________
From: Aurelien Jarno [aurelien@aurel32.net]
Sent: Sunday, March 24, 2013 11:27 AM
To: Petar Jovanovic
Cc: qemu-devel@nongnu.org; Petar Jovanovic; riku.voipio@linaro.org; Richard Henderson; Blue Swirl
Subject: Re: [PATCH] linux-user: improve target_to_host_sock_type conversion for MIPS

> Not related to your changes, but note that ARCH_HAS_SOCKET_TYPES is not
> defined anywhere contrary to what is said in the comment. It might be a
> good idea to revive it, as at least alpha and sparc also have the issue.

I have just revived it. It is defined for MIPS, ALPHA and SPARC.
I will update the patch with new changes today.

> +     * @SOCK_DGRAM - datagram (conn.less) socket
> +     * @SOCK_STREAM - stream (connection) socket
> +     * @SOCK_RAW - raw socket
> +     * @SOCK_RDM - reliably-delivered message
> +     * @SOCK_SEQPACKET - sequential packet socket
> +     * @SOCK_DCCP - Datagram Congestion Control Protocol socket
> +     * @SOCK_PACKET - linux specific way of getting packets at the dev level.
> +     *                For writing rarp and other similar things on the user
> +     *                level.
> +     * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
> +     * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
> +     */
> +    enum sock_type {
> +           TARGET_SOCK_DGRAM       = 1,
> +           TARGET_SOCK_STREAM      = 2,
> +           TARGET_SOCK_RAW         = 3,
> +           TARGET_SOCK_RDM         = 4,
> +           TARGET_SOCK_SEQPACKET   = 5,
> +           TARGET_SOCK_DCCP        = 6,
> +           TARGET_SOCK_PACKET      = 10,
> +           TARGET_SOCK_CLOEXEC     = 02000000,
> +           TARGET_SOCK_NONBLOCK    = 0200,
> +    };
> +
> +    #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
> +    #define TARGET_SOCK_TYPE_MASK    0xf  /* Covers up to TARGET_SOCK_MAX-1. */
>
>  #elif defined(TARGET_ALPHA)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index ee82a2d..0a9e6db 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1859,30 +1859,38 @@ static void unlock_iovec(struct iovec *vec, abi_ulong target_addr,
>      free(vec);
>  }
>
> -/* do_socket() Must return target values and target errnos. */
> -static abi_long do_socket(int domain, int type, int protocol)
> -{
>  #if defined(TARGET_MIPS)
> -    switch(type) {
> +static inline void target_to_host_sock_type(int *type)
> +{
> +    int host_type = 0;
> +    int target_type = *type;
> +
> +    switch (target_type & TARGET_SOCK_TYPE_MASK) {
>      case TARGET_SOCK_DGRAM:
> -        type = SOCK_DGRAM;
> +        host_type = SOCK_DGRAM;
>          break;
>      case TARGET_SOCK_STREAM:
> -        type = SOCK_STREAM;
> +        host_type = SOCK_STREAM;
>          break;
> -    case TARGET_SOCK_RAW:
> -        type = SOCK_RAW;
> -        break;
> -    case TARGET_SOCK_RDM:
> -        type = SOCK_RDM;
> -        break;
> -    case TARGET_SOCK_SEQPACKET:
> -        type = SOCK_SEQPACKET;
> -        break;
> -    case TARGET_SOCK_PACKET:
> -        type = SOCK_PACKET;
> +    default:
> +        host_type = target_type & TARGET_SOCK_TYPE_MASK;
>          break;
>      }

> I am not sure we want to really copy the type without more checking than
> the mask: a new value still within the mask limit could be added
> differently depending on the architecture.

I believe the current solution is applicable to all architectures that use
this function. 

> +    if (target_type & TARGET_SOCK_CLOEXEC) {
> +        host_type |= SOCK_CLOEXEC;
> +    }
> +    if (target_type & TARGET_SOCK_NONBLOCK) {
> +        host_type |= SOCK_NONBLOCK;
> +    }
> +    *type = host_type;

> Also I think the values should be checked in all cases returning -EINVAL
> if not supported. On other architecture where no translation is done,
> this is done by the host kernel. With the code above, unsupported
> arguments are simply ignored.

The kernel will return unsupported value if it does not support it.
One of the reasons to copy the type without more checking in other cases is
to let kernel return an error for unsupported types. Socket type is not a
'bit-flag', so conversion from target to host is tricky for irregular cases.

Petar

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-04-01 15:53 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-18 22:47 [Qemu-devel] [PATCH] linux-user: improve target_to_host_sock_type conversion for MIPS Petar Jovanovic
2013-03-24 10:27 ` Aurelien Jarno
2013-04-01 15:23   ` Petar Jovanovic

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.