qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] linux-user: correctly align types in thunking code
@ 2012-12-08 15:18 Laurent Vivier
  2012-12-08 16:40 ` Peter Maydell
  2012-12-08 16:55 ` Andreas Färber
  0 siblings, 2 replies; 9+ messages in thread
From: Laurent Vivier @ 2012-12-08 15:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: Riku Voipio, Laurent Vivier

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 thunk.h |   22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/thunk.h b/thunk.h
index 87025c3..d3e9f3d 100644
--- a/thunk.h
+++ b/thunk.h
@@ -151,20 +151,32 @@ static inline int thunk_type_align(const argtype *type_ptr, int is_host)
     case TYPE_CHAR:
         return 1;
     case TYPE_SHORT:
-        return 2;
+        if (is_host) {
+            return __alignof__(short);
+        } else {
+            return TARGET_SHORT_ALIGNMENT;
+        }
     case TYPE_INT:
-        return 4;
+        if (is_host) {
+            return __alignof__(int);
+        } else {
+            return TARGET_INT_ALIGNMENT;
+        }
     case TYPE_LONGLONG:
     case TYPE_ULONGLONG:
-        return 8;
+        if (is_host) {
+            return __alignof__(long long);
+        } else {
+            return TARGET_LLONG_ALIGNMENT;
+        }
     case TYPE_LONG:
     case TYPE_ULONG:
     case TYPE_PTRVOID:
     case TYPE_PTR:
         if (is_host) {
-            return sizeof(void *);
+            return __alignof__(long);
         } else {
-            return TARGET_ABI_BITS / 8;
+            return TARGET_LONG_ALIGNMENT;
         }
         break;
     case TYPE_OLDDEVT:
-- 
1.7.10.4

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

* Re: [Qemu-devel] [PATCH] linux-user: correctly align types in thunking code
  2012-12-08 15:18 Laurent Vivier
@ 2012-12-08 16:40 ` Peter Maydell
  2012-12-08 17:10   ` Laurent Vivier
  2012-12-08 16:55 ` Andreas Färber
  1 sibling, 1 reply; 9+ messages in thread
From: Peter Maydell @ 2012-12-08 16:40 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: Riku Voipio, qemu-devel

On 8 December 2012 15:18, Laurent Vivier <laurent@vivier.eu> wrote:
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>

This kind of patch really needs an explanation (and ideally
test case) of what the bug is that it is attempting to fix...

thanks
-- PMM

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

* Re: [Qemu-devel] [PATCH] linux-user: correctly align types in thunking code
  2012-12-08 15:18 Laurent Vivier
  2012-12-08 16:40 ` Peter Maydell
@ 2012-12-08 16:55 ` Andreas Färber
  2012-12-08 17:13   ` Laurent Vivier
  1 sibling, 1 reply; 9+ messages in thread
From: Andreas Färber @ 2012-12-08 16:55 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: Peter Maydell, Riku Voipio, qemu-devel

Am 08.12.2012 16:18, schrieb Laurent Vivier:
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  thunk.h |   22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/thunk.h b/thunk.h
> index 87025c3..d3e9f3d 100644
> --- a/thunk.h
> +++ b/thunk.h
> @@ -151,20 +151,32 @@ static inline int thunk_type_align(const argtype *type_ptr, int is_host)
>      case TYPE_CHAR:
>          return 1;
>      case TYPE_SHORT:
> -        return 2;
> +        if (is_host) {
> +            return __alignof__(short);

Might __alignof__() depend on a certain GCC version? Is it supported by
clang?

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

* Re: [Qemu-devel] [PATCH] linux-user: correctly align types in thunking code
  2012-12-08 16:40 ` Peter Maydell
@ 2012-12-08 17:10   ` Laurent Vivier
  0 siblings, 0 replies; 9+ messages in thread
From: Laurent Vivier @ 2012-12-08 17:10 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Riku Voipio, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 3441 bytes --]

Le samedi 08 décembre 2012 à 16:40 +0000, Peter Maydell a écrit :
> On 8 December 2012 15:18, Laurent Vivier <laurent@vivier.eu> wrote:
> > Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> 
> This kind of patch really needs an explanation (and ideally
> test case) of what the bug is that it is attempting to fix...

Yes... of course, but sometime no one reads my patches, so I was
lazy ;-)

The 3 first patches I sent today allow to run netstat and route in a
linux container with qemu linux-user (qemu-m68k in my case).

The first one, obviously, allows to have IP addresses in correct order
in the case of "netstat -nr".

The second one, allows to use the command "route". This is a follow up
of patch:

        commit c2e3dee6e03527baf8698698cce76b1a3174969a
        Author: Laurent Vivier <laurent@vivier.eu>
        Date:   Sun Feb 13 23:37:34 2011 +0100
        
            linux-user: Define target alignment size

In my case m68k aligns "int" on 2 not 4. You can check this with the
following program:

#include <stdio.h>
#include <net/route.h>
#include <stddef.h>

int main(void)
{
        struct rtentry rt;
        printf("rt_pad1 %ld %zd\n", offsetof(struct rtentry, rt_pad1),
                sizeof(rt.rt_pad1));
        printf("rt_dst %ld %zd\n", offsetof(struct rtentry, rt_dst),
                sizeof(rt.rt_dst));
        printf("rt_gateway %ld %zd\n", offsetof(struct rtentry, rt_gateway),
                sizeof(rt.rt_gateway));
        printf("rt_genmask %ld %zd\n", offsetof(struct rtentry, rt_genmask),
                sizeof(rt.rt_genmask));
        printf("rt_flags %ld %zd\n", offsetof(struct rtentry, rt_flags),
                sizeof(rt.rt_flags));
        printf("rt_pad2 %ld %zd\n", offsetof(struct rtentry, rt_pad2),
                sizeof(rt.rt_pad2));
        printf("rt_pad3 %ld %zd\n", offsetof(struct rtentry, rt_pad3),
                sizeof(rt.rt_pad3));
        printf("rt_pad4 %ld %zd\n", offsetof(struct rtentry, rt_pad4),
                sizeof(rt.rt_pad4));
        printf("rt_metric %ld %zd\n", offsetof(struct rtentry, rt_metric),
                sizeof(rt.rt_metric));
        printf("rt_dev %ld %zd\n", offsetof(struct rtentry, rt_dev),
                sizeof(rt.rt_dev));
}

On x86_64:

        rt_pad1 0 8
        rt_dst 8 16
        rt_gateway 24 16
        rt_genmask 40 16
        rt_flags 56 2
        rt_pad2 58 2
        rt_pad3 64 8
        rt_pad4 74 6
        rt_metric 80 2
        rt_dev 88 8

on m68k:

        rt_pad1 0 4
        rt_dst 4 16
        rt_gateway 20 16
        rt_genmask 36 16
        rt_flags 52 2
        rt_pad2 54 2
        rt_pad3 56 4
        rt_pad4 62 2
        rt_metric 64 2
        rt_dev 66 4


The third one, allows to set the interface for the command "route", for
instance : route add -net default gw 10.0.3.1 eth0

Obviously, If patches seem correct for everyone, I can resend them with
comments and in a serie.

Bonus: To test this, find attached a little script that will compile
qemu-m68k, install debian etch-m68k in a linux container. Then you will
be able to run debian m68k system with "sudo lxc-start -n virtm68k".
(tested on an ubuntu 12.10, you should check that lxc creates a lxcbr0
bridge with IP address 10.0.3.1). Check variable at the beginning for
the paths used.

Regards,
Laurent
--
"Just play. Have fun. Enjoy the game."

[-- Attachment #2: create-m68k-lxc.sh --]
[-- Type: application/x-shellscript, Size: 4150 bytes --]

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

* Re: [Qemu-devel] [PATCH] linux-user: correctly align types in thunking code
  2012-12-08 16:55 ` Andreas Färber
@ 2012-12-08 17:13   ` Laurent Vivier
  0 siblings, 0 replies; 9+ messages in thread
From: Laurent Vivier @ 2012-12-08 17:13 UTC (permalink / raw)
  To: Andreas Färber; +Cc: Peter Maydell, Riku Voipio, qemu-devel

Le samedi 08 décembre 2012 à 17:55 +0100, Andreas Färber a écrit :
> Am 08.12.2012 16:18, schrieb Laurent Vivier:
> > Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> > ---
> >  thunk.h |   22 +++++++++++++++++-----
> >  1 file changed, 17 insertions(+), 5 deletions(-)
> > 
> > diff --git a/thunk.h b/thunk.h
> > index 87025c3..d3e9f3d 100644
> > --- a/thunk.h
> > +++ b/thunk.h
> > @@ -151,20 +151,32 @@ static inline int thunk_type_align(const argtype *type_ptr, int is_host)
> >      case TYPE_CHAR:
> >          return 1;
> >      case TYPE_SHORT:
> > -        return 2;
> > +        if (is_host) {
> > +            return __alignof__(short);
> 
> Might __alignof__() depend on a certain GCC version? Is it supported by
> clang?

I'm a big fan of copy&paste: I took them from linux-user/syscall.c :

static const StructEntry struct_termios_def = {
    .convert = { host_to_target_termios, target_to_host_termios },
    .size = { sizeof(struct target_termios), sizeof(struct host_termios) },
    .align = { __alignof__(struct target_termios), __alignof__(struct host_termios) },
};

Regards,
Laurent

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

* [Qemu-devel] [PATCH] linux-user: correctly align types in thunking code
@ 2012-12-20 20:55 Laurent Vivier
  2013-01-01 23:08 ` Laurent Vivier
  0 siblings, 1 reply; 9+ messages in thread
From: Laurent Vivier @ 2012-12-20 20:55 UTC (permalink / raw)
  To: qemu-devel; +Cc: Riku Voipio, Laurent Vivier

This is a follow up
of patch:

        commit c2e3dee6e03527baf8698698cce76b1a3174969a
        Author: Laurent Vivier <laurent@vivier.eu>
        Date:   Sun Feb 13 23:37:34 2011 +0100

            linux-user: Define target alignment size

In my case m68k aligns "int" on 2 not 4. You can check this with the
following program:

int main(void)
{
        struct rtentry rt;
        printf("rt_pad1 %ld %zd\n", offsetof(struct rtentry, rt_pad1),
                sizeof(rt.rt_pad1));
        printf("rt_dst %ld %zd\n", offsetof(struct rtentry, rt_dst),
                sizeof(rt.rt_dst));
        printf("rt_gateway %ld %zd\n", offsetof(struct rtentry, rt_gateway),
                sizeof(rt.rt_gateway));
        printf("rt_genmask %ld %zd\n", offsetof(struct rtentry, rt_genmask),
                sizeof(rt.rt_genmask));
        printf("rt_flags %ld %zd\n", offsetof(struct rtentry, rt_flags),
                sizeof(rt.rt_flags));
        printf("rt_pad2 %ld %zd\n", offsetof(struct rtentry, rt_pad2),
                sizeof(rt.rt_pad2));
        printf("rt_pad3 %ld %zd\n", offsetof(struct rtentry, rt_pad3),
                sizeof(rt.rt_pad3));
        printf("rt_pad4 %ld %zd\n", offsetof(struct rtentry, rt_pad4),
                sizeof(rt.rt_pad4));
        printf("rt_metric %ld %zd\n", offsetof(struct rtentry, rt_metric),
                sizeof(rt.rt_metric));
        printf("rt_dev %ld %zd\n", offsetof(struct rtentry, rt_dev),
                sizeof(rt.rt_dev));
        printf("rt_mtu %ld %zd\n", offsetof(struct rtentry, rt_mtu),
                sizeof(rt.rt_mtu));
        printf("rt_window %ld %zd\n", offsetof(struct rtentry, rt_window),
                sizeof(rt.rt_window));
        printf("rt_irtt %ld %zd\n", offsetof(struct rtentry, rt_irtt),
                sizeof(rt.rt_irtt));
}

And result is :

i386

rt_pad1 0 4
rt_dst 4 16
rt_gateway 20 16
rt_genmask 36 16
rt_flags 52 2
rt_pad2 54 2
rt_pad3 56 4
rt_pad4 62 2
rt_metric 64 2
rt_dev 68 4
rt_mtu 72 4
rt_window 76 4
rt_irtt 80 2

m68k

rt_pad1 0 4
rt_dst 4 16
rt_gateway 20 16
rt_genmask 36 16
rt_flags 52 2
rt_pad2 54 2
rt_pad3 56 4
rt_pad4 62 2
rt_metric 64 2
rt_dev 66 4
rt_mtu 70 4
rt_window 74 4
rt_irtt 78 2

This affects the "route" command :

WITHOUT this patch:

$ sudo route add -net default gw 10.0.3.1 window 1024 irtt 2 eth0
$ netstat -nr
Kernel IP routing table
Destination     Gateway         Genmask         Flags   MSS Window  irtt Iface
0.0.0.0         10.0.3.1        0.0.0.0         UG        0 67108866  32768 eth0
10.0.3.0        0.0.0.0         255.255.255.0   U         0 0          0 eth0

WITH this patch:

$ sudo route add -net default gw 10.0.3.1 window 1024 irtt 2 eth0
$ netstat -nr
Kernel IP routing table
Destination     Gateway         Genmask         Flags   MSS Window  irtt Iface
0.0.0.0         10.0.3.1        0.0.0.0         UG        0 1024       2 eth0
10.0.3.0        0.0.0.0         255.255.255.0   U         0 0          0 eth0

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 include/exec/user/thunk.h |   22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/include/exec/user/thunk.h b/include/exec/user/thunk.h
index 87025c3..d3e9f3d 100644
--- a/include/exec/user/thunk.h
+++ b/include/exec/user/thunk.h
@@ -151,20 +151,32 @@ static inline int thunk_type_align(const argtype *type_ptr, int is_host)
     case TYPE_CHAR:
         return 1;
     case TYPE_SHORT:
-        return 2;
+        if (is_host) {
+            return __alignof__(short);
+        } else {
+            return TARGET_SHORT_ALIGNMENT;
+        }
     case TYPE_INT:
-        return 4;
+        if (is_host) {
+            return __alignof__(int);
+        } else {
+            return TARGET_INT_ALIGNMENT;
+        }
     case TYPE_LONGLONG:
     case TYPE_ULONGLONG:
-        return 8;
+        if (is_host) {
+            return __alignof__(long long);
+        } else {
+            return TARGET_LLONG_ALIGNMENT;
+        }
     case TYPE_LONG:
     case TYPE_ULONG:
     case TYPE_PTRVOID:
     case TYPE_PTR:
         if (is_host) {
-            return sizeof(void *);
+            return __alignof__(long);
         } else {
-            return TARGET_ABI_BITS / 8;
+            return TARGET_LONG_ALIGNMENT;
         }
         break;
     case TYPE_OLDDEVT:
-- 
1.7.10.4

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

* Re: [Qemu-devel] [PATCH] linux-user: correctly align types in thunking code
  2012-12-20 20:55 Laurent Vivier
@ 2013-01-01 23:08 ` Laurent Vivier
  0 siblings, 0 replies; 9+ messages in thread
From: Laurent Vivier @ 2013-01-01 23:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: Riku Voipio

Ping !

Le jeudi 20 décembre 2012 à 21:55 +0100, Laurent Vivier a écrit :
> This is a follow up
> of patch:
> 
>         commit c2e3dee6e03527baf8698698cce76b1a3174969a
>         Author: Laurent Vivier <laurent@vivier.eu>
>         Date:   Sun Feb 13 23:37:34 2011 +0100
> 
>             linux-user: Define target alignment size
> 
> In my case m68k aligns "int" on 2 not 4. You can check this with the
> following program:
> 
> int main(void)
> {
>         struct rtentry rt;
>         printf("rt_pad1 %ld %zd\n", offsetof(struct rtentry, rt_pad1),
>                 sizeof(rt.rt_pad1));
>         printf("rt_dst %ld %zd\n", offsetof(struct rtentry, rt_dst),
>                 sizeof(rt.rt_dst));
>         printf("rt_gateway %ld %zd\n", offsetof(struct rtentry, rt_gateway),
>                 sizeof(rt.rt_gateway));
>         printf("rt_genmask %ld %zd\n", offsetof(struct rtentry, rt_genmask),
>                 sizeof(rt.rt_genmask));
>         printf("rt_flags %ld %zd\n", offsetof(struct rtentry, rt_flags),
>                 sizeof(rt.rt_flags));
>         printf("rt_pad2 %ld %zd\n", offsetof(struct rtentry, rt_pad2),
>                 sizeof(rt.rt_pad2));
>         printf("rt_pad3 %ld %zd\n", offsetof(struct rtentry, rt_pad3),
>                 sizeof(rt.rt_pad3));
>         printf("rt_pad4 %ld %zd\n", offsetof(struct rtentry, rt_pad4),
>                 sizeof(rt.rt_pad4));
>         printf("rt_metric %ld %zd\n", offsetof(struct rtentry, rt_metric),
>                 sizeof(rt.rt_metric));
>         printf("rt_dev %ld %zd\n", offsetof(struct rtentry, rt_dev),
>                 sizeof(rt.rt_dev));
>         printf("rt_mtu %ld %zd\n", offsetof(struct rtentry, rt_mtu),
>                 sizeof(rt.rt_mtu));
>         printf("rt_window %ld %zd\n", offsetof(struct rtentry, rt_window),
>                 sizeof(rt.rt_window));
>         printf("rt_irtt %ld %zd\n", offsetof(struct rtentry, rt_irtt),
>                 sizeof(rt.rt_irtt));
> }
> 
> And result is :
> 
> i386
> 
> rt_pad1 0 4
> rt_dst 4 16
> rt_gateway 20 16
> rt_genmask 36 16
> rt_flags 52 2
> rt_pad2 54 2
> rt_pad3 56 4
> rt_pad4 62 2
> rt_metric 64 2
> rt_dev 68 4
> rt_mtu 72 4
> rt_window 76 4
> rt_irtt 80 2
> 
> m68k
> 
> rt_pad1 0 4
> rt_dst 4 16
> rt_gateway 20 16
> rt_genmask 36 16
> rt_flags 52 2
> rt_pad2 54 2
> rt_pad3 56 4
> rt_pad4 62 2
> rt_metric 64 2
> rt_dev 66 4
> rt_mtu 70 4
> rt_window 74 4
> rt_irtt 78 2
> 
> This affects the "route" command :
> 
> WITHOUT this patch:
> 
> $ sudo route add -net default gw 10.0.3.1 window 1024 irtt 2 eth0
> $ netstat -nr
> Kernel IP routing table
> Destination     Gateway         Genmask         Flags   MSS Window  irtt Iface
> 0.0.0.0         10.0.3.1        0.0.0.0         UG        0 67108866  32768 eth0
> 10.0.3.0        0.0.0.0         255.255.255.0   U         0 0          0 eth0
> 
> WITH this patch:
> 
> $ sudo route add -net default gw 10.0.3.1 window 1024 irtt 2 eth0
> $ netstat -nr
> Kernel IP routing table
> Destination     Gateway         Genmask         Flags   MSS Window  irtt Iface
> 0.0.0.0         10.0.3.1        0.0.0.0         UG        0 1024       2 eth0
> 10.0.3.0        0.0.0.0         255.255.255.0   U         0 0          0 eth0
> 
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  include/exec/user/thunk.h |   22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/include/exec/user/thunk.h b/include/exec/user/thunk.h
> index 87025c3..d3e9f3d 100644
> --- a/include/exec/user/thunk.h
> +++ b/include/exec/user/thunk.h
> @@ -151,20 +151,32 @@ static inline int thunk_type_align(const argtype *type_ptr, int is_host)
>      case TYPE_CHAR:
>          return 1;
>      case TYPE_SHORT:
> -        return 2;
> +        if (is_host) {
> +            return __alignof__(short);
> +        } else {
> +            return TARGET_SHORT_ALIGNMENT;
> +        }
>      case TYPE_INT:
> -        return 4;
> +        if (is_host) {
> +            return __alignof__(int);
> +        } else {
> +            return TARGET_INT_ALIGNMENT;
> +        }
>      case TYPE_LONGLONG:
>      case TYPE_ULONGLONG:
> -        return 8;
> +        if (is_host) {
> +            return __alignof__(long long);
> +        } else {
> +            return TARGET_LLONG_ALIGNMENT;
> +        }
>      case TYPE_LONG:
>      case TYPE_ULONG:
>      case TYPE_PTRVOID:
>      case TYPE_PTR:
>          if (is_host) {
> -            return sizeof(void *);
> +            return __alignof__(long);
>          } else {
> -            return TARGET_ABI_BITS / 8;
> +            return TARGET_LONG_ALIGNMENT;
>          }
>          break;
>      case TYPE_OLDDEVT:

-- 
"Just play. Have fun. Enjoy the game."
- Michael Jordan
"Just play. Have fun. Enjoy the game."
- Michael Jordan

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

* [Qemu-devel] [PATCH] linux-user: correctly align types in thunking code
@ 2018-05-10 20:59 Laurent Vivier
  2018-05-11 10:43 ` Peter Maydell
  0 siblings, 1 reply; 9+ messages in thread
From: Laurent Vivier @ 2018-05-10 20:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: Riku Voipio, Laurent Vivier

This is a follow up
of patch:

        commit c2e3dee6e03527baf8698698cce76b1a3174969a
        Author: Laurent Vivier <laurent@vivier.eu>
        Date:   Sun Feb 13 23:37:34 2011 +0100

            linux-user: Define target alignment size

In my case m68k aligns "int" on 2 not 4. You can check this with the
following program:

int main(void)
{
        struct rtentry rt;
        printf("rt_pad1 %ld %zd\n", offsetof(struct rtentry, rt_pad1),
                sizeof(rt.rt_pad1));
        printf("rt_dst %ld %zd\n", offsetof(struct rtentry, rt_dst),
                sizeof(rt.rt_dst));
        printf("rt_gateway %ld %zd\n", offsetof(struct rtentry, rt_gateway),
                sizeof(rt.rt_gateway));
        printf("rt_genmask %ld %zd\n", offsetof(struct rtentry, rt_genmask),
                sizeof(rt.rt_genmask));
        printf("rt_flags %ld %zd\n", offsetof(struct rtentry, rt_flags),
                sizeof(rt.rt_flags));
        printf("rt_pad2 %ld %zd\n", offsetof(struct rtentry, rt_pad2),
                sizeof(rt.rt_pad2));
        printf("rt_pad3 %ld %zd\n", offsetof(struct rtentry, rt_pad3),
                sizeof(rt.rt_pad3));
        printf("rt_pad4 %ld %zd\n", offsetof(struct rtentry, rt_pad4),
                sizeof(rt.rt_pad4));
        printf("rt_metric %ld %zd\n", offsetof(struct rtentry, rt_metric),
                sizeof(rt.rt_metric));
        printf("rt_dev %ld %zd\n", offsetof(struct rtentry, rt_dev),
                sizeof(rt.rt_dev));
        printf("rt_mtu %ld %zd\n", offsetof(struct rtentry, rt_mtu),
                sizeof(rt.rt_mtu));
        printf("rt_window %ld %zd\n", offsetof(struct rtentry, rt_window),
                sizeof(rt.rt_window));
        printf("rt_irtt %ld %zd\n", offsetof(struct rtentry, rt_irtt),
                sizeof(rt.rt_irtt));
}

And result is :

i386

rt_pad1 0 4
rt_dst 4 16
rt_gateway 20 16
rt_genmask 36 16
rt_flags 52 2
rt_pad2 54 2
rt_pad3 56 4
rt_pad4 62 2
rt_metric 64 2
rt_dev 68 4
rt_mtu 72 4
rt_window 76 4
rt_irtt 80 2

m68k

rt_pad1 0 4
rt_dst 4 16
rt_gateway 20 16
rt_genmask 36 16
rt_flags 52 2
rt_pad2 54 2
rt_pad3 56 4
rt_pad4 62 2
rt_metric 64 2
rt_dev 66 4
rt_mtu 70 4
rt_window 74 4
rt_irtt 78 2

This affects the "route" command :

WITHOUT this patch:

$ sudo route add -net default gw 10.0.3.1 window 1024 irtt 2 eth0
$ netstat -nr
Kernel IP routing table
Destination     Gateway         Genmask         Flags   MSS Window  irtt Iface
0.0.0.0         10.0.3.1        0.0.0.0         UG        0 67108866  32768 eth0
10.0.3.0        0.0.0.0         255.255.255.0   U         0 0          0 eth0

WITH this patch:

$ sudo route add -net default gw 10.0.3.1 window 1024 irtt 2 eth0
$ netstat -nr
Kernel IP routing table
Destination     Gateway         Genmask         Flags   MSS Window  irtt Iface
0.0.0.0         10.0.3.1        0.0.0.0         UG        0 1024       2 eth0
10.0.3.0        0.0.0.0         255.255.255.0   U         0 0          0 eth0

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 include/exec/user/thunk.h | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/include/exec/user/thunk.h b/include/exec/user/thunk.h
index f19ef4b230..8f55b233b3 100644
--- a/include/exec/user/thunk.h
+++ b/include/exec/user/thunk.h
@@ -149,20 +149,32 @@ static inline int thunk_type_align(const argtype *type_ptr, int is_host)
     case TYPE_CHAR:
         return 1;
     case TYPE_SHORT:
-        return 2;
+        if (is_host) {
+            return __alignof__(short);
+        } else {
+            return ABI_SHORT_ALIGNMENT;
+        }
     case TYPE_INT:
-        return 4;
+        if (is_host) {
+            return __alignof__(int);
+        } else {
+            return ABI_INT_ALIGNMENT;
+        }
     case TYPE_LONGLONG:
     case TYPE_ULONGLONG:
-        return 8;
+        if (is_host) {
+            return __alignof__(long long);
+        } else {
+            return ABI_LLONG_ALIGNMENT;
+        }
     case TYPE_LONG:
     case TYPE_ULONG:
     case TYPE_PTRVOID:
     case TYPE_PTR:
         if (is_host) {
-            return sizeof(void *);
+            return __alignof__(long);
         } else {
-            return TARGET_ABI_BITS / 8;
+            return ABI_LONG_ALIGNMENT;
         }
         break;
     case TYPE_OLDDEVT:
-- 
2.14.3

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

* Re: [Qemu-devel] [PATCH] linux-user: correctly align types in thunking code
  2018-05-10 20:59 [Qemu-devel] [PATCH] linux-user: correctly align types in thunking code Laurent Vivier
@ 2018-05-11 10:43 ` Peter Maydell
  0 siblings, 0 replies; 9+ messages in thread
From: Peter Maydell @ 2018-05-11 10:43 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: QEMU Developers, Riku Voipio

On 10 May 2018 at 21:59, Laurent Vivier <laurent@vivier.eu> wrote:
> This is a follow up
> of patch:
>
>         commit c2e3dee6e03527baf8698698cce76b1a3174969a
>         Author: Laurent Vivier <laurent@vivier.eu>
>         Date:   Sun Feb 13 23:37:34 2011 +0100
>
>             linux-user: Define target alignment size
>
> In my case m68k aligns "int" on 2 not 4. You can check this with the
> following program:
>
> int main(void)
> {
>         struct rtentry rt;
>         printf("rt_pad1 %ld %zd\n", offsetof(struct rtentry, rt_pad1),
>                 sizeof(rt.rt_pad1));
>         printf("rt_dst %ld %zd\n", offsetof(struct rtentry, rt_dst),
>                 sizeof(rt.rt_dst));
>         printf("rt_gateway %ld %zd\n", offsetof(struct rtentry, rt_gateway),
>                 sizeof(rt.rt_gateway));
>         printf("rt_genmask %ld %zd\n", offsetof(struct rtentry, rt_genmask),
>                 sizeof(rt.rt_genmask));
>         printf("rt_flags %ld %zd\n", offsetof(struct rtentry, rt_flags),
>                 sizeof(rt.rt_flags));
>         printf("rt_pad2 %ld %zd\n", offsetof(struct rtentry, rt_pad2),
>                 sizeof(rt.rt_pad2));
>         printf("rt_pad3 %ld %zd\n", offsetof(struct rtentry, rt_pad3),
>                 sizeof(rt.rt_pad3));
>         printf("rt_pad4 %ld %zd\n", offsetof(struct rtentry, rt_pad4),
>                 sizeof(rt.rt_pad4));
>         printf("rt_metric %ld %zd\n", offsetof(struct rtentry, rt_metric),
>                 sizeof(rt.rt_metric));
>         printf("rt_dev %ld %zd\n", offsetof(struct rtentry, rt_dev),
>                 sizeof(rt.rt_dev));
>         printf("rt_mtu %ld %zd\n", offsetof(struct rtentry, rt_mtu),
>                 sizeof(rt.rt_mtu));
>         printf("rt_window %ld %zd\n", offsetof(struct rtentry, rt_window),
>                 sizeof(rt.rt_window));
>         printf("rt_irtt %ld %zd\n", offsetof(struct rtentry, rt_irtt),
>                 sizeof(rt.rt_irtt));
> }
>
> And result is :
>
> i386
>
> rt_pad1 0 4
> rt_dst 4 16
> rt_gateway 20 16
> rt_genmask 36 16
> rt_flags 52 2
> rt_pad2 54 2
> rt_pad3 56 4
> rt_pad4 62 2
> rt_metric 64 2
> rt_dev 68 4
> rt_mtu 72 4
> rt_window 76 4
> rt_irtt 80 2
>
> m68k
>
> rt_pad1 0 4
> rt_dst 4 16
> rt_gateway 20 16
> rt_genmask 36 16
> rt_flags 52 2
> rt_pad2 54 2
> rt_pad3 56 4
> rt_pad4 62 2
> rt_metric 64 2
> rt_dev 66 4
> rt_mtu 70 4
> rt_window 74 4
> rt_irtt 78 2
>
> This affects the "route" command :
>
> WITHOUT this patch:
>
> $ sudo route add -net default gw 10.0.3.1 window 1024 irtt 2 eth0
> $ netstat -nr
> Kernel IP routing table
> Destination     Gateway         Genmask         Flags   MSS Window  irtt Iface
> 0.0.0.0         10.0.3.1        0.0.0.0         UG        0 67108866  32768 eth0
> 10.0.3.0        0.0.0.0         255.255.255.0   U         0 0          0 eth0
>
> WITH this patch:
>
> $ sudo route add -net default gw 10.0.3.1 window 1024 irtt 2 eth0
> $ netstat -nr
> Kernel IP routing table
> Destination     Gateway         Genmask         Flags   MSS Window  irtt Iface
> 0.0.0.0         10.0.3.1        0.0.0.0         UG        0 1024       2 eth0
> 10.0.3.0        0.0.0.0         255.255.255.0   U         0 0          0 eth0
>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  include/exec/user/thunk.h | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/include/exec/user/thunk.h b/include/exec/user/thunk.h
> index f19ef4b230..8f55b233b3 100644
> --- a/include/exec/user/thunk.h
> +++ b/include/exec/user/thunk.h
> @@ -149,20 +149,32 @@ static inline int thunk_type_align(const argtype *type_ptr, int is_host)
>      case TYPE_CHAR:
>          return 1;
>      case TYPE_SHORT:
> -        return 2;
> +        if (is_host) {
> +            return __alignof__(short);
> +        } else {
> +            return ABI_SHORT_ALIGNMENT;
> +        }
>      case TYPE_INT:
> -        return 4;
> +        if (is_host) {
> +            return __alignof__(int);
> +        } else {
> +            return ABI_INT_ALIGNMENT;
> +        }
>      case TYPE_LONGLONG:
>      case TYPE_ULONGLONG:
> -        return 8;
> +        if (is_host) {
> +            return __alignof__(long long);
> +        } else {
> +            return ABI_LLONG_ALIGNMENT;
> +        }
>      case TYPE_LONG:
>      case TYPE_ULONG:
>      case TYPE_PTRVOID:
>      case TYPE_PTR:
>          if (is_host) {
> -            return sizeof(void *);
> +            return __alignof__(long);
>          } else {
> -            return TARGET_ABI_BITS / 8;
> +            return ABI_LONG_ALIGNMENT;
>          }
>          break;
>      case TYPE_OLDDEVT:
> --

This definitely looks like the right thing. As well as m68k,
this will also affect sh4 and i386 (where long long is 4
aligned but we were saying it should be 8 aligned) -- I wonder
if that fixes any bugs...

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

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

end of thread, other threads:[~2018-05-11 10:43 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-05-10 20:59 [Qemu-devel] [PATCH] linux-user: correctly align types in thunking code Laurent Vivier
2018-05-11 10:43 ` Peter Maydell
  -- strict thread matches above, loose matches on Subject: below --
2012-12-20 20:55 Laurent Vivier
2013-01-01 23:08 ` Laurent Vivier
2012-12-08 15:18 Laurent Vivier
2012-12-08 16:40 ` Peter Maydell
2012-12-08 17:10   ` Laurent Vivier
2012-12-08 16:55 ` Andreas Färber
2012-12-08 17:13   ` Laurent Vivier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).