On 8/14/20 11:01 PM, 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.
Ok.
>
> Second, I would prefer that all methods be non-null, so that you don't need to
> check that either.
This contrasts a bit with the previous comments by Paolo to try to express only
the "interesting" bits in the CpusAccel struct in each module.
> 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().
>
>
> r~
>
That was my initial approach in very old versions of the series (all fields as
explicit and mandatory),
I changed direction there due to Paolo's comments. For me both solutions are
fine, each has its merits and downsides.
Thanks,
Claudio
