[Qemu-devel] [PATCH v2 0/5] Drop cpu_to_*w, *_to_cpup, document cpu_to_*, *_to_cpu

[Qemu-devel] [PATCH v2 0/5] Drop cpu_to_*w, *_to_cpup, document cpu_to_*, *_to_cpu

[Peter Maydell]

This patchset removes the final uses of the deprecated
cpu_to_*w() and *_to_cpup() conversion functions, and
then removes their implementations. We can then document
the remaining cpu_to_*() and *_to_cpu() conversion functions.

These are all the same as v1, except that:
 * qcow2 patch is upstream
 * I fixed the silly mistake in the documentation of
   the prototypes of the conversion functions in the last patch
 * rebased

All the patches have been reviewed, so I'm planning to just
commit these to master.

thanks
-- PMM


Peter Maydell (5):
  fsdev/9p-iov-marshal.c: Don't use cpu_to_*w() functions
  hw/bt: Don't use cpu_to_*w() and *_to_cpup()
  bswap.h: Remove unused cpu_to_*w() and *_to_cpup()
  bswap.h: Fix comment typo
  bswap.h: Document cpu_to_* and *_to_cpu conversion functions

 fsdev/9p-iov-marshal.c | 18 +++++--------
 hw/bt/hci.c            | 10 +++-----
 hw/bt/l2cap.c          | 12 ++++-----
 include/qemu/bswap.h   | 70 ++++++++++++++++++++++++++++++++++++++++++--------
 4 files changed, 74 insertions(+), 36 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH v2 1/5] fsdev/9p-iov-marshal.c: Don't use cpu_to_*w() functions

[Peter Maydell]

Don't use the cpu_to_*w() functions, which we are trying to deprecate.
Instead just use cpu_to_*() to do the byteswap, which brings the
code in the marshal function in line with that in the unmarshal.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 fsdev/9p-iov-marshal.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/fsdev/9p-iov-marshal.c b/fsdev/9p-iov-marshal.c
index 584082b..663cad5 100644
--- a/fsdev/9p-iov-marshal.c
+++ b/fsdev/9p-iov-marshal.c
@@ -207,31 +207,25 @@ ssize_t v9fs_iov_vmarshal(struct iovec *in_sg, int in_num, size_t offset,
             break;
         }
         case 'w': {
-            uint16_t val;
+            uint16_t val = va_arg(ap, int);
             if (bswap) {
-                cpu_to_le16w(&val, va_arg(ap, int));
-            } else {
-                val =  va_arg(ap, int);
+                val = cpu_to_le16(val);
             }
             copied = v9fs_pack(in_sg, in_num, offset, &val, sizeof(val));
             break;
         }
         case 'd': {
-            uint32_t val;
+            uint32_t val = va_arg(ap, uint32_t);
             if (bswap) {
-                cpu_to_le32w(&val, va_arg(ap, uint32_t));
-            } else {
-                val =  va_arg(ap, uint32_t);
+                val = cpu_to_le32(val);
             }
             copied = v9fs_pack(in_sg, in_num, offset, &val, sizeof(val));
             break;
         }
         case 'q': {
-            uint64_t val;
+            uint64_t val = va_arg(ap, uint64_t);
             if (bswap) {
-                cpu_to_le64w(&val, va_arg(ap, uint64_t));
-            } else {
-                val =  va_arg(ap, uint64_t);
+                val = cpu_to_le64(val);
             }
             copied = v9fs_pack(in_sg, in_num, offset, &val, sizeof(val));
             break;
-- 
1.9.1

[Qemu-devel] [PATCH v2 2/5] hw/bt: Don't use cpu_to_*w() and *_to_cpup()

[Peter Maydell]

Don't use cpu_to_*w() and *_to_cpup() to do byte-swapped loads
and stores; instead use ld*_p() and st*_p() which correctly handle
misaligned accesses.

Bring the HNDL() macro into line with how we deal with
PARAMHANDLE(), by using cpu_to_le16() rather than an ifdef
HOST_WORDS_BIGENDIAN.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 hw/bt/hci.c   | 10 +++-------
 hw/bt/l2cap.c | 12 ++++++------
 2 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/hw/bt/hci.c b/hw/bt/hci.c
index 7d52205..351123f 100644
--- a/hw/bt/hci.c
+++ b/hw/bt/hci.c
@@ -426,11 +426,7 @@ static void bt_submit_raw_acl(struct bt_piconet_s *net, int length, uint8_t *dat
  * be continuously allocated.  We do it though, to preserve similar
  * behaviour between hosts.  Some things, like the BD_ADDR cannot be
  * preserved though (for example if a real hci is used).  */
-#ifdef HOST_WORDS_BIGENDIAN
-# define HNDL(raw)	bswap16(raw)
-#else
-# define HNDL(raw)	(raw)
-#endif
+#define HNDL(raw) cpu_to_le16(raw)
 
 static const uint8_t bt_event_reserved_mask[8] = {
     0xff, 0x9f, 0xfb, 0xff, 0x07, 0x18, 0x00, 0x00,
@@ -1504,8 +1500,8 @@ static void bt_submit_hci(struct HCIInfo *info,
         return;
 
 #define PARAM(cmd, param)	(((cmd##_cp *) data)->param)
-#define PARAM16(cmd, param)	le16_to_cpup(&PARAM(cmd, param))
-#define PARAMHANDLE(cmd)	HNDL(PARAM(cmd, handle))
+#define PARAM16(cmd, param) lduw_le_p(&PARAM(cmd, param))
+#define PARAMHANDLE(cmd) PARAM16(cmd, handle)
 #define LENGTH_CHECK(cmd)	if (length < sizeof(cmd##_cp)) goto short_hci
     /* Note: the supported commands bitmask in bt_hci_read_local_commands_rp
      * needs to be updated every time a command is implemented here!  */
diff --git a/hw/bt/l2cap.c b/hw/bt/l2cap.c
index dfc95ed..e342045 100644
--- a/hw/bt/l2cap.c
+++ b/hw/bt/l2cap.c
@@ -526,9 +526,9 @@ static int l2cap_channel_config(struct l2cap_instance_s *l2cap,
             }
 
             /* MTU */
-            val = le16_to_cpup((void *) opt->val);
+            val = lduw_le_p(opt->val);
             if (val < ch->min_mtu) {
-                cpu_to_le16w((void *) opt->val, ch->min_mtu);
+                stw_le_p(opt->val, ch->min_mtu);
                 result = L2CAP_CONF_UNACCEPT;
                 break;
             }
@@ -543,7 +543,7 @@ static int l2cap_channel_config(struct l2cap_instance_s *l2cap,
             }
 
             /* Flush Timeout */
-            val = le16_to_cpup((void *) opt->val);
+            val = lduw_le_p(opt->val);
             if (val < 0x0001) {
                 opt->val[0] = 0xff;
                 opt->val[1] = 0xff;
@@ -987,7 +987,7 @@ static void l2cap_bframe_in(struct l2cap_chan_s *ch, uint16_t cid,
 static void l2cap_iframe_in(struct l2cap_chan_s *ch, uint16_t cid,
                 const l2cap_hdr *hdr, int len)
 {
-    uint16_t fcs = le16_to_cpup((void *) (hdr->data + len - 2));
+    uint16_t fcs = lduw_le_p(hdr->data + len - 2);
 
     if (len < 4)
         goto len_error;
@@ -1002,7 +1002,7 @@ static void l2cap_iframe_in(struct l2cap_chan_s *ch, uint16_t cid,
             /* TODO: Signal an error? */
             return;
         }
-        l2cap_sframe_in(ch, le16_to_cpup((void *) hdr->data));
+        l2cap_sframe_in(ch, lduw_le_p(hdr->data));
         return;
     }
 
@@ -1022,7 +1022,7 @@ static void l2cap_iframe_in(struct l2cap_chan_s *ch, uint16_t cid,
         if (len - 6 > ch->mps)
             goto len_error;
 
-        ch->len_total = le16_to_cpup((void *) (hdr->data + 2));
+        ch->len_total = lduw_le_p(hdr->data + 2);
         if (len >= 6 + ch->len_total)
             goto seg_error;
 
-- 
1.9.1

[Qemu-devel] [PATCH v2 3/5] bswap.h: Remove unused cpu_to_*w() and *_to_cpup()

[Peter Maydell]

Now that all uses of cpu_to_*w() and *_to_cpup() have been replaced
with either ld*_p()/st*_p() or by doing direct dereferences and
using the cpu_to_*()/*_to_cpu() byteswap functions, we can remove
the unused implementations.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 include/qemu/bswap.h | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index ce3c42e..3b48cf4 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -99,16 +99,6 @@ static inline void endian ## size ## _to_cpus(type *p)\
 static inline void cpu_to_ ## endian ## size ## s(type *p)\
 {\
     glue(endian, _bswaps)(p, size);\
-}\
-\
-static inline type endian ## size ## _to_cpup(const type *p)\
-{\
-    return glue(glue(endian, size), _to_cpu)(*p);\
-}\
-\
-static inline void cpu_to_ ## endian ## size ## w(type *p, type v)\
-{\
-    *p = glue(glue(cpu_to_, endian), size)(v);\
 }
 
 CPU_CONVERT(be, 16, uint16_t)
-- 
1.9.1

[Qemu-devel] [PATCH v2 4/5] bswap.h: Fix comment typo

[Peter Maydell]

Fix a typo in a comment.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Stefan Weil <sw@weilnetz.de>
---
 include/qemu/bswap.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index 3b48cf4..a17d9aa 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -116,7 +116,7 @@ static inline uint32_t qemu_bswap_len(uint32_t value, int len)
 }
 
 /*
- * Same as cpu_to_le{16,23}, except that gcc will figure the result is
+ * Same as cpu_to_le{16,32}, except that gcc will figure the result is
  * a compile-time constant if you pass in a constant.  So this can be
  * used to initialize static variables.
  */
-- 
1.9.1

[Qemu-devel] [PATCH v2 5/5] bswap.h: Document cpu_to_* and *_to_cpu conversion functions

[Peter Maydell]

Add a documentation comment describing the functions for
converting between the cpu and little or bigendian formats.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 include/qemu/bswap.h | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index a17d9aa..09c78fd 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -80,6 +80,64 @@ static inline void bswap64s(uint64_t *s)
 #define be_bswaps(p, size) do { *p = glue(bswap, size)(*p); } while(0)
 #endif
 
+/**
+ * Endianness conversion functions between host cpu and specified endianness.
+ * (We list the complete set of prototypes produced by the macros below
+ * to assist people who search the headers to find their definitions.)
+ *
+ * uint16_t le16_to_cpu(uint16_t v);
+ * uint32_t le32_to_cpu(uint32_t v);
+ * uint64_t le64_to_cpu(uint64_t v);
+ * uint16_t be16_to_cpu(uint16_t v);
+ * uint32_t be32_to_cpu(uint32_t v);
+ * uint64_t be64_to_cpu(uint64_t v);
+ *
+ * Convert the value @v from the specified format to the native
+ * endianness of the host CPU by byteswapping if necessary, and
+ * return the converted value.
+ *
+ * uint16_t cpu_to_le16(uint16_t v);
+ * uint32_t cpu_to_le32(uint32_t v);
+ * uint64_t cpu_to_le64(uint64_t v);
+ * uint16_t cpu_to_be16(uint16_t v);
+ * uint32_t cpu_to_be32(uint32_t v);
+ * uint64_t cpu_to_be64(uint64_t v);
+ *
+ * Convert the value @v from the native endianness of the host CPU to
+ * the specified format by byteswapping if necessary, and return
+ * the converted value.
+ *
+ * void le16_to_cpus(uint16_t *v);
+ * void le32_to_cpus(uint32_t *v);
+ * void le64_to_cpus(uint64_t *v);
+ * void be16_to_cpus(uint16_t *v);
+ * void be32_to_cpus(uint32_t *v);
+ * void be64_to_cpus(uint64_t *v);
+ *
+ * Do an in-place conversion of the value pointed to by @v from the
+ * specified format to the native endianness of the host CPU.
+ *
+ * void cpu_to_le16s(uint16_t *v);
+ * void cpu_to_le32s(uint32_t *v);
+ * void cpu_to_le64s(uint64_t *v);
+ * void cpu_to_be16s(uint16_t *v);
+ * void cpu_to_be32s(uint32_t *v);
+ * void cpu_to_be64s(uint64_t *v);
+ *
+ * Do an in-place conversion of the value pointed to by @v from the
+ * native endianness of the host CPU to the specified format.
+ *
+ * Both X_to_cpu() and cpu_to_X() perform the same operation; you
+ * should use whichever one is better documenting of the function your
+ * code is performing.
+ *
+ * Do not use these functions for conversion of values which are in guest
+ * memory, since the data may not be sufficiently aligned for the host CPU's
+ * load and store instructions. Instead you should use the ld*_p() and
+ * st*_p() functions, which perform loads and stores of data of any
+ * required size and endianness and handle possible misalignment.
+ */
+
 #define CPU_CONVERT(endian, size, type)\
 static inline type endian ## size ## _to_cpu(type v)\
 {\
-- 
1.9.1

Re: [Qemu-devel] [PATCH v2 0/5] Drop cpu_to_*w, *_to_cpup, document cpu_to_*, *_to_cpu

[Peter Maydell]

On 7 July 2016 at 17:20, Peter Maydell <peter.maydell@linaro.org> wrote:
> This patchset removes the final uses of the deprecated
> cpu_to_*w() and *_to_cpup() conversion functions, and
> then removes their implementations. We can then document
> the remaining cpu_to_*() and *_to_cpu() conversion functions.
>
> These are all the same as v1, except that:
>  * qcow2 patch is upstream
>  * I fixed the silly mistake in the documentation of
>    the prototypes of the conversion functions in the last patch
>  * rebased
>
> All the patches have been reviewed, so I'm planning to just
> commit these to master.

Applied, thanks
-- PMM