Re: [PATCH v5 10/14] cpus: add handle_interrupt to the CpusAccel interface

[Roman Bolshakov]

On Fri, Aug 14, 2020 at 02:01:01PM -0700, Richard Henderson wrote:
> On 8/12/20 11:32 AM, Claudio Fontana wrote:
> > +static void generic_handle_interrupt(CPUState *cpu, int mask)
> > +{
> > +    cpu->interrupt_request |= mask;
> > +
> > +    if (!qemu_cpu_is_self(cpu)) {
> > +        qemu_cpu_kick(cpu);
> > +    }
> > +}
> > +
> > +void cpu_interrupt(CPUState *cpu, int mask)
> > +{
> > +    if (cpus_accel && cpus_accel->handle_interrupt) {
> > +        cpus_accel->handle_interrupt(cpu, mask);
> > +    } else {
> > +        generic_handle_interrupt(cpu, mask);
> > +    }
> > +}
> 
> First, by this point you have converted all of the accelerators, so I would
> expect cpus_accel to always be non-null.  I would expect a patch immediately
> preceding this one to place an assert to that effect somewhere in the startup
> code, and to remove all of the checks.
> 
> Second, I would prefer that all methods be non-null, so that you don't need to
> check that either.  This patch would add generic_handle_interrupt (perhaps
> named cpus_accel_default_handle_interrupt declared in sysemu/cpus.h?) to the
> CpusAccel structure of all except TCG.
> 
> Similarly for all other methods that are checking non-null-ness of the method
> pointer.  Perhaps assert non-null for each method in cpus_register_accel().
> 
> 

I concur with that. It's similar to my comment to the previous revision
of the series.

Regards,
Roman