| Message ID | 1435330053-18733-9-git-send-email-fred.konrad@greensocs.com |
|---|---|
| State | New |
| Headers | show |
On 26/06/2015 16:47, fred.konrad@greensocs.com wrote: > From: KONRAD Frederic <fred.konrad@greensocs.com> > > This removes exit_request global and adds a variable in CPUState for this. > Only the flag for the first cpu is used for the moment as we are still with one > TCG thread. I think this should also be added to CPUThread. Paolo > Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com> > --- > cpu-exec.c | 15 --------------- > cpus.c | 17 ++++++++++++++--- > 2 files changed, 14 insertions(+), 18 deletions(-) > > diff --git a/cpu-exec.c b/cpu-exec.c > index 5d9b518..0644383 100644 > --- a/cpu-exec.c > +++ b/cpu-exec.c > @@ -364,8 +364,6 @@ static void cpu_handle_debug_exception(CPUArchState *env) > > /* main execution loop */ > > -volatile sig_atomic_t exit_request; > - > int cpu_exec(CPUArchState *env) > { > CPUState *cpu = ENV_GET_CPU(env); > @@ -394,20 +392,8 @@ int cpu_exec(CPUArchState *env) > > current_cpu = cpu; > > - /* As long as current_cpu is null, up to the assignment just above, > - * requests by other threads to exit the execution loop are expected to > - * be issued using the exit_request global. We must make sure that our > - * evaluation of the global value is performed past the current_cpu > - * value transition point, which requires a memory barrier as well as > - * an instruction scheduling constraint on modern architectures. */ > - smp_mb(); > - > rcu_read_lock(); > > - if (unlikely(exit_request)) { > - cpu->exit_request = 1; > - } > - > cc->cpu_exec_enter(cpu); > > /* Calculate difference between guest clock and host clock. > @@ -496,7 +482,6 @@ int cpu_exec(CPUArchState *env) > } > } > if (unlikely(cpu->exit_request)) { > - cpu->exit_request = 0; > cpu->exception_index = EXCP_INTERRUPT; > cpu_loop_exit(cpu); > } > diff --git a/cpus.c b/cpus.c > index 23c316c..2541c56 100644 > --- a/cpus.c > +++ b/cpus.c > @@ -137,6 +137,8 @@ typedef struct TimersState { > } TimersState; > > static TimersState timers_state; > +/* CPU associated to this thread. */ > +static __thread CPUState *tcg_thread_cpu; > > int64_t cpu_get_icount_raw(void) > { > @@ -661,12 +663,18 @@ static void cpu_handle_guest_debug(CPUState *cpu) > cpu->stopped = true; > } > > +/** > + * cpu_signal > + * Signal handler when using TCG. > + */ > static void cpu_signal(int sig) > { > if (current_cpu) { > cpu_exit(current_cpu); > } > - exit_request = 1; > + > + /* FIXME: We might want to check if the cpu is running? */ > + tcg_thread_cpu->exit_request = true; > } > > #ifdef CONFIG_LINUX > @@ -1031,6 +1039,7 @@ static void *qemu_tcg_cpu_thread_fn(void *arg) > { > CPUState *cpu = arg; > > + tcg_thread_cpu = cpu; > qemu_tcg_init_cpu_signals(); > qemu_thread_get_self(cpu->thread); > > @@ -1393,7 +1402,8 @@ static void tcg_exec_all(void) > if (next_cpu == NULL) { > next_cpu = first_cpu; > } > - for (; next_cpu != NULL && !exit_request; next_cpu = CPU_NEXT(next_cpu)) { > + for (; next_cpu != NULL && !first_cpu->exit_request; > + next_cpu = CPU_NEXT(next_cpu)) { > CPUState *cpu = next_cpu; > CPUArchState *env = cpu->env_ptr; > > @@ -1410,7 +1420,8 @@ static void tcg_exec_all(void) > break; > } > } > - exit_request = 0; > + > + first_cpu->exit_request = 0; > } > > void list_cpus(FILE *f, fprintf_function cpu_fprintf, const char *optarg) >
fred.konrad@greensocs.com writes: > From: KONRAD Frederic <fred.konrad@greensocs.com> > > This removes exit_request global and adds a variable in CPUState for this. > Only the flag for the first cpu is used for the moment as we are still with one > TCG thread. > > Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com> > --- > cpu-exec.c | 15 --------------- > cpus.c | 17 ++++++++++++++--- > 2 files changed, 14 insertions(+), 18 deletions(-) > > diff --git a/cpu-exec.c b/cpu-exec.c > index 5d9b518..0644383 100644 > --- a/cpu-exec.c > +++ b/cpu-exec.c > @@ -364,8 +364,6 @@ static void cpu_handle_debug_exception(CPUArchState *env) > > /* main execution loop */ > > -volatile sig_atomic_t exit_request; > - > int cpu_exec(CPUArchState *env) > { > CPUState *cpu = ENV_GET_CPU(env); > @@ -394,20 +392,8 @@ int cpu_exec(CPUArchState *env) > > current_cpu = cpu; > > - /* As long as current_cpu is null, up to the assignment just above, > - * requests by other threads to exit the execution loop are expected to > - * be issued using the exit_request global. We must make sure that our > - * evaluation of the global value is performed past the current_cpu > - * value transition point, which requires a memory barrier as well as > - * an instruction scheduling constraint on modern architectures. */ > - smp_mb(); > - > rcu_read_lock(); > > - if (unlikely(exit_request)) { > - cpu->exit_request = 1; > - } > - > cc->cpu_exec_enter(cpu); > > /* Calculate difference between guest clock and host clock. > @@ -496,7 +482,6 @@ int cpu_exec(CPUArchState *env) > } > } > if (unlikely(cpu->exit_request)) { > - cpu->exit_request = 0; > cpu->exception_index = EXCP_INTERRUPT; > cpu_loop_exit(cpu); > } > diff --git a/cpus.c b/cpus.c > index 23c316c..2541c56 100644 > --- a/cpus.c > +++ b/cpus.c > @@ -137,6 +137,8 @@ typedef struct TimersState { > } TimersState; > > static TimersState timers_state; > +/* CPU associated to this thread. */ > +static __thread CPUState *tcg_thread_cpu; > > int64_t cpu_get_icount_raw(void) > { > @@ -661,12 +663,18 @@ static void cpu_handle_guest_debug(CPUState *cpu) > cpu->stopped = true; > } > > +/** > + * cpu_signal > + * Signal handler when using TCG. > + */ > static void cpu_signal(int sig) > { > if (current_cpu) { > cpu_exit(current_cpu); > } > - exit_request = 1; > + > + /* FIXME: We might want to check if the cpu is running? */ > + tcg_thread_cpu->exit_request = true; I guess the potential problem is race conditions here? What happens if the cpu is signalled by two different threads for two different reasons? > } > > #ifdef CONFIG_LINUX > @@ -1031,6 +1039,7 @@ static void *qemu_tcg_cpu_thread_fn(void *arg) > { > CPUState *cpu = arg; > > + tcg_thread_cpu = cpu; > qemu_tcg_init_cpu_signals(); > qemu_thread_get_self(cpu->thread); > > @@ -1393,7 +1402,8 @@ static void tcg_exec_all(void) > if (next_cpu == NULL) { > next_cpu = first_cpu; > } > - for (; next_cpu != NULL && !exit_request; next_cpu = CPU_NEXT(next_cpu)) { > + for (; next_cpu != NULL && !first_cpu->exit_request; > + next_cpu = CPU_NEXT(next_cpu)) { > CPUState *cpu = next_cpu; > CPUArchState *env = cpu->env_ptr; > > @@ -1410,7 +1420,8 @@ static void tcg_exec_all(void) > break; > } > } > - exit_request = 0; > + > + first_cpu->exit_request = 0; > } > > void list_cpus(FILE *f, fprintf_function cpu_fprintf, const char *optarg)
On 07/07/2015 15:04, Alex Bennée wrote: > fred.konrad@greensocs.com writes: > >> From: KONRAD Frederic <fred.konrad@greensocs.com> >> >> This removes exit_request global and adds a variable in CPUState for this. >> Only the flag for the first cpu is used for the moment as we are still with one >> TCG thread. >> >> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com> >> --- >> cpu-exec.c | 15 --------------- >> cpus.c | 17 ++++++++++++++--- >> 2 files changed, 14 insertions(+), 18 deletions(-) >> >> diff --git a/cpu-exec.c b/cpu-exec.c >> index 5d9b518..0644383 100644 >> --- a/cpu-exec.c >> +++ b/cpu-exec.c >> @@ -364,8 +364,6 @@ static void cpu_handle_debug_exception(CPUArchState *env) >> >> /* main execution loop */ >> >> -volatile sig_atomic_t exit_request; >> - >> int cpu_exec(CPUArchState *env) >> { >> CPUState *cpu = ENV_GET_CPU(env); >> @@ -394,20 +392,8 @@ int cpu_exec(CPUArchState *env) >> >> current_cpu = cpu; >> >> - /* As long as current_cpu is null, up to the assignment just above, >> - * requests by other threads to exit the execution loop are expected to >> - * be issued using the exit_request global. We must make sure that our >> - * evaluation of the global value is performed past the current_cpu >> - * value transition point, which requires a memory barrier as well as >> - * an instruction scheduling constraint on modern architectures. */ >> - smp_mb(); >> - >> rcu_read_lock(); >> >> - if (unlikely(exit_request)) { >> - cpu->exit_request = 1; >> - } >> - >> cc->cpu_exec_enter(cpu); >> >> /* Calculate difference between guest clock and host clock. >> @@ -496,7 +482,6 @@ int cpu_exec(CPUArchState *env) >> } >> } >> if (unlikely(cpu->exit_request)) { >> - cpu->exit_request = 0; >> cpu->exception_index = EXCP_INTERRUPT; >> cpu_loop_exit(cpu); >> } >> diff --git a/cpus.c b/cpus.c >> index 23c316c..2541c56 100644 >> --- a/cpus.c >> +++ b/cpus.c >> @@ -137,6 +137,8 @@ typedef struct TimersState { >> } TimersState; >> >> static TimersState timers_state; >> +/* CPU associated to this thread. */ >> +static __thread CPUState *tcg_thread_cpu; >> >> int64_t cpu_get_icount_raw(void) >> { >> @@ -661,12 +663,18 @@ static void cpu_handle_guest_debug(CPUState *cpu) >> cpu->stopped = true; >> } >> >> +/** >> + * cpu_signal >> + * Signal handler when using TCG. >> + */ >> static void cpu_signal(int sig) >> { >> if (current_cpu) { >> cpu_exit(current_cpu); >> } >> - exit_request = 1; >> + >> + /* FIXME: We might want to check if the cpu is running? */ >> + tcg_thread_cpu->exit_request = true; > I guess the potential problem is race conditions here? What happens if > the cpu is signalled by two different threads for two different reasons? Hmmm yes, I need to take a look at that and check all the reason why it should exit. But maybe it's OK, the first time the cpu get the signal it will set exit_request.. and the second times as well? >> } >> >> #ifdef CONFIG_LINUX >> @@ -1031,6 +1039,7 @@ static void *qemu_tcg_cpu_thread_fn(void *arg) >> { >> CPUState *cpu = arg; >> >> + tcg_thread_cpu = cpu; >> qemu_tcg_init_cpu_signals(); >> qemu_thread_get_self(cpu->thread); >> >> @@ -1393,7 +1402,8 @@ static void tcg_exec_all(void) >> if (next_cpu == NULL) { >> next_cpu = first_cpu; >> } >> - for (; next_cpu != NULL && !exit_request; next_cpu = CPU_NEXT(next_cpu)) { >> + for (; next_cpu != NULL && !first_cpu->exit_request; >> + next_cpu = CPU_NEXT(next_cpu)) { >> CPUState *cpu = next_cpu; >> CPUArchState *env = cpu->env_ptr; >> >> @@ -1410,7 +1420,8 @@ static void tcg_exec_all(void) >> break; >> } >> } >> - exit_request = 0; >> + >> + first_cpu->exit_request = 0; >> } >> >> void list_cpus(FILE *f, fprintf_function cpu_fprintf, const char *optarg)
diff --git a/cpu-exec.c b/cpu-exec.c index 5d9b518..0644383 100644 --- a/cpu-exec.c +++ b/cpu-exec.c @@ -364,8 +364,6 @@ static void cpu_handle_debug_exception(CPUArchState *env) /* main execution loop */ -volatile sig_atomic_t exit_request; - int cpu_exec(CPUArchState *env) { CPUState *cpu = ENV_GET_CPU(env); @@ -394,20 +392,8 @@ int cpu_exec(CPUArchState *env) current_cpu = cpu; - /* As long as current_cpu is null, up to the assignment just above, - * requests by other threads to exit the execution loop are expected to - * be issued using the exit_request global. We must make sure that our - * evaluation of the global value is performed past the current_cpu - * value transition point, which requires a memory barrier as well as - * an instruction scheduling constraint on modern architectures. */ - smp_mb(); - rcu_read_lock(); - if (unlikely(exit_request)) { - cpu->exit_request = 1; - } - cc->cpu_exec_enter(cpu); /* Calculate difference between guest clock and host clock. @@ -496,7 +482,6 @@ int cpu_exec(CPUArchState *env) } } if (unlikely(cpu->exit_request)) { - cpu->exit_request = 0; cpu->exception_index = EXCP_INTERRUPT; cpu_loop_exit(cpu); } diff --git a/cpus.c b/cpus.c index 23c316c..2541c56 100644 --- a/cpus.c +++ b/cpus.c @@ -137,6 +137,8 @@ typedef struct TimersState { } TimersState; static TimersState timers_state; +/* CPU associated to this thread. */ +static __thread CPUState *tcg_thread_cpu; int64_t cpu_get_icount_raw(void) { @@ -661,12 +663,18 @@ static void cpu_handle_guest_debug(CPUState *cpu) cpu->stopped = true; } +/** + * cpu_signal + * Signal handler when using TCG. + */ static void cpu_signal(int sig) { if (current_cpu) { cpu_exit(current_cpu); } - exit_request = 1; + + /* FIXME: We might want to check if the cpu is running? */ + tcg_thread_cpu->exit_request = true; } #ifdef CONFIG_LINUX @@ -1031,6 +1039,7 @@ static void *qemu_tcg_cpu_thread_fn(void *arg) { CPUState *cpu = arg; + tcg_thread_cpu = cpu; qemu_tcg_init_cpu_signals(); qemu_thread_get_self(cpu->thread); @@ -1393,7 +1402,8 @@ static void tcg_exec_all(void) if (next_cpu == NULL) { next_cpu = first_cpu; } - for (; next_cpu != NULL && !exit_request; next_cpu = CPU_NEXT(next_cpu)) { + for (; next_cpu != NULL && !first_cpu->exit_request; + next_cpu = CPU_NEXT(next_cpu)) { CPUState *cpu = next_cpu; CPUArchState *env = cpu->env_ptr; @@ -1410,7 +1420,8 @@ static void tcg_exec_all(void) break; } } - exit_request = 0; + + first_cpu->exit_request = 0; } void list_cpus(FILE *f, fprintf_function cpu_fprintf, const char *optarg)