Re: [PATCH v1 01/21] accel/tcg: Change interrupt/exception handling to remove implied BQL

[Paolo Bonzini]

On 08/08/20 00:18, Robert Foley wrote:
> 2) Another perhaps cleaner option is to add a new cpu class function
> ->do_interrupt_locked.
>    This lets callers like *_cpu_exec_interrupt call to ->do_interrupt_locked
>    with lock held and solves the issue without resorting to conditional 
> locking.
> 
>    Another benefit we could gain from this approach is to simplify our 
> solution
>    overall by adding a common do_interrupt function.
> 
>    void cpu_common_do_interrupt(CPUState *cs)
>    {
>         CPUClass *cc = CPU_GET_CLASS(cpu);
>         qemu_mutex_lock_iothread();
>         cc->do_interrupt_locked(cpu);
>         qemu_mutex_unlock_iothread();
>     }
>    cc->do_interrupt would be set to cpu_common_do_interrupt by default
> in cpu_class_init.
>    In other words, the base cpu class would handle holding the BQL for us,
>    and we would not need to implement a new *_do_interrupt function
> for each arch.
> 
> We are thinking that 2) would be a good option.

Yes, it is.  The only slight complication is that you'd have both
->do_interrupt and ->do_interrupt_locked so you probably should add some
consistency check, for example

    /*
     * cc->do_interrupt_locked should only be needed if
     * the class uses cpu_common_do_interrupt.
     */
    assert(cc->do_interrupt == cpu_common_do_interrupt ||
           !cc->do_interrupt_locked);

Therefore, a variant is to add ->do_interrupt_locked to ARMCPUClass and
CRISCPUClass (target/avr/helper.c can just call
avr_cpu_do_interrupt_locked, because that's the only value that
cc->do_interrupt can have).  Then ARM and CRIS can have a do_interrupt
like you wrote above:

void arm_do_interrupt(CPUState *cs)
{
    ARMCPUClass *acc = ARM_CPU_GET_CLASS(cs);
    qemu_mutex_lock_iothread();
    acc->do_interrupt_locked(cpu);
    qemu_mutex_unlock_iothread();
}

with a small duplication between ARM and CRIS but on the other hand a
simpler definition of the common CPUClass.

Paolo