[Qemu-devel] [PATCH 1/4] qdev: device capabilities

[Qemu-devel] [PATCH 1/4] qdev: device capabilities

[Gerd Hoffmann]

This patch adds device capabilities to qdev devices.  This is the
core code, following patches will add the individual capabilities
and tag drivers.

The capabilities will be printed by '-device ?' and 'info qdm", so
users and management apps can use it.

Future plans:  I plan to use them to get rid off some hard-coded
lists in qemu by using capabilities instead: pci nic list, watchdog
list, maybe more.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/qdev.c |   19 ++++++++++++++++++-
 hw/qdev.h |    5 +++++
 2 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index c1a7779..568d249 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -107,8 +107,11 @@ DeviceState *qdev_create(BusState *bus, const char *name)
 
 static int qdev_print_devinfo(DeviceInfo *info, char *dest, int len)
 {
+    static const char *capname[] = {
+    };
+    const char *sep;
     int pos = 0;
-    int ret;
+    int ret,i;
 
     ret = snprintf(dest+pos, len-pos, "name \"%s\", bus %s",
                    info->name, info->bus_info->name);
@@ -125,6 +128,20 @@ static int qdev_print_devinfo(DeviceInfo *info, char *dest, int len)
         ret = snprintf(dest+pos, len-pos, ", no-user");
         pos += MIN(len-pos,ret);
     }
+    if (info->caps) {
+        ret = snprintf(dest+pos, len-pos, ", caps \"");
+        pos += MIN(len-pos,ret);
+        sep = "";
+        for (i = 0; i < ARRAY_SIZE(capname); i++) {
+            if (!(info->caps & (1 << i)))
+                continue;
+            ret = snprintf(dest+pos, len-pos, "%s%s", sep, capname[i]);
+            pos += MIN(len-pos,ret);
+            sep = ",";
+        }
+        ret = snprintf(dest+pos, len-pos, "\"");
+        pos += MIN(len-pos,ret);
+    }
     return pos;
 }
 
diff --git a/hw/qdev.h b/hw/qdev.h
index 204c4e5..7beb756 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -102,6 +102,10 @@ typedef void (*qdev_initfn)(DeviceState *dev, DeviceInfo *info);
 typedef void (*SCSIAttachFn)(DeviceState *host, BlockDriverState *bdrv,
               int unit);
 
+enum DeviceCapBits {
+    dummy
+};
+
 struct DeviceInfo {
     const char *name;
     const char *alias;
@@ -109,6 +113,7 @@ struct DeviceInfo {
     size_t size;
     Property *props;
     int no_user;
+    uint32_t caps;
 
     /* Private to qdev / bus.  */
     qdev_initfn init;
-- 
1.6.2.5

[Qemu-devel] [PATCH 2/4] qdev: add audio capability

[Gerd Hoffmann]

... and tag sound drivers (well, only the pci ones as the others are not
yet converted to qdev).

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/ac97.c   |    1 +
 hw/es1370.c |    1 +
 hw/qdev.c   |    1 +
 hw/qdev.h   |    4 +++-
 4 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/hw/ac97.c b/hw/ac97.c
index 6c818c9..8d9c202 100644
--- a/hw/ac97.c
+++ b/hw/ac97.c
@@ -1368,6 +1368,7 @@ static PCIDeviceInfo ac97_info = {
     .qdev.name    = "AC97",
     .qdev.desc    = "Intel 82801AA AC97 Audio",
     .qdev.size    = sizeof (PCIAC97LinkState),
+    .qdev.caps    = DEV_CAP_AUDIO,
     .init         = ac97_initfn,
 };
 
diff --git a/hw/es1370.c b/hw/es1370.c
index 5c9af0e..028db61 100644
--- a/hw/es1370.c
+++ b/hw/es1370.c
@@ -1056,6 +1056,7 @@ static PCIDeviceInfo es1370_info = {
     .qdev.name    = "ES1370",
     .qdev.desc    = "ENSONIQ AudioPCI ES1370",
     .qdev.size    = sizeof (PCIES1370State),
+    .qdev.caps    = DEV_CAP_AUDIO,
     .init         = es1370_initfn,
 };
 
diff --git a/hw/qdev.c b/hw/qdev.c
index 568d249..e501610 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -108,6 +108,7 @@ DeviceState *qdev_create(BusState *bus, const char *name)
 static int qdev_print_devinfo(DeviceInfo *info, char *dest, int len)
 {
     static const char *capname[] = {
+        [ DEV_CAP_BIT_AUDIO       ] = "audio",
     };
     const char *sep;
     int pos = 0;
diff --git a/hw/qdev.h b/hw/qdev.h
index 7beb756..90a4446 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -103,9 +103,11 @@ typedef void (*SCSIAttachFn)(DeviceState *host, BlockDriverState *bdrv,
               int unit);
 
 enum DeviceCapBits {
-    dummy
+    DEV_CAP_BIT_AUDIO      = 0,
 };
 
+#define DEV_CAP_AUDIO      (1 << DEV_CAP_BIT_AUDIO)
+
 struct DeviceInfo {
     const char *name;
     const char *alias;
-- 
1.6.2.5

[Qemu-devel] [PATCH 3/4] qdev: add ethernet capability

[Gerd Hoffmann]

... and tag ethernet drivers.  Also add some descriptions while being
at it.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/e1000.c      |    2 ++
 hw/eepro100.c   |    6 ++++++
 hw/ne2000.c     |    2 ++
 hw/pcnet.c      |    3 +++
 hw/qdev.c       |    1 +
 hw/qdev.h       |    2 ++
 hw/rtl8139.c    |    2 ++
 hw/virtio-pci.c |    1 +
 8 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/hw/e1000.c b/hw/e1000.c
index b0542d7..8ed84ce 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -1136,7 +1136,9 @@ static void pci_e1000_init(PCIDevice *pci_dev)
 
 static PCIDeviceInfo e1000_info = {
     .qdev.name = "e1000",
+    .qdev.desc = "Intel Gigabit Ethernet",
     .qdev.size = sizeof(E1000State),
+    .qdev.caps = DEV_CAP_ETHERNET,
     .init      = pci_e1000_init,
 };
 
diff --git a/hw/eepro100.c b/hw/eepro100.c
index ec31a6a..0c7db58 100644
--- a/hw/eepro100.c
+++ b/hw/eepro100.c
@@ -1794,15 +1794,21 @@ static void pci_i82559er_init(PCIDevice *dev)
 static PCIDeviceInfo eepro100_info[] = {
     {
         .qdev.name = "i82551",
+        .qdev.desc = "Intel EtherExpress PRO 100",
         .qdev.size = sizeof(PCIEEPRO100State),
+        .qdev.caps = DEV_CAP_ETHERNET,
         .init      = pci_i82551_init,
     },{
         .qdev.name = "i82557b",
+        .qdev.desc = "Intel EtherExpress PRO 100",
         .qdev.size = sizeof(PCIEEPRO100State),
+        .qdev.caps = DEV_CAP_ETHERNET,
         .init      = pci_i82557b_init,
     },{
         .qdev.name = "i82559er",
+        .qdev.desc = "Intel EtherExpress PRO 100",
         .qdev.size = sizeof(PCIEEPRO100State),
+        .qdev.caps = DEV_CAP_ETHERNET,
         .init      = pci_i82559er_init,
     },{
         /* end of list */
diff --git a/hw/ne2000.c b/hw/ne2000.c
index b9c018a..78e80b8 100644
--- a/hw/ne2000.c
+++ b/hw/ne2000.c
@@ -834,7 +834,9 @@ static void pci_ne2000_init(PCIDevice *pci_dev)
 
 static PCIDeviceInfo ne2000_info = {
     .qdev.name = "ne2k_pci",
+    .qdev.desc = "NE2000 PCI Ethernet",
     .qdev.size = sizeof(PCINE2000State),
+    .qdev.caps = DEV_CAP_ETHERNET,
     .init      = pci_ne2000_init,
 };
 
diff --git a/hw/pcnet.c b/hw/pcnet.c
index 637dcfb..2e070f1 100644
--- a/hw/pcnet.c
+++ b/hw/pcnet.c
@@ -2144,6 +2144,7 @@ static SysBusDeviceInfo lance_info = {
     .init = lance_init,
     .qdev.name  = "lance",
     .qdev.size  = sizeof(SysBusPCNetState),
+    .qdev.caps  = DEV_CAP_ETHERNET,
     .qdev.props = (Property[]) {
         DEFINE_PROP_PTR("dma", SysBusPCNetState, state.dma_opaque),
         DEFINE_PROP_END_OF_LIST(),
@@ -2154,7 +2155,9 @@ static SysBusDeviceInfo lance_info = {
 
 static PCIDeviceInfo pcnet_info = {
     .qdev.name = "pcnet",
+    .qdev.desc = "PCnet Lance Ethernet",
     .qdev.size = sizeof(PCIPCNetState),
+    .qdev.caps = DEV_CAP_ETHERNET,
     .init      = pci_pcnet_init,
 };
 
diff --git a/hw/qdev.c b/hw/qdev.c
index e501610..c026305 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -109,6 +109,7 @@ static int qdev_print_devinfo(DeviceInfo *info, char *dest, int len)
 {
     static const char *capname[] = {
         [ DEV_CAP_BIT_AUDIO       ] = "audio",
+        [ DEV_CAP_BIT_ETHERNET    ] = "ethernet",
     };
     const char *sep;
     int pos = 0;
diff --git a/hw/qdev.h b/hw/qdev.h
index 90a4446..ee282f0 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -104,9 +104,11 @@ typedef void (*SCSIAttachFn)(DeviceState *host, BlockDriverState *bdrv,
 
 enum DeviceCapBits {
     DEV_CAP_BIT_AUDIO      = 0,
+    DEV_CAP_BIT_ETHERNET   = 1,
 };
 
 #define DEV_CAP_AUDIO      (1 << DEV_CAP_BIT_AUDIO)
+#define DEV_CAP_ETHERNET   (1 << DEV_CAP_BIT_ETHERNET)
 
 struct DeviceInfo {
     const char *name;
diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index fcd6d95..dbb2d95 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -3501,7 +3501,9 @@ static void pci_rtl8139_init(PCIDevice *dev)
 
 static PCIDeviceInfo rtl8139_info = {
     .qdev.name = "rtl8139",
+    .qdev.desc = "Realtek RTL8139 Fast Ethernet",
     .qdev.size = sizeof(PCIRTL8139State),
+    .qdev.caps = DEV_CAP_ETHERNET,
     .init      = pci_rtl8139_init,
 };
 
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 8b57dfc..7b41e2d 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -509,6 +509,7 @@ static PCIDeviceInfo virtio_info[] = {
     },{
         .qdev.name  = "virtio-net-pci",
         .qdev.size  = sizeof(VirtIOPCIProxy),
+        .qdev.caps  = DEV_CAP_ETHERNET,
         .init       = virtio_net_init_pci,
         .qdev.props = (Property[]) {
             DEFINE_PROP_HEX32("vectors", VirtIOPCIProxy, nvectors,
-- 
1.6.2.5

[Qemu-devel] [PATCH 4/4] qdev: add display capability

[Gerd Hoffmann]

... and tag devices.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/cirrus_vga.c |    1 +
 hw/qdev.c       |    1 +
 hw/qdev.h       |    2 ++
 hw/syborg_fb.c  |    1 +
 hw/tcx.c        |    1 +
 hw/vga.c        |    1 +
 hw/vmware_vga.c |    1 +
 7 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index 95d822a..65dd8a0 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -3345,6 +3345,7 @@ void pci_cirrus_vga_init(PCIBus *bus)
 static PCIDeviceInfo cirrus_vga_info = {
     .qdev.name    = "Cirrus VGA",
     .qdev.size    = sizeof(PCICirrusVGAState),
+    .qdev.caps    = DEV_CAP_DISPLAY,
     .init         = pci_cirrus_vga_initfn,
     .config_write = pci_cirrus_write_config,
 };
diff --git a/hw/qdev.c b/hw/qdev.c
index c026305..b15408d 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -110,6 +110,7 @@ static int qdev_print_devinfo(DeviceInfo *info, char *dest, int len)
     static const char *capname[] = {
         [ DEV_CAP_BIT_AUDIO       ] = "audio",
         [ DEV_CAP_BIT_ETHERNET    ] = "ethernet",
+        [ DEV_CAP_BIT_DISPLAY     ] = "display",
     };
     const char *sep;
     int pos = 0;
diff --git a/hw/qdev.h b/hw/qdev.h
index ee282f0..ea38a88 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -105,10 +105,12 @@ typedef void (*SCSIAttachFn)(DeviceState *host, BlockDriverState *bdrv,
 enum DeviceCapBits {
     DEV_CAP_BIT_AUDIO      = 0,
     DEV_CAP_BIT_ETHERNET   = 1,
+    DEV_CAP_BIT_DISPLAY    = 2,
 };
 
 #define DEV_CAP_AUDIO      (1 << DEV_CAP_BIT_AUDIO)
 #define DEV_CAP_ETHERNET   (1 << DEV_CAP_BIT_ETHERNET)
+#define DEV_CAP_DISPLAY    (1 << DEV_CAP_BIT_DISPLAY)
 
 struct DeviceInfo {
     const char *name;
diff --git a/hw/syborg_fb.c b/hw/syborg_fb.c
index efa5c0e..9d9a07e 100644
--- a/hw/syborg_fb.c
+++ b/hw/syborg_fb.c
@@ -534,6 +534,7 @@ static SysBusDeviceInfo syborg_fb_info = {
     .init = syborg_fb_init,
     .qdev.name  = "syborg,framebuffer",
     .qdev.size  = sizeof(SyborgFBState),
+    .qdev.caps  = DEV_CAP_DISPLAY,
     .qdev.props = (Property[]) {
         DEFINE_PROP_UINT32("width",  SyborgFBState, cols, 0),
         DEFINE_PROP_UINT32("height", SyborgFBState, rows, 0),
diff --git a/hw/tcx.c b/hw/tcx.c
index 68dbf02..a6f29fc 100644
--- a/hw/tcx.c
+++ b/hw/tcx.c
@@ -645,6 +645,7 @@ static SysBusDeviceInfo tcx_info = {
     .init = tcx_init1,
     .qdev.name  = "SUNW,tcx",
     .qdev.size  = sizeof(TCXState),
+    .qdev.caps  = DEV_CAP_DISPLAY,
     .qdev.props = (Property[]) {
         DEFINE_PROP_TADDR("addr",      TCXState, addr,      -1),
         DEFINE_PROP_HEX32("vram_size", TCXState, vram_size, -1),
diff --git a/hw/vga.c b/hw/vga.c
index 4a0f197..7090075 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -2530,6 +2530,7 @@ int pci_vga_init(PCIBus *bus,
 static PCIDeviceInfo vga_info = {
     .qdev.name    = "VGA",
     .qdev.size    = sizeof(PCIVGAState),
+    .qdev.caps    = DEV_CAP_DISPLAY,
     .init         = pci_vga_initfn,
     .config_write = pci_vga_write_config,
     .qdev.props   = (Property[]) {
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 5ceebf1..0c7df8e 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -1246,6 +1246,7 @@ void pci_vmsvga_init(PCIBus *bus)
 static PCIDeviceInfo vmsvga_info = {
     .qdev.name    = "QEMUware SVGA",
     .qdev.size    = sizeof(struct pci_vmsvga_state_s),
+    .qdev.caps    = DEV_CAP_DISPLAY,
     .init         = pci_vmsvga_initfn,
 };
 
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH 4/4] qdev: add display capability

[Anthony Liguori]

Gerd Hoffmann wrote:
> ... and tag devices.
>   

Shouldn't there be a direct relationship between a capability and the 
interfaces the device accepts?

That is, instead of explicitly marking something as having a capability 
of { "ethernet" }, wouldn't you be able to infer that from the present 
of a VLANClientState property?

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 4/4] qdev: add display capability

[Gerd Hoffmann]

On 08/11/09 15:42, Anthony Liguori wrote:
> Gerd Hoffmann wrote:
>> ... and tag devices.
>
> Shouldn't there be a direct relationship between a capability and the
> interfaces the device accepts?

That is the case for some devices only.

> That is, instead of explicitly marking something as having a capability
> of { "ethernet" }, wouldn't you be able to infer that from the present
> of a VLANClientState property?

Works for ethernet (well, not yet, but most likely some day ...).

What property do you use to identify sound devices?
What property do you use to identify display devices?
What property do you use to identify watchdog devices?

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 4/4] qdev: add display capability

[Anthony Liguori]

Gerd Hoffmann wrote:
> On 08/11/09 15:42, Anthony Liguori wrote:
>> Gerd Hoffmann wrote:
>>> ... and tag devices.
>>
>> Shouldn't there be a direct relationship between a capability and the
>> interfaces the device accepts?
>
> That is the case for some devices only.

A device doesn't have a capability unless it implements the backend 
device interface, no?

That is, a device does not have the Display capability unless it returns 
a DisplayState.

>> That is, instead of explicitly marking something as having a capability
>> of { "ethernet" }, wouldn't you be able to infer that from the present
>> of a VLANClientState property?
>
> Works for ethernet (well, not yet, but most likely some day ...).
>
> What property do you use to identify sound devices?

A QEMUSoundCard * property (r/o).

> What property do you use to identify display devices?

A DisplayState * property (r/o).
> What property do you use to identify watchdog devices?

A WatchdogTimerModel * property (r/o).

Regards,

Anthony Liguori
> cheers,
>   Gerd

Re: [Qemu-devel] [PATCH 4/4] qdev: add display capability

[Gerd Hoffmann]

On 08/11/09 17:57, Anthony Liguori wrote:
> Gerd Hoffmann wrote:
>> That is the case for some devices only.
>
> A device doesn't have a capability unless it implements the backend
> device interface, no?

Right.  Although that doesn't imply there is a property needed for that.

>> What property do you use to identify display devices?
>
> A DisplayState * property (r/o).

Ok, that is the backend side of display devices.  It would somehow make 
sense to export that as property like we do today for DriveInfo and 
CharDriverState, although there is no need right now to have that 
accessable as property.


>> What property do you use to identify sound devices?
>
> A QEMUSoundCard * property (r/o).

That is a linked list of all sound card device instances.

>> What property do you use to identify watchdog devices?
>
> A WatchdogTimerModel * property (r/o).

That is a linked list of all watchdog device models.

Why on earth these should become device properties?
That doesn't make sense to me at all.

Oh, and the WatchdogTimerModel is the one I actually want to kill off 
some day (after converting watchdogs to qdev).  There is no need for a 
private device model list, we can use the qdev list instead and filter 
for DeviceInfo->caps & DEV_CAP_WATCHDOG.

cheers,
   Gerd