Re: [Qemu-devel] Memory API: handling unassigned physical memory

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Blue Swirl]

On Mon, Apr 30, 2012 at 13:52, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> On 30/04/12 14:27, Peter Maydell wrote:
>
> Hi Peter,
>
>
>>> IMO the best fix is to unsysbus the device and qomify it instead.  This
>>> way we're 100% flexible in how we can attach it.
>>
>>
>> You don't need to wait for QOM to grow enough features to
>> replace sysbus. If you don't like what sysbus_mmio_map() does, you
>
>
> Oh - so does this mean that QOM is not feature-complete?

At least 'Pin' class to replace GPIOs and IRQs is not in HEAD (but
Anthony already has a tree somewhere) and I can't remember what was
the plan for buses.

>
>
>> can always use sysbus_mmio_get_region() to get the MemoryRegion* and
>> then deal with it however you need to. This is the standard way
>> to deal with "I have a sysbus device which I want to map into my
>> custom container object".
>
>
> I already have code to do this for the sun4m-only hardware modelled on
> sysbus_mmio_map() like this:
>
>
> static void sbus_mmio_map(void *sbus, SysBusDevice *s, int n,
> target_phys_addr_t addr)
> {
>    MemoryRegion *sbus_mem, *mem;
>    target_phys_addr_t sbus_base;
>    SBusState *sbus_state = FROM_SYSBUS(SBusState, (SysBusDevice *)sbus);
>
>    sbus_mem = sysbus_mmio_get_region((SysBusDevice *)sbus, 0);
>    mem = sysbus_mmio_get_region(s, n);
>
>    /* SBus addresses are physical addresses, so subtract start of region */
>    sbus_base = sbus_state->base;
>    memory_region_add_subregion(sbus_mem, addr - sbus_base, mem);
> }
>
>
> The key problem is that this doesn't worked with shared peripherals, such as
> the ESP device which is also used on various PPC Mac models as well as
> SPARC. That's because its init function looks like this:
>
>
>
> void esp_init(target_phys_addr_t espaddr, int it_shift,
>              ESPDMAMemoryReadWriteFunc dma_memory_read,
>              ESPDMAMemoryReadWriteFunc dma_memory_write,
>              void *dma_opaque, qemu_irq irq, qemu_irq *reset,
>              qemu_irq *dma_enable)
> {
>    ...
>    ...
>    sysbus_mmio_map(s, 0, espaddr);
>    ...
> }
>
>
> Therefore I can't change it to my (modified) sbus_mmio_map() function
> because it would break other non-SPARC platforms, and AIUI there is nothing
> in the memory API that allows me to move a subregion to a different
> MemoryRegion parent, even if I can get a reference to it with
> sysbus_mmio_get_region() after the sysbus_mmio_map() call - or have I
> misunderstood something?

Sysbus is used as a generic class for motherboard devices, there is an
assumption that there is no higher level bus. What we need here is a
full blown bus. The translations and mappigs between bus addresses and
motherboard addresses should be done in a Sysbus to SBus bridge
device, just like PCI host bridges do.

>
>
> ATB,
>
> Mark.
>

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Anthony Liguori]

On 05/01/2012 01:57 AM, Blue Swirl wrote:
> On Mon, Apr 30, 2012 at 13:52, Mark Cave-Ayland
> <mark.cave-ayland@ilande.co.uk>  wrote:
>> On 30/04/12 14:27, Peter Maydell wrote:
>>
>> Hi Peter,
>>
>>
>>>> IMO the best fix is to unsysbus the device and qomify it instead.  This
>>>> way we're 100% flexible in how we can attach it.
>>>
>>>
>>> You don't need to wait for QOM to grow enough features to
>>> replace sysbus. If you don't like what sysbus_mmio_map() does, you
>>
>>
>> Oh - so does this mean that QOM is not feature-complete?
>
> At least 'Pin' class to replace GPIOs and IRQs is not in HEAD (but
> Anthony already has a tree somewhere) and I can't remember what was
> the plan for buses.

I'm planning on pushing bus support in before I freeze 1.1.

Regards,

Anthony Liguori

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Mark Cave-Ayland]

On 01/05/12 07:57, Blue Swirl wrote:

>> Therefore I can't change it to my (modified) sbus_mmio_map() function
>> because it would break other non-SPARC platforms, and AIUI there is nothing
>> in the memory API that allows me to move a subregion to a different
>> MemoryRegion parent, even if I can get a reference to it with
>> sysbus_mmio_get_region() after the sysbus_mmio_map() call - or have I
>> misunderstood something?
>
> Sysbus is used as a generic class for motherboard devices, there is an
> assumption that there is no higher level bus. What we need here is a
> full blown bus. The translations and mappigs between bus addresses and
> motherboard addresses should be done in a Sysbus to SBus bridge
> device, just like PCI host bridges do.

Since SBus is mapped directly to physical addresses, is this mapping not 
just 1:1? Or does it make sense to re-work all the offsets of the 
various peripherals to be from the base address of the first slot?


ATB,

Mark.

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Bob Breuer]

On 5/1/2012 1:48 PM, Mark Cave-Ayland wrote:
> On 01/05/12 07:57, Blue Swirl wrote:
> 
>>> Therefore I can't change it to my (modified) sbus_mmio_map() function
>>> because it would break other non-SPARC platforms, and AIUI there is
>>> nothing
>>> in the memory API that allows me to move a subregion to a different
>>> MemoryRegion parent, even if I can get a reference to it with
>>> sysbus_mmio_get_region() after the sysbus_mmio_map() call - or have I
>>> misunderstood something?
>>
>> Sysbus is used as a generic class for motherboard devices, there is an
>> assumption that there is no higher level bus. What we need here is a
>> full blown bus. The translations and mappigs between bus addresses and
>> motherboard addresses should be done in a Sysbus to SBus bridge
>> device, just like PCI host bridges do.
> 
> Since SBus is mapped directly to physical addresses, is this mapping not
> just 1:1? Or does it make sense to re-work all the offsets of the
> various peripherals to be from the base address of the first slot?

It would be nice to have the device offsets be relative to the slot.
User-pluggable sbus devices should be possible.

I've just pushed an update of a dbri sbus device model to github (
https://github.com/breuerr/qemu/commits/dbri-pre2 ).  This device was
built-in to at least the SS-20, but also available as an sbus add-in
card (SunLink ISDN).  There's an fcode rom so it can be probed by OBP,
and if we could get something like "-device SUNW,,DBRIe,slot=1" to work,
then a user could add it to most of the sun4m machine models.

Bob

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Blue Swirl]

On Wed, May 2, 2012 at 3:15 PM, Bob Breuer <breuerr@mc.net> wrote:
> On 5/1/2012 1:48 PM, Mark Cave-Ayland wrote:
>> On 01/05/12 07:57, Blue Swirl wrote:
>>
>>>> Therefore I can't change it to my (modified) sbus_mmio_map() function
>>>> because it would break other non-SPARC platforms, and AIUI there is
>>>> nothing
>>>> in the memory API that allows me to move a subregion to a different
>>>> MemoryRegion parent, even if I can get a reference to it with
>>>> sysbus_mmio_get_region() after the sysbus_mmio_map() call - or have I
>>>> misunderstood something?
>>>
>>> Sysbus is used as a generic class for motherboard devices, there is an
>>> assumption that there is no higher level bus. What we need here is a
>>> full blown bus. The translations and mappigs between bus addresses and
>>> motherboard addresses should be done in a Sysbus to SBus bridge
>>> device, just like PCI host bridges do.
>>
>> Since SBus is mapped directly to physical addresses, is this mapping not
>> just 1:1? Or does it make sense to re-work all the offsets of the
>> various peripherals to be from the base address of the first slot?
>
> It would be nice to have the device offsets be relative to the slot.
> User-pluggable sbus devices should be possible.

Yes, currently all devices are fixed but for example display adapter
should be pluggable and only on-board devices should be fixed.

> I've just pushed an update of a dbri sbus device model to github (
> https://github.com/breuerr/qemu/commits/dbri-pre2 ).  This device was
> built-in to at least the SS-20, but also available as an sbus add-in
> card (SunLink ISDN).  There's an fcode rom so it can be probed by OBP,
> and if we could get something like "-device SUNW,,DBRIe,slot=1" to work,
> then a user could add it to most of the sun4m machine models.

Nice work.

>
> Bob

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Blue Swirl]

On Tue, May 1, 2012 at 6:48 PM, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> On 01/05/12 07:57, Blue Swirl wrote:
>
>>> Therefore I can't change it to my (modified) sbus_mmio_map() function
>>> because it would break other non-SPARC platforms, and AIUI there is
>>> nothing
>>> in the memory API that allows me to move a subregion to a different
>>> MemoryRegion parent, even if I can get a reference to it with
>>> sysbus_mmio_get_region() after the sysbus_mmio_map() call - or have I
>>> misunderstood something?
>>
>>
>> Sysbus is used as a generic class for motherboard devices, there is an
>> assumption that there is no higher level bus. What we need here is a
>> full blown bus. The translations and mappigs between bus addresses and
>> motherboard addresses should be done in a Sysbus to SBus bridge
>> device, just like PCI host bridges do.
>
>
> Since SBus is mapped directly to physical addresses, is this mapping not
> just 1:1? Or does it make sense to re-work all the offsets of the various
> peripherals to be from the base address of the first slot?

The mapping is not direct, from device point of view there's IOMMU in
between and without help from IOMMU (or using the direct mode) the
device can't access the full 36 bit address space. The offset should
be from the start of the current slot, not first slot.

>
>
> ATB,
>
> Mark.

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Blue Swirl]

On Mon, Apr 30, 2012 at 14:33, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> On 30/04/12 15:03, Peter Maydell wrote:
>
>>> Therefore I can't change it to my (modified) sbus_mmio_map() function
>>> because it would break other non-SPARC platforms, and AIUI there is
>>> nothing
>>> in the memory API that allows me to move a subregion to a different
>>> MemoryRegion parent, even if I can get a reference to it with
>>> sysbus_mmio_get_region() after the sysbus_mmio_map() call - or have I
>>> misunderstood something?
>>
>>
>> Init functions like esp_init should be purely convenience functions
>> which create, set properties on, and map the sysbus/qdev device. If
>> they make use of private knowledge about the internals of the device
>> then this is wrong and should be fixed. For esp_init() it looks like
>> the handling of dma_memory_read, dma_memory_write, dma_opaque, it_shift
>> and dma_enabled are wrong.
>>
>> If you fix that then you can just ignore the convenience function,
>> and create, configure and map the device as appropriate for you.
>
>
> Right I think I'm starting to understand this now - in which case it becomes
> a matter of just copying a handful of lines within sun4m which is more
> bearable.
>
> In your view, would a suitable fix be to change dma_memory_read,
> dma_memory_write, dma_opaque, it_shift and dma_enabled to be qdev properties
> and modify esp_init() to return the qdev reference so they can be set by the
> caller?

There's an ongoing work to introduce IOMMUs by changing how DMA work
and this could simplify the DMA part. There's no clean way to use
function pointers in qdev.

For it_shift, a qdev or QOM property should be OK.

The signal dma_enabled should be eventually replaced by a Pin.

>
>
> ATB,
>
> Mark.
>

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Mark Cave-Ayland]

On 01/05/12 08:10, Blue Swirl wrote:

>> In your view, would a suitable fix be to change dma_memory_read,
>> dma_memory_write, dma_opaque, it_shift and dma_enabled to be qdev properties
>> and modify esp_init() to return the qdev reference so they can be set by the
>> caller?
>
> There's an ongoing work to introduce IOMMUs by changing how DMA work
> and this could simplify the DMA part. There's no clean way to use
> function pointers in qdev.

In my current working tree, I have actually managed to get this working 
with a customised qdev type macro and the standard qdev pointer type - 
so it may not be particularly great, but it works.

> For it_shift, a qdev or QOM property should be OK.
>
> The signal dma_enabled should be eventually replaced by a Pin.

And this is a sysbus concept, yes?


ATB,

Mark.

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Andreas Färber]

Am 01.05.2012 20:50, schrieb Mark Cave-Ayland:
> On 01/05/12 08:10, Blue Swirl wrote:
>> The signal dma_enabled should be eventually replaced by a Pin.
> 
> And this is a sysbus concept, yes?

No, it'll be a general DeviceState concept.
SysBus is supposed to die out gradually.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Avi Kivity]

On 04/30/2012 04:40 PM, Peter Maydell wrote:
> On 30 April 2012 14:36, Avi Kivity <avi@redhat.com> wrote:
> > On 04/30/2012 04:27 PM, Peter Maydell wrote:
> >> On 30 April 2012 14:23, Avi Kivity <avi@redhat.com> wrote:
> >> > IMO the best fix is to unsysbus the device and qomify it instead.  This
> >> > way we're 100% flexible in how we can attach it.
> >>
> >> You don't need to wait for QOM to grow enough features to
> >> replace sysbus. If you don't like what sysbus_mmio_map() does, you
> >> can always use sysbus_mmio_get_region() to get the MemoryRegion* and
> >> then deal with it however you need to. This is the standard way
> >> to deal with "I have a sysbus device which I want to map into my
> >> custom container object".
> >
> > I believe that API voids you warrantee.
>
> I wrote it for essentially the purpose described above :-)
> If you're the owner of the sysbus device in question then it's
> entirely fine as you are the one deciding whether to use the
> traditional map function or not.
>
> It's as good as we're going to get until QOM actually lets
> you export memory regions and pins, at which point we can just
> convert all the sysbus devices.

Sure.  But expect breakage if sysbus changes, for example dropping use
of get_system_memory().

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Peter Maydell]

On 1 May 2012 13:39, Avi Kivity <avi@redhat.com> wrote:
> On 04/30/2012 04:40 PM, Peter Maydell wrote:
>> I wrote it for essentially the purpose described above :-)
>> If you're the owner of the sysbus device in question then it's
>> entirely fine as you are the one deciding whether to use the
>> traditional map function or not.
>>
>> It's as good as we're going to get until QOM actually lets
>> you export memory regions and pins, at which point we can just
>> convert all the sysbus devices.
>
> Sure.  But expect breakage if sysbus changes, for example dropping use
> of get_system_memory().

That's OK, I'm happy to nak sysbus patches which break this
use case :-)

-- PMM

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Avi Kivity]

On 05/01/2012 03:41 PM, Peter Maydell wrote:
> On 1 May 2012 13:39, Avi Kivity <avi@redhat.com> wrote:
> > On 04/30/2012 04:40 PM, Peter Maydell wrote:
> >> I wrote it for essentially the purpose described above :-)
> >> If you're the owner of the sysbus device in question then it's
> >> entirely fine as you are the one deciding whether to use the
> >> traditional map function or not.
> >>
> >> It's as good as we're going to get until QOM actually lets
> >> you export memory regions and pins, at which point we can just
> >> convert all the sysbus devices.
> >
> > Sure.  But expect breakage if sysbus changes, for example dropping use
> > of get_system_memory().
>
> That's OK, I'm happy to nak sysbus patches which break this
> use case :-)

sysbus should just die.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Peter Maydell]

On 1 May 2012 13:42, Avi Kivity <avi@redhat.com> wrote:
> sysbus should just die.

Totally agreed. It's not going to go quietly though...

-- PMM

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Avi Kivity]

On 05/01/2012 03:43 PM, Peter Maydell wrote:
> On 1 May 2012 13:42, Avi Kivity <avi@redhat.com> wrote:
> > sysbus should just die.
>
> Totally agreed. It's not going to go quietly though...

Not if you keep suggesting workarounds when I tell unsuspecting
developers to qomify their devices.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Peter Maydell]

On 1 May 2012 13:48, Avi Kivity <avi@redhat.com> wrote:
> On 05/01/2012 03:43 PM, Peter Maydell wrote:
>> On 1 May 2012 13:42, Avi Kivity <avi@redhat.com> wrote:
>> > sysbus should just die.
>>
>> Totally agreed. It's not going to go quietly though...
>
> Not if you keep suggesting workarounds when I tell unsuspecting
> developers to qomify their devices.

When QOM supports (1) exporting gpio signals and (2)
exporting memory regions, then it will be providing the
main things that sysbus provides. (Sysbus today is essentially
"I need a device that exports some memory regions and some
I/O pins".) At that point it will be sensible to say "convert
your sysbus devices to QOM". Until QOM is actually able to
provide the functionality, it's not a workable replacement.

-- PMM

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Avi Kivity]

On 05/01/2012 03:49 PM, Peter Maydell wrote:
> On 1 May 2012 13:48, Avi Kivity <avi@redhat.com> wrote:
> > On 05/01/2012 03:43 PM, Peter Maydell wrote:
> >> On 1 May 2012 13:42, Avi Kivity <avi@redhat.com> wrote:
> >> > sysbus should just die.
> >>
> >> Totally agreed. It's not going to go quietly though...
> >
> > Not if you keep suggesting workarounds when I tell unsuspecting
> > developers to qomify their devices.
>
> When QOM supports (1) exporting gpio signals and (2)
> exporting memory regions, then it will be providing the
> main things that sysbus provides. (Sysbus today is essentially
> "I need a device that exports some memory regions and some
> I/O pins".) At that point it will be sensible to say "convert
> your sysbus devices to QOM". Until QOM is actually able to
> provide the functionality, it's not a workable replacement.

Doesn't property<MemoryRegion> (or however it's phrased) provide the
functionality for memory?

But I agree.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Anthony Liguori]

On 05/01/2012 08:01 AM, Avi Kivity wrote:
> On 05/01/2012 03:49 PM, Peter Maydell wrote:
>> On 1 May 2012 13:48, Avi Kivity<avi@redhat.com>  wrote:
>>> On 05/01/2012 03:43 PM, Peter Maydell wrote:
>>>> On 1 May 2012 13:42, Avi Kivity<avi@redhat.com>  wrote:
>>>>> sysbus should just die.
>>>>
>>>> Totally agreed. It's not going to go quietly though...
>>>
>>> Not if you keep suggesting workarounds when I tell unsuspecting
>>> developers to qomify their devices.
>>
>> When QOM supports (1) exporting gpio signals and

This is trivial.  It'll come in as soon as 1.2 opens up.  If folks want to start 
working on a branch with it:

https://github.com/aliguori/qemu/tree/qom-pin.4

>> (2) exporting memory regions, then it will be providing the
>> main things that sysbus provides.

This is a little tricky.  Here's the problems I've encountered so far:

a) A lot of devices need the equivalent of it_shift.  it_shift affects how 
addresses are decoded and the size of the memory region.  it_shift usually needs 
to be a device property.

Since we need to know the size of the memory region to initialize it, we need to 
know the value of it_shift before we can initialize it which means we have to 
delay the initialization of the mmemory region until realize.

I think a nice fix would be to make it_shift as memory region mutator and allow 
it to be set after initialization.

b) There's some duplication in MemoryRegions with respect to QOM.  Memory 
regions want to have a name but with QOM they'll be addressable via a path.  I 
go back and forth about how aggressively we want to refactor MemoryRegions.

If someone wants to spend some time converting MemoryRegion to QOM, that would 
certainly be helpful.  So far, I've been avoiding it but we'll eventually need 
to do it.

Regards,

Anthony Liguori

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Peter Maydell]

On 1 May 2012 14:50, Anthony Liguori <anthony@codemonkey.ws> wrote:
>> On 05/01/2012 03:49 PM, Peter Maydell wrote:
>>> When QOM supports (1) exporting gpio signals and
>
> This is trivial.  It'll come in as soon as 1.2 opens up.

You need to get it through code review first...

(just as a random example, this commit:
https://github.com/aliguori/qemu/commit/46483049813fca2c23fb7cd04eeab89905500c4c
is going down the wrong path because it's incorrectly introducing
knowledge of device internals to the utility i8259_init()
function.)

-- PMM

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Anthony Liguori]

On 05/01/2012 09:00 AM, Peter Maydell wrote:
> On 1 May 2012 14:50, Anthony Liguori<anthony@codemonkey.ws>  wrote:
>>> On 05/01/2012 03:49 PM, Peter Maydell wrote:
>>>> When QOM supports (1) exporting gpio signals and
>>
>> This is trivial.  It'll come in as soon as 1.2 opens up.
>
> You need to get it through code review first...

Obviously.

>
> (just as a random example, this commit:
> https://github.com/aliguori/qemu/commit/46483049813fca2c23fb7cd04eeab89905500c4c
> is going down the wrong path because it's incorrectly introducing
> knowledge of device internals to the utility i8259_init()
> function.)

Eh?

Do you mean:

-    qdev_connect_gpio_out(&dev->qdev, 0, irq_set[2]);
+    pin_connect_qemu_irq(&s->int_out[0], irq_set[2]);

There are three ways to do this:

1) pin_connect_qemu_irq(i8259_get_int_out(s), irq_set[2]);

2) pin_connect_qemu_irq(&s->int_out[0], irq_set[2]);

3) pin_connect_qemu_irq(PIN(object_get_child(s, "int_out[0]")), irq_set[2]);

Having converted a lot of devices by hand, I prefer (2).  I'd like to move to a 
model of:

typedef struct PICDevice
{
     DeviceState parent;

     Pin int_out[0];
     MemoryRegion io;

     /*< private >*/
     PICState *priv;
} PICDevice;

This let's us use (2), while still embedding devices and keeping the gory state 
details private to the implementation.

Regards,

Anthony Liguori

>
> -- PMM

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Peter Maydell]

On 1 May 2012 15:06, Anthony Liguori <anthony@codemonkey.ws> wrote:
> Do you mean:
>
> -    qdev_connect_gpio_out(&dev->qdev, 0, irq_set[2]);
> +    pin_connect_qemu_irq(&s->int_out[0], irq_set[2]);
>
> There are three ways to do this:
>
> 1) pin_connect_qemu_irq(i8259_get_int_out(s), irq_set[2]);

No good unless you're autogenerating all those accessor functions.

> 2) pin_connect_qemu_irq(&s->int_out[0], irq_set[2]);

Exposes the whole of the struct (including all the private
memmbers) to anything that wants to use it. As you say
later on in the email, this requires splitting everything up
into two structs, one for "publicly accessible" and one for
"private implementation"...which your series doesn't seem
to do at the moment.

> 3) pin_connect_qemu_irq(PIN(object_get_child(s, "int_out[0]")), irq_set[2]);

This is a bit verbose. Something more like
 pin_connect(s, "int_out[0]", self, "int_set[2]");
would be a bit less longwinded. Or even
 connect(TYPE_PIN, s, "int_out[0]", self, "int_set[2]");

(No, this doesn't do compile time type checking. I don't
think that's a problem particularly, or at least not enough
of one to justify not doing it this way. The object model we
have is dynamic and does things at runtime...)

-- PMM

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Anthony Liguori]

On 05/01/2012 09:20 AM, Peter Maydell wrote:
> On 1 May 2012 15:06, Anthony Liguori<anthony@codemonkey.ws>  wrote:
>> Do you mean:
>>
>> -    qdev_connect_gpio_out(&dev->qdev, 0, irq_set[2]);
>> +    pin_connect_qemu_irq(&s->int_out[0], irq_set[2]);
>>
>> There are three ways to do this:
>>
>> 1) pin_connect_qemu_irq(i8259_get_int_out(s), irq_set[2]);
>
> No good unless you're autogenerating all those accessor functions.
>
>> 2) pin_connect_qemu_irq(&s->int_out[0], irq_set[2]);
>
> Exposes the whole of the struct (including all the private
> memmbers) to anything that wants to use it. As you say
> later on in the email, this requires splitting everything up
> into two structs, one for "publicly accessible" and one for
> "private implementation"...which your series doesn't seem
> to do at the moment.
>
>> 3) pin_connect_qemu_irq(PIN(object_get_child(s, "int_out[0]")), irq_set[2]);
>
> This is a bit verbose. Something more like
>   pin_connect(s, "int_out[0]", self, "int_set[2]");

This is incorrect, it would have to be:

Error *err = NULL;

if (pin_connect(s, "in_out[0]", self, "int_set[2]", &err)) {
    error_propagate(errp, err);
    return;
}

> would be a bit less longwinded. Or even
>   connect(TYPE_PIN, s, "int_out[0]", self, "int_set[2]");

Not checking for failure is not an option.

> (No, this doesn't do compile time type checking. I don't
> think that's a problem particularly, or at least not enough
> of one to justify not doing it this way. The object model we
> have is dynamic and does things at runtime...)

Correctness is more important to me than brevity.

And really, we should focus on killing things like i8259_init().  These types of 
functions should go away in favor of proper modeling of SoCs/SuperIO chips.

Regards,

Anthony Liguori

>
> -- PMM

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Peter Maydell]

On 1 May 2012 16:09, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 05/01/2012 09:20 AM, Peter Maydell wrote:
>> This is a bit verbose. Something more like
>>  pin_connect(s, "int_out[0]", self, "int_set[2]");
>
> This is incorrect, it would have to be:
>
> Error *err = NULL;
>
> if (pin_connect(s, "in_out[0]", self, "int_set[2]", &err)) {
>   error_propagate(errp, err);
>   return;
>
> }
>
>> would be a bit less longwinded. Or even
>>  connect(TYPE_PIN, s, "int_out[0]", self, "int_set[2]");
>
>
> Not checking for failure is not an option.

The assumption is that failure to connect is a fatal error,
and we can happily just assert()/hw_error()/etc.
(Obviously we would provide APIs for the occasional use case
that really did want to do "connect if possible and do something
else in case of failure", but that's definitely not the
common case.)

>> (No, this doesn't do compile time type checking. I don't
>> think that's a problem particularly, or at least not enough
>> of one to justify not doing it this way. The object model we
>> have is dynamic and does things at runtime...)
>
> Correctness is more important to me than brevity.
>
> And really, we should focus on killing things like i8259_init().

Functions like i8259_init() exist precisely because
QOM/qdev don't provide brevity and people trying to
use these devices do in fact value brevity. That's why
I want the standard native "connect this thing to this
other thing" function to be short and simple.

-- PMM

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Anthony Liguori]

On 05/01/2012 10:20 AM, Peter Maydell wrote:
> On 1 May 2012 16:09, Anthony Liguori<anthony@codemonkey.ws>  wrote:
>> On 05/01/2012 09:20 AM, Peter Maydell wrote:
>>> This is a bit verbose. Something more like
>>>   pin_connect(s, "int_out[0]", self, "int_set[2]");
>>
>> This is incorrect, it would have to be:
>>
>> Error *err = NULL;
>>
>> if (pin_connect(s, "in_out[0]", self, "int_set[2]",&err)) {
>>    error_propagate(errp, err);
>>    return;
>>
>> }
>>
>>> would be a bit less longwinded. Or even
>>>   connect(TYPE_PIN, s, "int_out[0]", self, "int_set[2]");
>>
>>
>> Not checking for failure is not an option.
>
> The assumption is that failure to connect is a fatal error,
> and we can happily just assert()/hw_error()/etc.

So that means that we have a bug from someone miss-typing, instead of your 
hotplug attempt failing with an error, your entire guest is destroyed.  That 
doesn't sound very nice to me.

Device initialization should never exit() (unless you really hit a fatal error 
like OOM).

>>> (No, this doesn't do compile time type checking. I don't
>>> think that's a problem particularly, or at least not enough
>>> of one to justify not doing it this way. The object model we
>>> have is dynamic and does things at runtime...)
>>
>> Correctness is more important to me than brevity.
>>
>> And really, we should focus on killing things like i8259_init().
>
> Functions like i8259_init() exist precisely because
> QOM/qdev don't provide brevity and people trying to
> use these devices do in fact value brevity.

No, they exist because we aren't modeling correctly.

i8259_init() is doing a few different things at once.  Once you split things 
between init and realize, you no longer have long chunks of redundant code.

> That's why
> I want the standard native "connect this thing to this
> other thing" function to be short and simple.

With my previous proposal, it's just:

s->irq_in = &b->int_out[0];

This is why I like exposing public members in structures.  It's brief and safe.

Regards,

Anthony Liguori

>
> -- PMM

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Peter Maydell]

On 1 May 2012 16:26, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 05/01/2012 10:20 AM, Peter Maydell wrote:
>> The assumption is that failure to connect is a fatal error,
>> and we can happily just assert()/hw_error()/etc.
>
> So that means that we have a bug from someone miss-typing, instead of your
> hotplug attempt failing with an error, your entire guest is destroyed.  That
> doesn't sound very nice to me.

If you're trying to hotplug a buggily implemented device then
all bets are off, really.

> Device initialization should never exit() (unless you really hit a fatal
> error like OOM).
>
>
>>>> (No, this doesn't do compile time type checking. I don't
>>>> think that's a problem particularly, or at least not enough
>>>> of one to justify not doing it this way. The object model we
>>>> have is dynamic and does things at runtime...)
>>>
>>>
>>> Correctness is more important to me than brevity.
>>>
>>> And really, we should focus on killing things like i8259_init().
>>
>>
>> Functions like i8259_init() exist precisely because
>> QOM/qdev don't provide brevity and people trying to
>> use these devices do in fact value brevity.
>
>
> No, they exist because we aren't modeling correctly.

Most of them are simply convenience functions that just
do "create, configure, wire up, init". (The ones that do
more than that need fixing anyway.)

> i8259_init() is doing a few different things at once.
> Once you split things between init and realize, you no longer
> have long chunks of redundant code.

...you have redundant code scattered in multiple places instead?

>> That's why
>> I want the standard native "connect this thing to this
>> other thing" function to be short and simple.
>
>
> With my previous proposal, it's just:
>
> s->irq_in = &b->int_out[0];
>
> This is why I like exposing public members in structures.  It's brief and
> safe.

Obvious question: why isn't property setting done this way,
then? Surely it's equally brief and safe to say
 cpu->level = def_level;
rather than
 object_property_set_int(OBJECT(cpu), def->level, "level", &error);

I don't particularly object to this sort of "public struct
vs private struct" object model, it's just not what you've
actually implemented.

-- PMM

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Anthony Liguori]

[-- Attachment #1: Type: text/plain, Size: 2943 bytes --]

On May 1, 2012 10:37 AM, "Peter Maydell" <peter.maydell@linaro.org> wrote:
>
> On 1 May 2012 16:26, Anthony Liguori <anthony@codemonkey.ws> wrote:
> > On 05/01/2012 10:20 AM, Peter Maydell wrote:
> >> The assumption is that failure to connect is a fatal error,
> >> and we can happily just assert()/hw_error()/etc.
> >
> > So that means that we have a bug from someone miss-typing, instead of
your
> > hotplug attempt failing with an error, your entire guest is destroyed.
 That
> > doesn't sound very nice to me.
>
> If you're trying to hotplug a buggily implemented device then
> all bets are off, really.

Its never okay to kill a guest.  We should fail gracefully when possible or
simply avoid failing in the first place.

> > Device initialization should never exit() (unless you really hit a fatal
> > error like OOM).
> >
> >
> >>>> (No, this doesn't do compile time type checking. I don't
> >>>> think that's a problem particularly, or at least not enough
> >>>> of one to justify not doing it this way. The object model we
> >>>> have is dynamic and does things at runtime...)
> >>>
> >>>
> >>> Correctness is more important to me than brevity.
> >>>
> >>> And really, we should focus on killing things like i8259_init().
> >>
> >>
> >> Functions like i8259_init() exist precisely because
> >> QOM/qdev don't provide brevity and people trying to
> >> use these devices do in fact value brevity.
> >
> >
> > No, they exist because we aren't modeling correctly.
>
> Most of them are simply convenience functions that just
> do "create, configure, wire up, init". (The ones that do
> more than that need fixing anyway.
> > i8259_init() is doing a few different things at once.
> > Once you split things between init and realize, you no longer
> > have long chunks of redundant code.
>
> ...you have redundant code scattered in multiple places instead?

Redundant is the wrong word.  It seems like the code ought to be reduced to
a single function call but it really cannot.

>
> >> That's why
> >> I want the standard native "connect this thing to this
> >> other thing" function to be short and simple.
> >
> >
> > With my previous proposal, it's just:
> >
> > s->irq_in = &b->int_out[0];
> >
> > This is why I like exposing public members in structures.  It's brief
and
> > safe.
>
> Obvious question: why isn't property setting done this way,
> then? Surely it's equally brief and safe to say
>  cpu->level = def_level;
> rather than
>  object_property_set_int(OBJECT(cpu), def->level, "level", &error);

A primary design consideration for QOM was to *not* require using the
generic property interface to set properties. That doesnt mean that you
can/should access all properties this way. It depends on the specific
property.

> I don't particularly object to this sort of "public struct
> vs private struct" object model, it's just not what you've
> actually implemented.

One step at a time.

Regards,

Anthony Liguori

>
> -- PMM

[-- Attachment #2: Type: text/html, Size: 3992 bytes --]

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Mark Cave-Ayland]

On 01/05/12 16:20, Peter Maydell wrote:

>> Correctness is more important to me than brevity.
>>
>> And really, we should focus on killing things like i8259_init().
>
> Functions like i8259_init() exist precisely because
> QOM/qdev don't provide brevity and people trying to
> use these devices do in fact value brevity. That's why
> I want the standard native "connect this thing to this
> other thing" function to be short and simple.

My understanding was that the *_init() functions were legacy and 
shouldn't be used any more - at least I've started removing them and 
replacing them with the slighty more long-winded QOM versions in my 
working tree.

Or should I just leave them as they are copy the bits I need to a 
separate initialisation function?


ATB,

Mark.

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Peter Maydell]

On 1 May 2012 19:57, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> wrote:
> On 01/05/12 16:20, Peter Maydell wrote:
>
>>> Correctness is more important to me than brevity.
>>>
>>> And really, we should focus on killing things like i8259_init().
>>
>>
>> Functions like i8259_init() exist precisely because
>> QOM/qdev don't provide brevity and people trying to
>> use these devices do in fact value brevity. That's why
>> I want the standard native "connect this thing to this
>> other thing" function to be short and simple.
>
>
> My understanding was that the *_init() functions were legacy and shouldn't
> be used any more - at least I've started removing them and replacing them
> with the slighty more long-winded QOM versions in my working tree.

Yes, that is my opinion and is why I'm arguing with Anthony here...

-- PMM

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Avi Kivity]

On 05/01/2012 04:50 PM, Anthony Liguori wrote:
> On 05/01/2012 08:01 AM, Avi Kivity wrote:
>> On 05/01/2012 03:49 PM, Peter Maydell wrote:
>>> On 1 May 2012 13:48, Avi Kivity<avi@redhat.com>  wrote:
>>>> On 05/01/2012 03:43 PM, Peter Maydell wrote:
>>>>> On 1 May 2012 13:42, Avi Kivity<avi@redhat.com>  wrote:
>>>>>> sysbus should just die.
>>>>>
>>>>> Totally agreed. It's not going to go quietly though...
>>>>
>>>> Not if you keep suggesting workarounds when I tell unsuspecting
>>>> developers to qomify their devices.
>>>
>>> When QOM supports (1) exporting gpio signals and
>
> This is trivial.  It'll come in as soon as 1.2 opens up.  If folks
> want to start working on a branch with it:
>
> https://github.com/aliguori/qemu/tree/qom-pin.4
>
>>> (2) exporting memory regions, then it will be providing the
>>> main things that sysbus provides.
>
> This is a little tricky.  Here's the problems I've encountered so far:
>
> a) A lot of devices need the equivalent of it_shift.  it_shift affects
> how addresses are decoded and the size of the memory region.  it_shift
> usually needs to be a device property.
>
> Since we need to know the size of the memory region to initialize it,
> we need to know the value of it_shift before we can initialize it
> which means we have to delay the initialization of the mmemory region
> until realize.
>
> I think a nice fix would be to make it_shift as memory region mutator
> and allow it to be set after initialization.

Indeed I wanted to make it_shift as part of the memory core.  But a
mutator?  It doesn't change in real hardware, so this looks artificial. 
So far all mutators really change at runtime.

What is the problem with delaying region initialization until realize?

>
> b) There's some duplication in MemoryRegions with respect to QOM. 
> Memory regions want to have a name but with QOM they'll be addressable
> via a path.  I go back and forth about how aggressively we want to
> refactor MemoryRegions.

These days region names are purely for debugging.  The ABI bit was moved
to a separate function.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Anthony Liguori]

On 05/01/2012 09:09 AM, Avi Kivity wrote:
> On 05/01/2012 04:50 PM, Anthony Liguori wrote:
>> On 05/01/2012 08:01 AM, Avi Kivity wrote:
>>> On 05/01/2012 03:49 PM, Peter Maydell wrote:
>>>> On 1 May 2012 13:48, Avi Kivity<avi@redhat.com>   wrote:
>>>>> On 05/01/2012 03:43 PM, Peter Maydell wrote:
>>>>>> On 1 May 2012 13:42, Avi Kivity<avi@redhat.com>   wrote:
>>>>>>> sysbus should just die.
>>>>>>
>>>>>> Totally agreed. It's not going to go quietly though...
>>>>>
>>>>> Not if you keep suggesting workarounds when I tell unsuspecting
>>>>> developers to qomify their devices.
>>>>
>>>> When QOM supports (1) exporting gpio signals and
>>
>> This is trivial.  It'll come in as soon as 1.2 opens up.  If folks
>> want to start working on a branch with it:
>>
>> https://github.com/aliguori/qemu/tree/qom-pin.4
>>
>>>> (2) exporting memory regions, then it will be providing the
>>>> main things that sysbus provides.
>>
>> This is a little tricky.  Here's the problems I've encountered so far:
>>
>> a) A lot of devices need the equivalent of it_shift.  it_shift affects
>> how addresses are decoded and the size of the memory region.  it_shift
>> usually needs to be a device property.
>>
>> Since we need to know the size of the memory region to initialize it,
>> we need to know the value of it_shift before we can initialize it
>> which means we have to delay the initialization of the mmemory region
>> until realize.
>>
>> I think a nice fix would be to make it_shift as memory region mutator
>> and allow it to be set after initialization.
>
> Indeed I wanted to make it_shift as part of the memory core.  But a
> mutator?  It doesn't change in real hardware, so this looks artificial.
> So far all mutators really change at runtime.

QOM has a concept of initialization and realize.  You can change properties 
after initialization but not before realize.

So as long as it_shift can be set after initialization but before realize (which 
I think is roughly memory_region_add_subregion) it doesn't need to be a mutator.

> What is the problem with delaying region initialization until realize?

We need to initialize the MemoryRegion in order to expose it as a property.  We 
want to do that during initialize.  Here's an example:

qom-create isa-i8259 foo
qom-set /peripheral/foo/io it_shift 1
qom-set /peripheral/foo/realize true

For this to work, it_shift needs to be a QOM property of the "io" MemoryRegion. 
  The MemoryRegion needs to be created in instance_init.

>> b) There's some duplication in MemoryRegions with respect to QOM.
>> Memory regions want to have a name but with QOM they'll be addressable
>> via a path.  I go back and forth about how aggressively we want to
>> refactor MemoryRegions.
>
> These days region names are purely for debugging.  The ABI bit was moved
> to a separate function.

Fair enough.

BTW, in the branch I've posted, I've got a number of memory API conversions or 
removal of legacy interfaces.

Regards,

Anthony Liguori

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Avi Kivity]

On 05/01/2012 05:15 PM, Anthony Liguori wrote:
>>> I think a nice fix would be to make it_shift as memory region mutator
>>> and allow it to be set after initialization.
>>
>> Indeed I wanted to make it_shift as part of the memory core.  But a
>> mutator?  It doesn't change in real hardware, so this looks artificial.
>> So far all mutators really change at runtime.
>
>
> QOM has a concept of initialization and realize.  You can change
> properties after initialization but not before realize.
>
> So as long as it_shift can be set after initialization but before
> realize (which I think is roughly memory_region_add_subregion) it
> doesn't need to be a mutator.

Ok, good.

>
>> What is the problem with delaying region initialization until realize?
>
> We need to initialize the MemoryRegion in order to expose it as a
> property.  We want to do that during initialize.  Here's an example:
>
> qom-create isa-i8259 foo
> qom-set /peripheral/foo/io it_shift 1
> qom-set /peripheral/foo/realize true
>
> For this to work, it_shift needs to be a QOM property of the "io"
> MemoryRegion.  The MemoryRegion needs to be created in instance_init.

So it looks like we need two phase initialization for memory regions as
well?

Not so pretty.

>
>>> b) There's some duplication in MemoryRegions with respect to QOM.
>>> Memory regions want to have a name but with QOM they'll be addressable
>>> via a path.  I go back and forth about how aggressively we want to
>>> refactor MemoryRegions.
>>
>> These days region names are purely for debugging.  The ABI bit was moved
>> to a separate function.
>
> Fair enough.
>
> BTW, in the branch I've posted, I've got a number of memory API
> conversions or removal of legacy interfaces.

Nice.  But you use get_system_io(), which is bad.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Anthony Liguori]

On 05/01/2012 09:26 AM, Avi Kivity wrote:
> On 05/01/2012 05:15 PM, Anthony Liguori wrote:
>>>> I think a nice fix would be to make it_shift as memory region mutator
>>>> and allow it to be set after initialization.
>>>
>>> Indeed I wanted to make it_shift as part of the memory core.  But a
>>> mutator?  It doesn't change in real hardware, so this looks artificial.
>>> So far all mutators really change at runtime.
>>
>>
>> QOM has a concept of initialization and realize.  You can change
>> properties after initialization but not before realize.
>>
>> So as long as it_shift can be set after initialization but before
>> realize (which I think is roughly memory_region_add_subregion) it
>> doesn't need to be a mutator.
>
> Ok, good.
>
>>
>>> What is the problem with delaying region initialization until realize?
>>
>> We need to initialize the MemoryRegion in order to expose it as a
>> property.  We want to do that during initialize.  Here's an example:
>>
>> qom-create isa-i8259 foo
>> qom-set /peripheral/foo/io it_shift 1
>> qom-set /peripheral/foo/realize true
>>
>> For this to work, it_shift needs to be a QOM property of the "io"
>> MemoryRegion.  The MemoryRegion needs to be created in instance_init.
>
> So it looks like we need two phase initialization for memory regions as
> well?

Yes, exactly.  Converting MemoryRegion to QOM will give it a two phase 
initialization FWIW.

> Not so pretty.

It's unavoidable because we're dealing with a graph.  The connections between 
objects may form loops which effectively means that we have dependency loops. 
That means we need to be able to create all objects first and then establish the 
links between them.  This is why we need two stage initialization.

>>>> b) There's some duplication in MemoryRegions with respect to QOM.
>>>> Memory regions want to have a name but with QOM they'll be addressable
>>>> via a path.  I go back and forth about how aggressively we want to
>>>> refactor MemoryRegions.
>>>
>>> These days region names are purely for debugging.  The ABI bit was moved
>>> to a separate function.
>>
>> Fair enough.
>>
>> BTW, in the branch I've posted, I've got a number of memory API
>> conversions or removal of legacy interfaces.
>
> Nice.  But you use get_system_io(), which is bad.

Only in the device setup code.  That will all eventually get moved into a 
SuperIO chip.  The PC code needs massive amounts of refactoring.

Regards,

Anthony Liguori

>

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Avi Kivity]

On 04/30/2012 04:43 PM, Anthony Liguori wrote:
> On 04/30/2012 08:36 AM, Avi Kivity wrote:
>> On 04/30/2012 04:27 PM, Peter Maydell wrote:
>>> On 30 April 2012 14:23, Avi Kivity<avi@redhat.com>  wrote:
>>>> IMO the best fix is to unsysbus the device and qomify it instead. 
>>>> This
>>>> way we're 100% flexible in how we can attach it.
>>>
>>> You don't need to wait for QOM to grow enough features to
>>> replace sysbus. If you don't like what sysbus_mmio_map() does, you
>>> can always use sysbus_mmio_get_region() to get the MemoryRegion* and
>>> then deal with it however you need to. This is the standard way
>>> to deal with "I have a sysbus device which I want to map into my
>>> custom container object".
>>
>> I believe that API voids you warrantee.
>
> All that a "QOM" conversion would do is eliminate the use of sysbus
> and derive the object directly from DeviceState.  Then, you would map
> the MemoryRegion exported by the device directly.
>
> So sysbus_mmio_get_region() seems like the right API to use.
>

I think you're right.  The real difference is that there is no longer an
implied container region (which is just get_system_memory() in current
sysbus).

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Avi Kivity]

On 04/30/2012 04:40 PM, Mark Cave-Ayland wrote:
> On 30/04/12 14:23, Avi Kivity wrote:
>
> Hi Avi,
>
>>> My understanding based upon this is that it would be impossible to
>>> register a different parent MemoryRegion without duplicating the init
>>> function for all shared devices which seems undesirable :(
>>
>> What are the requirements?  You need a different catch-all mmio handler
>> for each slot?  Or would one catch-all mmio handler for all sysbus
>> devices suffice?
>
> A single catch-all for all sysbus devices would suffice, however I'm
> thinking it makes sense to have one MemoryRegion per slot so that the
> devices can register onto the bus using their "slot relative" address
> to allow for potentially moving hardware between slots.

Ok, so you have a hierarchy: bus -> slot -> devices in slot?  That
hierarchy is present in the hardware too, or that's how I interpret
"slot relative addresses".

>
>>> The only solution I can think of is to make sysbus_mmio_map() more
>>> intelligent so that instead of blindly adding the device to the "root"
>>> MemoryRegion, it traverses down the MemoryRegion hierarchy starting
>>> from the root to the furtherest leaf node it can find based upon the
>>> specified address and then adds the new subregion there. Hence if I
>>> add my SBus memory regions first before call the various peripheral
>>> init functions, everything should end up in the correct part of the
>>> memory tree.
>>>
>>
>> This solution attempts to reconstruct the memory hierarchy from the
>> address, instead of maintaining it in the device tree.
>
> So I guess that is bad...

Well, it's a lot of work -> bad.

>
>>> I believe this should preserve compatibility for existing sysbus API
>>> users with just a single "root" MemoryRegion, however due to lack of
>>> any documentation related to sysbus I'm not sure if this would break
>>> other platforms or maybe even violate one of its key design features?
>>
>> IMO the best fix is to unsysbus the device and qomify it instead.  This
>> way we're 100% flexible in how we can attach it.
>
> That's interesting - I didn't realise that sysbus is a legacy
> interface with respect to QOM. 

Maybe it's just wishful thinking on my part.  But I always saw sysbus as
a wrong interface - it corresponds to no real bus and doesn't allow
hierarchy, unlike our pci or even isa implementations.

> Is there any documentation related to this? Then again, converting all
> of the devices over to QOM and testing that it doesn't break all
> platforms/busses suddenly becomes a huge job...
>

You can just follow Peter's suggestion, although qomification would be
preferable IMO.

-- 
error compiling committee.c: too many arguments to function