diff mbox series

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

Message ID 20240930073450.33195-2-philmd@linaro.org (mailing list archive)
State New, archived
Headers show
Series hw: Add ld/st_endian() APIs | expand

Commit Message

Philippe Mathieu-Daudé Sept. 30, 2024, 7:34 a.m. UTC
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(+)

Comments

Pierrick Bouvier Oct. 1, 2024, 4:45 p.m. UTC | #1
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>
Philippe Mathieu-Daudé Oct. 3, 2024, 8:50 p.m. UTC | #2
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 */
Richard Henderson Oct. 3, 2024, 9:28 p.m. UTC | #3
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~
Philippe Mathieu-Daudé Oct. 3, 2024, 9:34 p.m. UTC | #4
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.
Richard Henderson Oct. 3, 2024, 9:37 p.m. UTC | #5
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~
Philippe Mathieu-Daudé Oct. 3, 2024, 9:46 p.m. UTC | #6
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)
diff mbox series

Patch

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 */