All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/8] qemu/bswap: Use compiler __builtin_bswap()
@ 2020-09-28 13:19 Philippe Mathieu-Daudé
  2020-09-28 13:19 ` [PATCH v2 1/8] qemu/bswap: Remove unused qemu_bswap_len() Philippe Mathieu-Daudé
                   ` (7 more replies)
  0 siblings, 8 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-28 13:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Alex Bennée, Kamil Rytarowski, Richard Henderson,
	Philippe Mathieu-Daudé

Implement Richard's suggestion to use __builtin_bswap().

const_XX() not updated. We could use le_bswap() definitions
but they are undefined, not sure yet what is the best way.
Meanwhile I figure this out, these patches are ready for review.

Since v1:
- Remove the Haiku/BSD ifdef'ry (Peter)
- Include the Haiku VM image from Alexander

Alexander von Gluck IV (1):
  tests/vm: Add Haiku test based on their vagrant images

Philippe Mathieu-Daudé (7):
  qemu/bswap: Remove unused qemu_bswap_len()
  qemu/bswap: Replace bswapXX() by compiler __builtin_bswap()
  qemu/bswap: Replace bswapXXs() by compiler __builtin_bswap()
  qemu/bswap: Remove <byteswap.h> dependency
  qemu/bswap: Use compiler __builtin_bswap() on Haiku
  qemu/bswap: Use compiler __builtin_bswap() on FreeBSD
  qemu/bswap: Use compiler __builtin_bswap() on NetBSD

 configure                 |  28 ---------
 include/qemu/bswap.h      |  79 +++++---------------------
 tests/keys/vagrant        |  27 +++++++++
 tests/keys/vagrant.pub    |   1 +
 tests/vm/Makefile.include |   3 +-
 tests/vm/basevm.py        |   5 +-
 tests/vm/haiku.x86_64     | 116 ++++++++++++++++++++++++++++++++++++++
 7 files changed, 164 insertions(+), 95 deletions(-)
 create mode 100644 tests/keys/vagrant
 create mode 100644 tests/keys/vagrant.pub
 create mode 100755 tests/vm/haiku.x86_64

-- 
2.26.2



^ permalink raw reply	[flat|nested] 23+ messages in thread
* [PATCH v2 1/8] qemu/bswap: Remove unused qemu_bswap_len()
  2020-09-28 13:19 [PATCH v2 0/8] qemu/bswap: Use compiler __builtin_bswap() Philippe Mathieu-Daudé
@ 2020-09-28 13:19 ` Philippe Mathieu-Daudé
  2020-09-28 13:19 ` [PATCH v2 2/8] qemu/bswap: Replace bswapXX() by compiler __builtin_bswap() Philippe Mathieu-Daudé
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-28 13:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Alex Bennée, Kamil Rytarowski, Richard Henderson,
	Philippe Mathieu-Daudé

Last use of qemu_bswap_len() has been removed in commit
e5fd1eb05ec ("apb: add busA qdev property to PBM PCI bridge").

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/qemu/bswap.h | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index 1d3e4c24e41..8b01c38040c 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -169,12 +169,6 @@ CPU_CONVERT(le, 16, uint16_t)
 CPU_CONVERT(le, 32, uint32_t)
 CPU_CONVERT(le, 64, uint64_t)
 
-/* len must be one of 1, 2, 4 */
-static inline uint32_t qemu_bswap_len(uint32_t value, int len)
-{
-    return bswap32(value) >> (32 - 8 * len);
-}
-
 /*
  * 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
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 23+ messages in thread
* [PATCH v2 2/8] qemu/bswap: Replace bswapXX() by compiler __builtin_bswap()
  2020-09-28 13:19 [PATCH v2 0/8] qemu/bswap: Use compiler __builtin_bswap() Philippe Mathieu-Daudé
  2020-09-28 13:19 ` [PATCH v2 1/8] qemu/bswap: Remove unused qemu_bswap_len() Philippe Mathieu-Daudé
@ 2020-09-28 13:19 ` Philippe Mathieu-Daudé
  2020-09-28 13:19 ` [PATCH v2 3/8] qemu/bswap: Replace bswapXXs() " Philippe Mathieu-Daudé
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-28 13:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Alex Bennée, Kamil Rytarowski, Richard Henderson,
	Philippe Mathieu-Daudé

Use the compiler built-in function to byte swap values,
as the compiler is clever and will fold constants.

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/qemu/bswap.h | 33 +++++++--------------------------
 1 file changed, 7 insertions(+), 26 deletions(-)

diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index 8b01c38040c..41131d3d76e 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -27,32 +27,13 @@ static inline uint64_t bswap64(uint64_t x)
 {
     return bswap_64(x);
 }
-# else
-static inline uint16_t bswap16(uint16_t x)
-{
-    return (((x & 0x00ff) << 8) |
-            ((x & 0xff00) >> 8));
-}
-
-static inline uint32_t bswap32(uint32_t x)
-{
-    return (((x & 0x000000ffU) << 24) |
-            ((x & 0x0000ff00U) <<  8) |
-            ((x & 0x00ff0000U) >>  8) |
-            ((x & 0xff000000U) >> 24));
-}
-
-static inline uint64_t bswap64(uint64_t x)
-{
-    return (((x & 0x00000000000000ffULL) << 56) |
-            ((x & 0x000000000000ff00ULL) << 40) |
-            ((x & 0x0000000000ff0000ULL) << 24) |
-            ((x & 0x00000000ff000000ULL) <<  8) |
-            ((x & 0x000000ff00000000ULL) >>  8) |
-            ((x & 0x0000ff0000000000ULL) >> 24) |
-            ((x & 0x00ff000000000000ULL) >> 40) |
-            ((x & 0xff00000000000000ULL) >> 56));
-}
+#else
+#undef  bswap16
+#define bswap16(_x) __builtin_bswap16(_x)
+#undef  bswap32
+#define bswap32(_x) __builtin_bswap32(_x)
+#undef  bswap64
+#define bswap64(_x) __builtin_bswap64(_x)
 #endif /* ! CONFIG_MACHINE_BSWAP_H */
 
 static inline void bswap16s(uint16_t *s)
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 23+ messages in thread
* [PATCH v2 3/8] qemu/bswap: Replace bswapXXs() by compiler __builtin_bswap()
  2020-09-28 13:19 [PATCH v2 0/8] qemu/bswap: Use compiler __builtin_bswap() Philippe Mathieu-Daudé
  2020-09-28 13:19 ` [PATCH v2 1/8] qemu/bswap: Remove unused qemu_bswap_len() Philippe Mathieu-Daudé
  2020-09-28 13:19 ` [PATCH v2 2/8] qemu/bswap: Replace bswapXX() by compiler __builtin_bswap() Philippe Mathieu-Daudé
@ 2020-09-28 13:19 ` Philippe Mathieu-Daudé
  2020-09-28 17:29   ` Thomas Huth
  2020-09-28 13:19 ` [PATCH v2 4/8] qemu/bswap: Remove <byteswap.h> dependency Philippe Mathieu-Daudé
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-28 13:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Alex Bennée, Kamil Rytarowski, Richard Henderson,
	Philippe Mathieu-Daudé

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/qemu/bswap.h | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index 41131d3d76e..fed96dddd7a 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -38,29 +38,31 @@ static inline uint64_t bswap64(uint64_t x)
 
 static inline void bswap16s(uint16_t *s)
 {
-    *s = bswap16(*s);
+    *s = __builtin_bswap16(*s);
 }
 
 static inline void bswap32s(uint32_t *s)
 {
-    *s = bswap32(*s);
+    *s = __builtin_bswap32(*s);
 }
 
 static inline void bswap64s(uint64_t *s)
 {
-    *s = bswap64(*s);
+    *s = __builtin_bswap64(*s);
 }
 
 #if defined(HOST_WORDS_BIGENDIAN)
 #define be_bswap(v, size) (v)
-#define le_bswap(v, size) glue(bswap, size)(v)
+#define le_bswap(v, size) glue(__builtin_bswap, size)(v)
 #define be_bswaps(v, size)
-#define le_bswaps(p, size) do { *p = glue(bswap, size)(*p); } while(0)
+#define le_bswaps(p, size) \
+            do { *p = glue(__builtin_bswap, size)(*p); } while (0)
 #else
 #define le_bswap(v, size) (v)
-#define be_bswap(v, size) glue(bswap, size)(v)
+#define be_bswap(v, size) glue(__builtin_bswap, size)(v)
 #define le_bswaps(v, size)
-#define be_bswaps(p, size) do { *p = glue(bswap, size)(*p); } while(0)
+#define be_bswaps(p, size) \
+            do { *p = glue(__builtin_bswap, size)(*p); } while (0)
 #endif
 
 /**
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 23+ messages in thread
* [PATCH v2 4/8] qemu/bswap: Remove <byteswap.h> dependency
  2020-09-28 13:19 [PATCH v2 0/8] qemu/bswap: Use compiler __builtin_bswap() Philippe Mathieu-Daudé
                   ` (2 preceding siblings ...)
  2020-09-28 13:19 ` [PATCH v2 3/8] qemu/bswap: Replace bswapXXs() " Philippe Mathieu-Daudé
@ 2020-09-28 13:19 ` Philippe Mathieu-Daudé
  2020-09-28 13:19 ` [PATCH v2 5/8] qemu/bswap: Use compiler __builtin_bswap() on Haiku Philippe Mathieu-Daudé
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-28 13:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Alex Bennée, Kamil Rytarowski, Richard Henderson,
	Philippe Mathieu-Daudé

Since commit efc6c070aca ("configure: Add a test for the minimum
compiler version") the minimum compiler version required for GCC
is 4.8, which supports __builtin_bswap().
Drop the <byteswap.h> dependency.

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 configure            | 13 -------------
 include/qemu/bswap.h | 17 -----------------
 2 files changed, 30 deletions(-)

diff --git a/configure b/configure
index e8e8e984f24..bff787daea7 100755
--- a/configure
+++ b/configure
@@ -4886,16 +4886,6 @@ if test "$docs" != "no" ; then
   fi
 fi
 
-# Search for bswap_32 function
-byteswap_h=no
-cat > $TMPC << EOF
-#include <byteswap.h>
-int main(void) { return bswap_32(0); }
-EOF
-if compile_prog "" "" ; then
-  byteswap_h=yes
-fi
-
 # Search for bswap32 function
 bswap_h=no
 cat > $TMPC << EOF
@@ -6789,9 +6779,6 @@ fi
 if test "$st_atim" = "yes" ; then
   echo "HAVE_STRUCT_STAT_ST_ATIM=y" >> $config_host_mak
 fi
-if test "$byteswap_h" = "yes" ; then
-  echo "CONFIG_BYTESWAP_H=y" >> $config_host_mak
-fi
 if test "$bswap_h" = "yes" ; then
   echo "CONFIG_MACHINE_BSWAP_H=y" >> $config_host_mak
 fi
diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index fed96dddd7a..55916670d39 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -10,23 +10,6 @@
 # include <sys/endian.h>
 #elif defined(__HAIKU__)
 # include <endian.h>
-#elif defined(CONFIG_BYTESWAP_H)
-# include <byteswap.h>
-
-static inline uint16_t bswap16(uint16_t x)
-{
-    return bswap_16(x);
-}
-
-static inline uint32_t bswap32(uint32_t x)
-{
-    return bswap_32(x);
-}
-
-static inline uint64_t bswap64(uint64_t x)
-{
-    return bswap_64(x);
-}
 #else
 #undef  bswap16
 #define bswap16(_x) __builtin_bswap16(_x)
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 23+ messages in thread
* [PATCH v2 5/8] qemu/bswap: Use compiler __builtin_bswap() on Haiku
  2020-09-28 13:19 [PATCH v2 0/8] qemu/bswap: Use compiler __builtin_bswap() Philippe Mathieu-Daudé
                   ` (3 preceding siblings ...)
  2020-09-28 13:19 ` [PATCH v2 4/8] qemu/bswap: Remove <byteswap.h> dependency Philippe Mathieu-Daudé
@ 2020-09-28 13:19 ` Philippe Mathieu-Daudé
  2020-09-28 14:09   ` David CARLIER
  2020-09-28 17:29   ` Thomas Huth
  2020-09-28 13:19 ` [PATCH v2 6/8] qemu/bswap: Use compiler __builtin_bswap() on FreeBSD Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  7 siblings, 2 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-28 13:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Philippe Mathieu-Daudé,
	Richard Henderson, David Carlier, Carlo Arenas, Kamil Rytarowski,
	Alex Bennée

Since commit efc6c070aca ("configure: Add a test for the minimum
compiler version") the minimum compiler version required for GCC
is 4.8, which supports __builtin_bswap().
Remove the Haiku specific ifdef'ry.

This reverts commit 652a46ebba970017c7a23767dcc983265cdb8eb7
("bswap.h: Include <endian.h> on Haiku for bswap operations").

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Cc: David Carlier <devnexen@gmail.com>
Cc: Carlo Arenas <carenas@gmail.com>
---
 include/qemu/bswap.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index 55916670d39..719d620bfe6 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -8,8 +8,6 @@
 # include <machine/bswap.h>
 #elif defined(__FreeBSD__)
 # include <sys/endian.h>
-#elif defined(__HAIKU__)
-# include <endian.h>
 #else
 #undef  bswap16
 #define bswap16(_x) __builtin_bswap16(_x)
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 23+ messages in thread
* [PATCH v2 6/8] qemu/bswap: Use compiler __builtin_bswap() on FreeBSD
  2020-09-28 13:19 [PATCH v2 0/8] qemu/bswap: Use compiler __builtin_bswap() Philippe Mathieu-Daudé
                   ` (4 preceding siblings ...)
  2020-09-28 13:19 ` [PATCH v2 5/8] qemu/bswap: Use compiler __builtin_bswap() on Haiku Philippe Mathieu-Daudé
@ 2020-09-28 13:19 ` Philippe Mathieu-Daudé
  2020-09-28 17:32   ` Thomas Huth
  2020-09-28 21:16   ` Ed Maste
  2020-09-28 13:19 ` [PATCH v2 7/8] qemu/bswap: Use compiler __builtin_bswap() on NetBSD Philippe Mathieu-Daudé
  2020-09-28 13:19 ` [RFC PATCH v2 8/8] tests/vm: Add Haiku test based on their vagrant images Philippe Mathieu-Daudé
  7 siblings, 2 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-28 13:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Ed Maste, Philippe Mathieu-Daudé,
	Richard Henderson, Kamil Rytarowski, Alex Bennée

Since commit efc6c070aca ("configure: Add a test for the minimum
compiler version") the minimum compiler version required for GCC
is 4.8, which supports __builtin_bswap().
Remove the FreeBSD specific ifdef'ry.

This reverts commit de03c3164accc21311c39327601fcdd95da301f3
("bswap: Fix build on FreeBSD 10.0").

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Cc: Ed Maste <emaste@freebsd.org>
---
 include/qemu/bswap.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index 719d620bfe6..1a297bfec22 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -6,8 +6,6 @@
 #ifdef CONFIG_MACHINE_BSWAP_H
 # include <sys/endian.h>
 # include <machine/bswap.h>
-#elif defined(__FreeBSD__)
-# include <sys/endian.h>
 #else
 #undef  bswap16
 #define bswap16(_x) __builtin_bswap16(_x)
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 23+ messages in thread
* [PATCH v2 7/8] qemu/bswap: Use compiler __builtin_bswap() on NetBSD
  2020-09-28 13:19 [PATCH v2 0/8] qemu/bswap: Use compiler __builtin_bswap() Philippe Mathieu-Daudé
                   ` (5 preceding siblings ...)
  2020-09-28 13:19 ` [PATCH v2 6/8] qemu/bswap: Use compiler __builtin_bswap() on FreeBSD Philippe Mathieu-Daudé
@ 2020-09-28 13:19 ` Philippe Mathieu-Daudé
  2020-09-28 21:51   ` Kamil Rytarowski
  2020-09-28 13:19 ` [RFC PATCH v2 8/8] tests/vm: Add Haiku test based on their vagrant images Philippe Mathieu-Daudé
  7 siblings, 1 reply; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-28 13:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Alex Bennée, Kamil Rytarowski, Richard Henderson,
	Philippe Mathieu-Daudé

Since commit efc6c070aca ("configure: Add a test for the minimum
compiler version") the minimum compiler version required for GCC
is 4.8, which supports __builtin_bswap().
Remove the NetBSD specific ifdef'ry.

This reverts commit 1360677cfe3ca8f945fa1de77823df21a77e4500
("makes NetBSD use the native bswap functions").

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 configure            | 15 ---------------
 include/qemu/bswap.h |  5 -----
 2 files changed, 20 deletions(-)

diff --git a/configure b/configure
index bff787daea7..1b0a02a0af8 100755
--- a/configure
+++ b/configure
@@ -4886,18 +4886,6 @@ if test "$docs" != "no" ; then
   fi
 fi
 
-# Search for bswap32 function
-bswap_h=no
-cat > $TMPC << EOF
-#include <sys/endian.h>
-#include <sys/types.h>
-#include <machine/bswap.h>
-int main(void) { return bswap32(0); }
-EOF
-if compile_prog "" "" ; then
-  bswap_h=yes
-fi
-
 ##########################################
 # Do we have libiscsi >= 1.9.0
 if test "$libiscsi" != "no" ; then
@@ -6779,9 +6767,6 @@ fi
 if test "$st_atim" = "yes" ; then
   echo "HAVE_STRUCT_STAT_ST_ATIM=y" >> $config_host_mak
 fi
-if test "$bswap_h" = "yes" ; then
-  echo "CONFIG_MACHINE_BSWAP_H=y" >> $config_host_mak
-fi
 if test "$curl" = "yes" ; then
   echo "CONFIG_CURL=y" >> $config_host_mak
   echo "CURL_CFLAGS=$curl_cflags" >> $config_host_mak
diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index 1a297bfec22..7e586531c09 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -3,17 +3,12 @@
 
 #include "fpu/softfloat-types.h"
 
-#ifdef CONFIG_MACHINE_BSWAP_H
-# include <sys/endian.h>
-# include <machine/bswap.h>
-#else
 #undef  bswap16
 #define bswap16(_x) __builtin_bswap16(_x)
 #undef  bswap32
 #define bswap32(_x) __builtin_bswap32(_x)
 #undef  bswap64
 #define bswap64(_x) __builtin_bswap64(_x)
-#endif /* ! CONFIG_MACHINE_BSWAP_H */
 
 static inline void bswap16s(uint16_t *s)
 {
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 23+ messages in thread
* [RFC PATCH v2 8/8] tests/vm: Add Haiku test based on their vagrant images
  2020-09-28 13:19 [PATCH v2 0/8] qemu/bswap: Use compiler __builtin_bswap() Philippe Mathieu-Daudé
                   ` (6 preceding siblings ...)
  2020-09-28 13:19 ` [PATCH v2 7/8] qemu/bswap: Use compiler __builtin_bswap() on NetBSD Philippe Mathieu-Daudé
@ 2020-09-28 13:19 ` Philippe Mathieu-Daudé
  7 siblings, 0 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-28 13:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Alexander von Gluck IV, Philippe Mathieu-Daudé,
	Richard Henderson, Philippe Mathieu-Daudé,
	Kamil Rytarowski, Alex Bennée

From: Alexander von Gluck IV <kallisti5@unixzen.com>

Signed-off-by: Alexander von Gluck IV <kallisti5@unixzen.com>
[PMD: Avoid to recreate the image each time]
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
RFC because Alexander will probably add more comments to this patch.

 tests/keys/vagrant        |  27 +++++++++
 tests/keys/vagrant.pub    |   1 +
 tests/vm/Makefile.include |   3 +-
 tests/vm/basevm.py        |   5 +-
 tests/vm/haiku.x86_64     | 116 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 149 insertions(+), 3 deletions(-)
 create mode 100644 tests/keys/vagrant
 create mode 100644 tests/keys/vagrant.pub
 create mode 100755 tests/vm/haiku.x86_64

diff --git a/tests/keys/vagrant b/tests/keys/vagrant
new file mode 100644
index 00000000000..7d6a083909e
--- /dev/null
+++ b/tests/keys/vagrant
@@ -0,0 +1,27 @@
+-----BEGIN RSA PRIVATE KEY-----
+MIIEogIBAAKCAQEA6NF8iallvQVp22WDkTkyrtvp9eWW6A8YVr+kz4TjGYe7gHzI
+w+niNltGEFHzD8+v1I2YJ6oXevct1YeS0o9HZyN1Q9qgCgzUFtdOKLv6IedplqoP
+kcmF0aYet2PkEDo3MlTBckFXPITAMzF8dJSIFo9D8HfdOV0IAdx4O7PtixWKn5y2
+hMNG0zQPyUecp4pzC6kivAIhyfHilFR61RGL+GPXQ2MWZWFYbAGjyiYJnAmCP3NO
+Td0jMZEnDkbUvxhMmBYSdETk1rRgm+R4LOzFUGaHqHDLKLX+FIPKcF96hrucXzcW
+yLbIbEgE98OHlnVYCzRdK8jlqm8tehUc9c9WhQIBIwKCAQEA4iqWPJXtzZA68mKd
+ELs4jJsdyky+ewdZeNds5tjcnHU5zUYE25K+ffJED9qUWICcLZDc81TGWjHyAqD1
+Bw7XpgUwFgeUJwUlzQurAv+/ySnxiwuaGJfhFM1CaQHzfXphgVml+fZUvnJUTvzf
+TK2Lg6EdbUE9TarUlBf/xPfuEhMSlIE5keb/Zz3/LUlRg8yDqz5w+QWVJ4utnKnK
+iqwZN0mwpwU7YSyJhlT4YV1F3n4YjLswM5wJs2oqm0jssQu/BT0tyEXNDYBLEF4A
+sClaWuSJ2kjq7KhrrYXzagqhnSei9ODYFShJu8UWVec3Ihb5ZXlzO6vdNQ1J9Xsf
+4m+2ywKBgQD6qFxx/Rv9CNN96l/4rb14HKirC2o/orApiHmHDsURs5rUKDx0f9iP
+cXN7S1uePXuJRK/5hsubaOCx3Owd2u9gD6Oq0CsMkE4CUSiJcYrMANtx54cGH7Rk
+EjFZxK8xAv1ldELEyxrFqkbE4BKd8QOt414qjvTGyAK+OLD3M2QdCQKBgQDtx8pN
+CAxR7yhHbIWT1AH66+XWN8bXq7l3RO/ukeaci98JfkbkxURZhtxV/HHuvUhnPLdX
+3TwygPBYZFNo4pzVEhzWoTtnEtrFueKxyc3+LjZpuo+mBlQ6ORtfgkr9gBVphXZG
+YEzkCD3lVdl8L4cw9BVpKrJCs1c5taGjDgdInQKBgHm/fVvv96bJxc9x1tffXAcj
+3OVdUN0UgXNCSaf/3A/phbeBQe9xS+3mpc4r6qvx+iy69mNBeNZ0xOitIjpjBo2+
+dBEjSBwLk5q5tJqHmy/jKMJL4n9ROlx93XS+njxgibTvU6Fp9w+NOFD/HvxB3Tcz
+6+jJF85D5BNAG3DBMKBjAoGBAOAxZvgsKN+JuENXsST7F89Tck2iTcQIT8g5rwWC
+P9Vt74yboe2kDT531w8+egz7nAmRBKNM751U/95P9t88EDacDI/Z2OwnuFQHCPDF
+llYOUI+SpLJ6/vURRbHSnnn8a/XG+nzedGH5JGqEJNQsz+xT2axM0/W/CRknmGaJ
+kda/AoGANWrLCz708y7VYgAtW2Uf1DPOIYMdvo6fxIB5i9ZfISgcJ/bbCUkFrhoH
++vq/5CIWxCPp0f85R4qxxQ5ihxJ0YDQT9Jpx4TMss4PSavPaBH3RXow5Ohe+bYoQ
+NE5OgEXk2wVfZczCZpigBKbKZHNYcelXtTt/nP3rsCuGcM4h53s=
+-----END RSA PRIVATE KEY-----
diff --git a/tests/keys/vagrant.pub b/tests/keys/vagrant.pub
new file mode 100644
index 00000000000..b8d012d787f
--- /dev/null
+++ b/tests/keys/vagrant.pub
@@ -0,0 +1 @@
+ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEA6NF8iallvQVp22WDkTkyrtvp9eWW6A8YVr+kz4TjGYe7gHzIw+niNltGEFHzD8+v1I2YJ6oXevct1YeS0o9HZyN1Q9qgCgzUFtdOKLv6IedplqoPkcmF0aYet2PkEDo3MlTBckFXPITAMzF8dJSIFo9D8HfdOV0IAdx4O7PtixWKn5y2hMNG0zQPyUecp4pzC6kivAIhyfHilFR61RGL+GPXQ2MWZWFYbAGjyiYJnAmCP3NOTd0jMZEnDkbUvxhMmBYSdETk1rRgm+R4LOzFUGaHqHDLKLX+FIPKcF96hrucXzcWyLbIbEgE98OHlnVYCzRdK8jlqm8tehUc9c9WhQ== well-known vagrant key for qemu-test, do not use on any machine exposed to an external network
diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
index 61f893ffdc0..e94d95ec541 100644
--- a/tests/vm/Makefile.include
+++ b/tests/vm/Makefile.include
@@ -4,7 +4,7 @@
 
 EFI_AARCH64 = $(wildcard $(BUILD_DIR)/pc-bios/edk2-aarch64-code.fd)
 
-IMAGES := freebsd netbsd openbsd centos fedora
+IMAGES := freebsd netbsd openbsd centos fedora haiku.x86_64
 ifneq ($(GENISOIMAGE),)
 IMAGES += ubuntu.i386 centos
 ifneq ($(EFI_AARCH64),)
@@ -41,6 +41,7 @@ endif
 else
 	@echo "  (install genisoimage to build centos/ubuntu images)"
 endif
+	@echo "  vm-build-haiku.x86_64           - Build QEMU in Haiku VM"
 	@echo ""
 	@echo "  vm-build-all                    - Build QEMU in all VMs"
 	@echo "  vm-clean-all                    - Clean up VM images"
diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index 3fac20e929a..00f1d5ca8da 100644
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -44,6 +44,7 @@
     'machine'         : 'pc',
     'guest_user'      : "qemu",
     'guest_pass'      : "qemupass",
+    'root_user'       : "root",
     'root_pass'       : "qemupass",
     'ssh_key_file'    : SSH_KEY_FILE,
     'ssh_pub_key_file': SSH_PUB_KEY_FILE,
@@ -245,13 +246,13 @@ def ssh(self, *cmd):
         return self._ssh_do(self._config["guest_user"], cmd, False)
 
     def ssh_root(self, *cmd):
-        return self._ssh_do("root", cmd, False)
+        return self._ssh_do(self._config["root_user"], cmd, False)
 
     def ssh_check(self, *cmd):
         self._ssh_do(self._config["guest_user"], cmd, True)
 
     def ssh_root_check(self, *cmd):
-        self._ssh_do("root", cmd, True)
+        self._ssh_do(self._config["root_user"], cmd, True)
 
     def build_image(self, img):
         raise NotImplementedError
diff --git a/tests/vm/haiku.x86_64 b/tests/vm/haiku.x86_64
new file mode 100755
index 00000000000..634ef774870
--- /dev/null
+++ b/tests/vm/haiku.x86_64
@@ -0,0 +1,116 @@
+#!/usr/bin/env python3
+#
+# Haiku VM image
+#
+# Copyright 2020 Haiku, Inc.
+#
+# Authors:
+#  Alexander von Gluck IV <kallisti5@unixzen.com>
+#
+# This code is licensed under the GPL version 2 or later.  See
+# the COPYING file in the top-level directory.
+#
+
+import os
+import re
+import sys
+import time
+import socket
+import subprocess
+import basevm
+
+VAGRANT_KEY_FILE = os.path.join(os.path.dirname(__file__),
+    "..", "keys", "vagrant")
+
+VAGRANT_PUB_KEY_FILE = os.path.join(os.path.dirname(__file__),
+    "..", "keys", "vagrant.pub")
+
+HAIKU_CONFIG = {
+    'cpu'             : "max",
+    'machine'         : 'pc',
+    'guest_user'      : "vagrant",
+    'guest_pass'      : "",
+    'root_user'       : "vagrant",
+    'root_pass'       : "",
+    'ssh_key_file'    : VAGRANT_KEY_FILE,
+    'ssh_pub_key_file': VAGRANT_PUB_KEY_FILE,
+    'memory'          : "4G",
+    'extra_args'      : [],
+    'qemu_args'       : "-device VGA",
+    'dns'             : "",
+    'ssh_port'        : 0,
+    'install_cmds'    : "",
+    'boot_dev_type'   : "block",
+    'ssh_timeout'     : 1,
+}
+
+class HaikuVM(basevm.BaseVM):
+    name = "haiku"
+    arch = "x86_64"
+
+    link = "https://app.vagrantup.com/haiku-os/boxes/r1beta2-x86_64/versions/20200702/providers/libvirt.box"
+    csum = "41c38b316e0cbdbc66b5dbaf3612b866700a4f35807cb1eb266a5bf83e9e68d5"
+
+    poweroff = "shutdown"
+
+    requirements = [
+        "devel:libbz2",
+        "devel:libcapstone",
+        "devel:libcurl",
+        "devel:libfdt",
+        "devel:libgcrypt",
+        "devel:libgl",
+        "devel:libglib_2.0",
+        "devel:libgnutls",
+        "devel:libgpg_error",
+        "devel:libintl",
+        "devel:libjpeg",
+        "devel:liblzo2",
+        "devel:libncursesw",
+        "devel:libnettle",
+        "devel:libpixman_1",
+        "devel:libpng16",
+        "devel:libsdl2_2.0",
+        "devel:libsnappy",
+        "devel:libssh2",
+        "devel:libtasn1",
+        "devel:libusb_1.0",
+        "devel:libz",
+        "setuptools_python3"
+    ]
+
+    # https://dev.haiku-os.org/ticket/16512 virtio disk1 shows up as 0 (reversed order)
+    BUILD_SCRIPT = """
+        set -e;
+        rm -rf /tmp/qemu-test.*
+        cd $(mktemp -d /tmp/qemu-test.XXXXXX);
+        mkdir src build; cd src;
+        tar -xf /dev/disk/virtual/virtio_block/0/raw;
+        cd ../build
+        ../src/configure --python=python3 {configure_opts};
+        make --output-sync -j{jobs} {target} {verbose};
+    """
+
+    def build_image(self, img):
+        self.print_step("Downloading disk image")
+        tarball = self._download_with_cache(self.link, sha256sum=self.csum)
+
+        self.print_step("Extracting disk image")
+
+        subprocess.check_call(["tar", "xzf", tarball, "./box.img", "-O"],
+                              stdout=open(img, 'wb'))
+
+        self.print_step("Preparing disk image")
+        self.boot(img)
+
+        # Wait for ssh to be available.
+        self.wait_ssh(wait_root=True, cmd="exit 0")
+
+        # Install packages
+        self.ssh_root("pkgman install -y %s" % " ".join(self.requirements))
+        self.graceful_shutdown()
+
+        self.print_step("All done")
+
+if __name__ == "__main__":
+    sys.exit(basevm.main(HaikuVM, config=HAIKU_CONFIG))
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 23+ messages in thread
* Re: [PATCH v2 5/8] qemu/bswap: Use compiler __builtin_bswap() on Haiku
  2020-09-28 13:19 ` [PATCH v2 5/8] qemu/bswap: Use compiler __builtin_bswap() on Haiku Philippe Mathieu-Daudé
@ 2020-09-28 14:09   ` David CARLIER
  2020-09-28 14:13     ` Daniel P. Berrangé
  2020-09-28 17:29   ` Thomas Huth
  1 sibling, 1 reply; 23+ messages in thread
From: David CARLIER @ 2020-09-28 14:09 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Fam Zheng, Richard Henderson, qemu-devel, Carlo Arenas,
	Kamil Rytarowski, Alex Bennée

Unfortunately it breaks the build.
Regards.

On Mon, 28 Sep 2020 at 14:20, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> Since commit efc6c070aca ("configure: Add a test for the minimum
> compiler version") the minimum compiler version required for GCC
> is 4.8, which supports __builtin_bswap().
> Remove the Haiku specific ifdef'ry.
>
> This reverts commit 652a46ebba970017c7a23767dcc983265cdb8eb7
> ("bswap.h: Include <endian.h> on Haiku for bswap operations").
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> Cc: David Carlier <devnexen@gmail.com>
> Cc: Carlo Arenas <carenas@gmail.com>
> ---
>  include/qemu/bswap.h | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
> index 55916670d39..719d620bfe6 100644
> --- a/include/qemu/bswap.h
> +++ b/include/qemu/bswap.h
> @@ -8,8 +8,6 @@
>  # include <machine/bswap.h>
>  #elif defined(__FreeBSD__)
>  # include <sys/endian.h>
> -#elif defined(__HAIKU__)
> -# include <endian.h>
>  #else
>  #undef  bswap16
>  #define bswap16(_x) __builtin_bswap16(_x)
> --
> 2.26.2
>


^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH v2 5/8] qemu/bswap: Use compiler __builtin_bswap() on Haiku
  2020-09-28 14:09   ` David CARLIER
@ 2020-09-28 14:13     ` Daniel P. Berrangé
  2020-09-28 14:18       ` Philippe Mathieu-Daudé
  2020-09-28 15:00       ` David CARLIER
  0 siblings, 2 replies; 23+ messages in thread
From: Daniel P. Berrangé @ 2020-09-28 14:13 UTC (permalink / raw)
  To: David CARLIER
  Cc: Fam Zheng, Alex Bennée, Richard Henderson, qemu-devel,
	Carlo Arenas, Kamil Rytarowski, Philippe Mathieu-Daudé

On Mon, Sep 28, 2020 at 03:09:01PM +0100, David CARLIER wrote:
> Unfortunately it breaks the build.

Can you provide details of the errors seen and toolchain versions.

I notice we don't have any CI support for Haiku right now, nor
any recipe in tests/vm/  for enabling users to setup a VM with
Haiku installed. This very much makes Haiku a second class
citizen right now in terms of QEMU's supported platforms, with
no expectation of whether it'll work at any point in time.


> On Mon, 28 Sep 2020 at 14:20, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> >
> > Since commit efc6c070aca ("configure: Add a test for the minimum
> > compiler version") the minimum compiler version required for GCC
> > is 4.8, which supports __builtin_bswap().
> > Remove the Haiku specific ifdef'ry.
> >
> > This reverts commit 652a46ebba970017c7a23767dcc983265cdb8eb7
> > ("bswap.h: Include <endian.h> on Haiku for bswap operations").
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > ---
> > Cc: David Carlier <devnexen@gmail.com>
> > Cc: Carlo Arenas <carenas@gmail.com>
> > ---
> >  include/qemu/bswap.h | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
> > index 55916670d39..719d620bfe6 100644
> > --- a/include/qemu/bswap.h
> > +++ b/include/qemu/bswap.h
> > @@ -8,8 +8,6 @@
> >  # include <machine/bswap.h>
> >  #elif defined(__FreeBSD__)
> >  # include <sys/endian.h>
> > -#elif defined(__HAIKU__)
> > -# include <endian.h>
> >  #else
> >  #undef  bswap16
> >  #define bswap16(_x) __builtin_bswap16(_x)
> > --
> > 2.26.2
> >
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH v2 5/8] qemu/bswap: Use compiler __builtin_bswap() on Haiku
  2020-09-28 14:13     ` Daniel P. Berrangé
@ 2020-09-28 14:18       ` Philippe Mathieu-Daudé
  2020-10-19 11:00         ` Philippe Mathieu-Daudé
  2020-09-28 15:00       ` David CARLIER
  1 sibling, 1 reply; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-28 14:18 UTC (permalink / raw)
  To: Daniel P. Berrangé, David CARLIER
  Cc: Fam Zheng, Richard Henderson, qemu-devel, Carlo Arenas,
	Kamil Rytarowski, Alex Bennée

On 9/28/20 4:13 PM, Daniel P. Berrangé wrote:
> On Mon, Sep 28, 2020 at 03:09:01PM +0100, David CARLIER wrote:
>> Unfortunately it breaks the build.
> 
> Can you provide details of the errors seen and toolchain versions.
> 
> I notice we don't have any CI support for Haiku right now, nor
> any recipe in tests/vm/  for enabling users to setup a VM with
> Haiku installed. This very much makes Haiku a second class
> citizen right now in terms of QEMU's supported platforms, with
> no expectation of whether it'll work at any point in time.

I provided the tests/vm/ build script from Alexander as patch 8/8
of this series:
https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg10022.html

This is what I used to test this patch/series.

> 
>> On Mon, 28 Sep 2020 at 14:20, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>>
>>> Since commit efc6c070aca ("configure: Add a test for the minimum
>>> compiler version") the minimum compiler version required for GCC
>>> is 4.8, which supports __builtin_bswap().
>>> Remove the Haiku specific ifdef'ry.
>>>
>>> This reverts commit 652a46ebba970017c7a23767dcc983265cdb8eb7
>>> ("bswap.h: Include <endian.h> on Haiku for bswap operations").
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> ---
>>> Cc: David Carlier <devnexen@gmail.com>
>>> Cc: Carlo Arenas <carenas@gmail.com>
>>> ---
>>>  include/qemu/bswap.h | 2 --
>>>  1 file changed, 2 deletions(-)
>>>
>>> diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
>>> index 55916670d39..719d620bfe6 100644
>>> --- a/include/qemu/bswap.h
>>> +++ b/include/qemu/bswap.h
>>> @@ -8,8 +8,6 @@
>>>  # include <machine/bswap.h>
>>>  #elif defined(__FreeBSD__)
>>>  # include <sys/endian.h>
>>> -#elif defined(__HAIKU__)
>>> -# include <endian.h>
>>>  #else
>>>  #undef  bswap16
>>>  #define bswap16(_x) __builtin_bswap16(_x)
>>> --
>>> 2.26.2
>>>
>>
> 
> Regards,
> Daniel
> 



^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH v2 5/8] qemu/bswap: Use compiler __builtin_bswap() on Haiku
  2020-09-28 14:13     ` Daniel P. Berrangé
  2020-09-28 14:18       ` Philippe Mathieu-Daudé
@ 2020-09-28 15:00       ` David CARLIER
  1 sibling, 0 replies; 23+ messages in thread
From: David CARLIER @ 2020-09-28 15:00 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: Fam Zheng, Alex Bennée, Richard Henderson, qemu-devel,
	Carlo Arenas, Kamil Rytarowski, Philippe Mathieu-Daudé

errors as
...
expected '=',   ',', ';', 'asm' r "__attribute__' before "builtin_swap16'
...

base gcc 8.3

On Mon, 28 Sep 2020 at 15:14, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Mon, Sep 28, 2020 at 03:09:01PM +0100, David CARLIER wrote:
> > Unfortunately it breaks the build.
>
> Can you provide details of the errors seen and toolchain versions.
>
> I notice we don't have any CI support for Haiku right now, nor
> any recipe in tests/vm/  for enabling users to setup a VM with
> Haiku installed. This very much makes Haiku a second class
> citizen right now in terms of QEMU's supported platforms, with
> no expectation of whether it'll work at any point in time.
>
>
> > On Mon, 28 Sep 2020 at 14:20, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> > >
> > > Since commit efc6c070aca ("configure: Add a test for the minimum
> > > compiler version") the minimum compiler version required for GCC
> > > is 4.8, which supports __builtin_bswap().
> > > Remove the Haiku specific ifdef'ry.
> > >
> > > This reverts commit 652a46ebba970017c7a23767dcc983265cdb8eb7
> > > ("bswap.h: Include <endian.h> on Haiku for bswap operations").
> > >
> > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > > ---
> > > Cc: David Carlier <devnexen@gmail.com>
> > > Cc: Carlo Arenas <carenas@gmail.com>
> > > ---
> > >  include/qemu/bswap.h | 2 --
> > >  1 file changed, 2 deletions(-)
> > >
> > > diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
> > > index 55916670d39..719d620bfe6 100644
> > > --- a/include/qemu/bswap.h
> > > +++ b/include/qemu/bswap.h
> > > @@ -8,8 +8,6 @@
> > >  # include <machine/bswap.h>
> > >  #elif defined(__FreeBSD__)
> > >  # include <sys/endian.h>
> > > -#elif defined(__HAIKU__)
> > > -# include <endian.h>
> > >  #else
> > >  #undef  bswap16
> > >  #define bswap16(_x) __builtin_bswap16(_x)
> > > --
> > > 2.26.2
> > >
> >
>
> Regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
>


^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH v2 3/8] qemu/bswap: Replace bswapXXs() by compiler __builtin_bswap()
  2020-09-28 13:19 ` [PATCH v2 3/8] qemu/bswap: Replace bswapXXs() " Philippe Mathieu-Daudé
@ 2020-09-28 17:29   ` Thomas Huth
  0 siblings, 0 replies; 23+ messages in thread
From: Thomas Huth @ 2020-09-28 17:29 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Fam Zheng, Richard Henderson, Kamil Rytarowski, Alex Bennée

On 28/09/2020 15.19, Philippe Mathieu-Daudé wrote:
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  include/qemu/bswap.h | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
> index 41131d3d76e..fed96dddd7a 100644
> --- a/include/qemu/bswap.h
> +++ b/include/qemu/bswap.h
> @@ -38,29 +38,31 @@ static inline uint64_t bswap64(uint64_t x)
>  
>  static inline void bswap16s(uint16_t *s)
>  {
> -    *s = bswap16(*s);
> +    *s = __builtin_bswap16(*s);
>  }
>  
>  static inline void bswap32s(uint32_t *s)
>  {
> -    *s = bswap32(*s);
> +    *s = __builtin_bswap32(*s);
>  }
>  
>  static inline void bswap64s(uint64_t *s)
>  {
> -    *s = bswap64(*s);
> +    *s = __builtin_bswap64(*s);
>  }
>  
>  #if defined(HOST_WORDS_BIGENDIAN)
>  #define be_bswap(v, size) (v)
> -#define le_bswap(v, size) glue(bswap, size)(v)
> +#define le_bswap(v, size) glue(__builtin_bswap, size)(v)
>  #define be_bswaps(v, size)
> -#define le_bswaps(p, size) do { *p = glue(bswap, size)(*p); } while(0)
> +#define le_bswaps(p, size) \
> +            do { *p = glue(__builtin_bswap, size)(*p); } while (0)
>  #else
>  #define le_bswap(v, size) (v)
> -#define be_bswap(v, size) glue(bswap, size)(v)
> +#define be_bswap(v, size) glue(__builtin_bswap, size)(v)
>  #define le_bswaps(v, size)
> -#define be_bswaps(p, size) do { *p = glue(bswap, size)(*p); } while(0)
> +#define be_bswaps(p, size) \
> +            do { *p = glue(__builtin_bswap, size)(*p); } while (0)
>  #endif

What's the advantage of the renaming here if bswap is already #defined
to the builtin function (or another useful function from a system
header)? This just looks like code churn to me?

 Thomas



^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH v2 5/8] qemu/bswap: Use compiler __builtin_bswap() on Haiku
  2020-09-28 13:19 ` [PATCH v2 5/8] qemu/bswap: Use compiler __builtin_bswap() on Haiku Philippe Mathieu-Daudé
  2020-09-28 14:09   ` David CARLIER
@ 2020-09-28 17:29   ` Thomas Huth
  1 sibling, 0 replies; 23+ messages in thread
From: Thomas Huth @ 2020-09-28 17:29 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Fam Zheng, Richard Henderson, Kamil Rytarowski, Carlo Arenas,
	David Carlier, Alex Bennée

On 28/09/2020 15.19, Philippe Mathieu-Daudé wrote:
> Since commit efc6c070aca ("configure: Add a test for the minimum
> compiler version") the minimum compiler version required for GCC
> is 4.8, which supports __builtin_bswap().
> Remove the Haiku specific ifdef'ry.
> 
> This reverts commit 652a46ebba970017c7a23767dcc983265cdb8eb7
> ("bswap.h: Include <endian.h> on Haiku for bswap operations").
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> Cc: David Carlier <devnexen@gmail.com>
> Cc: Carlo Arenas <carenas@gmail.com>
> ---
>  include/qemu/bswap.h | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
> index 55916670d39..719d620bfe6 100644
> --- a/include/qemu/bswap.h
> +++ b/include/qemu/bswap.h
> @@ -8,8 +8,6 @@
>  # include <machine/bswap.h>
>  #elif defined(__FreeBSD__)
>  # include <sys/endian.h>
> -#elif defined(__HAIKU__)
> -# include <endian.h>
>  #else
>  #undef  bswap16
>  #define bswap16(_x) __builtin_bswap16(_x)

Why don't we simply always use the builtin functions on all systems? I
assume the compiler can handle these the best in all cases... or do you
see any advantage in using <machine/bswap.h> or <sys/endian.h> in
certain cases?

 Thomas


 Thomas



^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH v2 6/8] qemu/bswap: Use compiler __builtin_bswap() on FreeBSD
  2020-09-28 13:19 ` [PATCH v2 6/8] qemu/bswap: Use compiler __builtin_bswap() on FreeBSD Philippe Mathieu-Daudé
@ 2020-09-28 17:32   ` Thomas Huth
  2020-09-28 17:57     ` Philippe Mathieu-Daudé
  2020-09-28 21:16   ` Ed Maste
  1 sibling, 1 reply; 23+ messages in thread
From: Thomas Huth @ 2020-09-28 17:32 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Fam Zheng, Richard Henderson, Kamil Rytarowski, Ed Maste,
	Alex Bennée

On 28/09/2020 15.19, Philippe Mathieu-Daudé wrote:
> Since commit efc6c070aca ("configure: Add a test for the minimum
> compiler version") the minimum compiler version required for GCC
> is 4.8, which supports __builtin_bswap().
> Remove the FreeBSD specific ifdef'ry.
> 
> This reverts commit de03c3164accc21311c39327601fcdd95da301f3
> ("bswap: Fix build on FreeBSD 10.0").
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> Cc: Ed Maste <emaste@freebsd.org>
> ---
>  include/qemu/bswap.h | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
> index 719d620bfe6..1a297bfec22 100644
> --- a/include/qemu/bswap.h
> +++ b/include/qemu/bswap.h
> @@ -6,8 +6,6 @@
>  #ifdef CONFIG_MACHINE_BSWAP_H
>  # include <sys/endian.h>
>  # include <machine/bswap.h>
> -#elif defined(__FreeBSD__)
> -# include <sys/endian.h>

Ah, well, no I get it ... you're removing this stuff bit by bit. Quite
confusing, IMHO, I'd remove them all in one patch instead.

 Thomas



^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH v2 6/8] qemu/bswap: Use compiler __builtin_bswap() on FreeBSD
  2020-09-28 17:32   ` Thomas Huth
@ 2020-09-28 17:57     ` Philippe Mathieu-Daudé
  2020-09-28 17:58       ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-28 17:57 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel
  Cc: Fam Zheng, Richard Henderson, Kamil Rytarowski, Ed Maste,
	Alex Bennée

On 9/28/20 7:32 PM, Thomas Huth wrote:
> On 28/09/2020 15.19, Philippe Mathieu-Daudé wrote:
>> Since commit efc6c070aca ("configure: Add a test for the minimum
>> compiler version") the minimum compiler version required for GCC
>> is 4.8, which supports __builtin_bswap().
>> Remove the FreeBSD specific ifdef'ry.
>>
>> This reverts commit de03c3164accc21311c39327601fcdd95da301f3
>> ("bswap: Fix build on FreeBSD 10.0").
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>> Cc: Ed Maste <emaste@freebsd.org>
>> ---
>>  include/qemu/bswap.h | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
>> index 719d620bfe6..1a297bfec22 100644
>> --- a/include/qemu/bswap.h
>> +++ b/include/qemu/bswap.h
>> @@ -6,8 +6,6 @@
>>  #ifdef CONFIG_MACHINE_BSWAP_H
>>  # include <sys/endian.h>
>>  # include <machine/bswap.h>
>> -#elif defined(__FreeBSD__)
>> -# include <sys/endian.h>
> 
> Ah, well, no I get it ... you're removing this stuff bit by bit. Quite
> confusing, IMHO, I'd remove them all in one patch instead.

Sure. I did it that way because I was testing OS after OS, when
one was successful I committed the change and went for the next
one.

> 
>  Thomas
> 



^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH v2 6/8] qemu/bswap: Use compiler __builtin_bswap() on FreeBSD
  2020-09-28 17:57     ` Philippe Mathieu-Daudé
@ 2020-09-28 17:58       ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-28 17:58 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel
  Cc: Fam Zheng, Richard Henderson, Kamil Rytarowski, Ed Maste,
	Alex Bennée

On 9/28/20 7:57 PM, Philippe Mathieu-Daudé wrote:
> On 9/28/20 7:32 PM, Thomas Huth wrote:
>> On 28/09/2020 15.19, Philippe Mathieu-Daudé wrote:
>>> Since commit efc6c070aca ("configure: Add a test for the minimum
>>> compiler version") the minimum compiler version required for GCC
>>> is 4.8, which supports __builtin_bswap().
>>> Remove the FreeBSD specific ifdef'ry.
>>>
>>> This reverts commit de03c3164accc21311c39327601fcdd95da301f3
>>> ("bswap: Fix build on FreeBSD 10.0").
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> ---
>>> Cc: Ed Maste <emaste@freebsd.org>
>>> ---
>>>  include/qemu/bswap.h | 2 --
>>>  1 file changed, 2 deletions(-)
>>>
>>> diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
>>> index 719d620bfe6..1a297bfec22 100644
>>> --- a/include/qemu/bswap.h
>>> +++ b/include/qemu/bswap.h
>>> @@ -6,8 +6,6 @@
>>>  #ifdef CONFIG_MACHINE_BSWAP_H
>>>  # include <sys/endian.h>
>>>  # include <machine/bswap.h>
>>> -#elif defined(__FreeBSD__)
>>> -# include <sys/endian.h>
>>
>> Ah, well, no I get it ... you're removing this stuff bit by bit. Quite
>> confusing, IMHO, I'd remove them all in one patch instead.

Actually I thought it would be easier for each OS maintainer to
review the corresponding patch.

> 
> Sure. I did it that way because I was testing OS after OS, when
> one was successful I committed the change and went for the next
> one.
> 
>>
>>  Thomas
>>
> 



^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH v2 6/8] qemu/bswap: Use compiler __builtin_bswap() on FreeBSD
  2020-09-28 13:19 ` [PATCH v2 6/8] qemu/bswap: Use compiler __builtin_bswap() on FreeBSD Philippe Mathieu-Daudé
  2020-09-28 17:32   ` Thomas Huth
@ 2020-09-28 21:16   ` Ed Maste
  1 sibling, 0 replies; 23+ messages in thread
From: Ed Maste @ 2020-09-28 21:16 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Fam Zheng, Richard Henderson, Kamil Rytarowski, Alex Bennée,
	qemu-devel

On Mon, 28 Sep 2020 at 09:20, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> Since commit efc6c070aca ("configure: Add a test for the minimum
> compiler version") the minimum compiler version required for GCC
> is 4.8, which supports __builtin_bswap().
> Remove the FreeBSD specific ifdef'ry.
>
> This reverts commit de03c3164accc21311c39327601fcdd95da301f3
> ("bswap: Fix build on FreeBSD 10.0").
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Ed Maste <emaste@FreeBSD.org>

Aside, to test building with this change on Clang 11 (default compiler
in FreeBSD-CURRENT) I needed a few other changes to avoid warnings:

hw/s390x/ipl.h, hw/usb/dev-uas.c variable sized type warnings

../hw/usb/dev-uas.c:157:31: error: field 'status' with variable sized
type 'uas_iu' not at the end of a struct or class is a GNU extension
[-Werror,-Wgnu-variable-sized-type-not-at-end]
    uas_iu                    status;
                              ^

target/s390x/cpu_models.c pointer to smaller integer type cast

../target/s390x/cpu_models.c:984:21: error: cast to smaller integer
type 'S390Feat' from 'void *' [-Werror,-Wvoid-pointer-to-enum-cast]
    S390Feat feat = (S390Feat) opaque;
                    ^~~~~~~~~~~~~~~~~


^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH v2 7/8] qemu/bswap: Use compiler __builtin_bswap() on NetBSD
  2020-09-28 13:19 ` [PATCH v2 7/8] qemu/bswap: Use compiler __builtin_bswap() on NetBSD Philippe Mathieu-Daudé
@ 2020-09-28 21:51   ` Kamil Rytarowski
  2020-09-29  8:58     ` Peter Maydell
  0 siblings, 1 reply; 23+ messages in thread
From: Kamil Rytarowski @ 2020-09-28 21:51 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Fam Zheng, Richard Henderson, Kamil Rytarowski, Alex Bennée

On 28.09.2020 15:19, Philippe Mathieu-Daudé wrote:
> Since commit efc6c070aca ("configure: Add a test for the minimum
> compiler version") the minimum compiler version required for GCC
> is 4.8, which supports __builtin_bswap().
> Remove the NetBSD specific ifdef'ry.
> 
> This reverts commit 1360677cfe3ca8f945fa1de77823df21a77e4500
> ("makes NetBSD use the native bswap functions").
> 

Personally, I prefer using the system headers. but if you want to use
the GCC builtins, please go for it.

> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  configure            | 15 ---------------
>  include/qemu/bswap.h |  5 -----
>  2 files changed, 20 deletions(-)
> 
> diff --git a/configure b/configure
> index bff787daea7..1b0a02a0af8 100755
> --- a/configure
> +++ b/configure
> @@ -4886,18 +4886,6 @@ if test "$docs" != "no" ; then
>    fi
>  fi
>  
> -# Search for bswap32 function
> -bswap_h=no
> -cat > $TMPC << EOF
> -#include <sys/endian.h>
> -#include <sys/types.h>
> -#include <machine/bswap.h>
> -int main(void) { return bswap32(0); }
> -EOF
> -if compile_prog "" "" ; then
> -  bswap_h=yes
> -fi
> -
>  ##########################################
>  # Do we have libiscsi >= 1.9.0
>  if test "$libiscsi" != "no" ; then
> @@ -6779,9 +6767,6 @@ fi
>  if test "$st_atim" = "yes" ; then
>    echo "HAVE_STRUCT_STAT_ST_ATIM=y" >> $config_host_mak
>  fi
> -if test "$bswap_h" = "yes" ; then
> -  echo "CONFIG_MACHINE_BSWAP_H=y" >> $config_host_mak
> -fi
>  if test "$curl" = "yes" ; then
>    echo "CONFIG_CURL=y" >> $config_host_mak
>    echo "CURL_CFLAGS=$curl_cflags" >> $config_host_mak
> diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
> index 1a297bfec22..7e586531c09 100644
> --- a/include/qemu/bswap.h
> +++ b/include/qemu/bswap.h
> @@ -3,17 +3,12 @@
>  
>  #include "fpu/softfloat-types.h"
>  
> -#ifdef CONFIG_MACHINE_BSWAP_H
> -# include <sys/endian.h>
> -# include <machine/bswap.h>
> -#else
>  #undef  bswap16
>  #define bswap16(_x) __builtin_bswap16(_x)
>  #undef  bswap32
>  #define bswap32(_x) __builtin_bswap32(_x)
>  #undef  bswap64
>  #define bswap64(_x) __builtin_bswap64(_x)
> -#endif /* ! CONFIG_MACHINE_BSWAP_H */
>  
>  static inline void bswap16s(uint16_t *s)
>  {
> 



^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH v2 7/8] qemu/bswap: Use compiler __builtin_bswap() on NetBSD
  2020-09-28 21:51   ` Kamil Rytarowski
@ 2020-09-29  8:58     ` Peter Maydell
  2020-09-29 14:06       ` Kamil Rytarowski
  0 siblings, 1 reply; 23+ messages in thread
From: Peter Maydell @ 2020-09-29  8:58 UTC (permalink / raw)
  To: Kamil Rytarowski
  Cc: Fam Zheng, Richard Henderson, Philippe Mathieu-Daudé,
	QEMU Developers, Alex Bennée

On Mon, 28 Sep 2020 at 23:02, Kamil Rytarowski <kamil@netbsd.org> wrote:
>
> Personally, I prefer using the system headers. but if you want to use
> the GCC builtins, please go for it.

I'd agree if the system header approach was cross-platform
or if this was a BSD-only program or if we were aiming for
complete compiler-implementation independence, but since we
rely on GCC/clang all over the place already it seems nicer to
avoid all the machinery for identifying which of the multiple
different system header implementations is present, and
instead just have a single implementation that works on
all the hosts we care about...

thanks
-- PMM


^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH v2 7/8] qemu/bswap: Use compiler __builtin_bswap() on NetBSD
  2020-09-29  8:58     ` Peter Maydell
@ 2020-09-29 14:06       ` Kamil Rytarowski
  0 siblings, 0 replies; 23+ messages in thread
From: Kamil Rytarowski @ 2020-09-29 14:06 UTC (permalink / raw)
  To: Peter Maydell, Kamil Rytarowski
  Cc: Fam Zheng, Richard Henderson, Philippe Mathieu-Daudé,
	QEMU Developers, Alex Bennée

On 29.09.2020 10:58, Peter Maydell wrote:
> On Mon, 28 Sep 2020 at 23:02, Kamil Rytarowski <kamil@netbsd.org> wrote:
>>
>> Personally, I prefer using the system headers. but if you want to use
>> the GCC builtins, please go for it.
> 
> I'd agree if the system header approach was cross-platform
> or if this was a BSD-only program or if we were aiming for
> complete compiler-implementation independence, but since we
> rely on GCC/clang all over the place already it seems nicer to
> avoid all the machinery for identifying which of the multiple
> different system header implementations is present, and
> instead just have a single implementation that works on
> all the hosts we care about...
> 

This is already a part of POSIX:

https://www.austingroupbugs.net/view.php?id=162

We have got everything needed from the standard now to implement bswap
without relying on compiler builtins. Every modern enough POSIX-like OS
already ships with <endian.h>.

> thanks
> -- PMM
> 



^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH v2 5/8] qemu/bswap: Use compiler __builtin_bswap() on Haiku
  2020-09-28 14:18       ` Philippe Mathieu-Daudé
@ 2020-10-19 11:00         ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-19 11:00 UTC (permalink / raw)
  To: Daniel P. Berrangé, David CARLIER
  Cc: Fam Zheng, Thomas Huth, Richard Henderson, qemu-devel,
	Carlo Arenas, Kamil Rytarowski, Gerd Hoffmann, Alex Bennée

On 9/28/20 4:18 PM, Philippe Mathieu-Daudé wrote:
> On 9/28/20 4:13 PM, Daniel P. Berrangé wrote:
>> On Mon, Sep 28, 2020 at 03:09:01PM +0100, David CARLIER wrote:
>>> Unfortunately it breaks the build.
>>
>> Can you provide details of the errors seen and toolchain versions.
>>
>> I notice we don't have any CI support for Haiku right now, nor
>> any recipe in tests/vm/  for enabling users to setup a VM with
>> Haiku installed. This very much makes Haiku a second class
>> citizen right now in terms of QEMU's supported platforms, with
>> no expectation of whether it'll work at any point in time.
> 
> I provided the tests/vm/ build script from Alexander as patch 8/8
> of this series:
> https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg10022.html
> 
> This is what I used to test this patch/series.

I suppose the Haiku VM patch got lost, so I'll repost it separate.

Which tree should I Cc:? Ah, Alex testing tree :)


^ permalink raw reply	[flat|nested] 23+ messages in thread
end of thread, other threads:[~2020-10-19 11:02 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-28 13:19 [PATCH v2 0/8] qemu/bswap: Use compiler __builtin_bswap() Philippe Mathieu-Daudé
2020-09-28 13:19 ` [PATCH v2 1/8] qemu/bswap: Remove unused qemu_bswap_len() Philippe Mathieu-Daudé
2020-09-28 13:19 ` [PATCH v2 2/8] qemu/bswap: Replace bswapXX() by compiler __builtin_bswap() Philippe Mathieu-Daudé
2020-09-28 13:19 ` [PATCH v2 3/8] qemu/bswap: Replace bswapXXs() " Philippe Mathieu-Daudé
2020-09-28 17:29   ` Thomas Huth
2020-09-28 13:19 ` [PATCH v2 4/8] qemu/bswap: Remove <byteswap.h> dependency Philippe Mathieu-Daudé
2020-09-28 13:19 ` [PATCH v2 5/8] qemu/bswap: Use compiler __builtin_bswap() on Haiku Philippe Mathieu-Daudé
2020-09-28 14:09   ` David CARLIER
2020-09-28 14:13     ` Daniel P. Berrangé
2020-09-28 14:18       ` Philippe Mathieu-Daudé
2020-10-19 11:00         ` Philippe Mathieu-Daudé
2020-09-28 15:00       ` David CARLIER
2020-09-28 17:29   ` Thomas Huth
2020-09-28 13:19 ` [PATCH v2 6/8] qemu/bswap: Use compiler __builtin_bswap() on FreeBSD Philippe Mathieu-Daudé
2020-09-28 17:32   ` Thomas Huth
2020-09-28 17:57     ` Philippe Mathieu-Daudé
2020-09-28 17:58       ` Philippe Mathieu-Daudé
2020-09-28 21:16   ` Ed Maste
2020-09-28 13:19 ` [PATCH v2 7/8] qemu/bswap: Use compiler __builtin_bswap() on NetBSD Philippe Mathieu-Daudé
2020-09-28 21:51   ` Kamil Rytarowski
2020-09-29  8:58     ` Peter Maydell
2020-09-29 14:06       ` Kamil Rytarowski
2020-09-28 13:19 ` [RFC PATCH v2 8/8] tests/vm: Add Haiku test based on their vagrant images Philippe Mathieu-Daudé

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.