On Tue, Jun 14, 2011 at 4:21 PM, Anthony Liguori <anth...@codemonkey.ws> wrote: > On 06/13/2011 03:59 PM, Blue Swirl wrote: >> >> On Sun, Jun 12, 2011 at 10:21 PM, Anthony Liguori<aligu...@us.ibm.com> >> wrote: >>> >>> On 06/12/2011 12:12 PM, Avi Kivity wrote: >>>> >>>> On 06/10/2011 06:43 PM, Anthony Liguori wrote: >>>>> >>>>>> 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. >>>> >>>> Right. As an example, you configure PCI interrupt routing and the memory >>>> controller by writing to a PCI device, which logically doesn't have >>>> access to any of this stuff if it's behind the PCI bus. >>>> >>>> However, I don't think buses should die. They should be available as an >>>> easy way to model the devices that do follow the rules. But we should >>>> also expose everything else for the exceptional cases. >>>> >>>>> 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. >>>> >>>> I think qdev buses are useful as long as they don't enforce their >>>> interfaces. That is, a qdev that is a child of a qbus has access to the >>>> qbus's interfaces, but also access to other stuff. >>> >>> I see two independent data structures. The first is the "instantiation >>> tree". >>> >>> The instantiation tree may look like this: >>> >>> +-- i440fx >>> | | >>> | +-- PIIX3 >>> | | | >>> | | +-- mc146818a >>> | | +-- uart >>> | | +-- DMA >>> | | +-- keyboard controller >>> | | +-- (remaining platform ISA devices >>> | | >>> | +-- UHCI USB controller >>> | +-- IDE controller >>> | >>> +-- e1000 >>> +-- cirrus-vga >>> +-- virtio-balloon-pci >>> +-- IDE disk0 >>> >>> Instantiating i440fx makes a bunch of default stuff. This is >>> composition. >>> Everything else requires explicit instantiation. This is, strictly >>> speaking, the parent/child relationships. If you destroy i440fx, all of >>> it's children have to also go away (by definition). Nothing about bus >>> relationship is implied here. Even if i440fx exposes a PCI bus, the >>> PIIX3 >>> is a child of i440fx even though e1000 is not (even if they're both PCI >>> devices). >> >> I actually like this slot idea in place of buses. But wouldn't there >> be two classes of devices (or two APIs), slot devices and composable >> devices? > > All devices have properties. We have this today in qdev. What's missing is > to have a properties who's type is a socket for another device. We really > want to be able to do: > > static DeviceInfo i440fx_info = { > .name = "i440fx", > .props = (Property[]){ > DEFINE_PROP_PLUG(I440FXState, piix3), > DEFINE_PROP_SOCKET(I440FXState, slot[0]), > DEFINE_PROP_SOCKET(I440FXState, slot[1]), > ... > }, > }; > > Which suggests that we really need to move away from declarative device > definitions. It makes it hard to have variable numbers of properties.
I'd suppose the number of slots could be fixed. Then the declaration could be DEFINE_PROP_SOCKETS(I440FXState, slot, 32), > In this case, piix3 would be defined as: > > struct I440FXState { > PIIX3 piix3; > PCIDevice slots[32]; > }; > > Which suggests we need an initfn to do the following: > > void i440fx_initfn(...) { > qdev_init_inplace(&dev->piix3, "PIIX3"); > dev->slot[1] = &dev->piix3->bus; > } > > This gets hard to do well in C though. I'm not sure how we could make > DEFINE_PROP_PLUG/SOCKET type safe. DEFINE_PROP_PCI_SOCKET()?