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

[Paolo Bonzini]

On 05/08/20 21:18, Richard Henderson wrote:
> On 8/5/20 11:12 AM, Robert Foley wrote:
>> This change removes the implied BQL from the cpu_handle_interrupt,
>> and cpu_handle_exception paths. This BQL acquire is being pushed
>> down into the per arch implementation.
>>
>> Signed-off-by: Robert Foley <robert.foley@linaro.org>
>> ---
>>  accel/tcg/cpu-exec.c | 19 +++++++++++--------
>>  1 file changed, 11 insertions(+), 8 deletions(-)
>>
>> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
>> index 80d0e649b2..8e2bfd97a1 100644
>> --- a/accel/tcg/cpu-exec.c
>> +++ b/accel/tcg/cpu-exec.c
>> @@ -517,9 +517,7 @@ static inline bool cpu_handle_exception(CPUState *cpu, 
>> int *ret)
>>  #else
>>          if (replay_exception()) {
>>              CPUClass *cc = CPU_GET_CLASS(cpu);
>> -            qemu_mutex_lock_iothread();
>>              cc->do_interrupt(cpu);
>> -            qemu_mutex_unlock_iothread();
>>              cpu->exception_index = -1;
>>  
> 
> This patch is not bisectable.  The removal of the lock here needs to happen at
> the end, or something.

Indeed the series should be structured like this:

1) rename all *_do_interrupt functions to *_do_interrupt_locked

2) add back *_do_interrupt that takes the BQL and calls
*_do_interrupt_locked, point ->do_interrupt to it, remove the BQL from
cpu-exec.c

3) modify the cpu_mutex and BQL critical sections around
->cpu_exec_interrupt, so that the BQL critical section covers just the
call to ->cpu_exec_interrupt.  Document which fields are now covered by
cpu_mutex.

4/5) same as 1/2 for ->cpu_exec_interrupt

Patches 1/2 would be pretty large, but they're trivial to review just by
grepping for "->do_interrupt\s*=", and likewise for 4/5.

Thanks,

Paolo