[Qemu-devel] [PATCH 0/9] Remove legacy unaligned bswap functions

[Qemu-devel] [PATCH 0/9] Remove legacy unaligned bswap functions

[Peter Maydell]

The bswap.h header includes a set of "legacy unaligned functions"
that (since commit c732a52d3 at the beginning of this year) are
just wrappers for underlying {ld,st}<type> functions. The legacy
functions aren't used in many places, so just replace all their
uses with uses of the new-style {ld,st} functions; this lets us
remove the legacy wrappers altogether.

Since we know the {ld,st}* routines are definitely functions,
we can in the process remove some casts which were left over
from when the legacy unaligned functions were previously macros.

The patchset is divided up by function being removed, rather
than by which device/subsystem is being fixed; I think this way
round is easier to review since you only have to keep one
substitution in your head when reading a patch.

Peter Maydell (9):
  bswap.h: Remove cpu_to_le16wu()
  bswap.h: Remove cpu_to_le32wu()
  bswap.h: Remove le16_to_cpupu()
  bswap.h: Remove le32_to_cpupu()
  bswap.h: Remove be32_to_cpupu()
  bswap.h: Remove cpu_to_be16wu()
  bswap.h: Remove cpu_to_be32wu()
  bswap.h: Remove cpu_to_be64wu()
  bswap.h: Remove cpu_to_32wu()

 block/qcow2-cluster.c     |    2 +-
 hw/acpi/core.c            |    3 +--
 hw/block/cdrom.c          |   10 +++++-----
 hw/display/vga_template.h |   14 ++++++++------
 hw/ide/atapi.c            |   16 +++++++--------
 hw/net/e1000.c            |   22 +++++++++------------
 hw/net/ne2000.c           |    4 ++--
 hw/pci/pcie_aer.c         |    4 ++--
 include/hw/pci/pci.h      |    8 ++++----
 include/qemu/bswap.h      |   47 ---------------------------------------------
 10 files changed, 40 insertions(+), 90 deletions(-)

-- 
1.7.9.5

[Qemu-devel] [PATCH 1/9] bswap.h: Remove cpu_to_le16wu()

[Peter Maydell]

Replace the legacy cpu_to_le16wu() with stw_le_p().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/acpi/core.c       |    4 ++--
 include/hw/pci/pci.h |    2 +-
 include/qemu/bswap.h |    5 -----
 3 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index b07feda..e84a577 100644
--- a/hw/acpi/core.c
+++ b/hw/acpi/core.c
@@ -170,8 +170,8 @@ static void acpi_table_install(const char unsigned *blob, size_t bloblen,
     }
 
     /* increase number of tables */
-    cpu_to_le16wu((uint16_t *)acpi_tables,
-                  le16_to_cpupu((uint16_t *)acpi_tables) + 1u);
+    stw_le_p(acpi_tables,
+             le16_to_cpupu((uint16_t *)acpi_tables) + 1u);
 
     /* Update the header fields. The strings need not be NUL-terminated. */
     changed_fields = 0;
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index ccec2ba..ae82064 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -422,7 +422,7 @@ pci_get_byte(const uint8_t *config)
 static inline void
 pci_set_word(uint8_t *config, uint16_t val)
 {
-    cpu_to_le16wu((uint16_t *)config, val);
+    stw_le_p(config, val);
 }
 
 static inline uint16_t
diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index 14a5f65..ada1084 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -412,11 +412,6 @@ static inline void stfq_be_p(void *ptr, float64 v)
 
 /* Legacy unaligned versions.  Note that we never had a complete set.  */
 
-static inline void cpu_to_le16wu(uint16_t *p, uint16_t v)
-{
-    stw_le_p(p, v);
-}
-
 static inline void cpu_to_le32wu(uint32_t *p, uint32_t v)
 {
     stl_le_p(p, v);
-- 
1.7.9.5

[Qemu-devel] [PATCH 2/9] bswap.h: Remove cpu_to_le32wu()

[Peter Maydell]

Replace the legacy cpu_to_le32wu() with stl_le_p().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/net/ne2000.c      |    2 +-
 include/hw/pci/pci.h |    2 +-
 include/qemu/bswap.h |    5 -----
 3 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/hw/net/ne2000.c b/hw/net/ne2000.c
index 31afd28..e7865e9 100644
--- a/hw/net/ne2000.c
+++ b/hw/net/ne2000.c
@@ -467,7 +467,7 @@ static inline void ne2000_mem_writel(NE2000State *s, uint32_t addr,
     addr &= ~1; /* XXX: check exact behaviour if not even */
     if (addr < 32 ||
         (addr >= NE2000_PMEM_START && addr < NE2000_MEM_SIZE)) {
-        cpu_to_le32wu((uint32_t *)(s->mem + addr), val);
+        stl_le_p(s->mem + addr, val);
     }
 }
 
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index ae82064..2de27c5 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -434,7 +434,7 @@ pci_get_word(const uint8_t *config)
 static inline void
 pci_set_long(uint8_t *config, uint32_t val)
 {
-    cpu_to_le32wu((uint32_t *)config, val);
+    stl_le_p(config, val);
 }
 
 static inline uint32_t
diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index ada1084..d0c4ff0 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -412,11 +412,6 @@ static inline void stfq_be_p(void *ptr, float64 v)
 
 /* Legacy unaligned versions.  Note that we never had a complete set.  */
 
-static inline void cpu_to_le32wu(uint32_t *p, uint32_t v)
-{
-    stl_le_p(p, v);
-}
-
 static inline uint16_t le16_to_cpupu(const uint16_t *p)
 {
     return lduw_le_p(p);
-- 
1.7.9.5

[Qemu-devel] [PATCH 3/9] bswap.h: Remove le16_to_cpupu()

[Peter Maydell]

Replace the legacy le16_to_cpupu() with lduw_le_p().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/acpi/core.c       |    3 +--
 include/hw/pci/pci.h |    2 +-
 include/qemu/bswap.h |    5 -----
 3 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index e84a577..f8f45ab 100644
--- a/hw/acpi/core.c
+++ b/hw/acpi/core.c
@@ -170,8 +170,7 @@ static void acpi_table_install(const char unsigned *blob, size_t bloblen,
     }
 
     /* increase number of tables */
-    stw_le_p(acpi_tables,
-             le16_to_cpupu((uint16_t *)acpi_tables) + 1u);
+    stw_le_p(acpi_tables, lduw_le_p(acpi_tables) + 1u);
 
     /* Update the header fields. The strings need not be NUL-terminated. */
     changed_fields = 0;
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 2de27c5..e5231df 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -428,7 +428,7 @@ pci_set_word(uint8_t *config, uint16_t val)
 static inline uint16_t
 pci_get_word(const uint8_t *config)
 {
-    return le16_to_cpupu((const uint16_t *)config);
+    return lduw_le_p(config);
 }
 
 static inline void
diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index d0c4ff0..1c50002 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -412,11 +412,6 @@ static inline void stfq_be_p(void *ptr, float64 v)
 
 /* Legacy unaligned versions.  Note that we never had a complete set.  */
 
-static inline uint16_t le16_to_cpupu(const uint16_t *p)
-{
-    return lduw_le_p(p);
-}
-
 static inline uint32_t le32_to_cpupu(const uint32_t *p)
 {
     return ldl_le_p(p);
-- 
1.7.9.5

[Qemu-devel] [PATCH 4/9] bswap.h: Remove le32_to_cpupu()

[Peter Maydell]

Replace the legacy le32_to_cpupu() with ldl_le_p().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/net/ne2000.c      |    2 +-
 include/hw/pci/pci.h |    2 +-
 include/qemu/bswap.h |    5 -----
 3 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/hw/net/ne2000.c b/hw/net/ne2000.c
index e7865e9..5088e65 100644
--- a/hw/net/ne2000.c
+++ b/hw/net/ne2000.c
@@ -497,7 +497,7 @@ static inline uint32_t ne2000_mem_readl(NE2000State *s, uint32_t addr)
     addr &= ~1; /* XXX: check exact behaviour if not even */
     if (addr < 32 ||
         (addr >= NE2000_PMEM_START && addr < NE2000_MEM_SIZE)) {
-        return le32_to_cpupu((uint32_t *)(s->mem + addr));
+        return ldl_le_p(s->mem + addr);
     } else {
         return 0xffffffff;
     }
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index e5231df..76d67a4 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -440,7 +440,7 @@ pci_set_long(uint8_t *config, uint32_t val)
 static inline uint32_t
 pci_get_long(const uint8_t *config)
 {
-    return le32_to_cpupu((const uint32_t *)config);
+    return ldl_le_p(config);
 }
 
 static inline void
diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index 1c50002..ac5b2e0 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -412,11 +412,6 @@ static inline void stfq_be_p(void *ptr, float64 v)
 
 /* Legacy unaligned versions.  Note that we never had a complete set.  */
 
-static inline uint32_t le32_to_cpupu(const uint32_t *p)
-{
-    return ldl_le_p(p);
-}
-
 static inline uint32_t be32_to_cpupu(const uint32_t *p)
 {
     return ldl_be_p(p);
-- 
1.7.9.5

[Qemu-devel] [PATCH 5/9] bswap.h: Remove be32_to_cpupu()

[Peter Maydell]

Replace the legacy be32_to_cpupu() with ldl_be_p().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/net/e1000.c       |    2 +-
 include/qemu/bswap.h |    5 -----
 2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index fdb1f89..f770f08 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -526,7 +526,7 @@ xmit_seg(E1000State *s)
         if (tp->tcp) {
             sofar = frames * tp->mss;
             cpu_to_be32wu((uint32_t *)(tp->data+css+4),	// seq
-                be32_to_cpupu((uint32_t *)(tp->data+css+4))+sofar);
+                ldl_be_p(tp->data+css+4)+sofar);
             if (tp->paylen - sofar > tp->mss)
                 tp->data[css + 13] &= ~9;		// PSH, FIN
         } else	// UDP
diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index ac5b2e0..9524931 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -412,11 +412,6 @@ static inline void stfq_be_p(void *ptr, float64 v)
 
 /* Legacy unaligned versions.  Note that we never had a complete set.  */
 
-static inline uint32_t be32_to_cpupu(const uint32_t *p)
-{
-    return ldl_be_p(p);
-}
-
 static inline void cpu_to_be16wu(uint16_t *p, uint16_t v)
 {
     stw_be_p(p, v);
-- 
1.7.9.5

[Qemu-devel] [PATCH 6/9] bswap.h: Remove cpu_to_be16wu()

[Peter Maydell]

Replace the legacy cpu_to_be16wu() with stw_be_p().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/block/cdrom.c     |    4 ++--
 hw/ide/atapi.c       |   16 ++++++++--------
 hw/net/e1000.c       |   19 ++++++++-----------
 include/qemu/bswap.h |    5 -----
 4 files changed, 18 insertions(+), 26 deletions(-)

diff --git a/hw/block/cdrom.c b/hw/block/cdrom.c
index 38469fa..5c69f34 100644
--- a/hw/block/cdrom.c
+++ b/hw/block/cdrom.c
@@ -77,7 +77,7 @@ int cdrom_read_toc(int nb_sectors, uint8_t *buf, int msf, int start_track)
         q += 4;
     }
     len = q - buf;
-    cpu_to_be16wu((uint16_t *)buf, len - 2);
+    stw_be_p(buf, len - 2);
     return len;
 }
 
@@ -150,6 +150,6 @@ int cdrom_read_toc_raw(int nb_sectors, uint8_t *buf, int msf, int session_num)
     }
 
     len = q - buf;
-    cpu_to_be16wu((uint16_t *)buf, len - 2);
+    stw_be_p(buf, len - 2);
     return len;
 }
diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 05e60b1..f7d2009 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -437,7 +437,7 @@ static int ide_dvd_read_structure(IDEState *s, int format,
                 cpu_to_ube32(buf + 16, total_sectors - 1); /* l0 end sector */
 
                 /* Size of buffer, not including 2 byte size field */
-                cpu_to_be16wu((uint16_t *)buf, 2048 + 2);
+                stw_be_p(buf, 2048 + 2);
 
                 /* 2k data + 4 byte header */
                 return (2048 + 4);
@@ -448,7 +448,7 @@ static int ide_dvd_read_structure(IDEState *s, int format,
             buf[5] = 0; /* no region restrictions */
 
             /* Size of buffer, not including 2 byte size field */
-            cpu_to_be16wu((uint16_t *)buf, 4 + 2);
+            stw_be_p(buf, 4 + 2);
 
             /* 4 byte header + 4 byte data */
             return (4 + 4);
@@ -458,7 +458,7 @@ static int ide_dvd_read_structure(IDEState *s, int format,
 
         case 0x04: /* DVD disc manufacturing information */
             /* Size of buffer, not including 2 byte size field */
-            cpu_to_be16wu((uint16_t *)buf, 2048 + 2);
+            stw_be_p(buf, 2048 + 2);
 
             /* 2k data + 4 byte header */
             return (2048 + 4);
@@ -471,22 +471,22 @@ static int ide_dvd_read_structure(IDEState *s, int format,
 
             buf[4] = 0x00; /* Physical format */
             buf[5] = 0x40; /* Not writable, is readable */
-            cpu_to_be16wu((uint16_t *)(buf + 6), 2048 + 4);
+            stw_be_p(buf + 6, 2048 + 4);
 
             buf[8] = 0x01; /* Copyright info */
             buf[9] = 0x40; /* Not writable, is readable */
-            cpu_to_be16wu((uint16_t *)(buf + 10), 4 + 4);
+            stw_be_p(buf + 10, 4 + 4);
 
             buf[12] = 0x03; /* BCA info */
             buf[13] = 0x40; /* Not writable, is readable */
-            cpu_to_be16wu((uint16_t *)(buf + 14), 188 + 4);
+            stw_be_p(buf + 14, 188 + 4);
 
             buf[16] = 0x04; /* Manufacturing info */
             buf[17] = 0x40; /* Not writable, is readable */
-            cpu_to_be16wu((uint16_t *)(buf + 18), 2048 + 4);
+            stw_be_p(buf + 18, 2048 + 4);
 
             /* Size of buffer, not including 2 byte size field */
-            cpu_to_be16wu((uint16_t *)buf, 16 + 2);
+            stw_be_p(buf, 16 + 2);
 
             /* data written + 4 byte header */
             return (16 + 4);
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index f770f08..b8e3ce9 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -451,8 +451,7 @@ putsum(uint8_t *data, uint32_t n, uint32_t sloc, uint32_t css, uint32_t cse)
         n = cse + 1;
     if (sloc < n-1) {
         sum = net_checksum_add(n-css, data+css);
-        cpu_to_be16wu((uint16_t *)(data + sloc),
-                      net_checksum_finish(sum));
+        stw_be_p(data + sloc, net_checksum_finish(sum));
     }
 }
 
@@ -513,13 +512,11 @@ xmit_seg(E1000State *s)
         DBGOUT(TXSUM, "frames %d size %d ipcss %d\n",
                frames, tp->size, css);
         if (tp->ip) {		// IPv4
-            cpu_to_be16wu((uint16_t *)(tp->data+css+2),
-                          tp->size - css);
-            cpu_to_be16wu((uint16_t *)(tp->data+css+4),
+            stw_be_p(tp->data+css+2, tp->size - css);
+            stw_be_p(tp->data+css+4,
                           be16_to_cpup((uint16_t *)(tp->data+css+4))+frames);
         } else			// IPv6
-            cpu_to_be16wu((uint16_t *)(tp->data+css+4),
-                          tp->size - css);
+            stw_be_p(tp->data+css+4, tp->size - css);
         css = tp->tucss;
         len = tp->size - css;
         DBGOUT(TXSUM, "tcp %d tucss %d len %d\n", tp->tcp, css, len);
@@ -530,14 +527,14 @@ xmit_seg(E1000State *s)
             if (tp->paylen - sofar > tp->mss)
                 tp->data[css + 13] &= ~9;		// PSH, FIN
         } else	// UDP
-            cpu_to_be16wu((uint16_t *)(tp->data+css+4), len);
+            stw_be_p(tp->data+css+4, len);
         if (tp->sum_needed & E1000_TXD_POPTS_TXSM) {
             unsigned int phsum;
             // add pseudo-header length before checksum calculation
             sp = (uint16_t *)(tp->data + tp->tucso);
             phsum = be16_to_cpup(sp) + len;
             phsum = (phsum >> 16) + (phsum & 0xffff);
-            cpu_to_be16wu(sp, phsum);
+            stw_be_p(sp, phsum);
         }
         tp->tso_frames++;
     }
@@ -606,9 +603,9 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
     if (vlan_enabled(s) && is_vlan_txd(txd_lower) &&
         (tp->cptse || txd_lower & E1000_TXD_CMD_EOP)) {
         tp->vlan_needed = 1;
-        cpu_to_be16wu((uint16_t *)(tp->vlan_header),
+        stw_be_p(tp->vlan_header,
                       le16_to_cpup((uint16_t *)(s->mac_reg + VET)));
-        cpu_to_be16wu((uint16_t *)(tp->vlan_header + 2),
+        stw_be_p(tp->vlan_header + 2,
                       le16_to_cpu(dp->upper.fields.special));
     }
         
diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index 9524931..3bb6b71 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -412,11 +412,6 @@ static inline void stfq_be_p(void *ptr, float64 v)
 
 /* Legacy unaligned versions.  Note that we never had a complete set.  */
 
-static inline void cpu_to_be16wu(uint16_t *p, uint16_t v)
-{
-    stw_be_p(p, v);
-}
-
 static inline void cpu_to_be32wu(uint32_t *p, uint32_t v)
 {
     stl_be_p(p, v);
-- 
1.7.9.5

[Qemu-devel] [PATCH 7/9] bswap.h: Remove cpu_to_be32wu()

[Peter Maydell]

Replace the legacy cpu_to_be32wu() with stl_be_p().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/block/cdrom.c     |    6 +++---
 hw/net/e1000.c       |    3 +--
 hw/pci/pcie_aer.c    |    4 ++--
 include/qemu/bswap.h |    5 -----
 4 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/hw/block/cdrom.c b/hw/block/cdrom.c
index 5c69f34..4e1019c 100644
--- a/hw/block/cdrom.c
+++ b/hw/block/cdrom.c
@@ -59,7 +59,7 @@ int cdrom_read_toc(int nb_sectors, uint8_t *buf, int msf, int start_track)
             q += 3;
         } else {
             /* sector 0 */
-            cpu_to_be32wu((uint32_t *)q, 0);
+            stl_be_p(q, 0);
             q += 4;
         }
     }
@@ -73,7 +73,7 @@ int cdrom_read_toc(int nb_sectors, uint8_t *buf, int msf, int start_track)
         lba_to_msf(q, nb_sectors);
         q += 3;
     } else {
-        cpu_to_be32wu((uint32_t *)q, nb_sectors);
+        stl_be_p(q, nb_sectors);
         q += 4;
     }
     len = q - buf;
@@ -127,7 +127,7 @@ int cdrom_read_toc_raw(int nb_sectors, uint8_t *buf, int msf, int session_num)
         lba_to_msf(q, nb_sectors);
         q += 3;
     } else {
-        cpu_to_be32wu((uint32_t *)q, nb_sectors);
+        stl_be_p(q, nb_sectors);
         q += 4;
     }
 
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index b8e3ce9..06a2f6e 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -522,8 +522,7 @@ xmit_seg(E1000State *s)
         DBGOUT(TXSUM, "tcp %d tucss %d len %d\n", tp->tcp, css, len);
         if (tp->tcp) {
             sofar = frames * tp->mss;
-            cpu_to_be32wu((uint32_t *)(tp->data+css+4),	// seq
-                ldl_be_p(tp->data+css+4)+sofar);
+            stl_be_p(tp->data+css+4, ldl_be_p(tp->data+css+4)+sofar); /* seq */
             if (tp->paylen - sofar > tp->mss)
                 tp->data[css + 13] &= ~9;		// PSH, FIN
         } else	// UDP
diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index ca762ab..723d88d 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -425,7 +425,7 @@ static void pcie_aer_update_log(PCIDevice *dev, const PCIEAERErr *err)
             /* 7.10.8 Header Log Register */
             uint8_t *header_log =
                 aer_cap + PCI_ERR_HEADER_LOG + i * sizeof err->header[0];
-            cpu_to_be32wu((uint32_t*)header_log, err->header[i]);
+            stl_be_p(header_log, err->header[i]);
         }
     } else {
         assert(!(err->flags & PCIE_AER_ERR_TLP_PREFIX_PRESENT));
@@ -439,7 +439,7 @@ static void pcie_aer_update_log(PCIDevice *dev, const PCIEAERErr *err)
             /* 7.10.12 tlp prefix log register */
             uint8_t *prefix_log =
                 aer_cap + PCI_ERR_TLP_PREFIX_LOG + i * sizeof err->prefix[0];
-            cpu_to_be32wu((uint32_t*)prefix_log, err->prefix[i]);
+            stl_be_p(prefix_log, err->prefix[i]);
         }
         errcap |= PCI_ERR_CAP_TLP;
     } else {
diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index 3bb6b71..b95cc73 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -412,11 +412,6 @@ static inline void stfq_be_p(void *ptr, float64 v)
 
 /* Legacy unaligned versions.  Note that we never had a complete set.  */
 
-static inline void cpu_to_be32wu(uint32_t *p, uint32_t v)
-{
-    stl_be_p(p, v);
-}
-
 static inline void cpu_to_be64wu(uint64_t *p, uint64_t v)
 {
     stq_be_p(p, v);
-- 
1.7.9.5

[Qemu-devel] [PATCH 8/9] bswap.h: Remove cpu_to_be64wu()

[Peter Maydell]

Replace the legacy cpu_to_be64wu() with stq_be_p().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 block/qcow2-cluster.c |    2 +-
 include/qemu/bswap.h  |    5 -----
 2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index cca76d4..363a725 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -92,7 +92,7 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
     /* set new table */
     BLKDBG_EVENT(bs->file, BLKDBG_L1_GROW_ACTIVATE_TABLE);
     cpu_to_be32w((uint32_t*)data, new_l1_size);
-    cpu_to_be64wu((uint64_t*)(data + 4), new_l1_table_offset);
+    stq_be_p(data + 4, new_l1_table_offset);
     ret = bdrv_pwrite_sync(bs->file, offsetof(QCowHeader, l1_size), data,sizeof(data));
     if (ret < 0) {
         goto fail;
diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index b95cc73..4cbd6e7 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -412,11 +412,6 @@ static inline void stfq_be_p(void *ptr, float64 v)
 
 /* Legacy unaligned versions.  Note that we never had a complete set.  */
 
-static inline void cpu_to_be64wu(uint64_t *p, uint64_t v)
-{
-    stq_be_p(p, v);
-}
-
 static inline void cpu_to_32wu(uint32_t *p, uint32_t v)
 {
     stl_p(p, v);
-- 
1.7.9.5

[Qemu-devel] [PATCH 9/9] bswap.h: Remove cpu_to_32wu()

[Peter Maydell]

Replace the legacy cpu_to_32wu() with stl_p().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/display/vga_template.h |   14 ++++++++------
 include/qemu/bswap.h      |    7 -------
 2 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/hw/display/vga_template.h b/hw/display/vga_template.h
index f6f6a01..6cfae56 100644
--- a/hw/display/vga_template.h
+++ b/hw/display/vga_template.h
@@ -113,20 +113,22 @@ static void glue(vga_draw_glyph9_, DEPTH)(uint8_t *d, int linesize,
     do {
         font_data = font_ptr[0];
 #if BPP == 1
-        cpu_to_32wu((uint32_t *)d, (dmask16[(font_data >> 4)] & xorcol) ^ bgcol);
+        stl_p((uint32_t *)d, (dmask16[(font_data >> 4)] & xorcol) ^ bgcol);
         v = (dmask16[(font_data >> 0) & 0xf] & xorcol) ^ bgcol;
-        cpu_to_32wu(((uint32_t *)d)+1, v);
+        stl_p(((uint32_t *)d)+1, v);
         if (dup9)
             ((uint8_t *)d)[8] = v >> (24 * (1 - BIG));
         else
             ((uint8_t *)d)[8] = bgcol;
 
 #elif BPP == 2
-        cpu_to_32wu(((uint32_t *)d)+0, (dmask4[(font_data >> 6)] & xorcol) ^ bgcol);
-        cpu_to_32wu(((uint32_t *)d)+1, (dmask4[(font_data >> 4) & 3] & xorcol) ^ bgcol);
-        cpu_to_32wu(((uint32_t *)d)+2, (dmask4[(font_data >> 2) & 3] & xorcol) ^ bgcol);
+        stl_p(((uint32_t *)d)+0, (dmask4[(font_data >> 6)] & xorcol) ^ bgcol);
+        stl_p(((uint32_t *)d)+1,
+              (dmask4[(font_data >> 4) & 3] & xorcol) ^ bgcol);
+        stl_p(((uint32_t *)d)+2,
+              (dmask4[(font_data >> 2) & 3] & xorcol) ^ bgcol);
         v = (dmask4[(font_data >> 0) & 3] & xorcol) ^ bgcol;
-        cpu_to_32wu(((uint32_t *)d)+3, v);
+        stl_p(((uint32_t *)d)+3, v);
         if (dup9)
             ((uint16_t *)d)[8] = v >> (16 * (1 - BIG));
         else
diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index 4cbd6e7..437b8e0 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -410,13 +410,6 @@ static inline void stfq_be_p(void *ptr, float64 v)
     stq_be_p(ptr, u.ll);
 }
 
-/* Legacy unaligned versions.  Note that we never had a complete set.  */
-
-static inline void cpu_to_32wu(uint32_t *p, uint32_t v)
-{
-    stl_p(p, v);
-}
-
 static inline unsigned long leul_to_cpu(unsigned long v)
 {
     /* In order to break an include loop between here and
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 2/9] bswap.h: Remove cpu_to_le32wu()

[Stefan Hajnoczi]

On Sun, Aug 25, 2013 at 03:59:30PM +0100, Peter Maydell wrote:
> Replace the legacy cpu_to_le32wu() with stl_le_p().
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/net/ne2000.c      |    2 +-
>  include/hw/pci/pci.h |    2 +-
>  include/qemu/bswap.h |    5 -----
>  3 files changed, 2 insertions(+), 7 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Re: [Qemu-devel] [PATCH 4/9] bswap.h: Remove le32_to_cpupu()

[Stefan Hajnoczi]

On Sun, Aug 25, 2013 at 03:59:32PM +0100, Peter Maydell wrote:
> Replace the legacy le32_to_cpupu() with ldl_le_p().
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/net/ne2000.c      |    2 +-
>  include/hw/pci/pci.h |    2 +-
>  include/qemu/bswap.h |    5 -----
>  3 files changed, 2 insertions(+), 7 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Re: [Qemu-devel] [PATCH 5/9] bswap.h: Remove be32_to_cpupu()

[Stefan Hajnoczi]

On Sun, Aug 25, 2013 at 03:59:33PM +0100, Peter Maydell wrote:
> Replace the legacy be32_to_cpupu() with ldl_be_p().
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/net/e1000.c       |    2 +-
>  include/qemu/bswap.h |    5 -----
>  2 files changed, 1 insertion(+), 6 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Re: [Qemu-devel] [PATCH 6/9] bswap.h: Remove cpu_to_be16wu()

[Stefan Hajnoczi]

On Sun, Aug 25, 2013 at 03:59:34PM +0100, Peter Maydell wrote:
> Replace the legacy cpu_to_be16wu() with stw_be_p().
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/block/cdrom.c     |    4 ++--
>  hw/ide/atapi.c       |   16 ++++++++--------
>  hw/net/e1000.c       |   19 ++++++++-----------
>  include/qemu/bswap.h |    5 -----
>  4 files changed, 18 insertions(+), 26 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Re: [Qemu-devel] [PATCH 7/9] bswap.h: Remove cpu_to_be32wu()

[Stefan Hajnoczi]

On Sun, Aug 25, 2013 at 03:59:35PM +0100, Peter Maydell wrote:
> Replace the legacy cpu_to_be32wu() with stl_be_p().
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/block/cdrom.c     |    6 +++---
>  hw/net/e1000.c       |    3 +--
>  hw/pci/pcie_aer.c    |    4 ++--
>  include/qemu/bswap.h |    5 -----
>  4 files changed, 6 insertions(+), 12 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Re: [Qemu-devel] [PATCH 8/9] bswap.h: Remove cpu_to_be64wu()

[Stefan Hajnoczi]

On Sun, Aug 25, 2013 at 03:59:36PM +0100, Peter Maydell wrote:
> Replace the legacy cpu_to_be64wu() with stq_be_p().
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  block/qcow2-cluster.c |    2 +-
>  include/qemu/bswap.h  |    5 -----
>  2 files changed, 1 insertion(+), 6 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Re: [Qemu-devel] [PATCH 0/9] Remove legacy unaligned bswap functions

[Richard Henderson]

On 08/25/2013 07:59 AM, Peter Maydell wrote:
> Peter Maydell (9):
>   bswap.h: Remove cpu_to_le16wu()
>   bswap.h: Remove cpu_to_le32wu()
>   bswap.h: Remove le16_to_cpupu()
>   bswap.h: Remove le32_to_cpupu()
>   bswap.h: Remove be32_to_cpupu()
>   bswap.h: Remove cpu_to_be16wu()
>   bswap.h: Remove cpu_to_be32wu()
>   bswap.h: Remove cpu_to_be64wu()
>   bswap.h: Remove cpu_to_32wu()

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

Re: [Qemu-devel] [PATCH 0/9] Remove legacy unaligned bswap functions

[Peter Maydell]

Ping!

thanks
-- PMM


On 25 August 2013 15:59, Peter Maydell <peter.maydell@linaro.org> wrote:
> The bswap.h header includes a set of "legacy unaligned functions"
> that (since commit c732a52d3 at the beginning of this year) are
> just wrappers for underlying {ld,st}<type> functions. The legacy
> functions aren't used in many places, so just replace all their
> uses with uses of the new-style {ld,st} functions; this lets us
> remove the legacy wrappers altogether.
>
> Since we know the {ld,st}* routines are definitely functions,
> we can in the process remove some casts which were left over
> from when the legacy unaligned functions were previously macros.
>
> The patchset is divided up by function being removed, rather
> than by which device/subsystem is being fixed; I think this way
> round is easier to review since you only have to keep one
> substitution in your head when reading a patch.
>
> Peter Maydell (9):
>   bswap.h: Remove cpu_to_le16wu()
>   bswap.h: Remove cpu_to_le32wu()
>   bswap.h: Remove le16_to_cpupu()
>   bswap.h: Remove le32_to_cpupu()
>   bswap.h: Remove be32_to_cpupu()
>   bswap.h: Remove cpu_to_be16wu()
>   bswap.h: Remove cpu_to_be32wu()
>   bswap.h: Remove cpu_to_be64wu()
>   bswap.h: Remove cpu_to_32wu()
>
>  block/qcow2-cluster.c     |    2 +-
>  hw/acpi/core.c            |    3 +--
>  hw/block/cdrom.c          |   10 +++++-----
>  hw/display/vga_template.h |   14 ++++++++------
>  hw/ide/atapi.c            |   16 +++++++--------
>  hw/net/e1000.c            |   22 +++++++++------------
>  hw/net/ne2000.c           |    4 ++--
>  hw/pci/pcie_aer.c         |    4 ++--
>  include/hw/pci/pci.h      |    8 ++++----
>  include/qemu/bswap.h      |   47 ---------------------------------------------
>  10 files changed, 40 insertions(+), 90 deletions(-)
>
> --
> 1.7.9.5

Re: [Qemu-devel] [PATCH 0/9] Remove legacy unaligned bswap functions

[Michael S. Tsirkin]

On Sun, Aug 25, 2013 at 03:59:28PM +0100, Peter Maydell wrote:
> The bswap.h header includes a set of "legacy unaligned functions"
> that (since commit c732a52d3 at the beginning of this year) are
> just wrappers for underlying {ld,st}<type> functions. The legacy
> functions aren't used in many places, so just replace all their
> uses with uses of the new-style {ld,st} functions; this lets us
> remove the legacy wrappers altogether.

I have a question on this: what happens if an unaligned
address is supplied?
In particular, if the address is supplied by the guest?

Esp the pci wrappers have many callers - were they all
audited?
I tried checking but there are many callers, will take
a while.

> Since we know the {ld,st}* routines are definitely functions,
> we can in the process remove some casts which were left over
> from when the legacy unaligned functions were previously macros.
> 
> The patchset is divided up by function being removed, rather
> than by which device/subsystem is being fixed; I think this way
> round is easier to review since you only have to keep one
> substitution in your head when reading a patch.
> 
> Peter Maydell (9):
>   bswap.h: Remove cpu_to_le16wu()
>   bswap.h: Remove cpu_to_le32wu()
>   bswap.h: Remove le16_to_cpupu()
>   bswap.h: Remove le32_to_cpupu()
>   bswap.h: Remove be32_to_cpupu()
>   bswap.h: Remove cpu_to_be16wu()
>   bswap.h: Remove cpu_to_be32wu()
>   bswap.h: Remove cpu_to_be64wu()
>   bswap.h: Remove cpu_to_32wu()
> 
>  block/qcow2-cluster.c     |    2 +-
>  hw/acpi/core.c            |    3 +--
>  hw/block/cdrom.c          |   10 +++++-----
>  hw/display/vga_template.h |   14 ++++++++------
>  hw/ide/atapi.c            |   16 +++++++--------
>  hw/net/e1000.c            |   22 +++++++++------------
>  hw/net/ne2000.c           |    4 ++--
>  hw/pci/pcie_aer.c         |    4 ++--
>  include/hw/pci/pci.h      |    8 ++++----
>  include/qemu/bswap.h      |   47 ---------------------------------------------
>  10 files changed, 40 insertions(+), 90 deletions(-)
> 
> -- 
> 1.7.9.5

Re: [Qemu-devel] [PATCH 0/9] Remove legacy unaligned bswap functions

[Peter Maydell]

On 9 September 2013 12:54, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Sun, Aug 25, 2013 at 03:59:28PM +0100, Peter Maydell wrote:
>> The bswap.h header includes a set of "legacy unaligned functions"
>> that (since commit c732a52d3 at the beginning of this year) are
>> just wrappers for underlying {ld,st}<type> functions. The legacy
>> functions aren't used in many places, so just replace all their
>> uses with uses of the new-style {ld,st} functions; this lets us
>> remove the legacy wrappers altogether.
>
> I have a question on this: what happens if an unaligned
> address is supplied?
> In particular, if the address is supplied by the guest?
>
> Esp the pci wrappers have many callers - were they all
> audited?
> I tried checking but there are many callers, will take
> a while.

This patchset has zero behavioural changes, because
literally all it is doing is taking trivial wrapper
functions like

static inline void cpu_to_le16wu(uint16_t *p, uint16_t v)
{
    stw_le_p(p, v);
}

and replacing the calls to the wrapper with direct
calls to the underlying function. There's no need
to audit calls to the callsites because there is
no semantic change and not even any implementation
change here.

There are no problems with unaligned accesses, because
the stw_le_p &c functions are designed to work for
unaligned accesses: they boil down to calls to
ldl_p/stw_p and friends, which use memcpy() to
ensure they work OK with unaligned accesses.

You can check all this by looking at bswap.h.

-- PMM

Re: [Qemu-devel] [PATCH 0/9] Remove legacy unaligned bswap functions

[Peter Maydell]

Ping^2, seven weeks after initial post and code review.

Frankly, our mechanism for committing patches which aren't
obviously localised to a specific subsystem sucks :-(
It seems like pretty much every time it takes multiple
pings and a month or two before anything gets into the tree.
(at least this one doesn't require a rebase...)

thanks
-- PMM

On 9 September 2013 11:38, Peter Maydell <peter.maydell@linaro.org> wrote:
> Ping!
>
> thanks
> -- PMM
>
>
> On 25 August 2013 15:59, Peter Maydell <peter.maydell@linaro.org> wrote:
>> The bswap.h header includes a set of "legacy unaligned functions"
>> that (since commit c732a52d3 at the beginning of this year) are
>> just wrappers for underlying {ld,st}<type> functions. The legacy
>> functions aren't used in many places, so just replace all their
>> uses with uses of the new-style {ld,st} functions; this lets us
>> remove the legacy wrappers altogether.
>>
>> Since we know the {ld,st}* routines are definitely functions,
>> we can in the process remove some casts which were left over
>> from when the legacy unaligned functions were previously macros.
>>
>> The patchset is divided up by function being removed, rather
>> than by which device/subsystem is being fixed; I think this way
>> round is easier to review since you only have to keep one
>> substitution in your head when reading a patch.
>>
>> Peter Maydell (9):
>>   bswap.h: Remove cpu_to_le16wu()
>>   bswap.h: Remove cpu_to_le32wu()
>>   bswap.h: Remove le16_to_cpupu()
>>   bswap.h: Remove le32_to_cpupu()
>>   bswap.h: Remove be32_to_cpupu()
>>   bswap.h: Remove cpu_to_be16wu()
>>   bswap.h: Remove cpu_to_be32wu()
>>   bswap.h: Remove cpu_to_be64wu()
>>   bswap.h: Remove cpu_to_32wu()
>>
>>  block/qcow2-cluster.c     |    2 +-
>>  hw/acpi/core.c            |    3 +--
>>  hw/block/cdrom.c          |   10 +++++-----
>>  hw/display/vga_template.h |   14 ++++++++------
>>  hw/ide/atapi.c            |   16 +++++++--------
>>  hw/net/e1000.c            |   22 +++++++++------------
>>  hw/net/ne2000.c           |    4 ++--
>>  hw/pci/pcie_aer.c         |    4 ++--
>>  include/hw/pci/pci.h      |    8 ++++----
>>  include/qemu/bswap.h      |   47 ---------------------------------------------
>>  10 files changed, 40 insertions(+), 90 deletions(-)
>>
>> --
>> 1.7.9.5

Re: [Qemu-devel] [PATCH 0/9] Remove legacy unaligned bswap functions

[Michael S. Tsirkin]

On Sun, Aug 25, 2013 at 03:59:28PM +0100, Peter Maydell wrote:
> The bswap.h header includes a set of "legacy unaligned functions"
> that (since commit c732a52d3 at the beginning of this year) are
> just wrappers for underlying {ld,st}<type> functions. The legacy
> functions aren't used in many places, so just replace all their
> uses with uses of the new-style {ld,st} functions; this lets us
> remove the legacy wrappers altogether.
> 
> Since we know the {ld,st}* routines are definitely functions,
> we can in the process remove some casts which were left over
> from when the legacy unaligned functions were previously macros.
> 
> The patchset is divided up by function being removed, rather
> than by which device/subsystem is being fixed; I think this way
> round is easier to review since you only have to keep one
> substitution in your head when reading a patch.

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

Sorry about the delay.

> Peter Maydell (9):
>   bswap.h: Remove cpu_to_le16wu()
>   bswap.h: Remove cpu_to_le32wu()
>   bswap.h: Remove le16_to_cpupu()
>   bswap.h: Remove le32_to_cpupu()
>   bswap.h: Remove be32_to_cpupu()
>   bswap.h: Remove cpu_to_be16wu()
>   bswap.h: Remove cpu_to_be32wu()
>   bswap.h: Remove cpu_to_be64wu()
>   bswap.h: Remove cpu_to_32wu()
> 
>  block/qcow2-cluster.c     |    2 +-
>  hw/acpi/core.c            |    3 +--
>  hw/block/cdrom.c          |   10 +++++-----
>  hw/display/vga_template.h |   14 ++++++++------
>  hw/ide/atapi.c            |   16 +++++++--------
>  hw/net/e1000.c            |   22 +++++++++------------
>  hw/net/ne2000.c           |    4 ++--
>  hw/pci/pcie_aer.c         |    4 ++--
>  include/hw/pci/pci.h      |    8 ++++----
>  include/qemu/bswap.h      |   47 ---------------------------------------------
>  10 files changed, 40 insertions(+), 90 deletions(-)
> 
> -- 
> 1.7.9.5

Re: [Qemu-devel] [PATCH 0/9] Remove legacy unaligned bswap functions

[Michael S. Tsirkin]

On Tue, Oct 15, 2013 at 02:36:58PM +0100, Peter Maydell wrote:
> Ping^2, seven weeks after initial post and code review.
> 
> Frankly, our mechanism for committing patches which aren't
> obviously localised to a specific subsystem sucks :-(
> It seems like pretty much every time it takes multiple
> pings and a month or two before anything gets into the tree.
> (at least this one doesn't require a rebase...)
> 
> thanks
> -- PMM

I think we should have a rule: no response within two weeks, any
maintainer can apply the patch.  We can make exceptions e.g. if
maintainer notifies list he's unavailable, etc, but that should be the
rule.

-- 
MST