[Qemu-devel] [PATCH 0/7] use QEMU_PACKED and QEMU_ALIGNED macros

[Qemu-devel] [PATCH 0/7] use QEMU_PACKED and QEMU_ALIGNED macros

[Philippe Mathieu-Daudé]

Still trying to reduce clang -Waddress-of-packed-member warnings.

Philippe Mathieu-Daudé (7):
  net: struct MACAddr can use the QEMU_PACKED macro
  net/eth: use the QEMU_PACKED macro
  net/rocker: use QEMU_PACKED and QEMU_ALIGNED() macros
  net/rocker: move struct rocker_desc to C file
  net/eepro100: use QEMU_ALIGNED() macro
  net/eepro100: reduce alignment to DWORD (32bit)
  i386/pc: use the QEMU_ALIGNED() macro

 hw/net/rocker/rocker_hw.h   | 15 +--------------
 include/net/eth.h           | 26 +++++++++++++-------------
 include/net/net.h           |  2 +-
 hw/i386/pc.c                |  4 ++--
 hw/net/eepro100.c           |  2 +-
 hw/net/rocker/rocker_desc.c | 13 +++++++++++++
 6 files changed, 31 insertions(+), 31 deletions(-)

-- 
2.15.1

[Qemu-devel] [PATCH 1/7] net: struct MACAddr can use the QEMU_PACKED macro

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/net/net.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net/net.h b/include/net/net.h
index 4afac1a9dd..70f5bb3419 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -23,7 +23,7 @@
 
 struct MACAddr {
     uint8_t a[6];
-};
+} QEMU_PACKED;
 
 /* qdev nic properties */
 
-- 
2.15.1

[Qemu-devel] [PATCH 2/7] net/eth: use the QEMU_PACKED macro

[Philippe Mathieu-Daudé]

---
 include/net/eth.h | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/include/net/eth.h b/include/net/eth.h
index 09054a506d..87c6499f18 100644
--- a/include/net/eth.h
+++ b/include/net/eth.h
@@ -36,12 +36,12 @@ struct eth_header {
     uint8_t  h_dest[ETH_ALEN];   /* destination eth addr */
     uint8_t  h_source[ETH_ALEN]; /* source ether addr    */
     uint16_t h_proto;            /* packet type ID field */
-};
+} QEMU_PACKED;
 
 struct vlan_header {
     uint16_t  h_tci;     /* priority and VLAN ID  */
     uint16_t  h_proto;   /* encapsulated protocol */
-};
+} QEMU_PACKED;
 
 struct ip_header {
     uint8_t  ip_ver_len;     /* version and header length */
@@ -53,9 +53,9 @@ struct ip_header {
     uint8_t  ip_p;           /* protocol */
     uint16_t ip_sum;         /* checksum */
     uint32_t ip_src, ip_dst; /* source and destination address */
-};
+} QEMU_PACKED;
 
-typedef struct tcp_header {
+typedef struct QEMU_PACKED tcp_header {
     uint16_t th_sport;          /* source port */
     uint16_t th_dport;          /* destination port */
     uint32_t th_seq;            /* sequence number */
@@ -77,14 +77,14 @@ typedef struct tcp_header {
 #define TCP_HEADER_DATA_OFFSET(tcp) \
     (((be16_to_cpu((tcp)->th_offset_flags) >> 12) & 0xf) << 2)
 
-typedef struct udp_header {
+typedef struct QEMU_PACKED udp_header {
     uint16_t uh_sport; /* source port */
     uint16_t uh_dport; /* destination port */
     uint16_t uh_ulen;  /* udp length */
     uint16_t uh_sum;   /* udp checksum */
 } udp_header;
 
-typedef struct ip_pseudo_header {
+typedef struct QEMU_PACKED ip_pseudo_header {
     uint32_t ip_src;
     uint32_t ip_dst;
     uint8_t  zeros;
@@ -116,9 +116,9 @@ struct ip6_header {
     } ip6_ctlun;
     struct in6_address ip6_src;    /* source address */
     struct in6_address ip6_dst;    /* destination address */
-};
+} QEMU_PACKED;
 
-typedef struct ip6_pseudo_header {
+typedef struct QEMU_PACKED ip6_pseudo_header {
     struct in6_address ip6_src;
     struct in6_address ip6_dst;
     uint32_t           len;
@@ -129,7 +129,7 @@ typedef struct ip6_pseudo_header {
 struct ip6_ext_hdr {
     uint8_t        ip6r_nxt;   /* next header */
     uint8_t        ip6r_len;   /* length in units of 8 octets */
-};
+} QEMU_PACKED;
 
 struct ip6_ext_hdr_routing {
     uint8_t     nxt;
@@ -137,21 +137,21 @@ struct ip6_ext_hdr_routing {
     uint8_t     rtype;
     uint8_t     segleft;
     uint8_t     rsvd[4];
-};
+} QEMU_PACKED;
 
 struct ip6_option_hdr {
 #define IP6_OPT_PAD1   (0x00)
 #define IP6_OPT_HOME   (0xC9)
     uint8_t type;
     uint8_t len;
-};
+} QEMU_PACKED;
 
 struct udp_hdr {
   uint16_t uh_sport;           /* source port */
   uint16_t uh_dport;           /* destination port */
   uint16_t uh_ulen;            /* udp length */
   uint16_t uh_sum;             /* udp checksum */
-};
+} QEMU_PACKED;
 
 struct tcp_hdr {
     u_short     th_sport;   /* source port */
@@ -180,7 +180,7 @@ struct tcp_hdr {
     u_short th_win;      /* window */
     u_short th_sum;      /* checksum */
     u_short th_urp;      /* urgent pointer */
-};
+} QEMU_PACKED;
 
 #define ip6_nxt      ip6_ctlun.ip6_un1.ip6_un1_nxt
 #define ip6_ecn_acc  ip6_ctlun.ip6_un3.ip6_un3_ecn
-- 
2.15.1

[Qemu-devel] [PATCH 3/7] net/rocker: use QEMU_PACKED and QEMU_ALIGNED() macros

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/net/rocker/rocker_hw.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/net/rocker/rocker_hw.h b/hw/net/rocker/rocker_hw.h
index 1786323fa4..8b4ce250d5 100644
--- a/hw/net/rocker/rocker_hw.h
+++ b/hw/net/rocker/rocker_hw.h
@@ -130,7 +130,7 @@ typedef struct rocker_desc {
     __le16 tlv_size;
     __le16 rsvd[5];   /* pad to 32 bytes */
     __le16 comp_err;
-} __attribute__((packed, aligned(8))) RockerDesc;
+} QEMU_PACKED QEMU_ALIGNED(8) RockerDesc;
 
 /*
  * Rocker TLV type fields
@@ -140,7 +140,7 @@ typedef struct rocker_tlv {
     __le32 type;
     __le16 len;
     __le16 rsvd;
-} __attribute__((packed, aligned(8))) RockerTlv;
+} QEMU_PACKED QEMU_ALIGNED(8) RockerTlv;
 
 /* cmd msg */
 enum {
-- 
2.15.1

[Qemu-devel] [PATCH 4/7] net/rocker: move struct rocker_desc to C file

[Philippe Mathieu-Daudé]

this is the only user.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/net/rocker/rocker_hw.h   | 13 -------------
 hw/net/rocker/rocker_desc.c | 13 +++++++++++++
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/hw/net/rocker/rocker_hw.h b/hw/net/rocker/rocker_hw.h
index 8b4ce250d5..daaaf4080f 100644
--- a/hw/net/rocker/rocker_hw.h
+++ b/hw/net/rocker/rocker_hw.h
@@ -119,19 +119,6 @@ enum {
  */
 #define ROCKER_RING_INDEX(reg) ((reg >> 5) & 0x7F)
 
-/*
- * Rocker DMA Descriptor
- */
-
-typedef struct rocker_desc {
-    __le64 buf_addr;
-    uint64_t cookie;
-    __le16 buf_size;
-    __le16 tlv_size;
-    __le16 rsvd[5];   /* pad to 32 bytes */
-    __le16 comp_err;
-} QEMU_PACKED QEMU_ALIGNED(8) RockerDesc;
-
 /*
  * Rocker TLV type fields
  */
diff --git a/hw/net/rocker/rocker_desc.c b/hw/net/rocker/rocker_desc.c
index b009da4f89..2f2de837f8 100644
--- a/hw/net/rocker/rocker_desc.c
+++ b/hw/net/rocker/rocker_desc.c
@@ -23,6 +23,19 @@
 #include "rocker_hw.h"
 #include "rocker_desc.h"
 
+/*
+ * Rocker DMA Descriptor
+ */
+
+typedef struct rocker_desc {
+    __le64 buf_addr;
+    uint64_t cookie;
+    __le16 buf_size;
+    __le16 tlv_size;
+    __le16 rsvd[5];   /* pad to 32 bytes */
+    __le16 comp_err;
+} QEMU_PACKED QEMU_ALIGNED(8) RockerDesc;
+
 struct desc_ring {
     hwaddr base_addr;
     uint32_t size;
-- 
2.15.1

[Qemu-devel] [PATCH 5/7] net/eepro100: use QEMU_ALIGNED() macro

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/net/eepro100.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index a07a63247e..61d767524f 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -266,7 +266,7 @@ typedef struct {
 
     /* Data in mem is always in the byte order of the controller (le).
      * It must be dword aligned to allow direct access to 32 bit values. */
-    uint8_t mem[PCI_MEM_SIZE] __attribute__((aligned(8)));
+    uint8_t mem[PCI_MEM_SIZE] QEMU_ALIGNED(8);
 
     /* Configuration bytes. */
     uint8_t configuration[22];
-- 
2.15.1

[Qemu-devel] [RFC PATCH 6/7] net/eepro100: reduce alignment to DWORD (32bit)

[Philippe Mathieu-Daudé]

as suggested in the comment.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/net/eepro100.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index 61d767524f..abb44710d9 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -266,7 +266,7 @@ typedef struct {
 
     /* Data in mem is always in the byte order of the controller (le).
      * It must be dword aligned to allow direct access to 32 bit values. */
-    uint8_t mem[PCI_MEM_SIZE] QEMU_ALIGNED(8);
+    uint8_t mem[PCI_MEM_SIZE] QEMU_ALIGNED(4);
 
     /* Configuration bytes. */
     uint8_t configuration[22];
-- 
2.15.1

[Qemu-devel] [PATCH 7/7] i386/pc: use the QEMU_ALIGNED() macro

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/i386/pc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 3fcf318a95..85d9454c71 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -93,12 +93,12 @@ struct e820_entry {
     uint64_t address;
     uint64_t length;
     uint32_t type;
-} QEMU_PACKED __attribute((__aligned__(4)));
+} QEMU_PACKED QEMU_ALIGNED(4);
 
 struct e820_table {
     uint32_t count;
     struct e820_entry entry[E820_NR_ENTRIES];
-} QEMU_PACKED __attribute((__aligned__(4)));
+} QEMU_PACKED QEMU_ALIGNED(4);
 
 static struct e820_table e820_reserve;
 static struct e820_entry *e820_table;
-- 
2.15.1

Re: [Qemu-devel] [RFC PATCH 6/7] net/eepro100: reduce alignment to DWORD (32bit)

[Paolo Bonzini]

On 08/01/2018 19:02, Philippe Mathieu-Daudé wrote:
>      /* Data in mem is always in the byte order of the controller (le).
>       * It must be dword aligned to allow direct access to 32 bit values. */
> -    uint8_t mem[PCI_MEM_SIZE] QEMU_ALIGNED(8);
> +    uint8_t mem[PCI_MEM_SIZE] QEMU_ALIGNED(4);

It's actually not needed at all, since accesses go through ld*_le_p and
st*_le_p (commit 4d9be25, "hw/net/eepro100.c: Don't use cpu_to_*w() and
*_to_cpup()", 2016-06-16).

Paolo

Re: [Qemu-devel] [RFC PATCH 6/7] net/eepro100: reduce alignment to DWORD (32bit)

[Philippe Mathieu-Daudé]

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

On 01/08/2018 03:10 PM, Paolo Bonzini wrote:
> On 08/01/2018 19:02, Philippe Mathieu-Daudé wrote:
>>      /* Data in mem is always in the byte order of the controller (le).
>>       * It must be dword aligned to allow direct access to 32 bit values. */
>> -    uint8_t mem[PCI_MEM_SIZE] QEMU_ALIGNED(8);
>> +    uint8_t mem[PCI_MEM_SIZE] QEMU_ALIGNED(4);
> 
> It's actually not needed at all, since accesses go through ld*_le_p and
> st*_le_p (commit 4d9be25, "hw/net/eepro100.c: Don't use cpu_to_*w() and
> *_to_cpup()", 2016-06-16).

Cool, I'll update this.

Thank you,

Phil.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [PATCH 7/7] i386/pc: use the QEMU_ALIGNED() macro

[Marcel Apfelbaum]

On 08/01/2018 20:02, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   hw/i386/pc.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 3fcf318a95..85d9454c71 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -93,12 +93,12 @@ struct e820_entry {
>       uint64_t address;
>       uint64_t length;
>       uint32_t type;
> -} QEMU_PACKED __attribute((__aligned__(4)));
> +} QEMU_PACKED QEMU_ALIGNED(4);
>   
>   struct e820_table {
>       uint32_t count;
>       struct e820_entry entry[E820_NR_ENTRIES];
> -} QEMU_PACKED __attribute((__aligned__(4)));
> +} QEMU_PACKED QEMU_ALIGNED(4);
>   
>   static struct e820_table e820_reserve;
>   static struct e820_entry *e820_table;
> 

Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>

Thanks,
Marcel

Re: [Qemu-devel] [PATCH 1/7] net: struct MACAddr can use the QEMU_PACKED macro

[Richard Henderson]

On 01/08/2018 10:02 AM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  include/net/net.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/net/net.h b/include/net/net.h
> index 4afac1a9dd..70f5bb3419 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -23,7 +23,7 @@
>  
>  struct MACAddr {
>      uint8_t a[6];
> -};
> +} QEMU_PACKED;

I suppose the arm32 abi does pad structs to a multiple of 4.  Does this
actually affect anything else?

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH 3/7] net/rocker: use QEMU_PACKED and QEMU_ALIGNED() macros

[Richard Henderson]

On 01/08/2018 10:02 AM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/net/rocker/rocker_hw.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH 4/7] net/rocker: move struct rocker_desc to C file

[Richard Henderson]

On 01/08/2018 10:02 AM, Philippe Mathieu-Daudé wrote:
> this is the only user.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/net/rocker/rocker_hw.h   | 13 -------------
>  hw/net/rocker/rocker_desc.c | 13 +++++++++++++
>  2 files changed, 13 insertions(+), 13 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH 5/7] net/eepro100: use QEMU_ALIGNED() macro

[Richard Henderson]

On 01/08/2018 10:02 AM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/net/eepro100.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH 7/7] i386/pc: use the QEMU_ALIGNED() macro

[Richard Henderson]

On 01/08/2018 10:02 AM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/i386/pc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~