Re: [PATCH 2/3] include/exec: Provide the tswap() functions for target independent code, too

["Cédric Le Goater" <clg@kaod.org>]

On 4/11/23 20:34, Thomas Huth wrote:
> In some cases of target independent code, it would be useful to have access
> to the functions that swap endianess in case it differs between guest and
> host. Thus re-implement the tswapXX() functions in a new header that can be
> included separately. The check whether the swapping is needed continues to
> be done at compile-time for target specific code, while it is done at
> run-time in target-independent code.


Reviewed-by: Cédric Le Goater <clg@kaod.org>

> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   include/exec/cpu-all.h | 64 +------------------------------------
>   include/exec/tswap.h   | 72 ++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 73 insertions(+), 63 deletions(-)
>   create mode 100644 include/exec/tswap.h
> 
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index 090922e4a8..ad824fee52 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -21,6 +21,7 @@
>   
>   #include "exec/cpu-common.h"
>   #include "exec/memory.h"
> +#include "exec/tswap.h"
>   #include "qemu/thread.h"
>   #include "hw/core/cpu.h"
>   #include "qemu/rcu.h"
> @@ -44,69 +45,6 @@
>   #define BSWAP_NEEDED

I wonder if we could get rid of the BSWAP_NEEDED define also.
(used load_aout() and ELF loaders)

Thanks,

C.



>   #endif
>   
> -#ifdef BSWAP_NEEDED
> -
> -static inline uint16_t tswap16(uint16_t s)
> -{
> -    return bswap16(s);
> -}
> -
> -static inline uint32_t tswap32(uint32_t s)
> -{
> -    return bswap32(s);
> -}
> -
> -static inline uint64_t tswap64(uint64_t s)
> -{
> -    return bswap64(s);
> -}
> -
> -static inline void tswap16s(uint16_t *s)
> -{
> -    *s = bswap16(*s);
> -}
> -
> -static inline void tswap32s(uint32_t *s)
> -{
> -    *s = bswap32(*s);
> -}
> -
> -static inline void tswap64s(uint64_t *s)
> -{
> -    *s = bswap64(*s);
> -}
> -
> -#else
> -
> -static inline uint16_t tswap16(uint16_t s)
> -{
> -    return s;
> -}
> -
> -static inline uint32_t tswap32(uint32_t s)
> -{
> -    return s;
> -}
> -
> -static inline uint64_t tswap64(uint64_t s)
> -{
> -    return s;
> -}
> -
> -static inline void tswap16s(uint16_t *s)
> -{
> -}
> -
> -static inline void tswap32s(uint32_t *s)
> -{
> -}
> -
> -static inline void tswap64s(uint64_t *s)
> -{
> -}
> -
> -#endif
> -
>   #if TARGET_LONG_SIZE == 4
>   #define tswapl(s) tswap32(s)
>   #define tswapls(s) tswap32s((uint32_t *)(s))
> diff --git a/include/exec/tswap.h b/include/exec/tswap.h
> new file mode 100644
> index 0000000000..68944a880b
> --- /dev/null
> +++ b/include/exec/tswap.h
> @@ -0,0 +1,72 @@
> +/*
> + * Macros for swapping a value if the endianness is different
> + * between the target and the host.
> + *
> + * SPDX-License-Identifier: LGPL-2.1-or-later
> + */
> +
> +#ifndef TSWAP_H
> +#define TSWAP_H
> +
> +#include "hw/core/cpu.h"
> +#include "qemu/bswap.h"
> +
> +/*
> + * If we're in target-specific code, we can hard-code the swapping
> + * condition, otherwise we have to do (slower) run-time checks.
> + */
> +#ifdef NEED_CPU_H
> +#define target_needs_bswap()  (HOST_BIG_ENDIAN != TARGET_BIG_ENDIAN)
> +#else
> +#define target_needs_bswap()  (target_words_bigendian() != HOST_BIG_ENDIAN)
> +#endif
> +
> +static inline uint16_t tswap16(uint16_t s)
> +{
> +    if (target_needs_bswap()) {
> +        return bswap16(s);
> +    } else {
> +        return s;
> +    }
> +}
> +
> +static inline uint32_t tswap32(uint32_t s)
> +{
> +    if (target_needs_bswap()) {
> +        return bswap32(s);
> +    } else {
> +        return s;
> +    }
> +}
> +
> +static inline uint64_t tswap64(uint64_t s)
> +{
> +    if (target_needs_bswap()) {
> +        return bswap64(s);
> +    } else {
> +        return s;
> +    }
> +}
> +
> +static inline void tswap16s(uint16_t *s)
> +{
> +    if (target_needs_bswap()) {
> +        *s = bswap16(*s);
> +    }
> +}
> +
> +static inline void tswap32s(uint32_t *s)
> +{
> +    if (target_needs_bswap()) {
> +        *s = bswap32(*s);
> +    }
> +}
> +
> +static inline void tswap64s(uint64_t *s)
> +{
> +    if (target_needs_bswap()) {
> +        *s = bswap64(*s);
> +    }
> +}
> +
> +#endif  /* TSWAP_H */