Re: [Qemu-devel] [PATCH v3 6/8] linux-user: update ARCH_HAS_SOCKET_TYPES use

2018-05-24 Thread Peter Maydell 
On 24 May 2018 at 16:50, Laurent Vivier  wrote:
> Le 24/05/2018 à 17:29, Laurent Vivier a écrit :
>> Le 21/05/2018 à 11:19, Peter Maydell a écrit :
>>> On 19 May 2018 at 10:29, Laurent Vivier  wrote:
 to be like in the kernel and rename it TARGET_ARCH_HAS_SOCKET_TYPES
>>>
>>> You could note in the commit message that this fixes our
>>> incorrect definition of TARGET_SOCK_CLOEXEC for SPARC.
>>
>> I agree
>
> In fact it doesn't change the value of TARGET_SOCK_CLOEXEC for SPARC,
> because original value is 02000 (0x40) and the new value from
> linux-user/socket.h is TARGET_O_CLOEXEC which is also 0x40 for SPARC.

Argh, octal vs hex. Thanks for checking that.

-- PMM

Re: [Qemu-devel] [PATCH v3 6/8] linux-user: update ARCH_HAS_SOCKET_TYPES use

2018-05-24 Thread Laurent Vivier 
Le 24/05/2018 à 17:29, Laurent Vivier a écrit :
> Le 21/05/2018 à 11:19, Peter Maydell a écrit :
>> On 19 May 2018 at 10:29, Laurent Vivier  wrote:
>>> to be like in the kernel and rename it TARGET_ARCH_HAS_SOCKET_TYPES
>>
>> You could note in the commit message that this fixes our
>> incorrect definition of TARGET_SOCK_CLOEXEC for SPARC.
> 
> I agree

In fact it doesn't change the value of TARGET_SOCK_CLOEXEC for SPARC,
because original value is 02000 (0x40) and the new value from
linux-user/socket.h is TARGET_O_CLOEXEC which is also 0x40 for SPARC.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v3 6/8] linux-user: update ARCH_HAS_SOCKET_TYPES use

2018-05-24 Thread Laurent Vivier 
Le 21/05/2018 à 11:19, Peter Maydell a écrit :
> On 19 May 2018 at 10:29, Laurent Vivier  wrote:
>> to be like in the kernel and rename it TARGET_ARCH_HAS_SOCKET_TYPES
> 
> You could note in the commit message that this fixes our
> incorrect definition of TARGET_SOCK_CLOEXEC for SPARC.

I agree

>> Signed-off-by: Laurent Vivier 
>> ---
>>  linux-user/alpha/sockbits.h | 36 +++---
>>  linux-user/hppa/sockbits.h  | 33 +++-
>>  linux-user/mips/sockbits.h  |  9 ---
>>  linux-user/socket.h | 62 
>> +++--
>>  linux-user/sparc/sockbits.h | 36 --
>>  5 files changed, 44 insertions(+), 132 deletions(-)
>>
>> diff --git a/linux-user/alpha/sockbits.h b/linux-user/alpha/sockbits.h
>> index 4db3e52b67..f5397dd875 100644
>> --- a/linux-user/alpha/sockbits.h
>> +++ b/linux-user/alpha/sockbits.h
>> @@ -75,39 +75,9 @@
>>  /* Instruct lower device to use last 4-bytes of skb data as FCS */
>>  #define TARGET_SO_NOFCS 43
>>
>> -/** sock_type - Socket types
>> - *
>> - * Please notice that for binary compat reasons ALPHA 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.
>> +/* TARGET_O_NONBLOCK clashes with the bits used for socket types.  
>> Therefore we
>> + * have to define SOCK_NONBLOCK to a different value here.
>>   */
>> +#define TARGET_SOCK_NONBLOCK   0x4000
> 
> This new value is different from the old value in the enum:
> 
>> -#define ARCH_HAS_SOCKET_TYPES  1
>> -
>> -enum sock_type {
>> -   TARGET_SOCK_STREAM  = 1,
>> -   TARGET_SOCK_DGRAM   = 2,
>> -   TARGET_SOCK_RAW = 3,
>> -   TARGET_SOCK_RDM = 4,
>> -   TARGET_SOCK_SEQPACKET   = 5,
>> -   TARGET_SOCK_DCCP= 6,
>> -   TARGET_SOCK_PACKET  = 10,
>> -   TARGET_SOCK_CLOEXEC = 01000,
>> -   TARGET_SOCK_NONBLOCK= 0100,
> 
> ...does it matter?

It's what we could think at first glance (I did), but

0x4000   (hex) is
0100 (octal)

Does this answer to your comment?

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v3 6/8] linux-user: update ARCH_HAS_SOCKET_TYPES use

2018-05-21 Thread Peter Maydell 
On 19 May 2018 at 10:29, Laurent Vivier  wrote:
> to be like in the kernel and rename it TARGET_ARCH_HAS_SOCKET_TYPES

You could note in the commit message that this fixes our
incorrect definition of TARGET_SOCK_CLOEXEC for SPARC.

> Signed-off-by: Laurent Vivier 
> ---
>  linux-user/alpha/sockbits.h | 36 +++---
>  linux-user/hppa/sockbits.h  | 33 +++-
>  linux-user/mips/sockbits.h  |  9 ---
>  linux-user/socket.h | 62 
> +++--
>  linux-user/sparc/sockbits.h | 36 --
>  5 files changed, 44 insertions(+), 132 deletions(-)
>
> diff --git a/linux-user/alpha/sockbits.h b/linux-user/alpha/sockbits.h
> index 4db3e52b67..f5397dd875 100644
> --- a/linux-user/alpha/sockbits.h
> +++ b/linux-user/alpha/sockbits.h
> @@ -75,39 +75,9 @@
>  /* Instruct lower device to use last 4-bytes of skb data as FCS */
>  #define TARGET_SO_NOFCS 43
>
> -/** sock_type - Socket types
> - *
> - * Please notice that for binary compat reasons ALPHA 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.
> +/* TARGET_O_NONBLOCK clashes with the bits used for socket types.  Therefore 
> we
> + * have to define SOCK_NONBLOCK to a different value here.
>   */
> +#define TARGET_SOCK_NONBLOCK   0x4000

This new value is different from the old value in the enum:

> -#define ARCH_HAS_SOCKET_TYPES  1
> -
> -enum sock_type {
> -   TARGET_SOCK_STREAM  = 1,
> -   TARGET_SOCK_DGRAM   = 2,
> -   TARGET_SOCK_RAW = 3,
> -   TARGET_SOCK_RDM = 4,
> -   TARGET_SOCK_SEQPACKET   = 5,
> -   TARGET_SOCK_DCCP= 6,
> -   TARGET_SOCK_PACKET  = 10,
> -   TARGET_SOCK_CLOEXEC = 01000,
> -   TARGET_SOCK_NONBLOCK= 0100,

...does it matter?

Neither are the same as Alpha's actual SOCK_NONBLOCK value,
which as far as I can tell is 4. The comment suggests that we're
deliberately avoiding a clash with the socket-type values, but
do you know how this works? I don't see how we can do the right
thing if we're not using the actual bit values the guest wants...

Otherwise
Reviewed-by: Peter Maydell 

thanks
-- PMM

[Qemu-devel] [PATCH v3 6/8] linux-user: update ARCH_HAS_SOCKET_TYPES use

2018-05-19 Thread Laurent Vivier 
to be like in the kernel and rename it TARGET_ARCH_HAS_SOCKET_TYPES

Signed-off-by: Laurent Vivier 
---
 linux-user/alpha/sockbits.h | 36 +++---
 linux-user/hppa/sockbits.h  | 33 +++-
 linux-user/mips/sockbits.h  |  9 ---
 linux-user/socket.h | 62 +++--
 linux-user/sparc/sockbits.h | 36 --
 5 files changed, 44 insertions(+), 132 deletions(-)

diff --git a/linux-user/alpha/sockbits.h b/linux-user/alpha/sockbits.h
index 4db3e52b67..f5397dd875 100644
--- a/linux-user/alpha/sockbits.h
+++ b/linux-user/alpha/sockbits.h
@@ -75,39 +75,9 @@
 /* Instruct lower device to use last 4-bytes of skb data as FCS */
 #define TARGET_SO_NOFCS 43
 
-/** sock_type - Socket types
- *
- * Please notice that for binary compat reasons ALPHA 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.
+/* TARGET_O_NONBLOCK clashes with the bits used for socket types.  Therefore we
+ * have to define SOCK_NONBLOCK to a different value here.
  */
+#define TARGET_SOCK_NONBLOCK   0x4000
 
-#define ARCH_HAS_SOCKET_TYPES  1
-
-enum sock_type {
-   TARGET_SOCK_STREAM  = 1,
-   TARGET_SOCK_DGRAM   = 2,
-   TARGET_SOCK_RAW = 3,
-   TARGET_SOCK_RDM = 4,
-   TARGET_SOCK_SEQPACKET   = 5,
-   TARGET_SOCK_DCCP= 6,
-   TARGET_SOCK_PACKET  = 10,
-   TARGET_SOCK_CLOEXEC = 01000,
-   TARGET_SOCK_NONBLOCK= 0100,
-};
-
-#define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
-#define TARGET_SOCK_TYPE_MASK0xf  /* Covers up to TARGET_SOCK_MAX-1. */
 #endif
diff --git a/linux-user/hppa/sockbits.h b/linux-user/hppa/sockbits.h
index 5044619e16..2641aea859 100644
--- a/linux-user/hppa/sockbits.h
+++ b/linux-user/hppa/sockbits.h
@@ -64,34 +64,7 @@
 
 #define TARGET_SO_CNX_ADVICE   0x402E
 
-/** sock_type - Socket types - default values
- *
- *
- * @SOCK_STREAM - stream (connection) socket
- * @SOCK_DGRAM - datagram (conn.less) 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.
+/* TARGET_O_NONBLOCK clashes with the bits used for socket types.  Therefore we
+ * have to define SOCK_NONBLOCK to a different value here.
  */
-enum sock_type {
-   TARGET_SOCK_STREAM  = 1,
-   TARGET_SOCK_DGRAM   = 2,
-   TARGET_SOCK_RAW = 3,
-   TARGET_SOCK_RDM = 4,
-   TARGET_SOCK_SEQPACKET   = 5,
-   TARGET_SOCK_DCCP= 6,
-   TARGET_SOCK_PACKET  = 10,
-   TARGET_SOCK_CLOEXEC = 01000,
-   TARGET_SOCK_NONBLOCK= 0x4000,
-};
-
-#define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
-#define TARGET_SOCK_TYPE_MASK0xf  /* Covers up to TARGET_SOCK_MAX-1. */
-
-#define ARCH_HAS_SOCKET_TYPES 1
+#define TARGET_SOCK_NONBLOCK   0x4000
diff --git a/linux-user/mips/sockbits.h b/linux-user/mips/sockbits.h
index 3fe5ac88e7..370d13ed86 100644
--- a/linux-user/mips/sockbits.h
+++ b/linux-user/mips/sockbits.h
@@ -91,7 +91,7 @@
  * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
  */
 
-#define ARCH_HAS_SOCKET_TYPES  1
+#define TARGET_ARCH_HAS_SOCKET_TYPES  1
 
 enum sock_type {
TARGET_SOCK_DGRAM   = 1,
@@ -101,10 +101,13 @@ enum sock_type {
TARGET_SOCK_SEQPACKET   = 5,
TARGET_SOCK_DCCP= 6,
TARGET_SOCK_PACKET  = 10,
-   TARGET_SOCK_CLOEXEC = 0200,
-   TARGET_SOCK_NONBLOCK= 0200,
 };
 
 #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
 #define TARGET_SOCK_TYPE_MASK0xf  /* Covers up to TARGET_SOCK_MAX-1. */
+
+/* Flags for socket, socketpair, paccept */
+#define TARGET_SOCK_CLOEXECTARGET_O_CLOEXEC
+#define TARGET_SOCK_NONBLOCK   TARGET_O_NONBLOCK
+
 #endif
diff --git a/linux-user/socket.h b/linux-user/socket.h
index 135f438bdf..4c0b5c2dfa 100644
--- a/linux-user/socket.h
+++ b/linux-user/socket.h
@@ -1,35 +1,37 @@
-
 #include "sockbits.h"
 
-#ifndef ARCH_HAS_SOCKET_TYPES
-/**