Re: [PATCH v1 04/21] target/avr: add BQL to do_interrupt and cpu_exec_interrupt

[Robert Foley]

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