[Qemu-devel] [PATCH] qdev: hook up i440fx pcihost to system bus.

[Qemu-devel] [PATCH] qdev: hook up i440fx pcihost to system bus.

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/pci_host.h |    1 +
 hw/piix_pci.c |   22 ++++++++++++++++++++--
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/hw/pci_host.h b/hw/pci_host.h
index 757b0e2..bd7adcb 100644
--- a/hw/pci_host.h
+++ b/hw/pci_host.h
@@ -36,6 +36,7 @@ do { printf("pci_host_data: " fmt , ## __VA_ARGS__); } while (0)
 #endif
 
 typedef struct {
+    DeviceState qdev;
     uint32_t config_reg;
     PCIBus *bus;
 } PCIHostState;
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 914a65a..8eed137 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -175,8 +175,8 @@ PCIBus *i440fx_init(PCIDevice **pi440fx_state, qemu_irq *pic)
     PCIDevice *d;
     I440FXState *s;
 
-    s = qemu_mallocz(sizeof(I440FXState));
-    b = pci_register_bus(NULL, "pci", 
+    s = container_of(qdev_create(NULL, "i440FX-pcihost"), I440FXState, qdev);
+    b = pci_register_bus(&s->qdev, "pci", 
                          piix3_set_irq, pci_slot_get_pirq, pic, 0, 4);
     s->bus = b;
 
@@ -206,6 +206,24 @@ PCIBus *i440fx_init(PCIDevice **pi440fx_state, qemu_irq *pic)
     return b;
 }
 
+static void i440fx_initfn(DeviceState *qdev, DeviceInfo *base)
+{
+    /* dummy for now ... */
+}
+
+static struct DeviceInfo i440fx_pcihost_devinfo = {
+    .init = i440fx_initfn,
+    .bus_type = BUS_TYPE_SYSTEM,
+};
+
+static void i440fx_register(void)
+{
+    qdev_register("i440FX-pcihost", sizeof(I440FXState),
+                  &i440fx_pcihost_devinfo);
+}
+
+device_init(i440fx_register);
+
 /* PIIX3 PCI to ISA bridge */
 
 static PCIDevice *piix3_dev;
-- 
1.6.2.2

Re: [Qemu-devel] [PATCH] qdev: hook up i440fx pcihost to system bus.

[Paul Brook]

> typedef struct {
> +    DeviceState qdev;
>...
>+static struct DeviceInfo i440fx_pcihost_devinfo = {
>+    .init = i440fx_initfn,
>+    .bus_type = BUS_TYPE_SYSTEM,
>+};

This is clearly a lie.

> +    s = container_of(qdev_create(NULL, "i440FX-pcihost"), I440FXState,

And this should be using FROM_SYSBUS.

Paul

Re: [Qemu-devel] [PATCH] qdev: hook up i440fx pcihost to system bus.

[Gerd Hoffmann]

On 06/08/09 15:04, Paul Brook wrote:
>> typedef struct {
>> +    DeviceState qdev;
>> ...
>> +static struct DeviceInfo i440fx_pcihost_devinfo = {
>> +    .init = i440fx_initfn,
>> +    .bus_type = BUS_TYPE_SYSTEM,
>> +};
>
> This is clearly a lie.

--verbose please.

Ok, looks like main_system_bus (qdev.c) is supposed to be the root bus. 
  Is that correct?  If so, then we'll have to link the (primary) pci bus 
to the system bus, so the devices show up in the device tree.

Or is the main_system_bus meant for embedded boards only which have some 
non-standard bus?  In that case we'll needs some way to tell qdev what 
the root bus is, so qdev can operate on the tree.  For now this is just 
printing the devices in "info qtree", but I'm sure one day we will do 
much more with the device tree.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH] qdev: hook up i440fx pcihost to system bus.

[Paul Brook]

On Monday 08 June 2009, Gerd Hoffmann wrote:
> On 06/08/09 15:04, Paul Brook wrote:
> >> typedef struct {
> >> +    DeviceState qdev;
> >> ...
> >> +static struct DeviceInfo i440fx_pcihost_devinfo = {
> >> +    .init = i440fx_initfn,
> >> +    .bus_type = BUS_TYPE_SYSTEM,
> >> +};
> >
> > This is clearly a lie.
>
> --verbose please.

If you're adding something to a system bus, it needs to be a system bus device 
(i.e. SysBusDevice and use sysbus_register_*). The FROM_SYSBUS and similar 
macros help to protect agains mixing the wrong things, hence my later comment.

You should never be setting bus_type directly. It will be done by the 
appropriate bus wrappers.  The comment at the top of qdev.c hints towards 
this:

    Devices will generally inherit from a particular bus (e.g. PCI or I2C)
    rather than this API directly.

Paul