[Qemu-devel] patch: e1000: fix unaligned access

[Qemu-devel] patch: e1000: fix unaligned access

[tgingold]

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

Hi,

as the tcp header may be unaligned, be32_to_cpup must be used instead of
be32_to_cpu.

Tristan.

[-- Attachment #2: e1000-unalign.diff --]
[-- Type: application/octet-stream, Size: 1353 bytes --]

--- /home/gingold/src/qemu/bswap.h	2007-09-16 23:07:48.000000000 +0200
+++ qemu/bswap.h	2008-03-18 06:46:23.000000000 +0100
@@ -132,6 +132,7 @@
 #define cpu_to_le32wu(p, v) cpu_to_le32w(p, v)
 #define le16_to_cpupu(p) le16_to_cpup(p)
 #define le32_to_cpupu(p) le32_to_cpup(p)
+#define be32_to_cpupu(p) le32_to_cpup(p)
 
 #define cpu_to_be16wu(p, v) cpu_to_be16w(p, v)
 #define cpu_to_be32wu(p, v) cpu_to_be32w(p, v)
@@ -168,6 +169,12 @@
     return p1[0] | (p1[1] << 8) | (p1[2] << 16) | (p1[3] << 24);
 }
 
+static inline uint32_t be32_to_cpupu(const uint32_t *p)
+{
+    const uint8_t *p1 = (const uint8_t *)p;
+    return p1[3] | (p1[2] << 8) | (p1[1] << 16) | (p1[0] << 24);
+}
+
 static inline void cpu_to_be16wu(uint16_t *p, uint16_t v)
 {
     uint8_t *p1 = (uint8_t *)p;
--- /home/gingold/src/qemu/hw/e1000.c	2008-03-13 20:18:26.000000000 +0100
+++ qemu/hw/e1000.c	2008-03-27 07:11:48.000000000 +0100
@@ -326,7 +326,7 @@
         if (tp->tcp) {
             sofar = frames * tp->mss;
             cpu_to_be32wu((uint32_t *)(tp->data+css+4),	// seq
-                be32_to_cpup((uint32_t *)(tp->data+css+4))+sofar);
+                be32_to_cpupu((uint32_t *)(tp->data+css+4))+sofar);
             if (tp->paylen - sofar > tp->mss)
                 tp->data[css + 13] &= ~9;		// PSH, FIN
         } else	// UDP

Re: [Qemu-devel] patch: e1000: fix unaligned access

[Laurent Vivier]

Le jeudi 27 mars 2008 à 16:59 +0100, tgingold@free.fr a écrit :
> --- /home/gingold/src/qemu/bswap.h      2007-09-16 23:07:48.000000000
> +0200
> +++ qemu/bswap.h        2008-03-18 06:46:23.000000000 +0100
> @@ -132,6 +132,7 @@
>  #define cpu_to_le32wu(p, v) cpu_to_le32w(p, v)
>  #define le16_to_cpupu(p) le16_to_cpup(p)
>  #define le32_to_cpupu(p) le32_to_cpup(p)
> +#define be32_to_cpupu(p) le32_to_cpup(p)

should be "#define be32_to_cpupu(p) be32_to_cpup(p)" ?

>  #define cpu_to_be16wu(p, v) cpu_to_be16w(p, v)
>  #define cpu_to_be32wu(p, v) cpu_to_be32w(p, v)
> @@ -168,6 +169,12 @@
>      return p1[0] | (p1[1] << 8) | (p1[2] << 16) | (p1[3] << 24);
>  }
>  
> +static inline uint32_t be32_to_cpupu(const uint32_t *p)
> +{
> +    const uint8_t *p1 = (const uint8_t *)p;
> +    return p1[3] | (p1[2] << 8) | (p1[1] << 16) | (p1[0] << 24);
> +}
> +
>  static inline void cpu_to_be16wu(uint16_t *p, uint16_t v)
>  {
>      uint8_t *p1 = (uint8_t *)p;
> --- /home/gingold/src/qemu/hw/e1000.c   2008-03-13 20:18:26.000000000
> +0100
> +++ qemu/hw/e1000.c     2008-03-27 07:11:48.000000000 +0100
> @@ -326,7 +326,7 @@
>          if (tp->tcp) {
>              sofar = frames * tp->mss;
>              cpu_to_be32wu((uint32_t *)(tp->data+css+4),        // seq
> -                be32_to_cpup((uint32_t *)(tp->data+css+4))+sofar);
> +                be32_to_cpupu((uint32_t *)(tp->data+css+4))+sofar);
>              if (tp->paylen - sofar > tp->mss)
>                  tp->data[css + 13] &= ~9;              // PSH, FIN
>          } else // UDP
> 
-- 
------------- Laurent.Vivier@bull.net ---------------
"The best way to predict the future is to invent it."
- Alan Kay

Re: [Qemu-devel] patch: e1000: fix unaligned access

[Tristan Gingold]

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

On Fri, Mar 28, 2008 at 01:56:36AM +0100, Laurent Vivier wrote:
> > +#define be32_to_cpupu(p) le32_to_cpup(p)
> 
> should be "#define be32_to_cpupu(p) be32_to_cpup(p)" ?

Yes, thank you for the catch.  I didn't submit the right patch.  So here
is the correct one.

Tristan.

[-- Attachment #2: e1000-unalign.diff --]
[-- Type: text/plain, Size: 1319 bytes --]

--- /home/gingold/src/qemu/bswap.h	2007-09-16 23:07:48.000000000 +0200
+++ qemu/bswap.h	2008-03-18 06:46:23.000000000 +0100
@@ -132,6 +132,7 @@
 #define cpu_to_le32wu(p, v) cpu_to_le32w(p, v)
 #define le16_to_cpupu(p) le16_to_cpup(p)
 #define le32_to_cpupu(p) le32_to_cpup(p)
+#define be32_to_cpupu(p) be32_to_cpup(p)
 
 #define cpu_to_be16wu(p, v) cpu_to_be16w(p, v)
 #define cpu_to_be32wu(p, v) cpu_to_be32w(p, v)
@@ -168,6 +169,12 @@
     return p1[0] | (p1[1] << 8) | (p1[2] << 16) | (p1[3] << 24);
 }
 
+static inline uint32_t be32_to_cpupu(const uint32_t *p)
+{
+    const uint8_t *p1 = (const uint8_t *)p;
+    return p1[3] | (p1[2] << 8) | (p1[1] << 16) | (p1[0] << 24);
+}
+
 static inline void cpu_to_be16wu(uint16_t *p, uint16_t v)
 {
     uint8_t *p1 = (uint8_t *)p;
--- /home/gingold/src/qemu/hw/e1000.c	2008-03-13 20:18:26.000000000 +0100
+++ qemu/hw/e1000.c	2008-03-27 07:11:48.000000000 +0100
@@ -326,7 +326,7 @@
         if (tp->tcp) {
             sofar = frames * tp->mss;
             cpu_to_be32wu((uint32_t *)(tp->data+css+4),	// seq
-                be32_to_cpup((uint32_t *)(tp->data+css+4))+sofar);
+                be32_to_cpupu((uint32_t *)(tp->data+css+4))+sofar);
             if (tp->paylen - sofar > tp->mss)
                 tp->data[css + 13] &= ~9;		// PSH, FIN
         } else	// UDP