Re: [Qemu-devel] [PATCH 5/6] PPC: e500: Support dynamically spawned sysbus devices

[Alexander Graf]

On 02.07.14 19:26, Scott Wood wrote:
> On Wed, 2014-07-02 at 19:12 +0200, Alexander Graf wrote:
> > On 02.07.14 00:50, Scott Wood wrote:
> > > Plus, let's please not hardcode any more addresses that are going to be
> > > a problem for giving guests a large amount of RAM (yes, CCSRBAR is also
> > > blocking that, but that has a TODO to parameterize).  How about
> > > 0xf00000000ULL?  Unless of course we're emulating an e500v1, which
> > > doesn't support 36-bit physical addressing.  Parameterization would help
> > > there as well.
> > I don't think we have to worry about e500v1. I'll change it :).
> We theoretically support it elsewhere...  Once parameterized, it
> shouldn't be hard to base the address for this, CCSRBAR, and similar
> things on whether MAS7 is supported.

It gets parametrized in the machine file, CPU selection comes after 
machine selection. So parameterizing it doesn't really solve it.

However, again, I don't think we have to worry about it.

> > > > @@ -122,6 +131,77 @@ static void dt_serial_create(void *fdt, unsigned long long 
> > > > offset,
> > > >        }
> > > >    }
> > > >    
> > > > +typedef struct PlatformDevtreeData {
> > > > +    void *fdt;
> > > > +    const char *mpic;
> > > > +    int irq_start;
> > > > +    const char *node;
> > > > +    int id;
> > > > +} PlatformDevtreeData;
> > > What is id?  How does irq_start work?
> > "id" is just a linear counter over all devices in the platform bus so
> > that if you need to have a unique identifier, you can have one.
> >
> > "irq_start" is the offset of the first mpic irq that's connected to the
> > platform bus.
> OK, but why is that here but no irq_end, and no address range?  How do
> allocations from the irq range happen?

There are 2 phases:

  1) Device association with the machine
  2) Device tree generation

The allocation of IRQ ranges happens during the association phase. That 
phase also updates all the hints in the devices to reflect their current 
IRQ (and MMIO) mappings. The device tree generation phase only needs to 
read those bits then - and add the IRQ offset to get from the "platform 
bus IRQ range" to "MPIC IRQ range".

> > > > +static int sysbus_device_create_devtree(Object *obj, void *opaque)
> > > > +{
> > > > +    PlatformDevtreeData *data = opaque;
> > > > +    Object *dev;
> > > > +    SysBusDevice *sbdev;
> > > > +    bool matched = false;
> > > > +
> > > > +    dev = object_dynamic_cast(obj, TYPE_SYS_BUS_DEVICE);
> > > > +    sbdev = (SysBusDevice *)dev;
> > > > +
> > > > +    if (!sbdev) {
> > > > +        /* Container, traverse it for children */
> > > > +        return object_child_foreach(obj, sysbus_device_create_devtree, data);
> > > > +    }
> > > > +
> > > > +    if (matched) {
> > > > +        data->id++;
> > > > +    } else {
> > > > +        error_report("Device %s is not supported by this machine yet.",
> > > > +                     qdev_fw_name(DEVICE(dev)));
> > > > +        exit(1);
> > > > +    }
> > > > +
> > > > +    return 0;
> > > > +}
> > > It's not clear to me how this function is creating a device tree node.
> > It's not yet - it's only the stub that allows to plug in specific device
> > code that then generates device tree nodes :).
> How does the plugging in work?
>
> It looks like all this does is increment id.

I'm not sure I understand. The plugging in is different code :). This 
really only does increment an id. Maybe I'll just remove it if it 
confuses you?


Alex