[Qemu-devel] [PATCH, RFC 0/5] Improve device info handling

[Qemu-devel] [PATCH, RFC 0/5] Improve device info handling

[Blue Swirl]

Hello,

Current monitor assumes certain functions are always available.
Because of the shortcomings of the API, devices have to keep some kind
of global state available with static variables, which is ugly.

Add info command registration to the API and make some devices use it.

User visible monitor commands change: 'info' is replaced by 'dev_info'
and also the parameter name may change, for example 'pic' becomes
'i8259.state'.

I'm not too happy about this, maybe 'info' should also handle
registered devices.

Blue Swirl (5):
  monitor: add device info infrastructure
  x86/Sparc32: use device info for pic and irq
  PCI: use device info
  x86: use device info for hpet
  PPC: use device info for CPU statistics

 cpu-all.h               |    3 --
 hw/an5206.c             |    9 ------
 hw/arm_pic.c            |   10 ------
 hw/cris_pic_cpu.c       |    5 ---
 hw/i8259.c              |   41 ++++++++++++++------------
 hw/microblaze_pic_cpu.c |    5 ---
 hw/pc.c                 |    7 ++++
 hw/pc.h                 |    2 -
 hw/pci.c                |   46 +++++++++++++++---------------
 hw/pci.h                |    2 -
 hw/shix.c               |   10 ------
 hw/slavio_intctl.c      |   16 ++++------
 hw/sun4c_intctl.c       |    6 ++-
 hw/sun4m.c              |   15 +---------
 hw/sun4m.h              |    8 -----
 hw/sun4u.c              |    8 -----
 monitor.c               |   72 ++++++++++++++++++++++++++--------------------
 monitor.h               |    5 +++
 qemu-monitor.hx         |   17 ++++------
 target-ppc/cpu.h        |    2 +
 target-ppc/helper.c     |    1 +
 target-ppc/translate.c  |   40 +++++++++++++++----------
 vl.c                    |    8 ++--
 23 files changed, 147 insertions(+), 191 deletions(-)

Re: [Qemu-devel] [PATCH, RFC 0/5] Improve device info handling

[Gerd Hoffmann]

   Hi,

> Add info command registration to the API and make some devices use it.

Jumping in here with a more general comment ...

I think right now we have _way_ to much register_something functions.
IMHO qdev allows us to kill off most of them.  We can stick function 
pointers (also VMstate pointers) into DeviceInfo instead of registering 
callbacks.

Short-term (while we are in the "convert-drivers-to-qdev" phase) that 
will just move the register calls from the driver code to generic qdev code.

Long-term we hopefully can kill the register calls altogether and walk 
the qdev device tree instead.

> User visible monitor commands change: 'info' is replaced by 'dev_info'
> and also the parameter name may change, for example 'pic' becomes
> 'i8259.state'.

Hmm, i8259 isn't converted to qdev yet, so the route outlined above 
above will not work (yet) for this device ...

cheers,
   Gerd

Re: [Qemu-devel] [PATCH, RFC 0/5] Improve device info handling

[Blue Swirl]

On Mon, Aug 31, 2009 at 11:12 AM, Gerd Hoffmann<kraxel@redhat.com> wrote:
>  Hi,
>
>> Add info command registration to the API and make some devices use it.
>
> Jumping in here with a more general comment ...
>
> I think right now we have _way_ to much register_something functions.
> IMHO qdev allows us to kill off most of them.  We can stick function
> pointers (also VMstate pointers) into DeviceInfo instead of registering
> callbacks.

Good idea. I wish reset could be handled also with a structure.

> Short-term (while we are in the "convert-drivers-to-qdev" phase) that will
> just move the register calls from the driver code to generic qdev code.
>
> Long-term we hopefully can kill the register calls altogether and walk the
> qdev device tree instead.

So at this stage, the registration function should take a structure
argument but later it would be sucked into qdev?

>> User visible monitor commands change: 'info' is replaced by 'dev_info'
>> and also the parameter name may change, for example 'pic' becomes
>> 'i8259.state'.
>
> Hmm, i8259 isn't converted to qdev yet, so the route outlined above above
> will not work (yet) for this device ...

There is also no qdev for pc.c. Maybe there should be one qdev for
each board? The higher level could set up common things like system
reset signal, memory, drives etc. Maybe even PCI.

Re: [Qemu-devel] [PATCH, RFC 0/5] Improve device info handling

[Gerd Hoffmann]

On 08/31/09 17:23, Blue Swirl wrote:
> On Mon, Aug 31, 2009 at 11:12 AM, Gerd Hoffmann<kraxel@redhat.com>  wrote:
>>   Hi,
>>
>>> Add info command registration to the API and make some devices use it.
>>
>> Jumping in here with a more general comment ...
>>
>> I think right now we have _way_ to much register_something functions.
>> IMHO qdev allows us to kill off most of them.  We can stick function
>> pointers (also VMstate pointers) into DeviceInfo instead of registering
>> callbacks.
>
> Good idea. I wish reset could be handled also with a structure.

reset is easy.  I'll send out a patch series shortly to make more clear 
what I'm talking about.

>> Short-term (while we are in the "convert-drivers-to-qdev" phase) that will
>> just move the register calls from the driver code to generic qdev code.
>>
>> Long-term we hopefully can kill the register calls altogether and walk the
>> qdev device tree instead.
>
> So at this stage, the registration function should take a structure
> argument but later it would be sucked into qdev?

Now: registration moves from drivers to qdev.c
Later: no registration needed any more, all info needed is in the qdev 
device tree.

>> Hmm, i8259 isn't converted to qdev yet, so the route outlined above above
>> will not work (yet) for this device ...
>
> There is also no qdev for pc.c. Maybe there should be one qdev for
> each board? The higher level could set up common things like system
> reset signal, memory, drives etc. Maybe even PCI.

IIRC openfirmware has one (or more?) device tree entries for memory, so 
this fits nicely into qdev.  Not sure how to handle that best for pc ...

drives are host side state, they don't go into qdev.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH, RFC 0/5] Improve device info handling

[Blue Swirl]

On Tue, Sep 1, 2009 at 10:54 AM, Gerd Hoffmann<kraxel@redhat.com> wrote:
> On 08/31/09 17:23, Blue Swirl wrote:
>>
>> On Mon, Aug 31, 2009 at 11:12 AM, Gerd Hoffmann<kraxel@redhat.com>  wrote:
>>>
>>>  Hi,
>>>
>>>> Add info command registration to the API and make some devices use it.
>>>
>>> Jumping in here with a more general comment ...
>>>
>>> I think right now we have _way_ to much register_something functions.
>>> IMHO qdev allows us to kill off most of them.  We can stick function
>>> pointers (also VMstate pointers) into DeviceInfo instead of registering
>>> callbacks.
>>
>> Good idea. I wish reset could be handled also with a structure.
>
> reset is easy.  I'll send out a patch series shortly to make more clear what
> I'm talking about.
>
>>> Short-term (while we are in the "convert-drivers-to-qdev" phase) that
>>> will
>>> just move the register calls from the driver code to generic qdev code.
>>>
>>> Long-term we hopefully can kill the register calls altogether and walk
>>> the
>>> qdev device tree instead.
>>
>> So at this stage, the registration function should take a structure
>> argument but later it would be sucked into qdev?
>
> Now: registration moves from drivers to qdev.c
> Later: no registration needed any more, all info needed is in the qdev
> device tree.
>
>>> Hmm, i8259 isn't converted to qdev yet, so the route outlined above above
>>> will not work (yet) for this device ...
>>
>> There is also no qdev for pc.c. Maybe there should be one qdev for
>> each board? The higher level could set up common things like system
>> reset signal, memory, drives etc. Maybe even PCI.
>
> IIRC openfirmware has one (or more?) device tree entries for memory, so this
> fits nicely into qdev.  Not sure how to handle that best for pc ...
>
> drives are host side state, they don't go into qdev.

My idea was that boards (like pc) should be in the long run
represented by a qdev. The higher level metamachine would instantiate
the "pc" qdev and plug in drives, memory, reset, power button, PCI
devices etc. Something like:

    DeviceState *dev;
    BoardDevice *s;

    dev = qdev_create(NULL, "pc");
    qdev_init(dev);
    s = board_from_qdev(dev);

    board_add_cpus(s, smp_cpus);
    board_add_memory(s, RAM_size);
    board_add_network(s, nd_tables[0]);
    board_add_block(s, drives_table[0]);
    board_add_serial(s, serial_hds[0]);
    board_add_display(s, ds); // DisplayState *

But shouldn't the drives, network and char devices be qdevs too,
wouldn't it help the metamachine (or "pc" device)?

Re: [Qemu-devel] [PATCH, RFC 0/5] Improve device info handling

[Gerd Hoffmann]

   Hi,

>> drives are host side state, they don't go into qdev.
>
> My idea was that boards (like pc) should be in the long run
> represented by a qdev.

The qdev root is the main_system_bus right now, I don't see a need to 
change that.  All core devices (cpus, memory, pic, ...) can be children 
of the main system bus.

> But shouldn't the drives, network and char devices be qdevs too,
> wouldn't it help the metamachine (or "pc" device)?

We have to clearly separate between host state and guest state.
qdev is about guest state.

A disk has two sides:  The host side (virtual drive foo is a lvm volume 
in raw format / a file in qcow2 format / a iso image / whatever else) 
and the guest side (this virtual drive is a master ide disk / scsi disk 
with id 3 / virtio disk / ...).  Only the later is represented by qdev. 
  The link between the two is a property.

Likewise for chardevs and network.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH, RFC 0/5] Improve device info handling

[Blue Swirl]

On Wed, Sep 2, 2009 at 9:35 AM, Gerd Hoffmann<kraxel@redhat.com> wrote:
>  Hi,
>
>>> drives are host side state, they don't go into qdev.
>>
>> My idea was that boards (like pc) should be in the long run
>> represented by a qdev.
>
> The qdev root is the main_system_bus right now, I don't see a need to change
> that.  All core devices (cpus, memory, pic, ...) can be children of the main
> system bus.

That level is too low, because the metamachine level would not be
interested in the internals of the pc device (except generic
parameters like memory, number of CPUs etc), but how it connects to
the host devices.

>> But shouldn't the drives, network and char devices be qdevs too,
>> wouldn't it help the metamachine (or "pc" device)?
>
> We have to clearly separate between host state and guest state.
> qdev is about guest state.

Yes, it's about guest state _now_, but why limit it to that? But if
host devices as a class are really different from guest device, we
could have something similar to qdev but for host devices. I still
don't see a need for a special type.

> A disk has two sides:  The host side (virtual drive foo is a lvm volume in
> raw format / a file in qcow2 format / a iso image / whatever else) and the
> guest side (this virtual drive is a master ide disk / scsi disk with id 3 /
> virtio disk / ...).  Only the later is represented by qdev.  The link
> between the two is a property.

There is no need for a link. Instead of the property stuff, the pc
qdev should make available mappable objects (drive placeholders), the
metamachine would plug in the host drives by mapping the drive to a
host drive. It's just like device vs. address: drive placeholder vs.
host drive.

In fact, maybe we should have more devices at metamachine level: "pc"
qdev and "host" qdev (hdev if you can convince me). Maybe other
virtual objects too, like vlans, monitor, gdbstub etc.

> Likewise for chardevs and network.

Consider vlan:
   DeviceState *gdev, *hdev, *vlan;
   VLAN *gvlan, *hvlan;

   hdev = qdev_create(NULL, "host");
   qdev_init(hdev);

   gdev = qdev_create(NULL, "pc");
   qdev_init(gdev);

   vlan = qdev_create(NULL, "vlan");
   qdev_init(gdev);
   gvlan = qdev_get_vlan_in(gdev, 0); // Analogous to gpio
   hvlan = qdev_get_vlan_in(hdev, 0);
   vlan_map(vlan, gvlan);
   vlan_map(vlan, hvlan);

Beautiful, isn't it? ;-)

Re: [Qemu-devel] [PATCH, RFC 0/5] Improve device info handling

[Gerd Hoffmann]

   Hi,

>> The qdev root is the main_system_bus right now, I don't see a need to change
>> that.  All core devices (cpus, memory, pic, ...) can be children of the main
>> system bus.
>
> That level is too low, because the metamachine level would not be
> interested in the internals of the pc device (except generic
> parameters like memory, number of CPUs etc), but how it connects to
> the host devices.

IMO the connection to the host device belongs to the guest device which 
needs the link.  i.e. the scsi-disk driver should handle that, not some 
metamachine.

>> We have to clearly separate between host state and guest state.
>> qdev is about guest state.
>
> Yes, it's about guest state _now_, but why limit it to that? But if
> host devices as a class are really different from guest device, we
> could have something similar to qdev but for host devices. I still
> don't see a need for a special type.

There are a number of differences.  guest devices belong to a emulated 
bus (ide/pci/usb/scsi/whatever).  host devices don't.  guest devices 
form a device tree.  host devices are simple flat lists.

It might make sense to create something simliar to qdev for the host 
devices.

>> A disk has two sides:  The host side (virtual drive foo is a lvm volume in
>> raw format / a file in qcow2 format / a iso image / whatever else) and the
>> guest side (this virtual drive is a master ide disk / scsi disk with id 3 /
>> virtio disk / ...).  Only the later is represented by qdev.  The link
>> between the two is a property.
>
> There is no need for a link. Instead of the property stuff, the pc
> qdev should make available mappable objects (drive placeholders),

The pc qdev doesn't know which "mappable objects" exist.  The knowledge 
is scattered all over the devices (ide / scsi / virtio-blk / usb-storage 
/ ...).  And not all of them are present in all configurations.

> the
> metamachine would plug in the host drives by mapping the drive to a
> host drive. It's just like device vs. address: drive placeholder vs.
> host drive.

It works the other way around:  We have a linked list of host drives 
(with names).  guest drives have a property, setting the property will 
lookup the host drive by name (only virtio-blk is merged, patches for 
scsi-disk & usb-storage are on the list).

> Consider vlan:
>     DeviceState *gdev, *hdev, *vlan;
>     VLAN *gvlan, *hvlan;
>
>     hdev = qdev_create(NULL, "host");
>     qdev_init(hdev);
>
>     gdev = qdev_create(NULL, "pc");
>     qdev_init(gdev);
>
>     vlan = qdev_create(NULL, "vlan");
>     qdev_init(gdev);
>     gvlan = qdev_get_vlan_in(gdev, 0); // Analogous to gpio

Same problem as with the drives: pc doesn't handle vlans, the nic driver 
somewhere down the device tree does.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH, RFC 0/5] Improve device info handling

[Blue Swirl]

On Wed, Sep 2, 2009 at 7:42 PM, Gerd Hoffmann<kraxel@redhat.com> wrote:
>  Hi,
>
>>> The qdev root is the main_system_bus right now, I don't see a need to
>>> change
>>> that.  All core devices (cpus, memory, pic, ...) can be children of the
>>> main
>>> system bus.
>>
>> That level is too low, because the metamachine level would not be
>> interested in the internals of the pc device (except generic
>> parameters like memory, number of CPUs etc), but how it connects to
>> the host devices.
>
> IMO the connection to the host device belongs to the guest device which
> needs the link.  i.e. the scsi-disk driver should handle that, not some
> metamachine.
>
>>> We have to clearly separate between host state and guest state.
>>> qdev is about guest state.
>>
>> Yes, it's about guest state _now_, but why limit it to that? But if
>> host devices as a class are really different from guest device, we
>> could have something similar to qdev but for host devices. I still
>> don't see a need for a special type.
>
> There are a number of differences.  guest devices belong to a emulated bus
> (ide/pci/usb/scsi/whatever).  host devices don't.  guest devices form a
> device tree.  host devices are simple flat lists.

Then the "host" qdev does not make much sense, current tables should be fine.

> It might make sense to create something simliar to qdev for the host
> devices.
>
>>> A disk has two sides:  The host side (virtual drive foo is a lvm volume
>>> in
>>> raw format / a file in qcow2 format / a iso image / whatever else) and
>>> the
>>> guest side (this virtual drive is a master ide disk / scsi disk with id 3
>>> /
>>> virtio disk / ...).  Only the later is represented by qdev.  The link
>>> between the two is a property.
>>
>> There is no need for a link. Instead of the property stuff, the pc
>> qdev should make available mappable objects (drive placeholders),
>
> The pc qdev doesn't know which "mappable objects" exist.  The knowledge is
> scattered all over the devices (ide / scsi / virtio-blk / usb-storage /
> ...).  And not all of them are present in all configurations.

The metamachine could scan the device tree using the future qdev
classes, but I didn't want that the metamachine messes with the
internals of pc qdev. It also seems artificial to float the knowledge
upwards.

>> the
>> metamachine would plug in the host drives by mapping the drive to a
>> host drive. It's just like device vs. address: drive placeholder vs.
>> host drive.
>
> It works the other way around:  We have a linked list of host drives (with
> names).  guest drives have a property, setting the property will lookup the
> host drive by name (only virtio-blk is merged, patches for scsi-disk &
> usb-storage are on the list).
>
>> Consider vlan:
>>    DeviceState *gdev, *hdev, *vlan;
>>    VLAN *gvlan, *hvlan;
>>
>>    hdev = qdev_create(NULL, "host");
>>    qdev_init(hdev);
>>
>>    gdev = qdev_create(NULL, "pc");
>>    qdev_init(gdev);
>>
>>    vlan = qdev_create(NULL, "vlan");
>>    qdev_init(gdev);
>>    gvlan = qdev_get_vlan_in(gdev, 0); // Analogous to gpio
>
> Same problem as with the drives: pc doesn't handle vlans, the nic driver
> somewhere down the device tree does.

There is still the reset signal case, monitor and displays. Reset
could be equally well handled by leaf devices instead of top of the
tree. Monitor could use host char devices, like it does now. Host
display devices can be grabbed by VGA etc.

So in summary I think my top-down approach is not very optimal.