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
