[Qemu-devel] [PATCH 3/5] cpu-exec: Move interrupt handling out of cpu_exec()

[Sergey Fedorov]

From: Sergey Fedorov 

Simplify cpu_exec() by extracting interrupt handling code outside of
cpu_exec() into a new static inline function cpu_handle_interrupt().

Signed-off-by: Sergey Fedorov 
Signed-off-by: Sergey Fedorov 
Reviewed-by: Richard Henderson  
---
 cpu-exec.c | 132 -
 1 file changed, 70 insertions(+), 62 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index f7e175e9ae1c..a053b5e73af1 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -438,6 +438,74 @@ static inline bool cpu_handle_exception(CPUState *cpu, int 
*ret)
 return false;
 }
 
+static inline void cpu_handle_interrupt(CPUState *cpu,
+TranslationBlock **last_tb)
+{
+CPUClass *cc = CPU_GET_CLASS(cpu);
+int interrupt_request = cpu->interrupt_request;
+
+if (unlikely(interrupt_request)) {
+if (unlikely(cpu->singlestep_enabled & SSTEP_NOIRQ)) {
+/* Mask out external interrupts for this step. */
+interrupt_request &= ~CPU_INTERRUPT_SSTEP_MASK;
+}
+if (interrupt_request & CPU_INTERRUPT_DEBUG) {
+cpu->interrupt_request &= ~CPU_INTERRUPT_DEBUG;
+cpu->exception_index = EXCP_DEBUG;
+cpu_loop_exit(cpu);
+}
+if (replay_mode == REPLAY_MODE_PLAY && !replay_has_interrupt()) {
+/* Do nothing */
+} else if (interrupt_request & CPU_INTERRUPT_HALT) {
+replay_interrupt();
+cpu->interrupt_request &= ~CPU_INTERRUPT_HALT;
+cpu->halted = 1;
+cpu->exception_index = EXCP_HLT;
+cpu_loop_exit(cpu);
+}
+#if defined(TARGET_I386)
+else if (interrupt_request & CPU_INTERRUPT_INIT) {
+X86CPU *x86_cpu = X86_CPU(cpu);
+CPUArchState *env = _cpu->env;
+replay_interrupt();
+cpu_svm_check_intercept_param(env, SVM_EXIT_INIT, 0);
+do_cpu_init(x86_cpu);
+cpu->exception_index = EXCP_HALTED;
+cpu_loop_exit(cpu);
+}
+#else
+else if (interrupt_request & CPU_INTERRUPT_RESET) {
+replay_interrupt();
+cpu_reset(cpu);
+cpu_loop_exit(cpu);
+}
+#endif
+/* The target hook has 3 exit conditions:
+   False when the interrupt isn't processed,
+   True when it is, and we should restart on a new TB,
+   and via longjmp via cpu_loop_exit.  */
+else {
+replay_interrupt();
+if (cc->cpu_exec_interrupt(cpu, interrupt_request)) {
+*last_tb = NULL;
+}
+}
+/* Don't use the cached interrupt_request value,
+   do_interrupt may have updated the EXITTB flag. */
+if (cpu->interrupt_request & CPU_INTERRUPT_EXITTB) {
+cpu->interrupt_request &= ~CPU_INTERRUPT_EXITTB;
+/* ensure that no TB jump will be modified as
+   the program flow was changed */
+*last_tb = NULL;
+}
+}
+if (unlikely(cpu->exit_request || replay_has_interrupt())) {
+cpu->exit_request = 0;
+cpu->exception_index = EXCP_INTERRUPT;
+cpu_loop_exit(cpu);
+}
+}
+
 /* main execution loop */
 
 int cpu_exec(CPUState *cpu)
@@ -447,7 +515,7 @@ int cpu_exec(CPUState *cpu)
 X86CPU *x86_cpu = X86_CPU(cpu);
 CPUArchState *env = _cpu->env;
 #endif
-int ret, interrupt_request;
+int ret;
 TranslationBlock *tb, *last_tb;
 int tb_exit = 0;
 SyncClocks sc;
@@ -486,67 +554,7 @@ int cpu_exec(CPUState *cpu)
 last_tb = NULL; /* forget the last executed TB after exception */
 cpu->tb_flushed = false; /* reset before first TB lookup */
 for(;;) {
-interrupt_request = cpu->interrupt_request;
-if (unlikely(interrupt_request)) {
-if (unlikely(cpu->singlestep_enabled & SSTEP_NOIRQ)) {
-/* Mask out external interrupts for this step. */
-interrupt_request &= ~CPU_INTERRUPT_SSTEP_MASK;
-}
-if (interrupt_request & CPU_INTERRUPT_DEBUG) {
-cpu->interrupt_request &= ~CPU_INTERRUPT_DEBUG;
-cpu->exception_index = EXCP_DEBUG;
-cpu_loop_exit(cpu);
-}
-if (replay_mode == REPLAY_MODE_PLAY
-&& !replay_has_interrupt()) {
-/* Do nothing */
-} else if (interrupt_request & CPU_INTERRUPT_HALT) {
-replay_interrupt();
-cpu->interrupt_request &= ~CPU_INTERRUPT_HALT;
-cpu->halted = 1;
-cpu->exception_index = EXCP_HLT;
-cpu_loop_exit(cpu);
-

Re: [Qemu-devel] [PATCH 3/5] cpu-exec: Move interrupt handling out of cpu_exec()

[Sergey Fedorov]

On 10/05/16 19:34, Richard Henderson wrote:
> On 05/10/2016 05:46 AM, Sergey Fedorov wrote:
>> From: Sergey Fedorov 
>>
>> Simplify cpu_exec() by extracting interrupt handling code outside of
>> cpu_exec() into a new static inline function cpu_handle_interrupt().
>>
>> Signed-off-by: Sergey Fedorov 
>> Signed-off-by: Sergey Fedorov 
>> ---
>>  cpu-exec.c | 132
>> -
>>  1 file changed, 70 insertions(+), 62 deletions(-)
>
> Reviewed-by: Richard Henderson  
>
>
>> +if (replay_mode == REPLAY_MODE_PLAY &&
>> !replay_has_interrupt()) {
>> +/* Do nothing */
>> +} else if (interrupt_request & CPU_INTERRUPT_HALT) {
>> +}
>> +else if (interrupt_request & CPU_INTERRUPT_RESET) {
>> +}
>> +else {
>> +replay_interrupt();
>> +if (cc->cpu_exec_interrupt(cpu, interrupt_request)) {
>> +*last_tb = NULL;
>> +}
>> +}
>> +/* Don't use the cached interrupt_request value,
>> +   do_interrupt may have updated the EXITTB flag. */
>> +if (cpu->interrupt_request & CPU_INTERRUPT_EXITTB) {
>
> Note for future cleanup: IMO this comment is cleaner if it's actually
> put where it's meaningful (and updated to reflect that do_interrupt no
> longer exists).  E.g.
>
> else {
> if (cc->cpu_exec_interrupt(cpu, interrupt_request)) {
> *last_tb = NULL;
> }
> /* Reload the interrupt_request value as it may have
>been updated by the target hook.  */
> interrupt_request = cpu->interrupt_request;
> }
> if (interupt_request & CPU_INTERRUPT_EXITTB) {
> ...
>
> But such a change of course belongs in a separate patch.

Cool, thanks for the suggestion. I've had feeling this could be
expressed in a better way, like you suggest :)

Kind regards,
Sergey

Re: [Qemu-devel] [PATCH 3/5] cpu-exec: Move interrupt handling out of cpu_exec()

[Richard Henderson]

On 05/10/2016 05:46 AM, Sergey Fedorov wrote:

From: Sergey Fedorov 

Simplify cpu_exec() by extracting interrupt handling code outside of
cpu_exec() into a new static inline function cpu_handle_interrupt().

Signed-off-by: Sergey Fedorov 
Signed-off-by: Sergey Fedorov 
---
 cpu-exec.c | 132 -
 1 file changed, 70 insertions(+), 62 deletions(-)


Reviewed-by: Richard Henderson  



+if (replay_mode == REPLAY_MODE_PLAY && !replay_has_interrupt()) {
+/* Do nothing */
+} else if (interrupt_request & CPU_INTERRUPT_HALT) {
+}
+else if (interrupt_request & CPU_INTERRUPT_RESET) {
+}
+else {
+replay_interrupt();
+if (cc->cpu_exec_interrupt(cpu, interrupt_request)) {
+*last_tb = NULL;
+}
+}
+/* Don't use the cached interrupt_request value,
+   do_interrupt may have updated the EXITTB flag. */
+if (cpu->interrupt_request & CPU_INTERRUPT_EXITTB) {


Note for future cleanup: IMO this comment is cleaner if it's actually put where 
it's meaningful (and updated to reflect that do_interrupt no longer exists).  E.g.


else {
if (cc->cpu_exec_interrupt(cpu, interrupt_request)) {
*last_tb = NULL;
}
/* Reload the interrupt_request value as it may have
   been updated by the target hook.  */
interrupt_request = cpu->interrupt_request;
}
if (interupt_request & CPU_INTERRUPT_EXITTB) {
...

But such a change of course belongs in a separate patch.


r~

[Qemu-devel] [PATCH 3/5] cpu-exec: Move interrupt handling out of cpu_exec()

[Sergey Fedorov]

From: Sergey Fedorov 

Simplify cpu_exec() by extracting interrupt handling code outside of
cpu_exec() into a new static inline function cpu_handle_interrupt().

Signed-off-by: Sergey Fedorov 
Signed-off-by: Sergey Fedorov 
---
 cpu-exec.c | 132 -
 1 file changed, 70 insertions(+), 62 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index ad95ace38dd9..0760d5dc274d 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -438,6 +438,74 @@ static inline bool cpu_handle_exception(CPUState *cpu, int 
*ret)
 return false;
 }
 
+static inline void cpu_handle_interrupt(CPUState *cpu,
+TranslationBlock **last_tb)
+{
+CPUClass *cc = CPU_GET_CLASS(cpu);
+int interrupt_request = cpu->interrupt_request;
+
+if (unlikely(interrupt_request)) {
+if (unlikely(cpu->singlestep_enabled & SSTEP_NOIRQ)) {
+/* Mask out external interrupts for this step. */
+interrupt_request &= ~CPU_INTERRUPT_SSTEP_MASK;
+}
+if (interrupt_request & CPU_INTERRUPT_DEBUG) {
+cpu->interrupt_request &= ~CPU_INTERRUPT_DEBUG;
+cpu->exception_index = EXCP_DEBUG;
+cpu_loop_exit(cpu);
+}
+if (replay_mode == REPLAY_MODE_PLAY && !replay_has_interrupt()) {
+/* Do nothing */
+} else if (interrupt_request & CPU_INTERRUPT_HALT) {
+replay_interrupt();
+cpu->interrupt_request &= ~CPU_INTERRUPT_HALT;
+cpu->halted = 1;
+cpu->exception_index = EXCP_HLT;
+cpu_loop_exit(cpu);
+}
+#if defined(TARGET_I386)
+else if (interrupt_request & CPU_INTERRUPT_INIT) {
+X86CPU *x86_cpu = X86_CPU(cpu);
+CPUArchState *env = _cpu->env;
+replay_interrupt();
+cpu_svm_check_intercept_param(env, SVM_EXIT_INIT, 0);
+do_cpu_init(x86_cpu);
+cpu->exception_index = EXCP_HALTED;
+cpu_loop_exit(cpu);
+}
+#else
+else if (interrupt_request & CPU_INTERRUPT_RESET) {
+replay_interrupt();
+cpu_reset(cpu);
+cpu_loop_exit(cpu);
+}
+#endif
+/* The target hook has 3 exit conditions:
+   False when the interrupt isn't processed,
+   True when it is, and we should restart on a new TB,
+   and via longjmp via cpu_loop_exit.  */
+else {
+replay_interrupt();
+if (cc->cpu_exec_interrupt(cpu, interrupt_request)) {
+*last_tb = NULL;
+}
+}
+/* Don't use the cached interrupt_request value,
+   do_interrupt may have updated the EXITTB flag. */
+if (cpu->interrupt_request & CPU_INTERRUPT_EXITTB) {
+cpu->interrupt_request &= ~CPU_INTERRUPT_EXITTB;
+/* ensure that no TB jump will be modified as
+   the program flow was changed */
+*last_tb = NULL;
+}
+}
+if (unlikely(cpu->exit_request || replay_has_interrupt())) {
+cpu->exit_request = 0;
+cpu->exception_index = EXCP_INTERRUPT;
+cpu_loop_exit(cpu);
+}
+}
+
 /* main execution loop */
 
 int cpu_exec(CPUState *cpu)
@@ -447,7 +515,7 @@ int cpu_exec(CPUState *cpu)
 X86CPU *x86_cpu = X86_CPU(cpu);
 CPUArchState *env = _cpu->env;
 #endif
-int ret, interrupt_request;
+int ret;
 TranslationBlock *tb, *last_tb;
 int tb_exit = 0;
 SyncClocks sc;
@@ -486,67 +554,7 @@ int cpu_exec(CPUState *cpu)
 last_tb = NULL; /* forget the last executed TB after exception */
 cpu->tb_flushed = false; /* reset before first TB lookup */
 for(;;) {
-interrupt_request = cpu->interrupt_request;
-if (unlikely(interrupt_request)) {
-if (unlikely(cpu->singlestep_enabled & SSTEP_NOIRQ)) {
-/* Mask out external interrupts for this step. */
-interrupt_request &= ~CPU_INTERRUPT_SSTEP_MASK;
-}
-if (interrupt_request & CPU_INTERRUPT_DEBUG) {
-cpu->interrupt_request &= ~CPU_INTERRUPT_DEBUG;
-cpu->exception_index = EXCP_DEBUG;
-cpu_loop_exit(cpu);
-}
-if (replay_mode == REPLAY_MODE_PLAY
-&& !replay_has_interrupt()) {
-/* Do nothing */
-} else if (interrupt_request & CPU_INTERRUPT_HALT) {
-replay_interrupt();
-cpu->interrupt_request &= ~CPU_INTERRUPT_HALT;
-cpu->halted = 1;
-cpu->exception_index = EXCP_HLT;
-cpu_loop_exit(cpu);
-}
-#if defined(TARGET_I386)
-