From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:39341)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <anthony@codemonkey.ws>) id 1QmCdg-0002n0-Vj
	for qemu-devel@nongnu.org; Wed, 27 Jul 2011 18:31:42 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <anthony@codemonkey.ws>) id 1QmCde-0004ZV-Qp
	for qemu-devel@nongnu.org; Wed, 27 Jul 2011 18:31:40 -0400
Received: from mail-gw0-f53.google.com ([74.125.83.53]:56116)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <anthony@codemonkey.ws>) id 1QmCde-0004Wv-KX
	for qemu-devel@nongnu.org; Wed, 27 Jul 2011 18:31:38 -0400
Received: by gwj20 with SMTP id 20so1672466gwj.12
	for <qemu-devel@nongnu.org>; Wed, 27 Jul 2011 15:31:34 -0700 (PDT)
Message-ID: <4E3091C2.3090605@codemonkey.ws>
Date: Wed, 27 Jul 2011 17:31:30 -0500
From: Anthony Liguori <anthony@codemonkey.ws>
MIME-Version: 1.0
References: <1311558293-5855-1-git-send-email-aliguori@us.ibm.com>	<4E2EBA1E.90006@redhat.com>
	<4E2EC90E.8090409@codemonkey.ws>
	<CAFEAcA9ScQ717m9sV8TPOtSodT4iHKyemy41nQoJMo35xkM4wg@mail.gmail.com>
In-Reply-To: <CAFEAcA9ScQ717m9sV8TPOtSodT4iHKyemy41nQoJMo35xkM4wg@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>, qemu-devel@nongnu.org

On 07/27/2011 04:33 PM, Peter Maydell wrote:
> On 26 July 2011 15:02, Anthony Liguori<anthony@codemonkey.ws>  wrote:
>> In my attempt at PCI modelling, I had something like:
>>
>> struct I440FX
>> {
>>    Device parent;
>>
>>    PciDevice slots[32];
>> };
>>
>> Which means that to attach to bus 0, slot 3, fn 0, you do:
>>
>> i440fx->slots[3] = mydevice
>
> So what I don't really understand about this model is why the PCI
> connection is "privileged" in the sense that 'mydevice' is actually
> an instance of something you can stick directly into one of the PCIBus
> device's slots.


Oh, it's not privileged at all.  It just happens to be that modelling 
something simple is simple.  That's goodness.

The example that I used for most of my early testing was meant to be a 
bit more interesting.  I created a Source, Sink, XorGate, AndGate, and 
OrGate and then used a config file to build a full working 8-bit adder.

You can see this at:

http://repo.or.cz/w/qemu/aliguori.git/blob/refs/heads/qdev2

Check out devices/gates and vm.cfg.

Here's a rough model of how it works:

struct XorGate
{
    Device parent;
    Pin out;
    Pin *in[2];
};

In XorGate's initfn, it registers three properties, 'in[0]' and 'in[1]' 
as 'socket<Pin>'s and 'out' as 'plug<Pin>'.

When XorGate is realized, it makes sure that both in[0] and in[1] have 
been set (sockets are RW whereas plugs are RO).  It then connects to the 
edge event on the in[0] and in[1] pins and when either event fires, it 
will compute a value, and set the level of out.

XorGate is-a Device, but so is Pin.  The following things are valid:

(qemu) plug_create xor-gate gate0
(qemu) plug_create xor-gate gate1
(qemu) plug_create xor-gate gate2
(qemu) plug_get gate1 out
gate1::out
(qemu) plug_set xor-gate in[0] gate1::out
(qemu) plug_get gate2 out
gate2::out
(qemu) plug_set xor-gate in[1] gate2::out

Which creates a simple tree of two xor gates who's outputs are tied to 
another xor gate.  Or:

            XorGate /
- XorGate /        \
           \
            XorGate /
                    \

The 'out' property of XorGate is really a child device.  All devices 
(all plugs for that matter) have a global namespace.  By convention, 
uniqueness in name is guaranteed for child devices by reserving the '::' 
separator and naming children based on 'parentname::propertyname'.  But 
for correctness, the right way to figure out the name of a child is by 
reading the property.

So to answer your above question, a multi PCI slot graphics adapter is 
simply:

struct GraphicsSlot {
    PciDevice parent;
};

struct MultislotGraphicsCard {
    Device parent;

    GraphicsSlot slot[2];
};

Then conceptually:

mgc = new MultislotGraphicsCard();

i440fx->slot[1] = mgc.slot[0];
i440fx->slot[2] = mgc.slot[1];

You do not need to ever assign a device directly to a socket.  This is a 
fundamental different between QOM and qdev.  There is no concept of 
"parent bus".  There's no concept of parent--even with composition. 
Devices can have links to zero or more other Devices.  They can also 
create devices within themselves but those devices don't have any 
special knowledge of that fact.


>  I would expect that 'mydevice' would have a bunch of
> (effectively) interfaces it exposed, and you'd connect i440fx->slots[3]
> to mydevice.pciconnector or something. [Which ought to both let the
> PCIBus call functions implemented by the device, and vice versa.]
>
> (What would a model of a two-PCI-slot graphics card look like?)
>
> Maybe it would be better to use some other example. After all, the
> cases like PCI are the ones our device model already handles pretty
> well, so the interesting cases to look at are where we're not so
> good at it. What does the implementation of omap2_gpio look like,
> for instance? (it's not qdev at the moment but there's a set of
> patches on the list which make it so; the guts of the qdevification
> are http://patchwork.ozlabs.org/patch/103905/ .)
>   That's a device which exposes:
>   * 4,5, or 6 MMIO regions (depending on how many 'modules' the
>     particular revision you ask for has)
>   * a variably sized array of gpio inputs
>   * ditto, gpio outputs

struct OmapGpio
{
     Device parent;

     size_t nb_regions;
     MemoryRegion *regions;

     size_t nb_out;
     Pin *out;

     size_t nb_in;
     Pin **in;
};

void omap_gpio_set_nb_out(OmapGpio *obj, size_t value);
void omap_gpio_set_nb_in(OmapGpio *obj, size_t value);
void omap_gpio_set_nb_regions(OmapGpio *obj, size_t value);

At initfn(), you register three integer properties, 'nb_out', 'nb_in', 
'nb_regions'.

In the respective setters, you allocate the appropriate array and then 
unregister the old properties, and then register teh new ones.  For 
instance:

void omap_gpio_set_nb_out(OmapGpio *obj, size_t value)
{
    /* remove old properties */
    for (i = 0; i < obj->nb_out; i++) {
        plug_del_property_plug(PLUG(obj), "out[%d]", i);
    }

    obj->out = realloc(obj->out, new_size);
    obj->nb_out = value;

    /* add new properties */
    for (i = 0; i < obj->nb_out; i++) {
        plug_add_property_plug(PLUG(obj), &obj->pin[i],
                               TYPE_PIN, "out[%d]", i);
    }
}

This would let you work with the device like this:

[OmapGpio "gpio"]
nb_out = 4
nb_in = 2
in[0] = "foo.out[0]"
in[1] = "foo.out[1]"

[OmapFoo "foo"]
bar = "gpio.out[3]"

It's exactly this use-case which makes GObject unsuitable.  Properties 
need to be associated with the instance, not the class.  qdev also 
suffers from this problem.

Regards,

Anthony Liguori

>   * for each 'module', three specific named outgoing gpio lines
>     "mpu_irq", "dsp_irq" and "wakeup"
>   * a number of omap_clk connections, where an omap_clk* represents
>     a connection to the OMAP clock tree, and in practice is an
>     interface where the omap_gpio can (a) call a function to
>     ask "what rate is this clock running at?" and (b) provide
>     a gpio line which will be raised when the clock ticks.
>     [omap_gpio doesn't actually use its clock connections but
>     other omap devices do; they're interesting because we have
>     function calls going in both directions over the interface.]
>
> None of these exposed interfaces are particularly obviously
> more important than any of the others -- they're just all
> things that might need to be wired up.
>
> I'm particularly interested in how much effort is involved
> in defining ad-hoc platform-specific interface types like
> omap_clk.
>
> -- PMM
>