| Message ID | 20200805181303.7822-5-robert.foley@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | accel/tcg: remove implied BQL from cpu_handle_interrupt/exception path | expand |
Hi Robert. I am sorry but how can I apply it? following this what I get error: patch failed: accel/tcg/cpu-exec.c:558 error: accel/tcg/cpu-exec.c: patch does not apply error: patch failed: target/arm/helper.c:9808 error: target/arm/helper.c: patch does not apply error: patch failed: target/ppc/excp_helper.c:1056 error: target/ppc/excp_helper.c: patch does not apply error: patch failed: target/sh4/helper.c:62 error: target/sh4/helper.c: patch does not apply error: patch failed: target/unicore32/softmmu.c:118 error: target/unicore32/softmmu.c: patch does not apply On Wed, Aug 5, 2020 at 9:17 PM Robert Foley <robert.foley@linaro.org> wrote: > This is part of a series of changes to remove the implied BQL > from the common code of cpu_handle_interrupt and > cpu_handle_exception. As part of removing the implied BQL > from the common code, we are pushing the BQL holding > down into the per-arch implementation functions of > do_interrupt and cpu_exec_interrupt. > > The purpose of this set of changes is to set the groundwork > so that an arch could move towards removing > the BQL from the cpu_handle_interrupt/exception paths. > > This approach was suggested by Paolo Bonzini. > For reference, here are two key posts in the discussion, explaining > the reasoning/benefits of this approach. > https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html > https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html > > Signed-off-by: Robert Foley <robert.foley@linaro.org> > --- > target/avr/helper.c | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/target/avr/helper.c b/target/avr/helper.c > index d96d14372b..f0d625c195 100644 > --- a/target/avr/helper.c > +++ b/target/avr/helper.c > @@ -30,6 +30,7 @@ bool avr_cpu_exec_interrupt(CPUState *cs, int > interrupt_request) > CPUClass *cc = CPU_GET_CLASS(cs); > AVRCPU *cpu = AVR_CPU(cs); > CPUAVRState *env = &cpu->env; > + qemu_mutex_lock_iothread(); > > if (interrupt_request & CPU_INTERRUPT_RESET) { > if (cpu_interrupts_enabled(env)) { > @@ -53,6 +54,7 @@ bool avr_cpu_exec_interrupt(CPUState *cs, int > interrupt_request) > ret = true; > } > } > + qemu_mutex_unlock_iothread(); > return ret; > } > > @@ -61,10 +63,15 @@ void avr_cpu_do_interrupt(CPUState *cs) > AVRCPU *cpu = AVR_CPU(cs); > CPUAVRState *env = &cpu->env; > > - uint32_t ret = env->pc_w; > + uint32_t ret; > int vector = 0; > int size = avr_feature(env, AVR_FEATURE_JMP_CALL) ? 2 : 1; > int base = 0; > + bool bql = !qemu_mutex_iothread_locked(); > + if (bql) { > + qemu_mutex_lock_iothread(); > + } > + ret = env->pc_w; > > if (cs->exception_index == EXCP_RESET) { > vector = 0; > @@ -87,6 +94,9 @@ void avr_cpu_do_interrupt(CPUState *cs) > env->sregI = 0; /* clear Global Interrupt Flag */ > > cs->exception_index = -1; > + if (bql) { > + qemu_mutex_unlock_iothread(); > + } > } > > int avr_cpu_memory_rw_debug(CPUState *cs, vaddr addr, uint8_t *buf, > -- > 2.17.1 > > -- Best Regards, Michael Rolnik <div dir="ltr">Hi Robert.<div><br></div><div>I am sorry but how can I apply it? following this what I get</div><div><br></div><div><font face="monospace">error: patch failed: accel/tcg/cpu-exec.c:558<br>error: accel/tcg/cpu-exec.c: patch does not apply<br>error: patch failed: target/arm/helper.c:9808<br>error: target/arm/helper.c: patch does not apply<br>error: patch failed: target/ppc/excp_helper.c:1056<br>error: target/ppc/excp_helper.c: patch does not apply<br>error: patch failed: target/sh4/helper.c:62<br>error: target/sh4/helper.c: patch does not apply<br>error: patch failed: target/unicore32/softmmu.c:118<br>error: target/unicore32/softmmu.c: patch does not apply</font><br></div><div><br></div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Aug 5, 2020 at 9:17 PM Robert Foley <<a href="mailto:robert.foley@linaro.org">robert.foley@linaro.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">This is part of a series of changes to remove the implied BQL<br> from the common code of cpu_handle_interrupt and<br> cpu_handle_exception. As part of removing the implied BQL<br> from the common code, we are pushing the BQL holding<br> down into the per-arch implementation functions of<br> do_interrupt and cpu_exec_interrupt.<br> <br> The purpose of this set of changes is to set the groundwork<br> so that an arch could move towards removing<br> the BQL from the cpu_handle_interrupt/exception paths.<br> <br> This approach was suggested by Paolo Bonzini.<br> For reference, here are two key posts in the discussion, explaining<br> the reasoning/benefits of this approach.<br> <a href="https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html" rel="noreferrer" target="_blank">https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html</a><br> <a href="https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html" rel="noreferrer" target="_blank">https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html</a><br> <br> Signed-off-by: Robert Foley <<a href="mailto:robert.foley@linaro.org" target="_blank">robert.foley@linaro.org</a>><br> ---<br> target/avr/helper.c | 12 +++++++++++-<br> 1 file changed, 11 insertions(+), 1 deletion(-)<br> <br> diff --git a/target/avr/helper.c b/target/avr/helper.c<br> index d96d14372b..f0d625c195 100644<br> --- a/target/avr/helper.c<br> +++ b/target/avr/helper.c<br> @@ -30,6 +30,7 @@ bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request)<br> CPUClass *cc = CPU_GET_CLASS(cs);<br> AVRCPU *cpu = AVR_CPU(cs);<br> CPUAVRState *env = &cpu->env;<br> + qemu_mutex_lock_iothread();<br> <br> if (interrupt_request & CPU_INTERRUPT_RESET) {<br> if (cpu_interrupts_enabled(env)) {<br> @@ -53,6 +54,7 @@ bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request)<br> ret = true;<br> }<br> }<br> + qemu_mutex_unlock_iothread();<br> return ret;<br> }<br> <br> @@ -61,10 +63,15 @@ void avr_cpu_do_interrupt(CPUState *cs)<br> AVRCPU *cpu = AVR_CPU(cs);<br> CPUAVRState *env = &cpu->env;<br> <br> - uint32_t ret = env->pc_w;<br> + uint32_t ret;<br> int vector = 0;<br> int size = avr_feature(env, AVR_FEATURE_JMP_CALL) ? 2 : 1;<br> int base = 0;<br> + bool bql = !qemu_mutex_iothread_locked();<br> + if (bql) {<br> + qemu_mutex_lock_iothread();<br> + }<br> + ret = env->pc_w;<br> <br> if (cs->exception_index == EXCP_RESET) {<br> vector = 0;<br> @@ -87,6 +94,9 @@ void avr_cpu_do_interrupt(CPUState *cs)<br> env->sregI = 0; /* clear Global Interrupt Flag */<br> <br> cs->exception_index = -1;<br> + if (bql) {<br> + qemu_mutex_unlock_iothread();<br> + }<br> }<br> <br> int avr_cpu_memory_rw_debug(CPUState *cs, vaddr addr, uint8_t *buf,<br> -- <br> 2.17.1<br> <br> </blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature">Best Regards,<br>Michael Rolnik</div>
On Thu, 6 Aug 2020 at 14:37, Michael Rolnik <mrolnik@gmail.com> wrote: > > Hi Robert. > > I am sorry but how can I apply it? following this what I get > > error: patch failed: accel/tcg/cpu-exec.c:558 > error: accel/tcg/cpu-exec.c: patch does not apply > error: patch failed: target/arm/helper.c:9808 > error: target/arm/helper.c: patch does not apply > error: patch failed: target/ppc/excp_helper.c:1056 > error: target/ppc/excp_helper.c: patch does not apply > error: patch failed: target/sh4/helper.c:62 > error: target/sh4/helper.c: patch does not apply > error: patch failed: target/unicore32/softmmu.c:118 > error: target/unicore32/softmmu.c: patch does not apply > Hi Michael, This patch is based on the per-cpu locks patch series: https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg05314.html Our current WIP tree for this interrupts patch is here: https://github.com/rf972/qemu/commits/int_core_v1.4 Also, just so you know, based on the initial feedback we are going to substantially change this series. Another version will be sent out in a few days. Thanks & Regards, -Rob > > > On Wed, Aug 5, 2020 at 9:17 PM Robert Foley <robert.foley@linaro.org> wrote: >> >> This is part of a series of changes to remove the implied BQL >> from the common code of cpu_handle_interrupt and >> cpu_handle_exception. As part of removing the implied BQL >> from the common code, we are pushing the BQL holding >> down into the per-arch implementation functions of >> do_interrupt and cpu_exec_interrupt. >> >> The purpose of this set of changes is to set the groundwork >> so that an arch could move towards removing >> the BQL from the cpu_handle_interrupt/exception paths. >> >> This approach was suggested by Paolo Bonzini. >> For reference, here are two key posts in the discussion, explaining >> the reasoning/benefits of this approach. >> https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html >> https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html >> >> Signed-off-by: Robert Foley <robert.foley@linaro.org> >> --- >> target/avr/helper.c | 12 +++++++++++- >> 1 file changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/target/avr/helper.c b/target/avr/helper.c >> index d96d14372b..f0d625c195 100644 >> --- a/target/avr/helper.c >> +++ b/target/avr/helper.c >> @@ -30,6 +30,7 @@ bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request) >> CPUClass *cc = CPU_GET_CLASS(cs); >> AVRCPU *cpu = AVR_CPU(cs); >> CPUAVRState *env = &cpu->env; >> + qemu_mutex_lock_iothread(); >> >> if (interrupt_request & CPU_INTERRUPT_RESET) { >> if (cpu_interrupts_enabled(env)) { >> @@ -53,6 +54,7 @@ bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request) >> ret = true; >> } >> } >> + qemu_mutex_unlock_iothread(); >> return ret; >> } >> >> @@ -61,10 +63,15 @@ void avr_cpu_do_interrupt(CPUState *cs) >> AVRCPU *cpu = AVR_CPU(cs); >> CPUAVRState *env = &cpu->env; >> >> - uint32_t ret = env->pc_w; >> + uint32_t ret; >> int vector = 0; >> int size = avr_feature(env, AVR_FEATURE_JMP_CALL) ? 2 : 1; >> int base = 0; >> + bool bql = !qemu_mutex_iothread_locked(); >> + if (bql) { >> + qemu_mutex_lock_iothread(); >> + } >> + ret = env->pc_w; >> >> if (cs->exception_index == EXCP_RESET) { >> vector = 0; >> @@ -87,6 +94,9 @@ void avr_cpu_do_interrupt(CPUState *cs) >> env->sregI = 0; /* clear Global Interrupt Flag */ >> >> cs->exception_index = -1; >> + if (bql) { >> + qemu_mutex_unlock_iothread(); >> + } >> } >> >> int avr_cpu_memory_rw_debug(CPUState *cs, vaddr addr, uint8_t *buf, >> -- >> 2.17.1 >> > > > -- > Best Regards, > Michael Rolnik
diff --git a/target/avr/helper.c b/target/avr/helper.c index d96d14372b..f0d625c195 100644 --- a/target/avr/helper.c +++ b/target/avr/helper.c @@ -30,6 +30,7 @@ bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request) CPUClass *cc = CPU_GET_CLASS(cs); AVRCPU *cpu = AVR_CPU(cs); CPUAVRState *env = &cpu->env; + qemu_mutex_lock_iothread(); if (interrupt_request & CPU_INTERRUPT_RESET) { if (cpu_interrupts_enabled(env)) { @@ -53,6 +54,7 @@ bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request) ret = true; } } + qemu_mutex_unlock_iothread(); return ret; } @@ -61,10 +63,15 @@ void avr_cpu_do_interrupt(CPUState *cs) AVRCPU *cpu = AVR_CPU(cs); CPUAVRState *env = &cpu->env; - uint32_t ret = env->pc_w; + uint32_t ret; int vector = 0; int size = avr_feature(env, AVR_FEATURE_JMP_CALL) ? 2 : 1; int base = 0; + bool bql = !qemu_mutex_iothread_locked(); + if (bql) { + qemu_mutex_lock_iothread(); + } + ret = env->pc_w; if (cs->exception_index == EXCP_RESET) { vector = 0; @@ -87,6 +94,9 @@ void avr_cpu_do_interrupt(CPUState *cs) env->sregI = 0; /* clear Global Interrupt Flag */ cs->exception_index = -1; + if (bql) { + qemu_mutex_unlock_iothread(); + } } int avr_cpu_memory_rw_debug(CPUState *cs, vaddr addr, uint8_t *buf,
This is part of a series of changes to remove the implied BQL from the common code of cpu_handle_interrupt and cpu_handle_exception. As part of removing the implied BQL from the common code, we are pushing the BQL holding down into the per-arch implementation functions of do_interrupt and cpu_exec_interrupt. The purpose of this set of changes is to set the groundwork so that an arch could move towards removing the BQL from the cpu_handle_interrupt/exception paths. This approach was suggested by Paolo Bonzini. For reference, here are two key posts in the discussion, explaining the reasoning/benefits of this approach. https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08731.html https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00044.html Signed-off-by: Robert Foley <robert.foley@linaro.org> --- target/avr/helper.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) -- 2.17.1