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

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

[Peter Maydell]

At the moment you can't really implement one sysbus device by saying
that it's composed of a set of other sysbus devices. This patch adds
new functions sysbus_pass_mmio() and sysbus_pass_one_irq() which
allow a sysbus device to delegate an MMIO or IRQ to another sysbus
device (The approach is inspired by the existing sysbus_pass_irq()
which lets a sysbus device delegate all its IRQs at once).

This works; the most obvious deficiency is that the subcomponent
device will still appear as its own device on the bus.

So: is this a reasonable solution to the problem, or an unacceptable
hack? Comments welcome :-)

The patchset includes a patch from Juha which implements resizing
and unmapping of sysbus MMIOs; this is sort of related but useful
on its own anyway [ie you're likely to see it again even if you
don't like patches 2 and 3 :-)]


Here's an example device init function that uses this:


static int omap3_hsusb_host_init(SysBusDevice *dev)
{
    OMAP3HSUSBHostState *s = FROM_SYSBUS(OMAP3HSUSBHostState, dev);
    SysBusDevice *ohci_busdev;

    /* Create the OHCI device which is our subcomponent */
    s->ohci_usb = qdev_create(dev->qdev.parent_bus, "sysbus-ohci");
    qdev_prop_set_uint32(s->ohci_usb, "num-ports", 3);
    qdev_prop_set_taddr(s->ohci_usb, "dma-offset", 0);
    qdev_init_nofail(s->ohci_usb);
    ohci_busdev = sysbus_from_qdev(s->ohci_usb);

    /* Our IRQ 0 is delegated to the OHCI; 1 and 2 we handle ourselves */
    sysbus_pass_one_irq(dev, ohci_busdev, 0);
    sysbus_init_irq(dev, &s->ehci_irq);
    sysbus_init_irq(dev, &s->tll_irq);
    /* MMIOs 0 and 1 created as usual and handled ourselves */
    sysbus_init_mmio(dev, 0x1000,
                     cpu_register_io_memory(omap3_hsusb_tll_readfn,
                                            omap3_hsusb_tll_writefn, s,
                                            DEVICE_NATIVE_ENDIAN));
    sysbus_init_mmio(dev, 0x400,
                     cpu_register_io_memory(omap3_hsusb_host_readfn,
                                            omap3_hsusb_host_writefn, s,
                                            DEVICE_NATIVE_ENDIAN));
    /* Resize the OHCI MMIO to fit our requirements, and expose it
     * as our MMIO 2
     */
    sysbus_mmio_resize(ohci_busdev, 0, 0x400);
    sysbus_pass_mmio(dev, ohci_busdev, 0);
    /* MMIO 3 is another normal case */
    sysbus_init_mmio(dev, 0x400,
                     cpu_register_io_memory(omap3_hsusb_ehci_readfn,
                                            omap3_hsusb_ehci_writefn, s,
                                            DEVICE_NATIVE_ENDIAN));
    vmstate_register(&dev->qdev, -1, &vmstate_omap3_hsusb_host, s);
    return 0;
}




Juha Riihimäki (1):
  sysbus: Add support for resizing and unmapping MMIOs

Peter Maydell (2):
  sysbus: Allow sysbus MMIO passthrough
  sysbus: Allow passthrough of single IRQ

 hw/sysbus.c |   72 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/sysbus.h |    6 +++++
 2 files changed, 78 insertions(+), 0 deletions(-)

[Qemu-devel] [PATCH RFC 1/3] sysbus: Add support for resizing and unmapping MMIOs

[Peter Maydell]

From: Juha Riihimäki <juha.riihimaki@nokia.com>

Add new functions sysbus_mmio_unmap() and sysbus_mmio_resize()
which allow a sysbus device user to trim the size of the
device's mmio region and also to unmap it.

The rationale here is twofold:
 * Some generic qdev devices might have an mmio region that
   doesn't match the specific implementation. For example
   usb-ohci creates an 0x1000 byte mmio for its registers,
   but the instantiation of the OHCI controller on OMAP3
   has only an 0x400 byte space in the memory map. Using
   resize lets us not worry about how overlapping mapped
   regions are resolved.
 * The OMAP GPMC (general purpose memory controller) has
   a set of registers which allow the guest to control
   whether and at what size various of its subdevices are
   mapped. At the moment QEMU's omap_gpmc model isn't
   qdevified, but when it and its children are this will
   be necessary.

Signed-off-by: Juha Riihimäki <juha.riihimaki@nokia.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/sysbus.c |   37 +++++++++++++++++++++++++++++++++++++
 hw/sysbus.h |    2 ++
 2 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/hw/sysbus.c b/hw/sysbus.c
index 2e22be7..01ebe47 100644
--- a/hw/sysbus.c
+++ b/hw/sysbus.c
@@ -39,6 +39,23 @@ void sysbus_connect_irq(SysBusDevice *dev, int n, qemu_irq irq)
     }
 }
 
+void sysbus_mmio_unmap(SysBusDevice *dev, int n)
+{
+    assert(n >= 0 && n < dev->num_mmio);
+
+    if (dev->mmio[n].addr == (target_phys_addr_t)-1) {
+        /* region already unmapped */
+        return;
+    }
+    if (dev->mmio[n].cb) {
+        dev->mmio[n].cb(dev, (target_phys_addr_t)-1);
+    } else {
+        cpu_register_physical_memory(dev->mmio[n].addr, dev->mmio[n].size,
+                                     IO_MEM_UNASSIGNED);
+    }
+    dev->mmio[n].addr = (target_phys_addr_t)-1;
+}
+
 void sysbus_mmio_map(SysBusDevice *dev, int n, target_phys_addr_t addr)
 {
     assert(n >= 0 && n < dev->num_mmio);
@@ -61,6 +78,26 @@ void sysbus_mmio_map(SysBusDevice *dev, int n, target_phys_addr_t addr)
     }
 }
 
+void sysbus_mmio_resize(SysBusDevice *dev, int n, target_phys_addr_t newsize)
+{
+    target_phys_addr_t addr;
+    assert(n >= 0 && n < dev->num_mmio);
+
+    if (newsize != dev->mmio[n].size) {
+        addr = dev->mmio[n].addr;
+        if (addr != (target_phys_addr_t)-1) {
+            /* The expected use case is that resizes will only happen
+             * on unmapped regions, but we handle the already-mapped
+             * case by temporarily unmapping and remapping.
+             */
+            sysbus_mmio_unmap(dev, n);
+        }
+        dev->mmio[n].size = newsize;
+        if (addr != (target_phys_addr_t)-1) {
+            sysbus_mmio_map(dev, n, addr);
+        }
+    }
+}
 
 /* Request an IRQ source.  The actual IRQ object may be populated later.  */
 void sysbus_init_irq(SysBusDevice *dev, qemu_irq *p)
diff --git a/hw/sysbus.h b/hw/sysbus.h
index 4e8cb16..70e2488 100644
--- a/hw/sysbus.h
+++ b/hw/sysbus.h
@@ -53,6 +53,8 @@ void sysbus_init_ioports(SysBusDevice *dev, pio_addr_t ioport, pio_addr_t size);
 
 void sysbus_connect_irq(SysBusDevice *dev, int n, qemu_irq irq);
 void sysbus_mmio_map(SysBusDevice *dev, int n, target_phys_addr_t addr);
+void sysbus_mmio_unmap(SysBusDevice *dev, int n);
+void sysbus_mmio_resize(SysBusDevice *dev, int n, target_phys_addr_t newsize);
 
 /* Legacy helper function for creating devices.  */
 DeviceState *sysbus_create_varargs(const char *name,
-- 
1.7.1

[Qemu-devel] [PATCH RFC 2/3] sysbus: Allow sysbus MMIO passthrough

[Peter Maydell]

Add new function sysbus_pass_mmio() to allow a sysbus device to
delegate MMIO to another sysbus device. This allows a limited
form of composition for sysbus devices.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/sysbus.c |   26 ++++++++++++++++++++++++++
 hw/sysbus.h |    3 +++
 2 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/hw/sysbus.c b/hw/sysbus.c
index 01ebe47..793b0c1 100644
--- a/hw/sysbus.c
+++ b/hw/sysbus.c
@@ -43,6 +43,10 @@ void sysbus_mmio_unmap(SysBusDevice *dev, int n)
 {
     assert(n >= 0 && n < dev->num_mmio);
 
+    if (dev->mmio[n].delegate) {
+        sysbus_mmio_unmap(dev->mmio[n].delegate, dev->mmio[n].delegate_mmio);
+        return;
+    }
     if (dev->mmio[n].addr == (target_phys_addr_t)-1) {
         /* region already unmapped */
         return;
@@ -60,6 +64,11 @@ void sysbus_mmio_map(SysBusDevice *dev, int n, target_phys_addr_t addr)
 {
     assert(n >= 0 && n < dev->num_mmio);
 
+    if (dev->mmio[n].delegate) {
+        sysbus_mmio_map(dev->mmio[n].delegate, dev->mmio[n].delegate_mmio,
+                        addr);
+        return;
+    }
     if (dev->mmio[n].addr == addr) {
         /* ??? region already mapped here.  */
         return;
@@ -83,6 +92,11 @@ void sysbus_mmio_resize(SysBusDevice *dev, int n, target_phys_addr_t newsize)
     target_phys_addr_t addr;
     assert(n >= 0 && n < dev->num_mmio);
 
+    if (dev->mmio[n].delegate) {
+        sysbus_mmio_resize(dev->mmio[n].delegate, dev->mmio[n].delegate_mmio,
+                           newsize);
+        return;
+    }
     if (newsize != dev->mmio[n].size) {
         addr = dev->mmio[n].addr;
         if (addr != (target_phys_addr_t)-1) {
@@ -127,6 +141,7 @@ void sysbus_init_mmio(SysBusDevice *dev, target_phys_addr_t size,
 
     assert(dev->num_mmio < QDEV_MAX_MMIO);
     n = dev->num_mmio++;
+    dev->mmio[n].delegate = 0;
     dev->mmio[n].addr = -1;
     dev->mmio[n].size = size;
     dev->mmio[n].iofunc = iofunc;
@@ -139,11 +154,22 @@ void sysbus_init_mmio_cb(SysBusDevice *dev, target_phys_addr_t size,
 
     assert(dev->num_mmio < QDEV_MAX_MMIO);
     n = dev->num_mmio++;
+    dev->mmio[n].delegate = 0;
     dev->mmio[n].addr = -1;
     dev->mmio[n].size = size;
     dev->mmio[n].cb = cb;
 }
 
+void sysbus_pass_mmio(SysBusDevice *dev, SysBusDevice *target, int target_mmio)
+{
+    int n;
+
+    assert(dev->num_mmio < QDEV_MAX_MMIO);
+    n = dev->num_mmio++;
+    dev->mmio[n].delegate = target;
+    dev->mmio[n].delegate_mmio = target_mmio;
+}
+
 void sysbus_init_ioports(SysBusDevice *dev, pio_addr_t ioport, pio_addr_t size)
 {
     pio_addr_t i;
diff --git a/hw/sysbus.h b/hw/sysbus.h
index 70e2488..789e4c5 100644
--- a/hw/sysbus.h
+++ b/hw/sysbus.h
@@ -19,6 +19,8 @@ struct SysBusDevice {
     qemu_irq *irqp[QDEV_MAX_IRQ];
     int num_mmio;
     struct {
+        SysBusDevice *delegate;
+        int delegate_mmio;
         target_phys_addr_t addr;
         target_phys_addr_t size;
         mmio_mapfunc cb;
@@ -46,6 +48,7 @@ void sysbus_init_mmio(SysBusDevice *dev, target_phys_addr_t size,
                       ram_addr_t iofunc);
 void sysbus_init_mmio_cb(SysBusDevice *dev, target_phys_addr_t size,
                             mmio_mapfunc cb);
+void sysbus_pass_mmio(SysBusDevice *dev, SysBusDevice *target, int target_mmio);
 void sysbus_init_irq(SysBusDevice *dev, qemu_irq *p);
 void sysbus_pass_irq(SysBusDevice *dev, SysBusDevice *target);
 void sysbus_init_ioports(SysBusDevice *dev, pio_addr_t ioport, pio_addr_t size);
-- 
1.7.1

[Qemu-devel] [PATCH RFC 3/3] sysbus: Allow passthrough of single IRQ

[Peter Maydell]

Add a sysbus_pass_one_irq() function to allow one sysbus device
to pass a single IRQ through to another. (It is already possible
to delegate all your IRQs to another device with sysbus_pass_irq().)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/sysbus.c |    9 +++++++++
 hw/sysbus.h |    1 +
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/hw/sysbus.c b/hw/sysbus.c
index 793b0c1..8211e8f 100644
--- a/hw/sysbus.c
+++ b/hw/sysbus.c
@@ -134,6 +134,15 @@ void sysbus_pass_irq(SysBusDevice *dev, SysBusDevice *target)
     }
 }
 
+void sysbus_pass_one_irq(SysBusDevice *dev, SysBusDevice *target, int tn)
+{
+    int n;
+
+    assert(dev->num_irq < QDEV_MAX_IRQ);
+    n = dev->num_irq++;
+    dev->irqp[n] = target->irqp[tn];
+}
+
 void sysbus_init_mmio(SysBusDevice *dev, target_phys_addr_t size,
                       ram_addr_t iofunc)
 {
diff --git a/hw/sysbus.h b/hw/sysbus.h
index 789e4c5..a6a873c 100644
--- a/hw/sysbus.h
+++ b/hw/sysbus.h
@@ -51,6 +51,7 @@ void sysbus_init_mmio_cb(SysBusDevice *dev, target_phys_addr_t size,
 void sysbus_pass_mmio(SysBusDevice *dev, SysBusDevice *target, int target_mmio);
 void sysbus_init_irq(SysBusDevice *dev, qemu_irq *p);
 void sysbus_pass_irq(SysBusDevice *dev, SysBusDevice *target);
+void sysbus_pass_one_irq(SysBusDevice *dev, SysBusDevice *target, int tn);
 void sysbus_init_ioports(SysBusDevice *dev, pio_addr_t ioport, pio_addr_t size);
 
 
-- 
1.7.1

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

[Jan Kiszka]

On 2011-06-08 13:33, Peter Maydell wrote:
> At the moment you can't really implement one sysbus device by saying
> that it's composed of a set of other sysbus devices. This patch adds
> new functions sysbus_pass_mmio() and sysbus_pass_one_irq() which
> allow a sysbus device to delegate an MMIO or IRQ to another sysbus
> device (The approach is inspired by the existing sysbus_pass_irq()
> which lets a sysbus device delegate all its IRQs at once).
> 
> This works; the most obvious deficiency is that the subcomponent
> device will still appear as its own device on the bus.
> 
> So: is this a reasonable solution to the problem, or an unacceptable
> hack? Comments welcome :-)

Sounds more like a little hack. :)

The relationships should be expressed via qdev, not yet another
sysbus-specific extension. Generally, many services of sysbus should
rather be generic qdev things.

Is there anything that today prevents creating a local bus and attaching
the component devices to that? If it's multi-bus support, that should to
be added anyway. Passing-through of MMIO and IRQs is still a worthwhile
generic service, then probably qbus associated.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

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

[Markus Armbruster]

Jan Kiszka <jan.kiszka@siemens.com> writes:

> On 2011-06-08 13:33, Peter Maydell wrote:
>> At the moment you can't really implement one sysbus device by saying
>> that it's composed of a set of other sysbus devices. This patch adds
>> new functions sysbus_pass_mmio() and sysbus_pass_one_irq() which
>> allow a sysbus device to delegate an MMIO or IRQ to another sysbus
>> device (The approach is inspired by the existing sysbus_pass_irq()
>> which lets a sysbus device delegate all its IRQs at once).
>> 
>> This works; the most obvious deficiency is that the subcomponent
>> device will still appear as its own device on the bus.
>> 
>> So: is this a reasonable solution to the problem, or an unacceptable
>> hack? Comments welcome :-)
>
> Sounds more like a little hack. :)
>
> The relationships should be expressed via qdev, not yet another
> sysbus-specific extension. Generally, many services of sysbus should
> rather be generic qdev things.

Examples?

> Is there anything that today prevents creating a local bus and attaching
> the component devices to that? If it's multi-bus support, that should to
> be added anyway. Passing-through of MMIO and IRQs is still a worthwhile
> generic service, then probably qbus associated.

Do you mean making the container device a sysbus-sysbus-bridge, then
hanging the component devices off the inner sysbus?

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

[Jan Kiszka]

On 2011-06-09 18:40, Markus Armbruster wrote:
> Jan Kiszka <jan.kiszka@siemens.com> writes:
> 
>> On 2011-06-08 13:33, Peter Maydell wrote:
>>> At the moment you can't really implement one sysbus device by saying
>>> that it's composed of a set of other sysbus devices. This patch adds
>>> new functions sysbus_pass_mmio() and sysbus_pass_one_irq() which
>>> allow a sysbus device to delegate an MMIO or IRQ to another sysbus
>>> device (The approach is inspired by the existing sysbus_pass_irq()
>>> which lets a sysbus device delegate all its IRQs at once).
>>>
>>> This works; the most obvious deficiency is that the subcomponent
>>> device will still appear as its own device on the bus.
>>>
>>> So: is this a reasonable solution to the problem, or an unacceptable
>>> hack? Comments welcome :-)
>>
>> Sounds more like a little hack. :)
>>
>> The relationships should be expressed via qdev, not yet another
>> sysbus-specific extension. Generally, many services of sysbus should
>> rather be generic qdev things.
> 
> Examples?

Resource management, e.g. IRQs. That will be useful for other types of
buses as well.

> 
>> Is there anything that today prevents creating a local bus and attaching
>> the component devices to that? If it's multi-bus support, that should to
>> be added anyway. Passing-through of MMIO and IRQs is still a worthwhile
>> generic service, then probably qbus associated.
> 
> Do you mean making the container device a sysbus-sysbus-bridge, then
> hanging the component devices off the inner sysbus?

And how to apply this concept on a composed PCI device e.g.?

Maybe we could define something like Linux' struct resource + a set of
helper services for it.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

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

[Markus Armbruster]

Jan Kiszka <jan.kiszka@siemens.com> writes:

> On 2011-06-09 18:40, Markus Armbruster wrote:
>> Jan Kiszka <jan.kiszka@siemens.com> writes:
>> 
>>> On 2011-06-08 13:33, Peter Maydell wrote:
>>>> At the moment you can't really implement one sysbus device by saying
>>>> that it's composed of a set of other sysbus devices. This patch adds
>>>> new functions sysbus_pass_mmio() and sysbus_pass_one_irq() which
>>>> allow a sysbus device to delegate an MMIO or IRQ to another sysbus
>>>> device (The approach is inspired by the existing sysbus_pass_irq()
>>>> which lets a sysbus device delegate all its IRQs at once).
>>>>
>>>> This works; the most obvious deficiency is that the subcomponent
>>>> device will still appear as its own device on the bus.
>>>>
>>>> So: is this a reasonable solution to the problem, or an unacceptable
>>>> hack? Comments welcome :-)
>>>
>>> Sounds more like a little hack. :)
>>>
>>> The relationships should be expressed via qdev, not yet another
>>> sysbus-specific extension. Generally, many services of sysbus should
>>> rather be generic qdev things.
>> 
>> Examples?
>
> 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.

A device or bus should be able to offer IRQ lines.  Feels generic, too.

Connecting the two is configuration.  Requires a suitable naming scheme
for IRQ lines.  Naming might have to include bus-specific sugar for
user-friendliness.  For instance, I'd rather express "isa-serial uses
ISA interrupt 4" as "irq=4" than as something like
"irq=PIIX3/isa.0:irq.4".

I doubt all resources are as generic as IRQ lines, but that's okay.

>>> Is there anything that today prevents creating a local bus and attaching
>>> the component devices to that? If it's multi-bus support, that should to
>>> be added anyway. Passing-through of MMIO and IRQs is still a worthwhile
>>> generic service, then probably qbus associated.
>> 
>> Do you mean making the container device a sysbus-sysbus-bridge, then
>> hanging the component devices off the inner sysbus?
>
> And how to apply this concept on a composed PCI device e.g.?

Make the PCI device provide a sysbus for its components?  Dunno...

See also thread "[RFC v4 00/12] ISA reconfigurability v4".

> Maybe we could define something like Linux' struct resource + a set of
> helper services for it.

Device component composition is not (yet?) covered by qdev.  Of course
we compose anyway, in various ad hoc ways.

One way is to put the components' state structs into the device's state
struct, and define suitable wrappers.  For instance, we have qdevs
"sysbus-fdc" and "isa-fdc".  They both contain the FDC proper as a
component: FDCtrlSysBus and FDCtrlISABus contain a FDCtrl member.
Simple enough.

A different way to adapt the same component to different buses can be
found in virtio: VirtIOPCIProxy and VirtIOS390Device both contain
pointers to VirtIODevice.  I found that one quite awkward to work with.

Yet another way can be found in usb-storage.  usb-storage expands into
two qdevs connected by a qbus: it provides a SCSI bus, and automatically
creates a single scsi-disk on that bus.  One of those tricks that look
cute initially, then create no end of trouble.

I figure a "qdevy" composition mechanism would be useful.  Needs
thought.

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

[Anthony Liguori]

On 06/10/2011 03:13 AM, Markus Armbruster wrote:
> Jan Kiszka<jan.kiszka@siemens.com>  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]

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.

> Connecting the two is configuration.  Requires a suitable naming scheme
> for IRQ lines.  Naming might have to include bus-specific sugar for
> user-friendliness.  For instance, I'd rather express "isa-serial uses
> ISA interrupt 4" as "irq=4" than as something like
> "irq=PIIX3/isa.0:irq.4".

That's just syntactic sugar.  It can live at a higher level than the 
qdev API.

> I doubt all resources are as generic as IRQ lines, but that's okay.

They pretty much are.  We're really just talking about referring to 
subcomponents of a device.  That's what's lacking today.

> Device component composition is not (yet?) covered by qdev.  Of course
> we compose anyway, in various ad hoc ways.

Busses need to die.  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).

Regards,

Anthony Liguori

> One way is to put the components' state structs into the device's state
> struct, and define suitable wrappers.  For instance, we have qdevs
> "sysbus-fdc" and "isa-fdc".  They both contain the FDC proper as a
> component: FDCtrlSysBus and FDCtrlISABus contain a FDCtrl member.
> Simple enough.
>
> A different way to adapt the same component to different buses can be
> found in virtio: VirtIOPCIProxy and VirtIOS390Device both contain
> pointers to VirtIODevice.  I found that one quite awkward to work with.
>
> Yet another way can be found in usb-storage.  usb-storage expands into
> two qdevs connected by a qbus: it provides a SCSI bus, and automatically
> creates a single scsi-disk on that bus.  One of those tricks that look
> cute initially, then create no end of trouble.
>
> I figure a "qdevy" composition mechanism would be useful.  Needs
> thought.

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

[Peter Maydell]

On 10 June 2011 13:51, Anthony Liguori <aliguori@us.ibm.com> wrote:
> On 06/10/2011 03:13 AM, Markus Armbruster wrote:
>>
>> Jan Kiszka<jan.kiszka@siemens.com>  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.

Actually, devices have input and output I/O signals (GPIOs, if you like).
A subset of these are IRQs. We already have some APIs in QEMU which
claim to be dealing with 'irq's but actually are just for wiring
up generic signals; I'd rather we didn't proliferate that terminology
confusion if possible. (And a single GPIO wire is just one kind of
thing you might want to link between two devices, obviously. MMIO is
another.)

> 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]

This makes the wiring of this signal look like a property of the
isa-serial device, which is a bit odd, since it's just as much
a property of the piix3. Actually it's neither, it's a property
of the machine model, and you might actually want a syntax a bit
more like:

 piix3 = piix3(property=value, property=value...);
 serial = isa-serial(property=value...);
 connect(serial.irq, piix3.irq[3]);

(in some mythical stitching language, which I think makes much
more sense than command line switches anyway.)

-- PMM

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

[Jan Kiszka]

On 2011-06-10 15:10, Peter Maydell wrote:
> On 10 June 2011 13:51, Anthony Liguori <aliguori@us.ibm.com> wrote:
>> On 06/10/2011 03:13 AM, Markus Armbruster wrote:
>>>
>>> Jan Kiszka<jan.kiszka@siemens.com>  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.

Generally true. Two exceptions still require to make the path explorable
/ transfer information in the reverse direction: IRQ de-coalescing and
physical device assignment. That's something a new generic API should
encapsulate so that we can stop peaking into the machine details.

>>
>> 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.
> 
> Actually, devices have input and output I/O signals (GPIOs, if you like).
> A subset of these are IRQs. We already have some APIs in QEMU which
> claim to be dealing with 'irq's but actually are just for wiring
> up generic signals; I'd rather we didn't proliferate that terminology
> confusion if possible. (And a single GPIO wire is just one kind of
> thing you might want to link between two devices, obviously. MMIO is
> another.)
> 
>> 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]
> 
> This makes the wiring of this signal look like a property of the
> isa-serial device, which is a bit odd, since it's just as much
> a property of the piix3. Actually it's neither, it's a property
> of the machine model, and you might actually want a syntax a bit
> more like:
> 
>  piix3 = piix3(property=value, property=value...);
>  serial = isa-serial(property=value...);
>  connect(serial.irq, piix3.irq[3]);

In fact, in the ISA case, it is a device property: The device, and only
the device decides which IRQ to use - from the bus it is attached to. So
attaching an ISA device to the bus of an ISA bridge like the PIIX3 and
selecting local IRQ 3 are the steps we can already express today.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

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

[Peter Maydell]

On 10 June 2011 14:43, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2011-06-10 15:10, Peter Maydell wrote:
>> This makes the wiring of this signal look like a property of the
>> isa-serial device, which is a bit odd, since it's just as much
>> a property of the piix3. Actually it's neither, it's a property
>> of the machine model, and you might actually want a syntax a bit
>> more like:
>>
>>  piix3 = piix3(property=value, property=value...);
>>  serial = isa-serial(property=value...);
>>  connect(serial.irq, piix3.irq[3]);
>
> In fact, in the ISA case, it is a device property: The device, and only
> the device decides which IRQ to use - from the bus it is attached to. So
> attaching an ISA device to the bus of an ISA bridge like the PIIX3 and
> selecting local IRQ 3 are the steps we can already express today.

Ah, in that case Anthony's suggestion of
  -device piix3,id=piix3 -device isa-serial,id=serial,irq=piix3.irq[3]
wrong in a different way -- the isa-serial shouldn't care
what other device is providing the ISA bus it is sitting on,
it just has a property of which ISA irq line it is using
(and rely on an isa bus abstraction to wire things up at
the machine model level). [As you say, this works now.]

But I think that's a non-typical case compared to the usual one
of "these wires are just hardwired this way by the machine".

-- PMM

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

[Anthony Liguori]

On 06/10/2011 08:10 AM, Peter Maydell wrote:
> On 10 June 2011 13:51, Anthony Liguori<aliguori@us.ibm.com>  wrote:
>> On 06/10/2011 03:13 AM, Markus Armbruster wrote:
>>>
>>> Jan Kiszka<jan.kiszka@siemens.com>    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.
>
> Actually, devices have input and output I/O signals (GPIOs, if you like).

Yes, I prefer the term Pin but since the discussion was using IRQs, I 
didn't want to rock the boat ;-)

> A subset of these are IRQs. We already have some APIs in QEMU which
> claim to be dealing with 'irq's but actually are just for wiring
> up generic signals; I'd rather we didn't proliferate that terminology
> confusion if possible. (And a single GPIO wire is just one kind of
> thing you might want to link between two devices, obviously. MMIO is
> another.)

Forget about MMIO.  From a device perspective, MMIO is an extremely high 
level concept.  MMIO almost always involves some higher level interface 
(like PCI, ISA, etc.).

That's really the interface that you want to model.  Sysbus ends up 
being treated as a generic bus in-lieu of implementing machine specific 
buses (probably because they're proprietary and undocumented).  That's 
sort of okay but we should treat that not as the parent bus of 
everything but as a generic bus type.

>> 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]
>
> This makes the wiring of this signal look like a property of the
> isa-serial device, which is a bit odd, since it's just as much
> a property of the piix3. Actually it's neither, it's a property
> of the machine model, and you might actually want a syntax a bit
> more like:

Think of it like a graph with directed paths.  The property is the end 
point of the connection.

But the key point is: the connections of the graph *IS* the machine model.

>
>   piix3 = piix3(property=value, property=value...);
>   serial = isa-serial(property=value...);
>   connect(serial.irq, piix3.irq[3]);

You can use a different verb but essentially, it's still:

serial.irq OPERATOR piix3.irq[3]

Picking a direction and using assignment is convenient syntactically.

Regards,

Anthony Liguori

>
> (in some mythical stitching language, which I think makes much
> more sense than command line switches anyway.)
>
> -- PMM

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

[Anthony Liguori]

On 06/10/2011 08:43 AM, Jan Kiszka wrote:
> On 2011-06-10 15:10, Peter Maydell wrote:
>> On 10 June 2011 13:51, Anthony Liguori<aliguori@us.ibm.com>  wrote:
>>> On 06/10/2011 03:13 AM, Markus Armbruster wrote:
>>>>
>>>> Jan Kiszka<jan.kiszka@siemens.com>   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.
>
> Generally true. Two exceptions still require to make the path explorable
> / transfer information in the reverse direction: IRQ de-coalescing and
> physical device assignment. That's something a new generic API should
> encapsulate so that we can stop peaking into the machine details.
>
>>>
>>> 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.
>>
>> Actually, devices have input and output I/O signals (GPIOs, if you like).
>> A subset of these are IRQs. We already have some APIs in QEMU which
>> claim to be dealing with 'irq's but actually are just for wiring
>> up generic signals; I'd rather we didn't proliferate that terminology
>> confusion if possible. (And a single GPIO wire is just one kind of
>> thing you might want to link between two devices, obviously. MMIO is
>> another.)
>>
>>> 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]
>>
>> This makes the wiring of this signal look like a property of the
>> isa-serial device, which is a bit odd, since it's just as much
>> a property of the piix3. Actually it's neither, it's a property
>> of the machine model, and you might actually want a syntax a bit
>> more like:
>>
>>   piix3 = piix3(property=value, property=value...);
>>   serial = isa-serial(property=value...);
>>   connect(serial.irq, piix3.irq[3]);
>
> In fact, in the ISA case, it is a device property: The device, and only
> the device decides which IRQ to use - from the bus it is attached to. So
> attaching an ISA device to the bus of an ISA bridge like the PIIX3 and
> selecting local IRQ 3 are the steps we can already express today.

If you really want to be pedantic, each ISA device has 5 input Pins that 
are supposed to correspond to IRQ 3, IRQ 4, IRQ 5, IRQ 6, and IRQ 7.

This could easily be modelled by doing the following:

  -device piix3,id=piix3 -device 
isa-serial,id=serial,irq[3]=piix3.irq[3],irq[4]=piix3.irq[4],...

But I don't think we benefit from modelling it this correctly.  The 
point is that the infrastructure could handle it though.

Regards,

Anthony Liguori

>
> Jan
>

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

[Jan Kiszka]

On 2011-06-10 16:12, Anthony Liguori wrote:
> On 06/10/2011 08:43 AM, Jan Kiszka wrote:
>> On 2011-06-10 15:10, Peter Maydell wrote:
>>> On 10 June 2011 13:51, Anthony Liguori<aliguori@us.ibm.com>  wrote:
>>>> On 06/10/2011 03:13 AM, Markus Armbruster wrote:
>>>>>
>>>>> Jan Kiszka<jan.kiszka@siemens.com>   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.
>>
>> Generally true. Two exceptions still require to make the path explorable
>> / transfer information in the reverse direction: IRQ de-coalescing and
>> physical device assignment. That's something a new generic API should
>> encapsulate so that we can stop peaking into the machine details.
>>
>>>>
>>>> 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.
>>>
>>> Actually, devices have input and output I/O signals (GPIOs, if you like).
>>> A subset of these are IRQs. We already have some APIs in QEMU which
>>> claim to be dealing with 'irq's but actually are just for wiring
>>> up generic signals; I'd rather we didn't proliferate that terminology
>>> confusion if possible. (And a single GPIO wire is just one kind of
>>> thing you might want to link between two devices, obviously. MMIO is
>>> another.)
>>>
>>>> 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]
>>>
>>> This makes the wiring of this signal look like a property of the
>>> isa-serial device, which is a bit odd, since it's just as much
>>> a property of the piix3. Actually it's neither, it's a property
>>> of the machine model, and you might actually want a syntax a bit
>>> more like:
>>>
>>>   piix3 = piix3(property=value, property=value...);
>>>   serial = isa-serial(property=value...);
>>>   connect(serial.irq, piix3.irq[3]);
>>
>> In fact, in the ISA case, it is a device property: The device, and only
>> the device decides which IRQ to use - from the bus it is attached to. So
>> attaching an ISA device to the bus of an ISA bridge like the PIIX3 and
>> selecting local IRQ 3 are the steps we can already express today.
> 
> If you really want to be pedantic, each ISA device has 5 input Pins that 
> are supposed to correspond to IRQ 3, IRQ 4, IRQ 5, IRQ 6, and IRQ 7.
> 
> This could easily be modelled by doing the following:
> 
>   -device piix3,id=piix3 -device 
> isa-serial,id=serial,irq[3]=piix3.irq[3],irq[4]=piix3.irq[4],...
> 
> But I don't think we benefit from modelling it this correctly.  The 
> point is that the infrastructure could handle it though.

I don't see the point of 'piix3.' in your IRQ assignment, though. It is
redundant to the bus assignment - as the ISA bus also routes interrupts.
There are surely also buses that don't. Still, interrupt routing via the
bus should be possible (as it avoids boilerplate code or configuration
redundancy).

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

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

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 10 June 2011 14:43, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On 2011-06-10 15:10, Peter Maydell wrote:
>>> This makes the wiring of this signal look like a property of the
>>> isa-serial device, which is a bit odd, since it's just as much
>>> a property of the piix3. Actually it's neither, it's a property
>>> of the machine model, and you might actually want a syntax a bit
>>> more like:
>>>
>>>  piix3 = piix3(property=value, property=value...);
>>>  serial = isa-serial(property=value...);
>>>  connect(serial.irq, piix3.irq[3]);
>>
>> In fact, in the ISA case, it is a device property: The device, and only
>> the device decides which IRQ to use - from the bus it is attached to. So
>> attaching an ISA device to the bus of an ISA bridge like the PIIX3 and
>> selecting local IRQ 3 are the steps we can already express today.
>
> Ah, in that case Anthony's suggestion of
>   -device piix3,id=piix3 -device isa-serial,id=serial,irq=piix3.irq[3]
> wrong in a different way -- the isa-serial shouldn't care
> what other device is providing the ISA bus it is sitting on,
> it just has a property of which ISA irq line it is using
> (and rely on an isa bus abstraction to wire things up at
> the machine model level). [As you say, this works now.]
>
> But I think that's a non-typical case compared to the usual one
> of "these wires are just hardwired this way by the machine".

IIRC, the PCI bus also provides a number of IRQ lines for the device to
tickle (INTA#..INTD#).  There's rarely a need to configure their use,
though.

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

[Anthony Liguori]

On 06/10/2011 09:18 AM, Jan Kiszka wrote:
> On 2011-06-10 16:12, Anthony Liguori wrote:
>> On 06/10/2011 08:43 AM, Jan Kiszka wrote:
>>    -device piix3,id=piix3 -device
>> isa-serial,id=serial,irq[3]=piix3.irq[3],irq[4]=piix3.irq[4],...
>>
>> But I don't think we benefit from modelling it this correctly.  The
>> point is that the infrastructure could handle it though.
>
> I don't see the point of 'piix3.' in your IRQ assignment, though. It is
> redundant to the bus assignment

Bus assignment is the problem.

IRQ routing cannot be tied strictly to buses if we want to be able to do 
composition.

You could do clever things syntactically but the general mechanism is 
needed is the ability to wire things in an arbitrary fashion.

Regards,

Anthony Liguori

  - as the ISA bus also routes interrupts.
> There are surely also buses that don't. Still, interrupt routing via the
> bus should be possible (as it avoids boilerplate code or configuration
> redundancy).
>
> Jan
>

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

[Anthony Liguori]

On 06/10/2011 08:50 AM, Peter Maydell wrote:
> On 10 June 2011 14:43, Jan Kiszka<jan.kiszka@siemens.com>  wrote:
>> On 2011-06-10 15:10, Peter Maydell wrote:
>>> This makes the wiring of this signal look like a property of the
>>> isa-serial device, which is a bit odd, since it's just as much
>>> a property of the piix3. Actually it's neither, it's a property
>>> of the machine model, and you might actually want a syntax a bit
>>> more like:
>>>
>>>   piix3 = piix3(property=value, property=value...);
>>>   serial = isa-serial(property=value...);
>>>   connect(serial.irq, piix3.irq[3]);
>>
>> In fact, in the ISA case, it is a device property: The device, and only
>> the device decides which IRQ to use - from the bus it is attached to. So
>> attaching an ISA device to the bus of an ISA bridge like the PIIX3 and
>> selecting local IRQ 3 are the steps we can already express today.
>
> Ah, in that case Anthony's suggestion of
>    -device piix3,id=piix3 -device isa-serial,id=serial,irq=piix3.irq[3]
> wrong in a different way -- the isa-serial shouldn't care
> what other device is providing the ISA bus it is sitting on,

If it really makes you feel bad, you could also do:

  -device piix3,id=piix3
  -device wire,id=wire,in=piix3.irq[3]
  -device isa-serial,id=serial,irq=wire.out

Regards,

Anthony Liguori

> it just has a property of which ISA irq line it is using
> (and rely on an isa bus abstraction to wire things up at
> the machine model level). [As you say, this works now.]
>
> But I think that's a non-typical case compared to the usual one
> of "these wires are just hardwired this way by the machine".
>
> -- PMM

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

[Anthony Liguori]

On 06/10/2011 09:22 AM, Markus Armbruster wrote:
> Peter Maydell<peter.maydell@linaro.org>  writes:
>
>> But I think that's a non-typical case compared to the usual one
>> of "these wires are just hardwired this way by the machine".
>
> IIRC, the PCI bus also provides a number of IRQ lines for the device to
> tickle (INTA#..INTD#).  There's rarely a need to configure their use,
> though.

Right.  So a PCI bus has 4 Pins (ignoring MSI) that a device can connect to.

This is a case where Pin is the wrong interface to model because the 
device decides which LNK to use.  We don't want to model every single 
wire with a 32-bit PCI bus such that we have to make dozens of 
connections.  We want to collapse all into a single higher level interface:

So you want something like:

  -device i440fx,id=pcibus,slot[3]=blk0
  -device virtio-pci-blk,id=blk0,bus=pcibus

And then when the virtio-pci-blk device is "realized" (qdev_initfn), it 
should use it's bus property and connect to the appropriate LNKs.

You could do the same for ISA, but since it's so common to have DIP 
switches on non-PnP ISA devices, and we call things ISA that aren't 
strictly ISA (yeah, yeah, let's not have this discussion again), 
actually wiring up the IRQs by hand instead of having it negotiated 
through the bus interface makes sense.

But note that the use of "bus" here is just a name.  It does not 
implement the notion of bus that qdev does and says nothing about device 
hierarchy.

Regards,

Anthony Liguori

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

[Markus Armbruster]

Anthony Liguori <aliguori@us.ibm.com> writes:

> On 06/10/2011 03:13 AM, Markus Armbruster wrote:
>> Jan Kiszka<jan.kiszka@siemens.com>  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 :)

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

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.

[...]
>> I doubt all resources are as generic as IRQ lines, but that's okay.
>
> They pretty much are.  We're really just talking about referring to
> subcomponents of a device.  That's what's lacking today.
>
>> Device component composition is not (yet?) covered by qdev.  Of course
>> we compose anyway, in various ad hoc ways.
>
> Busses need to die.

Well, I wish I was as certain as you about about what is very wrong,
what needs to die, and what needs to be done instead.

>                      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.
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.

What exactly is so very wrong about buses that they need to die?
Honest, non-rhetorical question.

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<aliguori@us.ibm.com>  writes:
>
>> On 06/10/2011 03:13 AM, Markus Armbruster wrote:
>>> Jan Kiszka<jan.kiszka@siemens.com>   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.

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

[Andreas Färber]

Am 10.06.2011 um 14:51 schrieb Anthony Liguori:

> 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.

For the former issue I sent a patch "qdev: Add helpers for reading  
properties" yesterday:

http://patchwork.ozlabs.org/patch/99536/

That allows to access properties of arbitrary DeviceState by name,  
e.g., qdev_prop_get_uint32(dev, "irq").

It currently depends on the open question of how to model bool  
properties but I can reorder the two if needed.

Andreas

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

[Avi Kivity]

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.  That makes the easy 
devices easy and the hard ones possible.

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

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

[Anthony Liguori]

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).

That said, there absolutely should be the following paths:

/i440fx/IDE controller/primary/master -> IDE disk0
/i440fx/slot3 -> cirrus-vga

The expression of bus should just be a bidirectional path (when that 
makes sense).  IOW:

/i440fx/slot3 -> cirrus-vga
/cirrus-vga/bus -> i440fx

There, of course, can be all sorts of crazy paths through the graph. 
The following should be valid:

/i440fx/slot2 -> IDE controller
/cirrus-vga/bus/slot2/primary/master

But separating out hw paths from instantiation tree has some nice 
characteristics.  The instantiation tree is the obvious place to 
implement live migration whereas reset would probably walk device paths.

Regards,

Anthony Liguori

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

[Avi Kivity]

On 06/12/2011 10:21 PM, Anthony Liguori wrote:
>>
>>> 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 bus/device relationship is not imposed, but may hold for some of the 
devices (but not others).

Another example of aggregation is PCI slots and functions.  A PCI device 
is composed of multiple functions that can be hotplugged as one, and 
share parts of the address.  But there is no "slot/function bus" involved.

>
> That said, there absolutely should be the following paths:
>
> /i440fx/IDE controller/primary/master -> IDE disk0
> /i440fx/slot3 -> cirrus-vga
>
> The expression of bus should just be a bidirectional path (when that 
> makes sense).  IOW:
>
> /i440fx/slot3 -> cirrus-vga
> /cirrus-vga/bus -> i440fx
>
> There, of course, can be all sorts of crazy paths through the graph. 
> The following should be valid:
>
> /i440fx/slot2 -> IDE controller
> /cirrus-vga/bus/slot2/primary/master
>
> But separating out hw paths from instantiation tree has some nice 
> characteristics.  The instantiation tree is the obvious place to 
> implement live migration whereas reset would probably walk device paths.

Agreed; and it's quite obvious as the bus has a RESET line but no 
relationship to live migration.

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

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

[Gleb Natapov]

On Fri, Jun 10, 2011 at 04:59:08PM +0200, Markus Armbruster wrote:
> Anthony Liguori <aliguori@us.ibm.com> writes:
> 
> > On 06/10/2011 03:13 AM, Markus Armbruster wrote:
> >> Jan Kiszka<jan.kiszka@siemens.com>  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 :)
> 
> An ISA device is always connected to all the ISA bus's interrupt lines.
> Device configuration determines how the device uses these lines.
> 
> The old (non-MSI) PCI interrupts are similar, I think.
> 

Each PCI card has 4 irq pins INTA/INTB/INTC/INTD (usually only INTA is
used). Chipset has PCI irq router with one or more inputs (PIIX3 has
4 PIRQ[A:D]#).  Wiring on the motherboard determines which irq pin is
connect to which PCI irq router input. Different slots usually connect
the same interrupt line to a different router input in order to spread
INTA of different cards between different inputs. PCI irq router is
configured to route each input pin to a different (or same) GSI. OS can
reconfigure irq router at will using AML methods if they are provided.

--
			Gleb.

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

[Anthony Liguori]

On 06/13/2011 03:05 AM, Avi Kivity wrote:
> On 06/12/2011 10:21 PM, Anthony Liguori wrote:
>>>
>>>> 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 bus/device relationship is not imposed, but may hold for some of the
> devices (but not others).
>
> Another example of aggregation is PCI slots and functions. A PCI device
> is composed of multiple functions that can be hotplugged as one, and
> share parts of the address. But there is no "slot/function bus" involved.

Correct.

This also hints at how hot plug could work.  If devices had properties 
of type socket that you could connect devices too, a device could 
conceivably lock the socket after the device is realized (becomes guest 
visible).

Sockets that aren't locked after realize are hot pluggable.  Hot 
plugging simply becomes making a connection post realize.  An address is 
implied by the property path.

This gives you a way to allow PCI devices to be plugged in sockets 
(including multifunction devices) while not allowing individual 
functions to be hot plugged.

Regards,

Anthony Liguori

Regards,

Anthony Liguori

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

[Blue Swirl]

On Sun, Jun 12, 2011 at 10:21 PM, Anthony Liguori <aliguori@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?

> That said, there absolutely should be the following paths:
>
> /i440fx/IDE controller/primary/master -> IDE disk0
> /i440fx/slot3 -> cirrus-vga
>
> The expression of bus should just be a bidirectional path (when that makes
> sense).  IOW:
>
> /i440fx/slot3 -> cirrus-vga
> /cirrus-vga/bus -> i440fx
>
> There, of course, can be all sorts of crazy paths through the graph. The
> following should be valid:
>
> /i440fx/slot2 -> IDE controller
> /cirrus-vga/bus/slot2/primary/master
>
> But separating out hw paths from instantiation tree has some nice
> characteristics.  The instantiation tree is the obvious place to implement
> live migration whereas reset would probably walk device paths.
>
> Regards,
>
> Anthony Liguori
>
>

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

[Anthony Liguori]

On 06/13/2011 03:59 PM, Blue Swirl wrote:
> On Sun, Jun 12, 2011 at 10:21 PM, Anthony Liguori<aliguori@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.

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.

Regards,

Anthony Liguori

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

[Blue Swirl]

On Tue, Jun 14, 2011 at 4:21 PM, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 06/13/2011 03:59 PM, Blue Swirl wrote:
>>
>> On Sun, Jun 12, 2011 at 10:21 PM, Anthony Liguori<aliguori@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()?

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

[Anthony Liguori]

On 06/15/2011 01:56 PM, Blue Swirl wrote:
> On Tue, Jun 14, 2011 at 4:21 PM, Anthony Liguori<anthony@codemonkey.ws>  wrote:
>>
>> 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),

That's fine for something like a PCI controller, but for something like 
a multiport network card, it's really desirable to be able to specify 
the number of ports as part of the config.  IOW:

  -device virtio-net-pci,ports=8,netdev[0]=tap0,netdev[1]=tap1,...

>> 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()?

Yeah, that's why I said, "hard to do well".  It makes it very hard to 
add new socket types.

Regards,

Anthony Liguori

>

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

[Blue Swirl]

On Wed, Jun 15, 2011 at 11:00 PM, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 06/15/2011 01:56 PM, Blue Swirl wrote:
>>
>> On Tue, Jun 14, 2011 at 4:21 PM, Anthony Liguori<anthony@codemonkey.ws>
>>  wrote:
>>>
>>> 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),
>
> That's fine for something like a PCI controller, but for something like a
> multiport network card, it's really desirable to be able to specify the
> number of ports as part of the config.  IOW:
>
>  -device virtio-net-pci,ports=8,netdev[0]=tap0,netdev[1]=tap1,...

In the PCI cases, I'd use the maximum allowed by PCI.

>>> 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()?
>
> Yeah, that's why I said, "hard to do well".  It makes it very hard to add
> new socket types.

PCI, USB, IDE, SCSI, SBus, what else? APICBus? I2C? 8 socket types
ought to be enough for anybody.

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

[Paul Brook]

> > Yeah, that's why I said, "hard to do well".  It makes it very hard to add
> > new socket types.
> 
> PCI, USB, IDE, SCSI, SBus, what else? APICBus? I2C? 8 socket types
> ought to be enough for anybody.

Off the top of my head: AClink (audio), i2s (audio), SSI/SSP (synchonous 
serial), Firewire, rs232, CAN, FibreChannel, ISA, PS2, ADB (apple desktop bus) 
and probably a bunch of others I've missed.  There's also a bunch of all-but 
extinct system architectures with interesting bus-level features (MCA, NuBus, 
etc.)

Paul

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

[Blue Swirl]

On Mon, Jun 20, 2011 at 6:23 PM, Paul Brook <paul@codesourcery.com> wrote:
>> > Yeah, that's why I said, "hard to do well".  It makes it very hard to add
>> > new socket types.
>>
>> PCI, USB, IDE, SCSI, SBus, what else? APICBus? I2C? 8 socket types
>> ought to be enough for anybody.
>
> Off the top of my head: AClink (audio), i2s (audio), SSI/SSP (synchonous
> serial), Firewire, rs232, CAN, FibreChannel, ISA, PS2, ADB (apple desktop bus)
> and probably a bunch of others I've missed.  There's also a bunch of all-but
> extinct system architectures with interesting bus-level features (MCA, NuBus,
> etc.)

Are these really buses with identifiable sockets? For example, it's
not possible to enumerate the users of ISA bus or RS-232.

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

[Avi Kivity]

On 06/21/2011 12:32 AM, Blue Swirl wrote:
> On Mon, Jun 20, 2011 at 6:23 PM, Paul Brook<paul@codesourcery.com>  wrote:
> >>  >  Yeah, that's why I said, "hard to do well".  It makes it very hard to add
> >>  >  new socket types.
> >>
> >>  PCI, USB, IDE, SCSI, SBus, what else? APICBus? I2C? 8 socket types
> >>  ought to be enough for anybody.
> >
> >  Off the top of my head: AClink (audio), i2s (audio), SSI/SSP (synchonous
> >  serial), Firewire, rs232, CAN, FibreChannel, ISA, PS2, ADB (apple desktop bus)
> >  and probably a bunch of others I've missed.  There's also a bunch of all-but
> >  extinct system architectures with interesting bus-level features (MCA, NuBus,
> >  etc.)
>
> Are these really buses with identifiable sockets? For example, it's
> not possible to enumerate the users of ISA bus or RS-232.

It's not possible from the guest, but it's possible from outside the guest.

RS-232 even supports hotplug.

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

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

[Paul Brook]

> On Mon, Jun 20, 2011 at 6:23 PM, Paul Brook <paul@codesourcery.com> wrote:
> >> > Yeah, that's why I said, "hard to do well".  It makes it very hard to
> >> > add new socket types.
> >> 
> >> PCI, USB, IDE, SCSI, SBus, what else? APICBus? I2C? 8 socket types
> >> ought to be enough for anybody.
> > 
> > Off the top of my head: AClink (audio), i2s (audio), SSI/SSP (synchonous
> > serial), Firewire, rs232, CAN, FibreChannel, ISA, PS2, ADB (apple desktop
> > bus) and probably a bunch of others I've missed.  There's also a bunch
> > of all-but extinct system architectures with interesting bus-level
> > features (MCA, NuBus, etc.)
> 
> Are these really buses with identifiable sockets? For example, it's
> not possible to enumerate the users of ISA bus or RS-232.

Why does that matter? The whole point of a "socket" is that it defines the 
interaction between two devices.  It allows the topology of a system to be 
expressed in a device agnostic manner.  Currently we only support a simple 
bus/device tree hierachy.  The fact that we also need to support additional  
single-bit unidirectional links (aka GPIO/IRQ) with a completely different 
topology is why this conversation started.

If done properly, sockets should entirely replace the existing bust/device 
split.  This just becomes a set of links between sockets of the appropriate 
type.

An actual bus topology implies a socket has more than a single point-point 
connection.  Whether we actually impletment this is something of an open 
question. Physical hardware always has a finite number of connection points so 
worst case you end up explicitly modelling that as a separate multiport "bus" 
device, or by daisy-chaining passthrough sockets.  If we do want to model a 
pure bus (without physically identifiable ports) then I'd probably go for a 
simple one-many connector, allowing the majority of the code to still treat it 
as a single link connection.  Device addressing can be handled by higher level 
bus/socket specific APIs.

Paul