Re: [PATCH v2 4/7] target: Push BQL on ->do_interrupt down into per-arch implementation

[Richard Henderson]

On 8/19/20 11:28 AM, Robert Foley wrote:
> avr is another exception.  avr, arm and cris all had a similar
> case where their *_cpu_exec_interrupt was calling to
> the CPUClass ->do_interrupt.  This causes an issue when we push
> the lock down since ->do_interrupt will try to acquire the BQL, but
> the calling context already has it.

Alpha is in this lest as well, correct?

> diff --git a/target/alpha/cpu.h b/target/alpha/cpu.h
> index 4c6753df34..be29bdd530 100644
> --- a/target/alpha/cpu.h
> +++ b/target/alpha/cpu.h
> @@ -276,7 +276,7 @@ struct AlphaCPU {
>  extern const VMStateDescription vmstate_alpha_cpu;
>  #endif
>  
> -void alpha_cpu_do_interrupt_locked(CPUState *cpu);
> +void alpha_cpu_do_interrupt(CPUState *cpu);
>  bool alpha_cpu_exec_interrupt(CPUState *cpu, int int_req);
>  void alpha_cpu_dump_state(CPUState *cs, FILE *f, int flags);
>  hwaddr alpha_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
> diff --git a/target/alpha/helper.c b/target/alpha/helper.c
> index ff9a2a7765..e497dd269e 100644
> --- a/target/alpha/helper.c
> +++ b/target/alpha/helper.c
> @@ -295,7 +295,7 @@ bool alpha_cpu_tlb_fill(CPUState *cs, vaddr addr, int 
> size,
>  }
>  #endif /* USER_ONLY */
>  
> -void alpha_cpu_do_interrupt_locked(CPUState *cs)
> +static void alpha_cpu_do_interrupt_locked(CPUState *cs)
>  {
>      AlphaCPU *cpu = ALPHA_CPU(cs);
>      CPUAlphaState *env = &cpu->env;
> @@ -407,6 +407,13 @@ void alpha_cpu_do_interrupt_locked(CPUState *cs)
>  #endif /* !USER_ONLY */
>  }
>  
> +void alpha_cpu_do_interrupt(CPUState *cs)
> +{
> +    qemu_mutex_lock_iothread();
> +    alpha_cpu_do_interrupt_locked(cs);
> +    qemu_mutex_unlock_iothread();
> +}
> +
>  bool alpha_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>  {
>      AlphaCPU *cpu = ALPHA_CPU(cs);

This rename should have been done in patch 1, as with all others.
Moreover, this leaves a bug in alpha_cpu_exec_interrupt in that it should be
calling alpha_cpu_do_interrupt_locked.

That seems to be the only instance of this mistake.


r~