diff mbox series

[RFC,26/34] Wrap target macros in functions

Message ID 20240119144024.14289-27-anjo@rev.ng (mailing list archive)
State New, archived
Headers show
Series Compile accel/tcg once (partially) | expand

Commit Message

Anton Johansson Jan. 19, 2024, 2:40 p.m. UTC 
Adds wrapper functions around common target specific macros required by
accel/tcg.

Signed-off-by: Anton Johansson <anjo@rev.ng>
---
 include/hw/core/cpu.h |  9 +++++++
 cpu-target.c          | 62 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+)

Comments

Philippe Mathieu-Daudé Jan. 23, 2024, 11:50 a.m. UTC | #1 
Hi Anton,

On 19/1/24 15:40, Anton Johansson wrote:
> Adds wrapper functions around common target specific macros required by
> accel/tcg.
> 
> Signed-off-by: Anton Johansson <anjo@rev.ng>
> ---
>   include/hw/core/cpu.h |  9 +++++++
>   cpu-target.c          | 62 +++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 71 insertions(+)
> 
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 57d100c203..a2d65c1d7a 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -26,6 +26,7 @@
>   #include "exec/vaddr.h"
>   #include "exec/memattrs.h"
>   #include "exec/tlb-common.h"
> +#include "exec/memop.h"
>   #include "qapi/qapi-types-run-state.h"
>   #include "qemu/bitmap.h"
>   #include "qemu/rcu_queue.h"
> @@ -1164,6 +1165,14 @@ void cpu_exec_unrealizefn(CPUState *cpu);
>    * what you are doing!
>    */
>   bool target_words_bigendian(void);
> +bool target_supports_mttcg(void);
> +bool target_has_precise_smc(void);
> +int target_long_bits(void);
> +int target_phys_addr_space_bits(void);
> +uint8_t target_insn_start_words(void);
> +uint8_t target_default_memory_order(void);
> +uint8_t target_tlb_dyn_max_bits(void);
> +MemOp target_endian_memory_order(void);

None of these helpers take argument. I don't understand
how they can be called in heterogeneous context.

>   const char *target_name(void);
>   
> diff --git a/cpu-target.c b/cpu-target.c
> index 1a8e730bed..6b67af7a51 100644
> --- a/cpu-target.c
> +++ b/cpu-target.c
> @@ -39,10 +39,13 @@
>   #include "exec/tb-flush.h"
>   #include "exec/translate-all.h"
>   #include "exec/log.h"
> +#include "exec/cpu-defs.h"
>   #include "hw/core/accel-cpu.h"
>   #include "trace/trace-root.h"
>   #include "qemu/accel.h"
>   #include "qemu/plugin.h"
> +#include "tcg/tcg-mo.h"
> +#include "tcg/insn-start-words.h"
>   
>   uintptr_t qemu_host_page_size;
>   intptr_t qemu_host_page_mask;
> @@ -416,6 +419,65 @@ bool target_words_bigendian(void)
>       return TARGET_BIG_ENDIAN;
>   }
>   
> +bool target_supports_mttcg(void)
> +{
> +#ifdef TARGET_SUPPORTS_MTTCG
> +# ifndef TCG_GUEST_DEFAULT_MO
> +#  error "TARGET_SUPPORTS_MTTCG without TCG_GUEST_DEFAULT_MO"
> +# endif
> +    return true;
> +#else
> +    return false;
> +#endif
> +}
> +
> +bool target_has_precise_smc(void)
> +{
> +#ifdef TARGET_HAS_PRECISE_SMC
> +    return true;
> +#else
> +    return false;
> +#endif
> +}
> +
> +int target_long_bits(void)
> +{
> +    return TARGET_LONG_BITS;
> +}
> +
> +int target_phys_addr_space_bits(void)
> +{
> +    return TARGET_PHYS_ADDR_SPACE_BITS;
> +}
> +
> +uint8_t target_insn_start_words(void)
> +{
> +    return TARGET_INSN_START_WORDS;
> +}
> +
> +uint8_t target_default_memory_order(void)
> +{
> +#ifdef TCG_GUEST_DEFAULT_MO
> +    return TCG_GUEST_DEFAULT_MO;
> +#else
> +    return TCG_MO_ALL;
> +#endif
> +}
> +
> +MemOp target_endian_memory_order(void)
> +{
> +    return MO_TE;
> +}
> +
> +uint8_t target_tlb_dyn_max_bits(void)
> +{
> +#if defined(CONFIG_SOFTMMU) && defined(CONFIG_TCG)
> +    return CPU_TLB_DYN_MAX_BITS;
> +#else
> +    return 0;
> +#endif
> +}
> +
>   const char *target_name(void)
>   {
>       return TARGET_NAME;
Anton Johansson Jan. 23, 2024, 12:12 p.m. UTC | #2 
On 23/01/24, Philippe Mathieu-Daudé wrote:
> Hi Anton,
> 
> On 19/1/24 15:40, Anton Johansson wrote:
> > Adds wrapper functions around common target specific macros required by
> > accel/tcg.
> > 
> > Signed-off-by: Anton Johansson <anjo@rev.ng>
> > ---
> >   include/hw/core/cpu.h |  9 +++++++
> >   cpu-target.c          | 62 +++++++++++++++++++++++++++++++++++++++++++
> >   2 files changed, 71 insertions(+)
> > 
> > diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> > index 57d100c203..a2d65c1d7a 100644
> > --- a/include/hw/core/cpu.h
> > +++ b/include/hw/core/cpu.h
> > @@ -26,6 +26,7 @@
> >   #include "exec/vaddr.h"
> >   #include "exec/memattrs.h"
> >   #include "exec/tlb-common.h"
> > +#include "exec/memop.h"
> >   #include "qapi/qapi-types-run-state.h"
> >   #include "qemu/bitmap.h"
> >   #include "qemu/rcu_queue.h"
> > @@ -1164,6 +1165,14 @@ void cpu_exec_unrealizefn(CPUState *cpu);
> >    * what you are doing!
> >    */
> >   bool target_words_bigendian(void);
> > +bool target_supports_mttcg(void);
> > +bool target_has_precise_smc(void);
> > +int target_long_bits(void);
> > +int target_phys_addr_space_bits(void);
> > +uint8_t target_insn_start_words(void);
> > +uint8_t target_default_memory_order(void);
> > +uint8_t target_tlb_dyn_max_bits(void);
> > +MemOp target_endian_memory_order(void);
> 
> None of these helpers take argument. I don't understand
> how they can be called in heterogeneous context.

No you're right, I was focused mostly on getting accel/tcg to compile 
with hetrogeneous being a goal downt the line.

I like the idea of moving these fields to a struct filled out per 
target, but dispatching would also work.
Richard Henderson Jan. 24, 2024, 2:50 a.m. UTC | #3 
On 1/23/24 22:12, Anton Johansson wrote:
> On 23/01/24, Philippe Mathieu-Daudé wrote:
>> Hi Anton,
>>
>> On 19/1/24 15:40, Anton Johansson wrote:
>>> Adds wrapper functions around common target specific macros required by
>>> accel/tcg.
>>>
>>> Signed-off-by: Anton Johansson <anjo@rev.ng>
>>> ---
>>>    include/hw/core/cpu.h |  9 +++++++
>>>    cpu-target.c          | 62 +++++++++++++++++++++++++++++++++++++++++++
>>>    2 files changed, 71 insertions(+)
>>>
>>> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
>>> index 57d100c203..a2d65c1d7a 100644
>>> --- a/include/hw/core/cpu.h
>>> +++ b/include/hw/core/cpu.h
>>> @@ -26,6 +26,7 @@
>>>    #include "exec/vaddr.h"
>>>    #include "exec/memattrs.h"
>>>    #include "exec/tlb-common.h"
>>> +#include "exec/memop.h"
>>>    #include "qapi/qapi-types-run-state.h"
>>>    #include "qemu/bitmap.h"
>>>    #include "qemu/rcu_queue.h"
>>> @@ -1164,6 +1165,14 @@ void cpu_exec_unrealizefn(CPUState *cpu);
>>>     * what you are doing!
>>>     */
>>>    bool target_words_bigendian(void);
>>> +bool target_supports_mttcg(void);
>>> +bool target_has_precise_smc(void);
>>> +int target_long_bits(void);
>>> +int target_phys_addr_space_bits(void);
>>> +uint8_t target_insn_start_words(void);
>>> +uint8_t target_default_memory_order(void);
>>> +uint8_t target_tlb_dyn_max_bits(void);
>>> +MemOp target_endian_memory_order(void);
>>
>> None of these helpers take argument. I don't understand
>> how they can be called in heterogeneous context.
> 
> No you're right, I was focused mostly on getting accel/tcg to compile
> with hetrogeneous being a goal downt the line.
> 
> I like the idea of moving these fields to a struct filled out per
> target, but dispatching would also work.

All of the bits that you're referencing in TCGContext, outside of compilation, should be 
treated the same way.  Like Phil, I'd prefer to move these once and get the API right.


r~
diff mbox series

Patch

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 57d100c203..a2d65c1d7a 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -26,6 +26,7 @@ 
 #include "exec/vaddr.h"
 #include "exec/memattrs.h"
 #include "exec/tlb-common.h"
+#include "exec/memop.h"
 #include "qapi/qapi-types-run-state.h"
 #include "qemu/bitmap.h"
 #include "qemu/rcu_queue.h"
@@ -1164,6 +1165,14 @@  void cpu_exec_unrealizefn(CPUState *cpu);
  * what you are doing!
  */
 bool target_words_bigendian(void);
+bool target_supports_mttcg(void);
+bool target_has_precise_smc(void);
+int target_long_bits(void);
+int target_phys_addr_space_bits(void);
+uint8_t target_insn_start_words(void);
+uint8_t target_default_memory_order(void);
+uint8_t target_tlb_dyn_max_bits(void);
+MemOp target_endian_memory_order(void);
 
 const char *target_name(void);
 
diff --git a/cpu-target.c b/cpu-target.c
index 1a8e730bed..6b67af7a51 100644
--- a/cpu-target.c
+++ b/cpu-target.c
@@ -39,10 +39,13 @@ 
 #include "exec/tb-flush.h"
 #include "exec/translate-all.h"
 #include "exec/log.h"
+#include "exec/cpu-defs.h"
 #include "hw/core/accel-cpu.h"
 #include "trace/trace-root.h"
 #include "qemu/accel.h"
 #include "qemu/plugin.h"
+#include "tcg/tcg-mo.h"
+#include "tcg/insn-start-words.h"
 
 uintptr_t qemu_host_page_size;
 intptr_t qemu_host_page_mask;
@@ -416,6 +419,65 @@  bool target_words_bigendian(void)
     return TARGET_BIG_ENDIAN;
 }
 
+bool target_supports_mttcg(void)
+{
+#ifdef TARGET_SUPPORTS_MTTCG
+# ifndef TCG_GUEST_DEFAULT_MO
+#  error "TARGET_SUPPORTS_MTTCG without TCG_GUEST_DEFAULT_MO"
+# endif
+    return true;
+#else
+    return false;
+#endif
+}
+
+bool target_has_precise_smc(void)
+{
+#ifdef TARGET_HAS_PRECISE_SMC
+    return true;
+#else
+    return false;
+#endif
+}
+
+int target_long_bits(void)
+{
+    return TARGET_LONG_BITS;
+}
+
+int target_phys_addr_space_bits(void)
+{
+    return TARGET_PHYS_ADDR_SPACE_BITS;
+}
+
+uint8_t target_insn_start_words(void)
+{
+    return TARGET_INSN_START_WORDS;
+}
+
+uint8_t target_default_memory_order(void)
+{
+#ifdef TCG_GUEST_DEFAULT_MO
+    return TCG_GUEST_DEFAULT_MO;
+#else
+    return TCG_MO_ALL;
+#endif
+}
+
+MemOp target_endian_memory_order(void)
+{
+    return MO_TE;
+}
+
+uint8_t target_tlb_dyn_max_bits(void)
+{
+#if defined(CONFIG_SOFTMMU) && defined(CONFIG_TCG)
+    return CPU_TLB_DYN_MAX_BITS;
+#else
+    return 0;
+#endif
+}
+
 const char *target_name(void)
 {
     return TARGET_NAME;