[PATCH 01/13] qemu/bswap: Introduce ld/st_endian_p() API

Philippe Mathieu-Daudé posted 13 patches 1 month, 3 weeks ago
[PATCH 01/13] qemu/bswap: Introduce ld/st_endian_p() API
Posted by Philippe Mathieu-Daudé 1 month, 3 weeks ago
Introduce the ld/st_endian_p() API, which takes an extra
boolean argument to dispatch to ld/st_{be,le}_p() methods.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
TODO: Update docstring regexp
---
 include/qemu/bswap.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index ad22910a5d..ec813a756d 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -433,4 +433,23 @@ DO_STN_LDN_P(be)
 #undef le_bswaps
 #undef be_bswaps
 
+#define lduw_endian_p(big_endian, p) \
+                     (big_endian) ? lduw_be_p(p) : lduw_le_p(p)
+#define ldsw_endian_p(big_endian, p) \
+                     (big_endian) ? ldsw_be_p(p) : ldsw_be_p(p)
+#define ldl_endian_p(big_endian, p) \
+                    (big_endian) ? ldl_be_p(p) : ldl_le_p(p)
+#define ldq_endian_p(big_endian, p) \
+                    (big_endian) ? ldq_be_p(p) : ldq_le_p(p)
+#define stw_endian_p(big_endian, p, v) \
+                    (big_endian) ? stw_be_p(p, v) : stw_le_p(p, v)
+#define stl_endian_p(big_endian, p, v) \
+                    (big_endian) ? stl_be_p(p, v) : stl_le_p(p, v)
+#define stq_endian_p(big_endian, p, v) \
+                    (big_endian) ? stq_be_p(p, v) : stq_le_p(p, v)
+#define ldn_endian_p(big_endian, p, sz) \
+                     (big_endian) ? ldn_be_p(p, sz) : ldn_le_p(p, sz)
+#define stn_endian_p(big_endian, p, sz, v) \
+                    (big_endian) ? stn_be_p(p, sz, v) : stn_le_p(p, sz, v)
+
 #endif /* BSWAP_H */
-- 
2.45.2


Re: [PATCH 01/13] qemu/bswap: Introduce ld/st_endian_p() API
Posted by Philippe Mathieu-Daudé 1 month, 3 weeks ago
On 30/9/24 09:34, Philippe Mathieu-Daudé wrote:
> Introduce the ld/st_endian_p() API, which takes an extra

Alternatively we could use ld/st_te_p() since we already
have ld/st_he_p() for host endianness.

> boolean argument to dispatch to ld/st_{be,le}_p() methods.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> TODO: Update docstring regexp
> ---
>   include/qemu/bswap.h | 19 +++++++++++++++++++
>   1 file changed, 19 insertions(+)
> 
> diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
> index ad22910a5d..ec813a756d 100644
> --- a/include/qemu/bswap.h
> +++ b/include/qemu/bswap.h
> @@ -433,4 +433,23 @@ DO_STN_LDN_P(be)
>   #undef le_bswaps
>   #undef be_bswaps
>   
> +#define lduw_endian_p(big_endian, p) \
> +                     (big_endian) ? lduw_be_p(p) : lduw_le_p(p)
> +#define ldsw_endian_p(big_endian, p) \
> +                     (big_endian) ? ldsw_be_p(p) : ldsw_be_p(p)
> +#define ldl_endian_p(big_endian, p) \
> +                    (big_endian) ? ldl_be_p(p) : ldl_le_p(p)
> +#define ldq_endian_p(big_endian, p) \
> +                    (big_endian) ? ldq_be_p(p) : ldq_le_p(p)
> +#define stw_endian_p(big_endian, p, v) \
> +                    (big_endian) ? stw_be_p(p, v) : stw_le_p(p, v)
> +#define stl_endian_p(big_endian, p, v) \
> +                    (big_endian) ? stl_be_p(p, v) : stl_le_p(p, v)
> +#define stq_endian_p(big_endian, p, v) \
> +                    (big_endian) ? stq_be_p(p, v) : stq_le_p(p, v)
> +#define ldn_endian_p(big_endian, p, sz) \
> +                     (big_endian) ? ldn_be_p(p, sz) : ldn_le_p(p, sz)
> +#define stn_endian_p(big_endian, p, sz, v) \
> +                    (big_endian) ? stn_be_p(p, sz, v) : stn_le_p(p, sz, v)
> +
>   #endif /* BSWAP_H */


Re: [PATCH 01/13] qemu/bswap: Introduce ld/st_endian_p() API
Posted by Richard Henderson 1 month, 3 weeks ago
On 10/3/24 13:50, Philippe Mathieu-Daudé wrote:
> On 30/9/24 09:34, Philippe Mathieu-Daudé wrote:
>> Introduce the ld/st_endian_p() API, which takes an extra
> 
> Alternatively we could use ld/st_te_p() since we already
> have ld/st_he_p() for host endianness.

That's what ld/st_p are -- target-specific, in exec/cpu-all.h.


r~

Re: [PATCH 01/13] qemu/bswap: Introduce ld/st_endian_p() API
Posted by Philippe Mathieu-Daudé 1 month, 3 weeks ago
On 3/10/24 23:28, Richard Henderson wrote:
> On 10/3/24 13:50, Philippe Mathieu-Daudé wrote:
>> On 30/9/24 09:34, Philippe Mathieu-Daudé wrote:
>>> Introduce the ld/st_endian_p() API, which takes an extra
>>
>> Alternatively we could use ld/st_te_p() since we already
>> have ld/st_he_p() for host endianness.
> 
> That's what ld/st_p are -- target-specific, in exec/cpu-all.h.

They are indeed *target-specific*, so we can not use them in
target-agnostic code.

By explicitly passing the endianness, ld/st_endian_p() API is
target-agnostic.

Re: [PATCH 01/13] qemu/bswap: Introduce ld/st_endian_p() API
Posted by Richard Henderson 1 month, 3 weeks ago
On 10/3/24 14:34, Philippe Mathieu-Daudé wrote:
> On 3/10/24 23:28, Richard Henderson wrote:
>> On 10/3/24 13:50, Philippe Mathieu-Daudé wrote:
>>> On 30/9/24 09:34, Philippe Mathieu-Daudé wrote:
>>>> Introduce the ld/st_endian_p() API, which takes an extra
>>>
>>> Alternatively we could use ld/st_te_p() since we already
>>> have ld/st_he_p() for host endianness.
>>
>> That's what ld/st_p are -- target-specific, in exec/cpu-all.h.
> 
> They are indeed *target-specific*, so we can not use them in
> target-agnostic code.
> 
> By explicitly passing the endianness, ld/st_endian_p() API is
> target-agnostic.

Then I miss whatever you meant here re st_te_p().
Care to elaborate?


r~

Re: [PATCH 01/13] qemu/bswap: Introduce ld/st_endian_p() API
Posted by Philippe Mathieu-Daudé 1 month, 3 weeks ago
On 3/10/24 23:37, Richard Henderson wrote:
> On 10/3/24 14:34, Philippe Mathieu-Daudé wrote:
>> On 3/10/24 23:28, Richard Henderson wrote:
>>> On 10/3/24 13:50, Philippe Mathieu-Daudé wrote:
>>>> On 30/9/24 09:34, Philippe Mathieu-Daudé wrote:
>>>>> Introduce the ld/st_endian_p() API, which takes an extra
>>>>
>>>> Alternatively we could use ld/st_te_p() since we already
>>>> have ld/st_he_p() for host endianness.
>>>
>>> That's what ld/st_p are -- target-specific, in exec/cpu-all.h.
>>
>> They are indeed *target-specific*, so we can not use them in
>> target-agnostic code.
>>
>> By explicitly passing the endianness, ld/st_endian_p() API is
>> target-agnostic.
> 
> Then I miss whatever you meant here re st_te_p().
> Care to elaborate?

I might had a bad start by adding this now endian-agnostic API
before removing the current endian-specific one.

Goal is instead of having machine code build twice, one for each
endianness, the same machine will be built once, but registering
2x machines. Endianness being a machine property, propagated to
the vCPUs and HW.

Instead of the following target-specific API:

   #if TARGET_BIG_ENDIAN
   #define stl_p(p, v) stl_be_p(p, v)
   #else
   #define stl_p(p, v) stl_le_p(p, v)
   #endif

I'm suggesting this target-agnostic one:

   #define stl_endian_p(big_endian, p, v) \
                       (big_endian) ? stl_be_p(p, v) : stl_le_p(p, v)

Re: [PATCH 01/13] qemu/bswap: Introduce ld/st_endian_p() API
Posted by Pierrick Bouvier 1 month, 3 weeks ago
On 9/30/24 00:34, Philippe Mathieu-Daudé wrote:
> Introduce the ld/st_endian_p() API, which takes an extra
> boolean argument to dispatch to ld/st_{be,le}_p() methods.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> TODO: Update docstring regexp
> ---
>   include/qemu/bswap.h | 19 +++++++++++++++++++
>   1 file changed, 19 insertions(+)
> 
> diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
> index ad22910a5d..ec813a756d 100644
> --- a/include/qemu/bswap.h
> +++ b/include/qemu/bswap.h
> @@ -433,4 +433,23 @@ DO_STN_LDN_P(be)
>   #undef le_bswaps
>   #undef be_bswaps
>   
> +#define lduw_endian_p(big_endian, p) \
> +                     (big_endian) ? lduw_be_p(p) : lduw_le_p(p)
> +#define ldsw_endian_p(big_endian, p) \
> +                     (big_endian) ? ldsw_be_p(p) : ldsw_be_p(p)
> +#define ldl_endian_p(big_endian, p) \
> +                    (big_endian) ? ldl_be_p(p) : ldl_le_p(p)
> +#define ldq_endian_p(big_endian, p) \
> +                    (big_endian) ? ldq_be_p(p) : ldq_le_p(p)
> +#define stw_endian_p(big_endian, p, v) \
> +                    (big_endian) ? stw_be_p(p, v) : stw_le_p(p, v)
> +#define stl_endian_p(big_endian, p, v) \
> +                    (big_endian) ? stl_be_p(p, v) : stl_le_p(p, v)
> +#define stq_endian_p(big_endian, p, v) \
> +                    (big_endian) ? stq_be_p(p, v) : stq_le_p(p, v)
> +#define ldn_endian_p(big_endian, p, sz) \
> +                     (big_endian) ? ldn_be_p(p, sz) : ldn_le_p(p, sz)
> +#define stn_endian_p(big_endian, p, sz, v) \
> +                    (big_endian) ? stn_be_p(p, sz, v) : stn_le_p(p, sz, v)
> +
>   #endif /* BSWAP_H */

May it be useful to have extra parenthesis around macro value to prevent 
any issue when using it?
((big_endian) ? stn_be_p(p, sz, v) : stn_le_p(p, sz, v))

Else,
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>