Re: [Qemu-devel] [PATCH v2] sysbus: add irq_routing_notifier

Peter Crosthwaite
2015-04-27T14:31:19Z

On Mon, Apr 27, 2015 at 6:37 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 27/04/2015 14:20, Eric Auger wrote:
>> On 04/27/2015 12:39 PM, Paolo Bonzini wrote:
>>>
>>>
>>> On 27/04/2015 10:26, Eric Auger wrote:
>>>>>> One of my long term goals is to try and get rid of sysbus IRQ
>>>>>> abstraction completely in favor of just qdev gpios. This means
>>>>>> features that apply to GPIOs automatically apply to IRQs and vice
>>>>>> versa. Can your notifier hook be pushed up to the qdev GPIO level to
>>>>>> make it more globally usable and avoid a new feature to sysbus IRQs?
>>>> Yes sure, I am going to put the notifier in DeviceClass then.
>>>
>>> I've thought too about this, and I'm not sure about it.
>>>
>>> It would mean you have to pass the gpio name (e.g.
>>> SYSBUS_DEVICE_GPIO_IRQ) to the hook, and in the case of sysbus IRQs this
>>> would leak the SYSBUS_DEVICE_GPIO_IRQ abstraction to the implementors of
>>> the hook.

That's OK IMO. SYSBUS_DEVICE_GPIO_IRQ was never intended to be
private. The semantics of it are something like "If you don't have
anything better to name your IRQ pin use this".

This adds the requirement on machine level code that you can't
consistently use sysbus_connect_irq for intc connection. But machines
should be able to connect any wires between any cores without having
to special case interrupts or chip-selects, resets or whatever. So the
name of the GPIO has to be exposed to the callback hook registration
if we want to break down this GPIO special casing. Names of interrupt
pins are system-level knowledge so I think this is all OK.

>> Hi Paolo,
>>
>> Currently my notifier has the following proto:
>> void (*connect_gpio_out_notifier)(DeviceState *dev, qemu_irq irq);
>>
>> It is sufficient for my need.
>>
>> is it really mandated to pass other qdev_connect_gpio_out_named args,
>> ie. name & n?
>
> It's an ugly situation. If you look at qdev_connect_gpio_out_named, it
> is really a thin wrapper around object_property_set_link. Just like
> Peter wasn't too happy with changing sysbus_connect_irq, the same
> objection would apply here. Callers of object_property_set_link should
> call the notifiers, not just those that use qdev_connect_gpio_out_named.
>
> This is why I originally asked you to look into using the check callback
> instead.
>

Is this still feasible? Pushing it up to the higher again to the QOM
level? I think this would be an ideal backend to the problem even if
we still go with a code-friendly sysbus frontend.

Regards,
Peter

> This is why I think it's better to keep the sysbus patch.
>
> Paolo
>