Re: [Qemu-devel] [PATCH RFC 0/3] basic support for composing sysbus devices

[Anthony Liguori]

On 06/10/2011 09:59 AM, Markus Armbruster wrote:
> Anthony Liguori<address@hidden>  writes:
>
> > On 06/10/2011 03:13 AM, Markus Armbruster wrote:
> > > Jan Kiszka<address@hidden>   writes:
> > > > Resource management, e.g. IRQs. That will be useful for other types of
> > > > buses as well.
> > >
> > > A device should be able to say "I need to be connected to an IRQ line".
> > > Feels generic to me.
> >
> > More specifically, a device has input IRQs.  A device has no idea what
> > number the IRQ is tied to.
> >
> > Devices may also have output IRQs.  At the qdev layer, we should be
> > able to connect an arbitrary output IRQ to an arbitrary input IRQ.
> >
> > So the crux of the problem is that:
> >
> >   -device isa-serial,id=serial,irq=3
> >
> > Is very wrong.  It ought to look something more like
> >
> >   -device piix3,id=piix3 -device isa-serial,id=serial,irq=piix3.irq[3]
>
> As Jan pointed out, ISA is a counter-example: your "very wrong" claim is
> actually wrong there :)

The wrongness comes from the fact that "irq=3" assumes that somehow the 
ISA device can get an IRQ from an index.

If instead the example was:

 -device piix3,id=isa0
 -device isa-serial,id=serial,irq=3,bus=isa0

With the implication isa-serial could use the "3" index to get an IRQ 
from it's bus at realize, like:

void isa_serial_initfn()
{
    ISAController *bus = qdev_get_isa_controller(dev, "bus");
    connect_irq(dev->irq, bus->irq[qdev_get_int(dev, "irq")]);
}

That is at least conceptually okay but I don't think it's best.  If you 
want to go this route, I'd actually model DIP switches or ISA plug and 
play.  Usually devices would only allow you to choose between two of the 
5 IRQs available.

But personally, the modelling I mentioned in another part of the thread 
where you connect the IRQs directly to the device seems like a better 
compromise to me.

> An ISA device is always connected to all the ISA bus's interrupt lines.
> Device configuration determines how the device uses these lines.

That's 100% correct.  But the "ISA bus" is just a grouping of interfaces 
to another device.  We shouldn't treat that as any more special than any 
other type of connection between devices.

> The old (non-MSI) PCI interrupts are similar, I think.
>
> Of course, "ISA IRQ 4" can be anything, depending on how the device
> providing the ISA bus is wired up.
>
> > IRQ forwarding becomes very easy in this model because your composite
> > qdev device has a set of input IRQs and then routes the input IRQs to
> > the appropriate input IRQs of the sub devices.
> >
> > The trouble is that I don't think we have a reasonable way to refer to
> > properties of other devices and we don't have names for all devices.
> > I think if we fix the later problem, the former problem becomes
> > easier.
>
> We already connect devices to other resources, such as block, char and
> network host parts.  The way we do it may not be "pure", but it works:
> we define the plug as property, and connect it to its socket by saying
> PROP-NAME=SOCK-NAME.  Note that the connection is made by core qdev;
> device model code is oblivious of it.  It just uses it.

Yes.  The problem with this is namespaces.  What you are referring to as 
"SOCK-NAME" is interpreted based on the property type.  Ideally, we'd 
have a single namespace for everything.

> >                       They can be replaced by having fixed "slots".
> > For instance, if you had a way of having a PCIDevice * property, the
> > I440FX could have 32 PCIDevice * properties.  That's how you would add
> > to a bus (and notice that it conveniently solves bus addressing in a
> > robust fashion).
>
> Assumes that bus addresses are isomorphic to [0..N], doesn't it?
>
> Not really the case for USB: we use a string of the form %d(.%d)* there.

Couple points there.

1) I didn't mean to suggest that the name "irq[3]" implies anything.  I 
sort of thing it should be just treated as a string.  It could as well 
be "irq[foo]".  I'm not 100% here though.

2) USB addressing is expressing an implicit topology of HUBs.  I think 
it's better to model those HUBs explicitly and leave convenience to 
syntactic sugar.  For instance:

 -device usb-uhci,id=uhci0,ports=4,port0=hub0
 -device usb-hub,id=hub0,ports=4,port1=tablet0
 -device usb-tablet,id=tablet0

Which is the expansion of:

 -tablet address=0.1

> Not my idea.
>
> A bus is just a standardized container for slots.  A single device can
> provide multiple buses.  Killing buses just unwraps the slots.  You lose
> the grouping.

The grouping is still there by virtue of the fact that the slots are all 
part of the same device.

> What exactly is so very wrong about buses that they need to die?

They force a device tree.  The device model shouldn't be a tree, but a 
directed graph.  It's perfectly fine to have a type called PCIBus that 
I440FX extends, but qdev shouldn't have explicit knowledge of something 
called a "bus" IMHO.  Doing this forces a limited mechanism of 
connecting devices because it creates an artificial tree (by implying a 
parent/child relationship).  It makes composition difficult if not 
impossible.

Regards,

Anthony Liguori

> Honest, non-rhetorical question.