On 23/05/2017 19:03, Bjorn Helgaas wrote:
> On Wed, May 17, 2017 at 04:56:08PM +0200, Marc Gonzalez wrote:
>> On 20/04/2017 16:28, Marc Gonzalez wrote:
>>
>>> +static int tango_set_affinity(struct irq_data *data,
>>> + const struct cpumask *mask, bool force)
>>> +{
>>> + return -EINVAL;
>>> +}
>>> +
>>> +static struct irq_chip tango_chip = {
>>> + .irq_ack = tango_ack,
>>> + .irq_mask = tango_mask,
>>> + .irq_unmask = tango_unmask,
>>> + .irq_set_affinity = tango_set_affinity,
>>> + .irq_compose_msi_msg = tango_compose_msi_msg,
>>> +};
>>
>> Hmmm... I'm wondering why .irq_set_affinity is required.
>>
>> static int setup_affinity(struct irq_desc *desc, struct cpumask *mask)
>> first calls __irq_can_set_affinity() to check whether
>> desc->irq_data.chip->irq_set_affinity) exists.
>>
>> then calls irq_do_set_affinity(&desc->irq_data, mask, false);
>> which calls chip->irq_set_affinity(data, mask, force);
>> = msi_domain_set_affinity()
>> which calls parent->chip->irq_set_affinity() unconditionally.
>>
>> Would it make sense to test that the callback is implemented
>> before calling it?
>>
>>
>> [ 0.723895] Unable to handle kernel NULL pointer dereference at virtual address 00000000
>
> I'm not sure what you're asking.
>
> Is this a bug report for the v4 tango driver?
No.
> Or are you asking whether msi_domain_set_affinity() should be changed
> to check whether parent->chip->irq_set_affinity is implemented?
Yes. The way things are implemented now, drivers are forced
to define an irq_set_affinity callback, even if it just returns
an error, otherwise, the kernel crashes, because of the
unconditional function pointer deref.
> msi_domain_set_affinity() has called parent->chip->irq_set_affinity()
> without checking since it was added in 2014 by f3cf8bb0d6c3 ("genirq: Add
> generic msi irq domain support"), so if there's a problem here, it's most
> likely in the tango code.
The issue is having to define an "empty" function.
(Unnecessary code bloat and maintenance.)
I'll send a patch illustrating exactly what I intended.
Regards.
