Re: IRQ handling (was [Qemu-devel] qemu Makefile.target vl.h hw/acpi.c hw/adlib.c ...)

["J. Mayer" <l_indien@magic.fr>]

On Sun, 2007-04-08 at 09:49 +0200, J. Mayer wrote:

> I'm sorry, I'm no sure I can (NDA rules....).
> Let's say it's a chips used in some consumer electronics products.

I gave this example to show that we cannot pre-determine any limit to
the number of PINs we have to manage, even if it's a poor design and
that no one need those 500 IRQ sources (50 would be sufficient if it was
well designed but that the way the actual hardware has been
designed...).

Here's an example which is documented:
A CHRP machine is supposed to have an OpenPIC controller (up to 32 IRQ
sources and up to 4 output IRQ pins) and 2 i8259. This simple machine
already has more than 32 IRQs.
Some may also have 2 cascaded OpenPICs.
It will also have one or more PCI controllers (which have it's own IRQ
controller, n inputs, 4 outputs, on actual hardware).
Some complex PCI devices may also have an internal IRQ controller if
they can generate a lot of different hardware events. Those devices can
even be hotplugged.

Then, can you pre-determine the maximum number of IRQ of a hardware
platform ? I can't, because it's conceptually impossible.

[....]

Ooops... Lots of typo in what I wrote....

> static inline void qemu_pin_set_state(struct PINState *st, int level)
> {
>       if (st->handler != NULL && level != st->level != level) {

I meant  .... && level != st->level) {

[...]

> static inline void qemu_ping_raise(struct PINState *st)

I meant qemu_pin_raise (not ping).
You can even have an allocator to ease sharing the information between
the devices, like:
static inline PINState *qemu_pin_alloc (void *opaque, int n, int
initial_level)
{
    PINState *st;

    st = qemu_mallocz(sizeof(PINState));
    if (st != NULL) {
        st->opaque = opaque;
        st->n = n;
        st->level = initial_level;
    }
}

And note that if you really want to emulate GPIOs, you may also need to
add a default_level field to the structure and a bitmask to know which
devices are really driving the PIN (as it reaches its default state only
when no one is driving it). and a new function to signal the case where
the PIN is beeing released (not drived) by a peripheral. And a
peripheral need to identify itself when calling the pin_raise,
pin_lower, pin_release function. The only limitation of this approach
may be the number of peripheral driving one single signal on the board,
which can not be > of the size of the bitfield. But you can have as many
handled signals as you need too.

Once again, one of the two structure is not needed (qemu_irq /
IRQState): there is only one physical state so having 2 different
description of the same thing is conceptually broken. Nothing can be
stored in a generic place as a PIN connection is a completely local
concept between a set of devices. And once again, IRQ are not bound to a
CPU. Storing any informations about IRQs in the CPU structure is
meaningless. Where do you store the IRQ informations when you have 2, 4
CPUs connected to one OpenPIC ? Choose a random CPU ? Only use one ?

Then, we can see the code that has been commited does not solve the
problems of generic signal routing and is conceptually completelly
broken.

One last point:
I thought more about the ppc_openpic_irq function and I have to admit
it's not the good approach.
Yes, this has to be changed to store the IRQ destination inside the
OpenPIC controller and directly call the ppc_set_irq callback. In fact,
there should be a function replacing the ppc_openpic_irq one that
handles the external exception sources and callback ppc_set_irq (this
function also manages internal hardware exception sources). The code in
hw/ppc.c would not be changed a lot but the OpenPIC would be more
generic and may allow some crazy schemes where the outputs would not be
connected to the same destination device (usually the CPU). And it may
make easier to use OpenPIC on hardware that are not PowerPC base (with
no hack), which is imho more important (OpenPIC was designed for and
used on AMD SMP PC platforms, originally). I'll change this, once the
IRQ problems will be solved (which seems urgent, more than anything
else).

-- 
J. Mayer <l_indien@magic.fr>
Never organized