[PATCH 20/24] exec: Declare abi_ptr type in its own 'tcg/abi_ptr.h' header

Philippe Mathieu-Daudé posted 24 patches 1 year, 11 months ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, Riku Voipio <riku.voipio@iki.fi>, Warner Losh <imp@bsdimp.com>, Kyle Evans <kevans@freebsd.org>, "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, BALATON Zoltan <balaton@eik.bme.hu>, Nicholas Piggin <npiggin@gmail.com>, Daniel Henrique Barboza <danielhb413@gmail.com>, "Cédric Le Goater" <clg@kaod.org>, David Gibson <david@gibson.dropbear.id.au>, Harsh Prateek Bora <harshpb@linux.ibm.com>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Thomas Huth <thuth@redhat.com>, Halil Pasic <pasic@linux.ibm.com>, Eric Farman <farman@linux.ibm.com>, David Hildenbrand <david@redhat.com>, Ilya Leoshkevich <iii@linux.ibm.com>, Peter Xu <peterx@redhat.com>, Laurent Vivier <laurent@vivier.eu>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Michael Rolnik <mrolnik@gmail.com>, Brian Cain <bcain@quicinc.com>, Eduardo Habkost <eduardo@habkost.net>, Song Gao <gaosong@loongson.cn>, Aurelien Jarno <aurelien@aurel32.net>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>, Huacai Chen <chenhuacai@kernel.org>, Chris Wulff <crwulff@gmail.com>, Marek Vasut <marex@denx.de>, Stafford Horne <shorne@gmail.com>, Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bin.meng@windriver.com>, Weiwei Li <liwei1518@gmail.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Yoshinori Sato <ysato@users.sourceforge.jp>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Artyom Tarasenko <atar4qemu@gmail.com>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Max Filippov <jcmvbkbc@gmail.com>
There is a newer version of this series
[PATCH 20/24] exec: Declare abi_ptr type in its own 'tcg/abi_ptr.h' header
Posted by Philippe Mathieu-Daudé 1 year, 11 months ago
The abi_ptr type is declared in "exec/cpu_ldst.h" with all
the load/store helpers. Some source files requiring abi_ptr
type don't need the load/store helpers. In order to simplify,
create a new "tcg/abi_ptr.h" header.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/exec/cpu_ldst.h   | 17 +++--------------
 include/exec/exec-all.h   |  1 +
 include/exec/translator.h |  5 ++++-
 include/tcg/abi_ptr.h     | 32 ++++++++++++++++++++++++++++++++
 accel/tcg/cputlb.c        |  1 +
 5 files changed, 41 insertions(+), 15 deletions(-)
 create mode 100644 include/tcg/abi_ptr.h

diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index 25e7239cc5..c69f02b699 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -65,18 +65,9 @@
 #include "exec/memopidx.h"
 #include "qemu/int128.h"
 #include "cpu.h"
+#include "tcg/abi_ptr.h"
 
 #if defined(CONFIG_USER_ONLY)
-/* sparc32plus has 64bit long but 32bit space address
- * this can make bad result with g2h() and h2g()
- */
-#if TARGET_VIRT_ADDR_SPACE_BITS <= 32
-typedef uint32_t abi_ptr;
-#define TARGET_ABI_FMT_ptr "%x"
-#else
-typedef uint64_t abi_ptr;
-#define TARGET_ABI_FMT_ptr "%"PRIx64
-#endif
 
 #ifndef TARGET_TAGGED_ADDRESSES
 static inline abi_ptr cpu_untagged_addr(CPUState *cs, abi_ptr x)
@@ -120,10 +111,8 @@ static inline bool guest_range_valid_untagged(abi_ulong start, abi_ulong len)
     assert(h2g_valid(x)); \
     h2g_nocheck(x); \
 })
-#else
-typedef target_ulong abi_ptr;
-#define TARGET_ABI_FMT_ptr TARGET_FMT_lx
-#endif
+
+#endif /* CONFIG_USER_ONLY */
 
 uint32_t cpu_ldub_data(CPUArchState *env, abi_ptr ptr);
 int cpu_ldsb_data(CPUArchState *env, abi_ptr ptr);
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index df3d93a2e2..6a634c0889 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -22,6 +22,7 @@
 
 #include "cpu.h"
 #if defined(CONFIG_USER_ONLY)
+#include "tcg/abi_ptr.h"
 #include "exec/cpu_ldst.h"
 #endif
 #include "exec/translation-block.h"
diff --git a/include/exec/translator.h b/include/exec/translator.h
index 6d3f59d095..16d2449292 100644
--- a/include/exec/translator.h
+++ b/include/exec/translator.h
@@ -19,7 +19,10 @@
  */
 
 #include "qemu/bswap.h"
-#include "exec/cpu_ldst.h"	/* for abi_ptr */
+#include "exec/cpu-common.h"
+#include "exec/cpu-defs.h"
+#include "cpu.h"
+#include "tcg/abi_ptr.h"
 
 /**
  * gen_intermediate_code
diff --git a/include/tcg/abi_ptr.h b/include/tcg/abi_ptr.h
new file mode 100644
index 0000000000..415e31cabb
--- /dev/null
+++ b/include/tcg/abi_ptr.h
@@ -0,0 +1,32 @@
+/*
+ * TCG abi_ptr type
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+#ifndef TCG_ABI_PTR_H
+#define TCG_ABI_PTR_H
+
+#include "cpu-param.h"
+
+#if defined(CONFIG_USER_ONLY)
+/* sparc32plus has 64bit long but 32bit space address
+ * this can make bad result with g2h() and h2g()
+ */
+#if TARGET_VIRT_ADDR_SPACE_BITS <= 32
+typedef uint32_t abi_ptr;
+#define TARGET_ABI_FMT_ptr "%x"
+#else
+typedef uint64_t abi_ptr;
+#define TARGET_ABI_FMT_ptr "%"PRIx64
+#endif
+
+#else /* !CONFIG_USER_ONLY */
+
+#include "exec/target_long.h"
+
+typedef target_ulong abi_ptr;
+#define TARGET_ABI_FMT_ptr TARGET_FMT_lx
+
+#endif /* !CONFIG_USER_ONLY */
+
+#endif
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index db3f93fda9..c4500d3261 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -41,6 +41,7 @@
 #ifdef CONFIG_PLUGIN
 #include "qemu/plugin-memory.h"
 #endif
+#include "tcg/abi_ptr.h"
 #include "tcg/tcg-ldst.h"
 #include "tcg/oversized-guest.h"
 
-- 
2.41.0


Re: [PATCH 20/24] exec: Declare abi_ptr type in its own 'tcg/abi_ptr.h' header
Posted by Richard Henderson 1 year, 11 months ago
On 12/11/23 13:19, Philippe Mathieu-Daudé wrote:
> The abi_ptr type is declared in "exec/cpu_ldst.h" with all
> the load/store helpers. Some source files requiring abi_ptr
> type don't need the load/store helpers. In order to simplify,
> create a new "tcg/abi_ptr.h" header.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/exec/cpu_ldst.h   | 17 +++--------------
>   include/exec/exec-all.h   |  1 +
>   include/exec/translator.h |  5 ++++-
>   include/tcg/abi_ptr.h     | 32 ++++++++++++++++++++++++++++++++
>   accel/tcg/cputlb.c        |  1 +
>   5 files changed, 41 insertions(+), 15 deletions(-)
>   create mode 100644 include/tcg/abi_ptr.h

I'm unconvinced about this being tcg related, per se.
Leave it in include/exec/, with exec/user/abitypes.h.

With that,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 20/24] exec: Declare abi_ptr type in its own 'tcg/abi_ptr.h' header
Posted by Philippe Mathieu-Daudé 1 year, 11 months ago
On 12/12/23 02:18, Richard Henderson wrote:
> On 12/11/23 13:19, Philippe Mathieu-Daudé wrote:
>> The abi_ptr type is declared in "exec/cpu_ldst.h" with all
>> the load/store helpers. Some source files requiring abi_ptr
>> type don't need the load/store helpers. In order to simplify,
>> create a new "tcg/abi_ptr.h" header.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   include/exec/cpu_ldst.h   | 17 +++--------------
>>   include/exec/exec-all.h   |  1 +
>>   include/exec/translator.h |  5 ++++-
>>   include/tcg/abi_ptr.h     | 32 ++++++++++++++++++++++++++++++++
>>   accel/tcg/cputlb.c        |  1 +
>>   5 files changed, 41 insertions(+), 15 deletions(-)
>>   create mode 100644 include/tcg/abi_ptr.h
> 
> I'm unconvinced about this being tcg related, per se.

Indeed:

$ git grep -w abi_ptr -- tcg
$

> Leave it in include/exec/, with exec/user/abitypes.h.
> 
> With that,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Thanks!


Re: [PATCH 20/24] exec: Declare abi_ptr type in its own 'tcg/abi_ptr.h' header
Posted by Philippe Mathieu-Daudé 1 year, 11 months ago
On 11/12/23 22:19, Philippe Mathieu-Daudé wrote:
> The abi_ptr type is declared in "exec/cpu_ldst.h" with all
> the load/store helpers. Some source files requiring abi_ptr
> type don't need the load/store helpers. In order to simplify,
> create a new "tcg/abi_ptr.h" header.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/exec/cpu_ldst.h   | 17 +++--------------
>   include/exec/exec-all.h   |  1 +
>   include/exec/translator.h |  5 ++++-
>   include/tcg/abi_ptr.h     | 32 ++++++++++++++++++++++++++++++++
>   accel/tcg/cputlb.c        |  1 +
>   5 files changed, 41 insertions(+), 15 deletions(-)
>   create mode 100644 include/tcg/abi_ptr.h
> 
> diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
> index 25e7239cc5..c69f02b699 100644
> --- a/include/exec/cpu_ldst.h
> +++ b/include/exec/cpu_ldst.h
> @@ -65,18 +65,9 @@
>   #include "exec/memopidx.h"
>   #include "qemu/int128.h"
>   #include "cpu.h"
> +#include "tcg/abi_ptr.h"
>   
>   #if defined(CONFIG_USER_ONLY)
> -/* sparc32plus has 64bit long but 32bit space address
> - * this can make bad result with g2h() and h2g()
> - */
> -#if TARGET_VIRT_ADDR_SPACE_BITS <= 32
> -typedef uint32_t abi_ptr;
> -#define TARGET_ABI_FMT_ptr "%x"
> -#else
> -typedef uint64_t abi_ptr;
> -#define TARGET_ABI_FMT_ptr "%"PRIx64
> -#endif
>   
>   #ifndef TARGET_TAGGED_ADDRESSES
>   static inline abi_ptr cpu_untagged_addr(CPUState *cs, abi_ptr x)
> @@ -120,10 +111,8 @@ static inline bool guest_range_valid_untagged(abi_ulong start, abi_ulong len)
>       assert(h2g_valid(x)); \
>       h2g_nocheck(x); \
>   })
> -#else
> -typedef target_ulong abi_ptr;
> -#define TARGET_ABI_FMT_ptr TARGET_FMT_lx
> -#endif
> +
> +#endif /* CONFIG_USER_ONLY */
>   
>   uint32_t cpu_ldub_data(CPUArchState *env, abi_ptr ptr);
>   int cpu_ldsb_data(CPUArchState *env, abi_ptr ptr);


> diff --git a/include/tcg/abi_ptr.h b/include/tcg/abi_ptr.h
> new file mode 100644
> index 0000000000..415e31cabb
> --- /dev/null
> +++ b/include/tcg/abi_ptr.h
> @@ -0,0 +1,32 @@
> +/*
> + * TCG abi_ptr type
> + *
> + * SPDX-License-Identifier: LGPL-2.1-or-later
> + */
> +#ifndef TCG_ABI_PTR_H
> +#define TCG_ABI_PTR_H
> +
> +#include "cpu-param.h"
> +
> +#if defined(CONFIG_USER_ONLY)
> +/* sparc32plus has 64bit long but 32bit space address

Forgot to squash:

-- >8 --
@@ -11,3 +11,4 @@
  #if defined(CONFIG_USER_ONLY)
-/* sparc32plus has 64bit long but 32bit space address
+/*
+ * sparc32plus has 64bit long but 32bit space address
   * this can make bad result with g2h() and h2g()
---

> + * this can make bad result with g2h() and h2g()
> + */
> +#if TARGET_VIRT_ADDR_SPACE_BITS <= 32
> +typedef uint32_t abi_ptr;
> +#define TARGET_ABI_FMT_ptr "%x"
> +#else
> +typedef uint64_t abi_ptr;
> +#define TARGET_ABI_FMT_ptr "%"PRIx64
> +#endif
> +
> +#else /* !CONFIG_USER_ONLY */
> +
> +#include "exec/target_long.h"
> +
> +typedef target_ulong abi_ptr;
> +#define TARGET_ABI_FMT_ptr TARGET_FMT_lx
> +
> +#endif /* !CONFIG_USER_ONLY */
> +
> +#endif