[PATCH v3 12/28] accel/tcg: Merge tcg_exec_init into tcg_init_machine

Richard Henderson posted 28 patches 4 years, 9 months ago
[PATCH v3 12/28] accel/tcg: Merge tcg_exec_init into tcg_init_machine
Posted by Richard Henderson 4 years, 9 months ago
There is only one caller, and shortly we will need access
to the MachineState, which tcg_init_machine already has.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/internal.h      |  2 ++
 include/sysemu/tcg.h      |  2 --
 accel/tcg/tcg-all.c       | 14 +++++++++++++-
 accel/tcg/translate-all.c | 21 ++-------------------
 4 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/accel/tcg/internal.h b/accel/tcg/internal.h
index e9c145e0fb..881bc1ede0 100644
--- a/accel/tcg/internal.h
+++ b/accel/tcg/internal.h
@@ -16,5 +16,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu, target_ulong pc,
                               int cflags);
 
 void QEMU_NORETURN cpu_io_recompile(CPUState *cpu, uintptr_t retaddr);
+void page_init(void);
+void tb_htable_init(void);
 
 #endif /* ACCEL_TCG_INTERNAL_H */
diff --git a/include/sysemu/tcg.h b/include/sysemu/tcg.h
index 00349fb18a..53352450ff 100644
--- a/include/sysemu/tcg.h
+++ b/include/sysemu/tcg.h
@@ -8,8 +8,6 @@
 #ifndef SYSEMU_TCG_H
 #define SYSEMU_TCG_H
 
-void tcg_exec_init(unsigned long tb_size, int splitwx);
-
 #ifdef CONFIG_TCG
 extern bool tcg_allowed;
 #define tcg_enabled() (tcg_allowed)
diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
index 30d81ff7f5..0e83acbfe5 100644
--- a/accel/tcg/tcg-all.c
+++ b/accel/tcg/tcg-all.c
@@ -32,6 +32,7 @@
 #include "qemu/error-report.h"
 #include "qemu/accel.h"
 #include "qapi/qapi-builtin-visit.h"
+#include "internal.h"
 
 struct TCGState {
     AccelState parent_obj;
@@ -109,8 +110,19 @@ static int tcg_init_machine(MachineState *ms)
 {
     TCGState *s = TCG_STATE(current_accel());
 
-    tcg_exec_init(s->tb_size * 1024 * 1024, s->splitwx_enabled);
+    tcg_allowed = true;
     mttcg_enabled = s->mttcg_enabled;
+
+    page_init();
+    tb_htable_init();
+    tcg_init(s->tb_size * 1024 * 1024, s->splitwx_enabled);
+
+#if defined(CONFIG_SOFTMMU)
+    /* There's no guest base to take into account, so go ahead and
+       initialize the prologue now.  */
+    tcg_prologue_init(tcg_ctx);
+#endif
+
     return 0;
 }
 
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index bebb3366c0..0c818c3618 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -408,7 +408,7 @@ bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc, bool will_exit)
     return false;
 }
 
-static void page_init(void)
+void page_init(void)
 {
     page_size_init();
     page_table_config_init();
@@ -907,30 +907,13 @@ static bool tb_cmp(const void *ap, const void *bp)
         a->page_addr[1] == b->page_addr[1];
 }
 
-static void tb_htable_init(void)
+void tb_htable_init(void)
 {
     unsigned int mode = QHT_MODE_AUTO_RESIZE;
 
     qht_init(&tb_ctx.htable, tb_cmp, CODE_GEN_HTABLE_SIZE, mode);
 }
 
-/* Must be called before using the QEMU cpus. 'tb_size' is the size
-   (in bytes) allocated to the translation buffer. Zero means default
-   size. */
-void tcg_exec_init(unsigned long tb_size, int splitwx)
-{
-    tcg_allowed = true;
-    page_init();
-    tb_htable_init();
-    tcg_init(tb_size, splitwx);
-
-#if defined(CONFIG_SOFTMMU)
-    /* There's no guest base to take into account, so go ahead and
-       initialize the prologue now.  */
-    tcg_prologue_init(tcg_ctx);
-#endif
-}
-
 /* call with @p->lock held */
 static inline void invalidate_page_bitmap(PageDesc *p)
 {
-- 
2.25.1


Re: [PATCH v3 12/28] accel/tcg: Merge tcg_exec_init into tcg_init_machine
Posted by Alex Bennée 4 years, 8 months ago
Richard Henderson <richard.henderson@linaro.org> writes:

> There is only one caller, and shortly we will need access
> to the MachineState, which tcg_init_machine already has.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  accel/tcg/internal.h      |  2 ++
>  include/sysemu/tcg.h      |  2 --
>  accel/tcg/tcg-all.c       | 14 +++++++++++++-
>  accel/tcg/translate-all.c | 21 ++-------------------
>  4 files changed, 17 insertions(+), 22 deletions(-)
>
> diff --git a/accel/tcg/internal.h b/accel/tcg/internal.h
> index e9c145e0fb..881bc1ede0 100644
> --- a/accel/tcg/internal.h
> +++ b/accel/tcg/internal.h
> @@ -16,5 +16,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu, target_ulong pc,
>                                int cflags);
>  
>  void QEMU_NORETURN cpu_io_recompile(CPUState *cpu, uintptr_t retaddr);
> +void page_init(void);
> +void tb_htable_init(void);
>  
>  #endif /* ACCEL_TCG_INTERNAL_H */
> diff --git a/include/sysemu/tcg.h b/include/sysemu/tcg.h
> index 00349fb18a..53352450ff 100644
> --- a/include/sysemu/tcg.h
> +++ b/include/sysemu/tcg.h
> @@ -8,8 +8,6 @@
>  #ifndef SYSEMU_TCG_H
>  #define SYSEMU_TCG_H
>  
> -void tcg_exec_init(unsigned long tb_size, int splitwx);
> -
>  #ifdef CONFIG_TCG
>  extern bool tcg_allowed;
>  #define tcg_enabled() (tcg_allowed)
> diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
> index 30d81ff7f5..0e83acbfe5 100644
> --- a/accel/tcg/tcg-all.c
> +++ b/accel/tcg/tcg-all.c
> @@ -32,6 +32,7 @@
>  #include "qemu/error-report.h"
>  #include "qemu/accel.h"
>  #include "qapi/qapi-builtin-visit.h"
> +#include "internal.h"
>  
>  struct TCGState {
>      AccelState parent_obj;
> @@ -109,8 +110,19 @@ static int tcg_init_machine(MachineState *ms)
>  {
>      TCGState *s = TCG_STATE(current_accel());
>  
> -    tcg_exec_init(s->tb_size * 1024 * 1024, s->splitwx_enabled);
> +    tcg_allowed = true;
>      mttcg_enabled = s->mttcg_enabled;
> +
> +    page_init();
> +    tb_htable_init();
> +    tcg_init(s->tb_size * 1024 * 1024, s->splitwx_enabled);
> +

nit: you could clean up to use unit.h definitions, i.e. tb->size * MiB

Otherwise:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée

Re: [PATCH v3 12/28] accel/tcg: Merge tcg_exec_init into tcg_init_machine
Posted by Richard Henderson 4 years, 8 months ago
On 6/8/21 4:55 AM, Alex Bennée wrote:
>> +    tcg_init(s->tb_size * 1024 * 1024, s->splitwx_enabled);
>> +
> 
> nit: you could clean up to use unit.h definitions, i.e. tb->size * MiB
> 
> Otherwise:
> 
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

Amusingly, I cleaned that up this week while rebasing.


r~

RE: [PATCH v3 12/28] accel/tcg: Merge tcg_exec_init into tcg_init_machine
Posted by Luis Fernando Fujita Pires 4 years, 8 months ago
From: Richard Henderson <richard.henderson@linaro.org>
> There is only one caller, and shortly we will need access to the MachineState,
> which tcg_init_machine already has.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  accel/tcg/internal.h      |  2 ++
>  include/sysemu/tcg.h      |  2 --
>  accel/tcg/tcg-all.c       | 14 +++++++++++++-
>  accel/tcg/translate-all.c | 21 ++-------------------
>  4 files changed, 17 insertions(+), 22 deletions(-)

Nitpicking: there's a comment in bsd-user/main.c's main() that should be updated now that tcg_exec_init() no longer exists. Currently:
934     /*
935      * Now that page sizes are configured in tcg_exec_init() we can do
936      * proper page alignment for guest_base.
937      */
938     guest_base = HOST_PAGE_ALIGN(guest_base);

Reviewed-by: Luis Pires <luis.pires@eldorado.org.br>

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>

Re: [PATCH v3 12/28] accel/tcg: Merge tcg_exec_init into tcg_init_machine
Posted by Richard Henderson 4 years, 8 months ago
On 6/9/21 7:58 AM, Luis Fernando Fujita Pires wrote:
> From: Richard Henderson <richard.henderson@linaro.org>
>> There is only one caller, and shortly we will need access to the MachineState,
>> which tcg_init_machine already has.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   accel/tcg/internal.h      |  2 ++
>>   include/sysemu/tcg.h      |  2 --
>>   accel/tcg/tcg-all.c       | 14 +++++++++++++-
>>   accel/tcg/translate-all.c | 21 ++-------------------
>>   4 files changed, 17 insertions(+), 22 deletions(-)
> 
> Nitpicking: there's a comment in bsd-user/main.c's main() that should be updated now that tcg_exec_init() no longer exists. Currently:
> 934     /*
> 935      * Now that page sizes are configured in tcg_exec_init() we can do
> 936      * proper page alignment for guest_base.
> 937      */
> 938     guest_base = HOST_PAGE_ALIGN(guest_base);

Thanks.  I think the comment was wrong, and has been wrong for a while.  The 
only thing that controls HOST_PAGE_ALIGN is command-line options.

I've just clipped tcg_exec_init out of the comment and will leave the rest to 
the guys that are modernizing bsd-user/.


r~