Re: [PATCH 1/2] include/exec: Make ld*_p and st*_p functions available for generic code, too

[Philippe Mathieu-Daudé]

Hi Thomas,

On 17/5/23 09:42, Thomas Huth wrote:
> This will allow to move more code into the target independent source set.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   include/exec/cpu-all.h | 25 ----------------
>   include/exec/tswap.h   | 66 ++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 66 insertions(+), 25 deletions(-)
>
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index ad824fee52..0daa4c06e5 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -55,31 +55,6 @@
>   #define bswaptls(s) bswap64s(s)
>   #endif
>   
> -/* Target-endianness CPU memory access functions. These fit into the
> - * {ld,st}{type}{sign}{size}{endian}_p naming scheme described in bswap.h.
> - */
> -#if TARGET_BIG_ENDIAN
> -#define lduw_p(p) lduw_be_p(p)
> -#define ldsw_p(p) ldsw_be_p(p)
> -#define ldl_p(p) ldl_be_p(p)
> -#define ldq_p(p) ldq_be_p(p)
> -#define stw_p(p, v) stw_be_p(p, v)
> -#define stl_p(p, v) stl_be_p(p, v)
> -#define stq_p(p, v) stq_be_p(p, v)
> -#define ldn_p(p, sz) ldn_be_p(p, sz)
> -#define stn_p(p, sz, v) stn_be_p(p, sz, v)
> -#else
> -#define lduw_p(p) lduw_le_p(p)
> -#define ldsw_p(p) ldsw_le_p(p)
> -#define ldl_p(p) ldl_le_p(p)
> -#define ldq_p(p) ldq_le_p(p)
> -#define stw_p(p, v) stw_le_p(p, v)
> -#define stl_p(p, v) stl_le_p(p, v)
> -#define stq_p(p, v) stq_le_p(p, v)
> -#define ldn_p(p, sz) ldn_le_p(p, sz)
> -#define stn_p(p, sz, v) stn_le_p(p, sz, v)
> -#endif
> -
>   /* MMU memory access macros */
>   
>   #if defined(CONFIG_USER_ONLY)
> diff --git a/include/exec/tswap.h b/include/exec/tswap.h
> index 68944a880b..2774820bbe 100644
> --- a/include/exec/tswap.h
> +++ b/include/exec/tswap.h
> @@ -69,4 +69,70 @@ static inline void tswap64s(uint64_t *s)
>       }
>   }
>   
> +/*
> + * Target-endianness CPU memory access functions. These fit into the
> + * {ld,st}{type}{sign}{size}{endian}_p naming scheme described in bswap.h.
> + */
> +
> +static inline int lduw_p(const void *ptr)
> +{
> +    return (uint16_t)tswap16(lduw_he_p(ptr));
> +}
> +
> +static inline int ldsw_p(const void *ptr)
> +{
> +    return (int16_t)tswap16(lduw_he_p(ptr));
> +}
> +
> +static inline int ldl_p(const void *ptr)
> +{
> +    return tswap32(ldl_he_p(ptr));
> +}
> +
> +static inline uint64_t ldq_p(const void *ptr)
> +{
> +    return tswap64(ldq_he_p(ptr));
> +}

Hmm I am a bit confused, I was working on removing the tswapXX API
from softmmu [*] (restricting it locally to gdbstub). Now I realize
commit 24be3369ad ("include/exec: Provide the tswap() functions for 
target independent code, too") exposes it furthermore. I thought the
ld/st API was clearer and enough for all our uses, but maybe I am
wrong and we need this API.

[*] 
https://lore.kernel.org/qemu-devel/20221213125218.39868-1-philmd@linaro.org/