diff mbox series

[09/13] exec/memory_ldst_phys: Introduce ld/st_endian_phys() API

Message ID 20240930073450.33195-10-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_phys() API, which takes an extra
boolean argument to dispatch to ld/st_{be,le}_phys() methods.

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

Comments

Pierrick Bouvier Oct. 1, 2024, 4:54 p.m. UTC | #1 
On 9/30/24 00:34, Philippe Mathieu-Daudé wrote:
> Introduce the ld/st_endian_phys() API, which takes an extra
> boolean argument to dispatch to ld/st_{be,le}_phys() methods.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> TODO: Update docstring regexp
> ---
>   include/exec/memory_ldst_phys.h.inc | 66 +++++++++++++++++++++++++++++
>   1 file changed, 66 insertions(+)
> 
> diff --git a/include/exec/memory_ldst_phys.h.inc b/include/exec/memory_ldst_phys.h.inc
> index ecd678610d..8ea162b40d 100644
> --- a/include/exec/memory_ldst_phys.h.inc
> +++ b/include/exec/memory_ldst_phys.h.inc
> @@ -74,6 +74,16 @@ static inline uint16_t glue(lduw_be_phys, SUFFIX)(ARG1_DECL, hwaddr addr)
>                                                  MEMTXATTRS_UNSPECIFIED, NULL);
>   }
>   
> +static inline uint16_t glue(lduw_endian_phys, SUFFIX)(bool big_endian,
> +                                                      ARG1_DECL, hwaddr addr)
> +{
> +    return big_endian
> +           ? glue(address_space_lduw_le, SUFFIX)(ARG1, addr,
> +                                                 MEMTXATTRS_UNSPECIFIED, NULL)
> +           : glue(address_space_lduw_be, SUFFIX)(ARG1, addr,
> +                                                 MEMTXATTRS_UNSPECIFIED, NULL);
> +}
> +
>   static inline uint32_t glue(ldl_le_phys, SUFFIX)(ARG1_DECL, hwaddr addr)
>   {
>       return glue(address_space_ldl_le, SUFFIX)(ARG1, addr,
> @@ -86,6 +96,16 @@ static inline uint32_t glue(ldl_be_phys, SUFFIX)(ARG1_DECL, hwaddr addr)
>                                                 MEMTXATTRS_UNSPECIFIED, NULL);
>   }
>   
> +static inline uint32_t glue(ldl_endian_phys, SUFFIX)(bool big_endian,
> +                                                     ARG1_DECL, hwaddr addr)
> +{
> +    return big_endian
> +           ? glue(address_space_ldl_le, SUFFIX)(ARG1, addr,
> +                                                MEMTXATTRS_UNSPECIFIED, NULL)
> +           : glue(address_space_ldl_be, SUFFIX)(ARG1, addr,
> +                                                MEMTXATTRS_UNSPECIFIED, NULL);
> +}
> +
>   static inline uint64_t glue(ldq_le_phys, SUFFIX)(ARG1_DECL, hwaddr addr)
>   {
>       return glue(address_space_ldq_le, SUFFIX)(ARG1, addr,
> @@ -98,6 +118,16 @@ static inline uint64_t glue(ldq_be_phys, SUFFIX)(ARG1_DECL, hwaddr addr)
>                                                 MEMTXATTRS_UNSPECIFIED, NULL);
>   }
>   
> +static inline uint32_t glue(ldq_endian_phys, SUFFIX)(bool big_endian,
> +                                                     ARG1_DECL, hwaddr addr)
> +{
> +    return big_endian
> +           ? glue(address_space_ldq_le, SUFFIX)(ARG1, addr,
> +                                                MEMTXATTRS_UNSPECIFIED, NULL)
> +           : glue(address_space_ldq_be, SUFFIX)(ARG1, addr,
> +                                                MEMTXATTRS_UNSPECIFIED, NULL);
> +}
> +
>   static inline void glue(stb_phys, SUFFIX)(ARG1_DECL, hwaddr addr, uint8_t val)
>   {
>       glue(address_space_stb, SUFFIX)(ARG1, addr, val,
> @@ -116,6 +146,18 @@ static inline void glue(stw_be_phys, SUFFIX)(ARG1_DECL, hwaddr addr, uint16_t va
>                                          MEMTXATTRS_UNSPECIFIED, NULL);
>   }
>   
> +static inline void glue(stw_endian_phys, SUFFIX)(bool big_endian, ARG1_DECL,
> +                                                 hwaddr addr, uint16_t val)
> +{
> +    if (big_endian) {
> +        glue(address_space_stw_be, SUFFIX)(ARG1, addr, val,
> +                                           MEMTXATTRS_UNSPECIFIED, NULL);
> +   } else {
> +        glue(address_space_stw_le, SUFFIX)(ARG1, addr, val,
> +                                           MEMTXATTRS_UNSPECIFIED, NULL);
> +    }
> +}
> +
>   static inline void glue(stl_le_phys, SUFFIX)(ARG1_DECL, hwaddr addr, uint32_t val)
>   {
>       glue(address_space_stl_le, SUFFIX)(ARG1, addr, val,
> @@ -128,6 +170,18 @@ static inline void glue(stl_be_phys, SUFFIX)(ARG1_DECL, hwaddr addr, uint32_t va
>                                          MEMTXATTRS_UNSPECIFIED, NULL);
>   }
>   
> +static inline void glue(stl_endian_phys, SUFFIX)(bool big_endian, ARG1_DECL,
> +                                                 hwaddr addr, uint32_t val)
> +{
> +    if (big_endian) {
> +        glue(address_space_stl_be, SUFFIX)(ARG1, addr, val,
> +                                           MEMTXATTRS_UNSPECIFIED, NULL);
> +   } else {
> +        glue(address_space_stl_le, SUFFIX)(ARG1, addr, val,
> +                                           MEMTXATTRS_UNSPECIFIED, NULL);
> +    }
> +}
> +
>   static inline void glue(stq_le_phys, SUFFIX)(ARG1_DECL, hwaddr addr, uint64_t val)
>   {
>       glue(address_space_stq_le, SUFFIX)(ARG1, addr, val,
> @@ -139,6 +193,18 @@ static inline void glue(stq_be_phys, SUFFIX)(ARG1_DECL, hwaddr addr, uint64_t va
>       glue(address_space_stq_be, SUFFIX)(ARG1, addr, val,
>                                          MEMTXATTRS_UNSPECIFIED, NULL);
>   }
> +
> +static inline void glue(stq_endian_phys, SUFFIX)(bool big_endian, ARG1_DECL,
> +                                                 hwaddr addr, uint64_t val)
> +{
> +    if (big_endian) {
> +        glue(address_space_stq_be, SUFFIX)(ARG1, addr, val,
> +                                           MEMTXATTRS_UNSPECIFIED, NULL);
> +   } else {
> +        glue(address_space_stq_le, SUFFIX)(ARG1, addr, val,
> +                                           MEMTXATTRS_UNSPECIFIED, NULL);
> +    }
> +}
>   #endif
>   
>   #undef ARG1_DECL

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Richard Henderson Oct. 3, 2024, 9:39 p.m. UTC | #2 
On 9/30/24 00:34, Philippe Mathieu-Daudé wrote:
> Introduce the ld/st_endian_phys() API, which takes an extra
> boolean argument to dispatch to ld/st_{be,le}_phys() methods.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> TODO: Update docstring regexp
> ---
>   include/exec/memory_ldst_phys.h.inc | 66 +++++++++++++++++++++++++++++
>   1 file changed, 66 insertions(+)
> 
> diff --git a/include/exec/memory_ldst_phys.h.inc b/include/exec/memory_ldst_phys.h.inc
> index ecd678610d..8ea162b40d 100644
> --- a/include/exec/memory_ldst_phys.h.inc
> +++ b/include/exec/memory_ldst_phys.h.inc
> @@ -74,6 +74,16 @@ static inline uint16_t glue(lduw_be_phys, SUFFIX)(ARG1_DECL, hwaddr addr)
>                                                  MEMTXATTRS_UNSPECIFIED, NULL);
>   }
>   
> +static inline uint16_t glue(lduw_endian_phys, SUFFIX)(bool big_endian,
> +                                                      ARG1_DECL, hwaddr addr)
> +{
> +    return big_endian
> +           ? glue(address_space_lduw_le, SUFFIX)(ARG1, addr,
> +                                                 MEMTXATTRS_UNSPECIFIED, NULL)
> +           : glue(address_space_lduw_be, SUFFIX)(ARG1, addr,
> +                                                 MEMTXATTRS_UNSPECIFIED, NULL);
> +}
> +

This is backward, using the le function for big_endian true.

> +static inline uint32_t glue(ldl_endian_phys, SUFFIX)(bool big_endian,
> +                                                     ARG1_DECL, hwaddr addr)
> +{
> +    return big_endian
> +           ? glue(address_space_ldl_le, SUFFIX)(ARG1, addr,
> +                                                MEMTXATTRS_UNSPECIFIED, NULL)
> +           : glue(address_space_ldl_be, SUFFIX)(ARG1, addr,
> +                                                MEMTXATTRS_UNSPECIFIED, NULL);
> +}

Likewise.


r~
Richard Henderson Oct. 3, 2024, 9:47 p.m. UTC | #3 
On 9/30/24 00:34, Philippe Mathieu-Daudé wrote:
> Introduce the ld/st_endian_phys() API, which takes an extra
> boolean argument to dispatch to ld/st_{be,le}_phys() methods.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> TODO: Update docstring regexp
> ---
>   include/exec/memory_ldst_phys.h.inc | 66 +++++++++++++++++++++++++++++
>   1 file changed, 66 insertions(+)
> 
> diff --git a/include/exec/memory_ldst_phys.h.inc b/include/exec/memory_ldst_phys.h.inc
> index ecd678610d..8ea162b40d 100644
> --- a/include/exec/memory_ldst_phys.h.inc
> +++ b/include/exec/memory_ldst_phys.h.inc
> @@ -74,6 +74,16 @@ static inline uint16_t glue(lduw_be_phys, SUFFIX)(ARG1_DECL, hwaddr addr)
>                                                  MEMTXATTRS_UNSPECIFIED, NULL);
>   }
>   
> +static inline uint16_t glue(lduw_endian_phys, SUFFIX)(bool big_endian,
> +                                                      ARG1_DECL, hwaddr addr)
> +{
> +    return big_endian
> +           ? glue(address_space_lduw_le, SUFFIX)(ARG1, addr,
> +                                                 MEMTXATTRS_UNSPECIFIED, NULL)
> +           : glue(address_space_lduw_be, SUFFIX)(ARG1, addr,
> +                                                 MEMTXATTRS_UNSPECIFIED, NULL);
> +}

Endian swap aside, I think you should expose this at the address_space_* level first, 
where the internals already have

     return glue(address_space_lduw_internal, SUFFIX)(ARG1, addr, attrs, result,
                                                      DEVICE_LITTLE_ENDIAN);

then you can pass big_endian directly (perhaps frobbed into DEVICE_* space).

That leaves one unconditional function call instead of two conditional calls.


r~
diff mbox series

Patch

diff --git a/include/exec/memory_ldst_phys.h.inc b/include/exec/memory_ldst_phys.h.inc
index ecd678610d..8ea162b40d 100644
--- a/include/exec/memory_ldst_phys.h.inc
+++ b/include/exec/memory_ldst_phys.h.inc
@@ -74,6 +74,16 @@  static inline uint16_t glue(lduw_be_phys, SUFFIX)(ARG1_DECL, hwaddr addr)
                                                MEMTXATTRS_UNSPECIFIED, NULL);
 }
 
+static inline uint16_t glue(lduw_endian_phys, SUFFIX)(bool big_endian,
+                                                      ARG1_DECL, hwaddr addr)
+{
+    return big_endian
+           ? glue(address_space_lduw_le, SUFFIX)(ARG1, addr,
+                                                 MEMTXATTRS_UNSPECIFIED, NULL)
+           : glue(address_space_lduw_be, SUFFIX)(ARG1, addr,
+                                                 MEMTXATTRS_UNSPECIFIED, NULL);
+}
+
 static inline uint32_t glue(ldl_le_phys, SUFFIX)(ARG1_DECL, hwaddr addr)
 {
     return glue(address_space_ldl_le, SUFFIX)(ARG1, addr,
@@ -86,6 +96,16 @@  static inline uint32_t glue(ldl_be_phys, SUFFIX)(ARG1_DECL, hwaddr addr)
                                               MEMTXATTRS_UNSPECIFIED, NULL);
 }
 
+static inline uint32_t glue(ldl_endian_phys, SUFFIX)(bool big_endian,
+                                                     ARG1_DECL, hwaddr addr)
+{
+    return big_endian
+           ? glue(address_space_ldl_le, SUFFIX)(ARG1, addr,
+                                                MEMTXATTRS_UNSPECIFIED, NULL)
+           : glue(address_space_ldl_be, SUFFIX)(ARG1, addr,
+                                                MEMTXATTRS_UNSPECIFIED, NULL);
+}
+
 static inline uint64_t glue(ldq_le_phys, SUFFIX)(ARG1_DECL, hwaddr addr)
 {
     return glue(address_space_ldq_le, SUFFIX)(ARG1, addr,
@@ -98,6 +118,16 @@  static inline uint64_t glue(ldq_be_phys, SUFFIX)(ARG1_DECL, hwaddr addr)
                                               MEMTXATTRS_UNSPECIFIED, NULL);
 }
 
+static inline uint32_t glue(ldq_endian_phys, SUFFIX)(bool big_endian,
+                                                     ARG1_DECL, hwaddr addr)
+{
+    return big_endian
+           ? glue(address_space_ldq_le, SUFFIX)(ARG1, addr,
+                                                MEMTXATTRS_UNSPECIFIED, NULL)
+           : glue(address_space_ldq_be, SUFFIX)(ARG1, addr,
+                                                MEMTXATTRS_UNSPECIFIED, NULL);
+}
+
 static inline void glue(stb_phys, SUFFIX)(ARG1_DECL, hwaddr addr, uint8_t val)
 {
     glue(address_space_stb, SUFFIX)(ARG1, addr, val,
@@ -116,6 +146,18 @@  static inline void glue(stw_be_phys, SUFFIX)(ARG1_DECL, hwaddr addr, uint16_t va
                                        MEMTXATTRS_UNSPECIFIED, NULL);
 }
 
+static inline void glue(stw_endian_phys, SUFFIX)(bool big_endian, ARG1_DECL,
+                                                 hwaddr addr, uint16_t val)
+{
+    if (big_endian) {
+        glue(address_space_stw_be, SUFFIX)(ARG1, addr, val,
+                                           MEMTXATTRS_UNSPECIFIED, NULL);
+   } else {
+        glue(address_space_stw_le, SUFFIX)(ARG1, addr, val,
+                                           MEMTXATTRS_UNSPECIFIED, NULL);
+    }
+}
+
 static inline void glue(stl_le_phys, SUFFIX)(ARG1_DECL, hwaddr addr, uint32_t val)
 {
     glue(address_space_stl_le, SUFFIX)(ARG1, addr, val,
@@ -128,6 +170,18 @@  static inline void glue(stl_be_phys, SUFFIX)(ARG1_DECL, hwaddr addr, uint32_t va
                                        MEMTXATTRS_UNSPECIFIED, NULL);
 }
 
+static inline void glue(stl_endian_phys, SUFFIX)(bool big_endian, ARG1_DECL,
+                                                 hwaddr addr, uint32_t val)
+{
+    if (big_endian) {
+        glue(address_space_stl_be, SUFFIX)(ARG1, addr, val,
+                                           MEMTXATTRS_UNSPECIFIED, NULL);
+   } else {
+        glue(address_space_stl_le, SUFFIX)(ARG1, addr, val,
+                                           MEMTXATTRS_UNSPECIFIED, NULL);
+    }
+}
+
 static inline void glue(stq_le_phys, SUFFIX)(ARG1_DECL, hwaddr addr, uint64_t val)
 {
     glue(address_space_stq_le, SUFFIX)(ARG1, addr, val,
@@ -139,6 +193,18 @@  static inline void glue(stq_be_phys, SUFFIX)(ARG1_DECL, hwaddr addr, uint64_t va
     glue(address_space_stq_be, SUFFIX)(ARG1, addr, val,
                                        MEMTXATTRS_UNSPECIFIED, NULL);
 }
+
+static inline void glue(stq_endian_phys, SUFFIX)(bool big_endian, ARG1_DECL,
+                                                 hwaddr addr, uint64_t val)
+{
+    if (big_endian) {
+        glue(address_space_stq_be, SUFFIX)(ARG1, addr, val,
+                                           MEMTXATTRS_UNSPECIFIED, NULL);
+   } else {
+        glue(address_space_stq_le, SUFFIX)(ARG1, addr, val,
+                                           MEMTXATTRS_UNSPECIFIED, NULL);
+    }
+}
 #endif
 
 #undef ARG1_DECL