qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [RFC] Introduce module API to QEMU
@ 2009-04-03  2:12 Anthony Liguori
  2009-04-03  2:29 ` malc
                   ` (4 more replies)
  0 siblings, 5 replies; 29+ messages in thread
From: Anthony Liguori @ 2009-04-03  2:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anthony Liguori, Paul Brook

N.B. This has nothing to do with plugins or making things available at runtime.

This patch introduces a module API similar to what's in the Linux kernel.  This
includes module_init/module_exit functions that register functions that are run
at init and exit respectively.

The real utility of this is that it provides a means for device modules to
register themselves on their own.  Included in this patch is the conversion to
make the PCI NIC adapters use this interface.

The result is that some really ugly dependencies between pci.[ch] and the
device models are eliminated making things generally nicer.

It uses __attribute__((section)) to make module_init/module_exit work.  I looked
at making this work by using a parser to find and extract all of these things.
I'm not sure I know a good way to force the names to be unique via CPP but in
the very least, I came to the determination that I would need to use something
like perl or python which would introduce a new dependency to the build.

I figured using the GCCism was a lesser burden since we don't attempt to support
any compiler other than GCC today.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

diff --git a/Makefile b/Makefile
index 2bee52c..a261560 100644
--- a/Makefile
+++ b/Makefile
@@ -84,7 +84,7 @@ OBJS+=usb-serial.o usb-net.o
 OBJS+=sd.o ssi-sd.o
 OBJS+=bt.o bt-host.o bt-vhci.o bt-l2cap.o bt-sdp.o bt-hci.o bt-hid.o usb-bt.o
 OBJS+=buffered_file.o migration.o migration-tcp.o net.o qemu-sockets.o
-OBJS+=qemu-char.o aio.o net-checksum.o savevm.o cache-utils.o
+OBJS+=qemu-char.o aio.o net-checksum.o savevm.o cache-utils.o module.o
 
 ifdef CONFIG_BRLAPI
 OBJS+= baum.o
diff --git a/hw/e1000.c b/hw/e1000.c
index 1644201..311ddf4 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -26,6 +26,7 @@
 #include "hw.h"
 #include "pci.h"
 #include "net.h"
+#include "module.h"
 
 #include "e1000_hw.h"
 
@@ -1044,7 +1045,7 @@ pci_e1000_uninit(PCIDevice *dev)
     return 0;
 }
 
-PCIDevice *
+static PCIDevice *
 pci_e1000_init(PCIBus *bus, NICInfo *nd, int devfn)
 {
     E1000State *d;
@@ -1106,3 +1107,12 @@ pci_e1000_init(PCIBus *bus, NICInfo *nd, int devfn)
 
     return (PCIDevice *)d;
 }
+
+static int e1000_init(void)
+{
+    pci_nic_register("e1000", pci_e1000_init);
+
+    return 0;
+}
+
+module_init(e1000_init);
diff --git a/hw/eepro100.c b/hw/eepro100.c
index 0a343df..5e8c412 100644
--- a/hw/eepro100.c
+++ b/hw/eepro100.c
@@ -44,6 +44,7 @@
 #include "pci.h"
 #include "net.h"
 #include "eeprom93xx.h"
+#include "module.h"
 
 /* Common declarations for all PCI devices. */
 
@@ -1761,20 +1762,31 @@ static PCIDevice *nic_init(PCIBus * bus, NICInfo * nd,
     return (PCIDevice *)d;
 }
 
-PCIDevice *pci_i82551_init(PCIBus * bus, NICInfo * nd, int devfn)
+static PCIDevice *pci_i82551_init(PCIBus * bus, NICInfo * nd, int devfn)
 {
     return nic_init(bus, nd, "i82551", i82551);
     //~ uint8_t *pci_conf = d->dev.config;
 }
 
-PCIDevice *pci_i82557b_init(PCIBus * bus, NICInfo * nd, int devfn)
+static PCIDevice *pci_i82557b_init(PCIBus * bus, NICInfo * nd, int devfn)
 {
     return nic_init(bus, nd, "i82557b", i82557B);
 }
 
-PCIDevice *pci_i82559er_init(PCIBus * bus, NICInfo * nd, int devfn)
+static PCIDevice *pci_i82559er_init(PCIBus * bus, NICInfo * nd, int devfn)
 {
     return nic_init(bus, nd, "i82559er", i82559ER);
 }
 
+static int eepro100_init(void)
+{
+    pci_nic_register("i82551", pci_i82551_init);
+    pci_nic_register("i82557b", pci_i82557b_init);
+    pci_nic_register("i82559er", pci_i82559er_init);
+
+    return 0;
+}
+
+module_init(eepro100_init);
+
 /* eof */
diff --git a/hw/etraxfs_eth.c b/hw/etraxfs_eth.c
index c87e55f..a03f383 100644
--- a/hw/etraxfs_eth.c
+++ b/hw/etraxfs_eth.c
@@ -560,8 +560,6 @@ void *etraxfs_eth_init(NICInfo *nd, CPUState *env,
 	struct etraxfs_dma_client *dma = NULL;	
 	struct fs_eth *eth = NULL;
 
-	qemu_check_nic_model(nd, "fseth");
-
 	dma = qemu_mallocz(sizeof *dma * 2);
 
 	eth = qemu_mallocz(sizeof *eth);
diff --git a/hw/mcf_fec.c b/hw/mcf_fec.c
index 413c569..49ae69b 100644
--- a/hw/mcf_fec.c
+++ b/hw/mcf_fec.c
@@ -446,8 +446,6 @@ void mcf_fec_init(NICInfo *nd, target_phys_addr_t base, qemu_irq *irq)
     mcf_fec_state *s;
     int iomemtype;
 
-    qemu_check_nic_model(nd, "mcf_fec");
-
     s = (mcf_fec_state *)qemu_mallocz(sizeof(mcf_fec_state));
     s->irq = irq;
     iomemtype = cpu_register_io_memory(0, mcf_fec_readfn,
diff --git a/hw/mipsnet.c b/hw/mipsnet.c
index 29bd9b8..f0df95e 100644
--- a/hw/mipsnet.c
+++ b/hw/mipsnet.c
@@ -236,8 +236,6 @@ void mipsnet_init (int base, qemu_irq irq, NICInfo *nd)
 {
     MIPSnetState *s;
 
-    qemu_check_nic_model(nd, "mipsnet");
-
     s = qemu_mallocz(sizeof(MIPSnetState));
 
     register_ioport_write(base, 36, 1, mipsnet_ioport_write, s);
diff --git a/hw/musicpal.c b/hw/musicpal.c
index 8eb1316..8758e13 100644
--- a/hw/musicpal.c
+++ b/hw/musicpal.c
@@ -719,8 +719,6 @@ static void mv88w8618_eth_init(NICInfo *nd, uint32_t base, qemu_irq irq)
     mv88w8618_eth_state *s;
     int iomemtype;
 
-    qemu_check_nic_model(nd, "mv88w8618");
-
     s = qemu_mallocz(sizeof(mv88w8618_eth_state));
     s->irq = irq;
     s->vc = qemu_new_vlan_client(nd->vlan, nd->model, nd->name,
diff --git a/hw/ne2000.c b/hw/ne2000.c
index 24a66bb..eb91115 100644
--- a/hw/ne2000.c
+++ b/hw/ne2000.c
@@ -25,6 +25,7 @@
 #include "pci.h"
 #include "pc.h"
 #include "net.h"
+#include "module.h"
 
 /* debug NE2000 card */
 //#define DEBUG_NE2000
@@ -722,8 +723,6 @@ void isa_ne2000_init(int base, qemu_irq irq, NICInfo *nd)
 {
     NE2000State *s;
 
-    qemu_check_nic_model(nd, "ne2k_isa");
-
     s = qemu_mallocz(sizeof(NE2000State));
 
     register_ioport_write(base, 16, 1, ne2000_ioport_write, s);
@@ -777,7 +776,7 @@ static void ne2000_map(PCIDevice *pci_dev, int region_num,
     register_ioport_read(addr + 0x1f, 1, 1, ne2000_reset_ioport_read, s);
 }
 
-PCIDevice *pci_ne2000_init(PCIBus *bus, NICInfo *nd, int devfn)
+static PCIDevice *pci_ne2000_init(PCIBus *bus, NICInfo *nd, int devfn)
 {
     PCINE2000State *d;
     NE2000State *s;
@@ -810,3 +809,12 @@ PCIDevice *pci_ne2000_init(PCIBus *bus, NICInfo *nd, int devfn)
 
     return (PCIDevice *)d;
 }
+
+static int ne2k_init(void)
+{
+    pci_nic_register("ne2k_pci", pci_ne2000_init);
+
+    return 0;
+}
+
+module_init(ne2k_init);
diff --git a/hw/pci.c b/hw/pci.c
index 18362d3..d62973f 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -25,7 +25,6 @@
 #include "pci.h"
 #include "monitor.h"
 #include "net.h"
-#include "virtio-net.h"
 #include "sysemu.h"
 
 //#define DEBUG_PCI
@@ -773,48 +772,90 @@ void pci_info(Monitor *mon)
     pci_for_each_device(0, pci_info_device);
 }
 
-static const char * const pci_nic_models[] = {
-    "ne2k_pci",
-    "i82551",
-    "i82557b",
-    "i82559er",
-    "rtl8139",
-    "e1000",
-    "pcnet",
-    "virtio",
-    NULL
-};
+typedef struct PCINICCallbacks
+{
+    const char *name;
+    PCINICInitFn initfn;
+    TAILQ_ENTRY(PCINICCallbacks) node;
+} PCINICCallbacks;
 
-typedef PCIDevice *(*PCINICInitFn)(PCIBus *, NICInfo *, int);
-
-static PCINICInitFn pci_nic_init_fns[] = {
-    pci_ne2000_init,
-    pci_i82551_init,
-    pci_i82557b_init,
-    pci_i82559er_init,
-    pci_rtl8139_init,
-    pci_e1000_init,
-    pci_pcnet_init,
-    virtio_net_init,
-    NULL
-};
+static TAILQ_HEAD(, PCINICCallbacks) pci_nic_callbacks;
+
+static void qemu_check_nic_model_list(NICInfo *nd,
+                                      const char *default_model)
+{
+    int exit_status = 0, first;
+    PCINICCallbacks *cb;
+
+    if (!nd->model)
+        nd->model = strdup(default_model);
+
+    if (strcmp(nd->model, "?") != 0) {
+        TAILQ_FOREACH(cb, &pci_nic_callbacks, node) {
+            if (strcmp(nd->model, cb->name) == 0)
+                return;
+        }
+
+        fprintf(stderr, "qemu: Unsupported NIC model: %s\n", nd->model);
+        exit_status = 1;
+    }
+
+    fprintf(stderr, "qemu: Supported NIC models: ");
+
+    first = 1;
+    TAILQ_FOREACH(cb, &pci_nic_callbacks, node) {
+        if (first)
+            first = 0;
+        else
+            fprintf(stderr, ",");
+        fprintf(stderr, "%s", cb->name);
+    }
+    fprintf(stderr, "\n");
+
+    exit(exit_status);
+}
+
+static void pci_nic_callbacks_init(void)
+{
+    static int inited;
+
+    if (!inited) {
+        TAILQ_INIT(&pci_nic_callbacks);
+        inited = 1;
+    }
+}
+
+void pci_nic_register(const char *name, PCINICInitFn fn)
+{
+    PCINICCallbacks *cb = qemu_mallocz(sizeof(*cb));
+
+    pci_nic_callbacks_init();
+
+    cb->name = name;
+    cb->initfn = fn;
+
+    TAILQ_INSERT_HEAD(&pci_nic_callbacks, cb, node);
+}
 
 /* Initialize a PCI NIC.  */
 PCIDevice *pci_nic_init(PCIBus *bus, NICInfo *nd, int devfn,
                   const char *default_model)
 {
     PCIDevice *pci_dev;
-    int i;
+    PCINICCallbacks *cb;
+
+    pci_nic_callbacks_init();
 
-    qemu_check_nic_model_list(nd, pci_nic_models, default_model);
+    qemu_check_nic_model_list(nd, default_model);
 
-    for (i = 0; pci_nic_models[i]; i++)
-        if (strcmp(nd->model, pci_nic_models[i]) == 0) {
-            pci_dev = pci_nic_init_fns[i](bus, nd, devfn);
+    TAILQ_FOREACH(cb, &pci_nic_callbacks, node) {
+        if (strcmp(nd->model, cb->name) == 0) {
+            pci_dev = cb->initfn(bus, nd, devfn);
             if (pci_dev)
                 nd->private = pci_dev;
             return pci_dev;
         }
+    }
 
     return NULL;
 }
diff --git a/hw/pci.h b/hw/pci.h
index 831f1b1..5e2afe9 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -179,8 +179,13 @@ typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num);
 PCIBus *pci_register_bus(pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
                          qemu_irq *pic, int devfn_min, int nirq);
 
+typedef PCIDevice *(*PCINICInitFn)(PCIBus *, NICInfo *, int);
+
+void pci_nic_register(const char *name, PCINICInitFn fn);
+
 PCIDevice *pci_nic_init(PCIBus *bus, NICInfo *nd, int devfn,
-                  const char *default_model);
+                        const char *default_model);
+
 void pci_data_write(void *opaque, uint32_t addr, uint32_t val, int len);
 uint32_t pci_data_read(void *opaque, uint32_t addr, int len);
 int pci_bus_num(PCIBus *s);
@@ -229,26 +234,6 @@ void usb_uhci_piix4_init(PCIBus *bus, int devfn);
 /* usb-ohci.c */
 void usb_ohci_init_pci(struct PCIBus *bus, int num_ports, int devfn);
 
-/* eepro100.c */
-
-PCIDevice *pci_i82551_init(PCIBus *bus, NICInfo *nd, int devfn);
-PCIDevice *pci_i82557b_init(PCIBus *bus, NICInfo *nd, int devfn);
-PCIDevice *pci_i82559er_init(PCIBus *bus, NICInfo *nd, int devfn);
-
-/* ne2000.c */
-
-PCIDevice *pci_ne2000_init(PCIBus *bus, NICInfo *nd, int devfn);
-
-/* rtl8139.c */
-
-PCIDevice *pci_rtl8139_init(PCIBus *bus, NICInfo *nd, int devfn);
-
-/* e1000.c */
-PCIDevice *pci_e1000_init(PCIBus *bus, NICInfo *nd, int devfn);
-
-/* pcnet.c */
-PCIDevice *pci_pcnet_init(PCIBus *bus, NICInfo *nd, int devfn);
-
 /* prep_pci.c */
 PCIBus *pci_prep_init(qemu_irq *pic);
 
diff --git a/hw/pcnet.c b/hw/pcnet.c
index 15167ac..65f6439 100644
--- a/hw/pcnet.c
+++ b/hw/pcnet.c
@@ -40,6 +40,7 @@
 #include "net.h"
 #include "qemu-timer.h"
 #include "qemu_socket.h"
+#include "module.h"
 
 //#define PCNET_DEBUG
 //#define PCNET_DEBUG_IO
@@ -1985,7 +1986,7 @@ static void pci_physical_memory_read(void *dma_opaque, target_phys_addr_t addr,
     cpu_physical_memory_read(addr, buf, len);
 }
 
-PCIDevice *pci_pcnet_init(PCIBus *bus, NICInfo *nd, int devfn)
+static PCIDevice *pci_pcnet_init(PCIBus *bus, NICInfo *nd, int devfn)
 {
     PCNetState *d;
     uint8_t *pci_conf;
@@ -2035,6 +2036,15 @@ PCIDevice *pci_pcnet_init(PCIBus *bus, NICInfo *nd, int devfn)
     return (PCIDevice *)d;
 }
 
+static int pcnet_mod_init(void)
+{
+    pci_nic_register("pcnet", pci_pcnet_init);
+
+    return 0;
+}
+
+module_init(pcnet_mod_init);
+
 /* SPARC32 interface */
 
 #if defined (TARGET_SPARC) && !defined(TARGET_SPARC64) // Avoid compile failure
@@ -2087,8 +2097,6 @@ void lance_init(NICInfo *nd, target_phys_addr_t leaddr, void *dma_opaque,
     PCNetState *d;
     int lance_io_memory;
 
-    qemu_check_nic_model(nd, "lance");
-
     d = qemu_mallocz(sizeof(PCNetState));
 
     lance_io_memory =
diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index 9fa69db..a0301e3 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -47,6 +47,7 @@
 #include "pci.h"
 #include "qemu-timer.h"
 #include "net.h"
+#include "module.h"
 
 /* debug RTL8139 card */
 //#define DEBUG_RTL8139 1
@@ -3414,7 +3415,7 @@ static void rtl8139_timer(void *opaque)
 }
 #endif /* RTL8139_ONBOARD_TIMER */
 
-PCIDevice *pci_rtl8139_init(PCIBus *bus, NICInfo *nd, int devfn)
+static PCIDevice *pci_rtl8139_init(PCIBus *bus, NICInfo *nd, int devfn)
 {
     PCIRTL8139State *d;
     RTL8139State *s;
@@ -3468,3 +3469,12 @@ PCIDevice *pci_rtl8139_init(PCIBus *bus, NICInfo *nd, int devfn)
 #endif /* RTL8139_ONBOARD_TIMER */
     return (PCIDevice *)d;
 }
+
+static int rtl8139_init(void)
+{
+    pci_nic_register("rtl8139", pci_rtl8139_init);
+
+    return 0;
+}
+
+module_init(rtl8139_init);
diff --git a/hw/smc91c111.c b/hw/smc91c111.c
index f5b29a7..27a3158 100644
--- a/hw/smc91c111.c
+++ b/hw/smc91c111.c
@@ -695,8 +695,6 @@ void smc91c111_init(NICInfo *nd, uint32_t base, qemu_irq irq)
     smc91c111_state *s;
     int iomemtype;
 
-    qemu_check_nic_model(nd, "smc91c111");
-
     s = (smc91c111_state *)qemu_mallocz(sizeof(smc91c111_state));
     iomemtype = cpu_register_io_memory(0, smc91c111_readfn,
                                        smc91c111_writefn, s);
diff --git a/hw/stellaris_enet.c b/hw/stellaris_enet.c
index 88c5620..a5cd163 100644
--- a/hw/stellaris_enet.c
+++ b/hw/stellaris_enet.c
@@ -389,8 +389,6 @@ void stellaris_enet_init(NICInfo *nd, uint32_t base, qemu_irq irq)
     stellaris_enet_state *s;
     int iomemtype;
 
-    qemu_check_nic_model(nd, "stellaris");
-
     s = (stellaris_enet_state *)qemu_mallocz(sizeof(stellaris_enet_state));
     iomemtype = cpu_register_io_memory(0, stellaris_enet_readfn,
                                        stellaris_enet_writefn, s);
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index ad55bb7..6b46a2f 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -15,6 +15,7 @@
 #include "net.h"
 #include "qemu-timer.h"
 #include "virtio-net.h"
+#include "module.h"
 
 #define VIRTIO_NET_VM_VERSION    6
 
@@ -560,7 +561,7 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
     return 0;
 }
 
-PCIDevice *virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn)
+static PCIDevice *virtio_net_probe(PCIBus *bus, NICInfo *nd, int devfn)
 {
     VirtIONet *n;
     static int virtio_net_id;
@@ -605,3 +606,11 @@ PCIDevice *virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn)
                     virtio_net_save, virtio_net_load, n);
     return (PCIDevice *)n;
 }
+
+static int virtio_net_init(void)
+{
+    pci_nic_register("virtio", virtio_net_probe);
+    return 0;
+}
+
+module_init(virtio_net_init);
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index 4e20349..390fe10 100644
--- a/hw/virtio-net.h
+++ b/hw/virtio-net.h
@@ -85,8 +85,6 @@ struct virtio_net_hdr_mrg_rxbuf
     uint16_t num_buffers;   /* Number of merged rx buffers */
 };
 
-PCIDevice *virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn);
-
 /*
  * Control virtqueue data structures
  *
diff --git a/module.c b/module.c
new file mode 100644
index 0000000..b011d30
--- /dev/null
+++ b/module.c
@@ -0,0 +1,63 @@
+/*
+ * QEMU Module Support
+ *
+ * Copyright IBM, Corp. 2009
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "module.h"
+
+int module_init_all(void)
+{
+    module_init_fn *initfn;
+    module_exit_fn *exitfn;
+    int ret = 0;
+
+    initfn = __start_initfuncs;
+    exitfn = __start_exitfuncs;
+    while (initfn != __stop_initfuncs) {
+        ret = (*initfn)();
+        if (ret != 0)
+            goto cleanup;
+        initfn++;
+        exitfn++;
+    }
+
+    return ret;
+
+cleanup:
+    while (exitfn != __start_exitfuncs) {
+        (*exitfn)();
+        exitfn--;
+    }
+
+    return ret;
+}
+
+void module_exit_all(void)
+{
+    module_exit_fn *exitfn = __stop_exitfuncs;
+
+    while (exitfn != __start_exitfuncs) {
+        exitfn--;
+        (*exitfn)();
+    }
+}
+
+static int dummy_init(void)
+{
+    return 0;
+}
+
+static void dummy_exit(void)
+{
+}
+
+module_init(dummy_init);
+module_exit(dummy_exit);
diff --git a/module.h b/module.h
new file mode 100644
index 0000000..eae224f
--- /dev/null
+++ b/module.h
@@ -0,0 +1,35 @@
+/*
+ * QEMU Module Support
+ *
+ * Copyright IBM, Corp. 2009
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef QEMU_MODULE_H
+#define QEMU_MODULE_H
+
+typedef int (*module_init_fn)(void);
+typedef void (*module_exit_fn)(void);
+
+#define module_init(func) \
+    static module_init_fn __attribute__((section("initfuncs"), used)) \
+           qemu__module__init__func = func
+
+#define module_exit(func) \
+    static module_exit_fn __attribute__((section("exitfuncs"), used)) \
+           qemu__module__exit__func = func
+
+extern module_init_fn __start_initfuncs[], __stop_initfuncs[];
+extern module_exit_fn __start_exitfuncs[], __stop_exitfuncs[];
+
+int module_init_all(void);
+
+void module_exit_all(void);
+
+#endif
diff --git a/net.c b/net.c
index 395ee4f..b258abd 100644
--- a/net.c
+++ b/net.c
@@ -1554,40 +1554,6 @@ static int nic_get_free_idx(void)
     return -1;
 }
 
-void qemu_check_nic_model(NICInfo *nd, const char *model)
-{
-    const char *models[2];
-
-    models[0] = model;
-    models[1] = NULL;
-
-    qemu_check_nic_model_list(nd, models, model);
-}
-
-void qemu_check_nic_model_list(NICInfo *nd, const char * const *models,
-                               const char *default_model)
-{
-    int i, exit_status = 0;
-
-    if (!nd->model)
-        nd->model = strdup(default_model);
-
-    if (strcmp(nd->model, "?") != 0) {
-        for (i = 0 ; models[i]; i++)
-            if (strcmp(nd->model, models[i]) == 0)
-                return;
-
-        fprintf(stderr, "qemu: Unsupported NIC model: %s\n", nd->model);
-        exit_status = 1;
-    }
-
-    fprintf(stderr, "qemu: Supported NIC models: ");
-    for (i = 0 ; models[i]; i++)
-        fprintf(stderr, "%s%c", models[i], models[i+1] ? ',' : '\n');
-
-    exit(exit_status);
-}
-
 int net_client_init(const char *device, const char *p)
 {
     char buf[1024];
diff --git a/net.h b/net.h
index 1a51be7..e5c373e 100644
--- a/net.h
+++ b/net.h
@@ -48,9 +48,6 @@ ssize_t qemu_sendv_packet(VLANClientState *vc, const struct iovec *iov,
                           int iovcnt);
 void qemu_send_packet(VLANClientState *vc, const uint8_t *buf, int size);
 void qemu_format_nic_info_str(VLANClientState *vc, uint8_t macaddr[6]);
-void qemu_check_nic_model(NICInfo *nd, const char *model);
-void qemu_check_nic_model_list(NICInfo *nd, const char * const *models,
-                               const char *default_model);
 void qemu_handler_true(void *opaque);
 
 void do_info_network(Monitor *mon);
diff --git a/vl.c b/vl.c
index 5e6c621..f282e4b 100644
--- a/vl.c
+++ b/vl.c
@@ -160,6 +160,8 @@ int main(int argc, char **argv)
 
 #include "qemu_socket.h"
 
+#include "module.h"
+
 #if defined(CONFIG_SLIRP)
 #include "libslirp.h"
 #endif
@@ -4304,6 +4306,11 @@ int main(int argc, char **argv, char **envp)
     }
 #endif
 
+    if (module_init_all() < 0) {
+        fprintf(stderr, "Module initialization failed\n");
+        exit(1);
+    }
+    atexit(module_exit_all);
     register_machines();
     machine = first_machine;
     cpu_model = NULL;

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [RFC] Introduce module API to QEMU
  2009-04-03  2:12 [Qemu-devel] [RFC] Introduce module API to QEMU Anthony Liguori
@ 2009-04-03  2:29 ` malc
  2009-04-03  3:36   ` Anthony Liguori
  2009-04-03  7:08 ` Mark McLoughlin
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 29+ messages in thread
From: malc @ 2009-04-03  2:29 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anthony Liguori, Paul Brook


Once again ISO/IEC 9899:1990 4.1.2
           ISO/IEC 9899:1999 7.1.3

-- 
mailto:av1474@comtv.ru

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [RFC] Introduce module API to QEMU
  2009-04-03  2:29 ` malc
@ 2009-04-03  3:36   ` Anthony Liguori
  2009-04-03  3:48     ` malc
  0 siblings, 1 reply; 29+ messages in thread
From: Anthony Liguori @ 2009-04-03  3:36 UTC (permalink / raw)
  To: malc; +Cc: qemu-devel, Paul Brook

malc wrote:
> Once again ISO/IEC 9899:1990 4.1.2
>   

I don't see a 4.1.2...  Section 4 is "Conformance".

>            ISO/IEC 9899:1999 7.1.3
>   

At any rate, I assume you're referring to:

> +extern module_init_fn __start_initfuncs[], __stop_initfuncs[];
> +extern module_exit_fn __start_exitfuncs[], __stop_exitfuncs[];

All sections have variable definitions that define their start and stop 
that are in the form __start_SECTION and __stop_SECTION.  The 
__attribute__((section("initfuncs"), used)) directive adds a new section 
(if necessary) named initfuncs and the linker will create these 
variables.  If you look closely, the patch doesn't define these 
variables, it just declares them because they're defined by the linker.

So this is exactly the sort of thing that the standard is there to 
protect :-)

Regards,

Anthony Liguori

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [RFC] Introduce module API to QEMU
  2009-04-03  3:36   ` Anthony Liguori
@ 2009-04-03  3:48     ` malc
  2009-04-03 12:59       ` Anthony Liguori
  0 siblings, 1 reply; 29+ messages in thread
From: malc @ 2009-04-03  3:48 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: qemu-devel, Paul Brook

On Thu, 2 Apr 2009, Anthony Liguori wrote:

> malc wrote:
> > Once again ISO/IEC 9899:1990 4.1.2
> >   
> 
> I don't see a 4.1.2...  Section 4 is "Conformance".

4.1.2 comes from C90

> >            ISO/IEC 9899:1999 7.1.3
> >   
>
> At any rate, I assume you're referring to:
> 
> > +extern module_init_fn __start_initfuncs[], __stop_initfuncs[];
> > +extern module_exit_fn __start_exitfuncs[], __stop_exitfuncs[];
> 
> All sections have variable definitions that define their start and stop 
> that are in the form __start_SECTION and __stop_SECTION.  The 
> __attribute__((section("initfuncs"), used)) directive adds a new section 
> (if necessary) named initfuncs and the linker will create these 
> variables.  If you look closely, the patch doesn't define these 
> variables, it just declares them because they're defined by the linker.
> 
> So this is exactly the sort of thing that the standard is there to 
> protect :-)

No. Those are reserved for _any_ use. For example, 6.2.5 states:

     31) An implementation may define new keywords that provide
         alternative ways to designate  a  basic  (or  any  other)
         type;  this does not violate the requirement that all
         basic   types   be   different.    Implementation-defined
         keywords  shall  have  the form of an identifier reserved
         for any use as described in 7.1.3.

-- 
mailto:av1474@comtv.ru

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [RFC] Introduce module API to QEMU
  2009-04-03  2:12 [Qemu-devel] [RFC] Introduce module API to QEMU Anthony Liguori
  2009-04-03  2:29 ` malc
@ 2009-04-03  7:08 ` Mark McLoughlin
  2009-04-03 13:01   ` Anthony Liguori
  2009-04-03 17:36   ` Anthony Liguori
  2009-04-03  7:50 ` Avi Kivity
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 29+ messages in thread
From: Mark McLoughlin @ 2009-04-03  7:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anthony Liguori, Paul Brook

Hey,

Generally looks good to me.

On Thu, 2009-04-02 at 21:12 -0500, Anthony Liguori wrote:
> diff --git a/hw/mcf_fec.c b/hw/mcf_fec.c
> index 413c569..49ae69b 100644
> --- a/hw/mcf_fec.c
> +++ b/hw/mcf_fec.c
> @@ -446,8 +446,6 @@ void mcf_fec_init(NICInfo *nd, target_phys_addr_t base, qemu_irq *irq)
>      mcf_fec_state *s;
>      int iomemtype;
>  
> -    qemu_check_nic_model(nd, "mcf_fec");

It's obscure, but this line does three things:

  1) Makes 'qemu-system-m68k -net nic,model=?' list the available model

  2) Makes 'qemu-system-m68k -net nic,model=e1000' fail

  3) Makes 'qemu-system-m68k -net nic' have sane 'info network' output 
     - i.e. the model is listed as mcf_fec

That goes for the other non-PCI NICs too.

Cheers,
Mark.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [RFC] Introduce module API to QEMU
  2009-04-03  2:12 [Qemu-devel] [RFC] Introduce module API to QEMU Anthony Liguori
  2009-04-03  2:29 ` malc
  2009-04-03  7:08 ` Mark McLoughlin
@ 2009-04-03  7:50 ` Avi Kivity
  2009-04-03 11:35 ` Paul Brook
  2009-04-03 14:04 ` Daniel Jacobowitz
  4 siblings, 0 replies; 29+ messages in thread
From: Avi Kivity @ 2009-04-03  7:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anthony Liguori, Paul Brook

Anthony Liguori wrote:
> It uses __attribute__((section)) to make module_init/module_exit work.  I looked
> at making this work by using a parser to find and extract all of these things.
> I'm not sure I know a good way to force the names to be unique via CPP but in
> the very least, I came to the determination that I would need to use something
> like perl or python which would introduce a new dependency to the build.
>
> I figured using the GCCism was a lesser burden since we don't attempt to support
> any compiler other than GCC today.
>   

If we're introducing a gccism, __attribute__((__constructor__)) is a 
lesser evil IMO, and much more understandable.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [RFC] Introduce module API to QEMU
  2009-04-03  2:12 [Qemu-devel] [RFC] Introduce module API to QEMU Anthony Liguori
                   ` (2 preceding siblings ...)
  2009-04-03  7:50 ` Avi Kivity
@ 2009-04-03 11:35 ` Paul Brook
  2009-04-03 12:57   ` Anthony Liguori
  2009-04-03 14:04 ` Daniel Jacobowitz
  4 siblings, 1 reply; 29+ messages in thread
From: Paul Brook @ 2009-04-03 11:35 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anthony Liguori

> This patch introduces a module API similar to what's in the Linux kernel. 
> This includes module_init/module_exit functions that register functions
> that are run at init and exit respectively.

Wouldn't it be much simpler to just have a list of device names, and assume 
the each device is implemented in $devicename., and provides 
$devicename_register ?

It's then an extremely simple shell script to collate and call these.

Paul

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [RFC] Introduce module API to QEMU
  2009-04-03 11:35 ` Paul Brook
@ 2009-04-03 12:57   ` Anthony Liguori
  2009-04-03 13:09     ` Paul Brook
  2009-04-03 17:12     ` Blue Swirl
  0 siblings, 2 replies; 29+ messages in thread
From: Anthony Liguori @ 2009-04-03 12:57 UTC (permalink / raw)
  To: qemu-devel

Paul Brook wrote:
>> This patch introduces a module API similar to what's in the Linux kernel. 
>> This includes module_init/module_exit functions that register functions
>> that are run at init and exit respectively.
>>     
>
> Wouldn't it be much simpler to just have a list of device names, and assume 
> the each device is implemented in $devicename., and provides 
> $devicename_register ?
>
> It's then an extremely simple shell script to collate and call these.
>   

It doesn't generalize very well.  For instance, the VNC server is not a 
device but it needs to be able to register as a DisplayState backend.

The other way to do this typically is to have module_init() functions 
that turn a function into a non-static function with some sort of 
annotation that disappears at compile time.  For instance:

module_init(virtnet_init);

Would become:

int qemu__module__init__virtnet_init(void)
{
     return virtnet_init();
}

with #define qemu__init__function

You then have something that searches for these functions.  The problem 
with this approach is that virtnet_init must be a unique name because 
it's a non-static.  You can add some uniqueness by playing with __LINE__ 
but that doesn't make any guarantees.

This is doable but IMHO a worse solution.  I'd rather use 
__attribute__((constructor)) for now and introduce the above only if we 
ever support something other than GCC.

Regards,

Anthony Liguori
> Paul
>
>
>   

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [RFC] Introduce module API to QEMU
  2009-04-03  3:48     ` malc
@ 2009-04-03 12:59       ` Anthony Liguori
  2009-04-03 17:00         ` malc
  0 siblings, 1 reply; 29+ messages in thread
From: Anthony Liguori @ 2009-04-03 12:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paul Brook

malc wrote:
> 4.1.2 comes from C90
>   

C99 would be a more reasonable thing to quote from since it updates C89.

>> So this is exactly the sort of thing that the standard is there to 
>> protect :-)
>>     
>
> No. Those are reserved for _any_ use. For example, 6.2.5 states:
>
>      31) An implementation may define new keywords that provide
>          alternative ways to designate  a  basic  (or  any  other)
>          type;  this does not violate the requirement that all
>          basic   types   be   different.    Implementation-defined
>          keywords  shall  have  the form of an identifier reserved
>          for any use as described in 7.1.3.
>   

I still think you're missing the fact that this is a variable that's 
provided by the compiler.  It's a GCC extension in the same way 
__attribute__ is.  The reason we have to declare the variables is 
because it's actually provided by the linker so GCC the front-end has no 
knowledge of these variables existence.

This isn't standard C99 but it certainly doesn't violate C99 either.

Regards,

Anthony Liguori

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [RFC] Introduce module API to QEMU
  2009-04-03  7:08 ` Mark McLoughlin
@ 2009-04-03 13:01   ` Anthony Liguori
  2009-04-03 17:36   ` Anthony Liguori
  1 sibling, 0 replies; 29+ messages in thread
From: Anthony Liguori @ 2009-04-03 13:01 UTC (permalink / raw)
  To: Mark McLoughlin, qemu-devel; +Cc: Paul Brook

Mark McLoughlin wrote:
> Hey,
>
> Generally looks good to me.
>
> On Thu, 2009-04-02 at 21:12 -0500, Anthony Liguori wrote:
>   
>> diff --git a/hw/mcf_fec.c b/hw/mcf_fec.c
>> index 413c569..49ae69b 100644
>> --- a/hw/mcf_fec.c
>> +++ b/hw/mcf_fec.c
>> @@ -446,8 +446,6 @@ void mcf_fec_init(NICInfo *nd, target_phys_addr_t base, qemu_irq *irq)
>>      mcf_fec_state *s;
>>      int iomemtype;
>>  
>> -    qemu_check_nic_model(nd, "mcf_fec");
>>     
>
> It's obscure, but this line does three things:
>
>   1) Makes 'qemu-system-m68k -net nic,model=?' list the available model
>
>   2) Makes 'qemu-system-m68k -net nic,model=e1000' fail
>
>   3) Makes 'qemu-system-m68k -net nic' have sane 'info network' output 
>      - i.e. the model is listed as mcf_fec
>
> That goes for the other non-PCI NICs too.
>   

Okay, I was afraid of this.  I'll make the non-PCI nics behave similar 
to the PCI ones.

Regards,

Anthony Liguori

> Cheers,
> Mark.
>
>
>
>   

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [RFC] Introduce module API to QEMU
  2009-04-03 12:57   ` Anthony Liguori
@ 2009-04-03 13:09     ` Paul Brook
  2009-04-03 14:10       ` Anthony Liguori
  2009-04-03 14:11       ` Anthony Liguori
  2009-04-03 17:12     ` Blue Swirl
  1 sibling, 2 replies; 29+ messages in thread
From: Paul Brook @ 2009-04-03 13:09 UTC (permalink / raw)
  To: qemu-devel

On Friday 03 April 2009, Anthony Liguori wrote:
> Paul Brook wrote:
> >> This patch introduces a module API similar to what's in the Linux
> >> kernel. This includes module_init/module_exit functions that register
> >> functions that are run at init and exit respectively.
> >
> > Wouldn't it be much simpler to just have a list of device names, and
> > assume the each device is implemented in $devicename., and provides
> > $devicename_register ?
> >
> > It's then an extremely simple shell script to collate and call these.
>
> It doesn't generalize very well.  For instance, the VNC server is not a
> device but it needs to be able to register as a DisplayState backend.

Hmm, this raises annother issue - we've got to be extremely careful about 
ordering. It's not inconcievable that the PCI support code would have 
constructors (e.g. to register a PCI bus type).

Paul

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [RFC] Introduce module API to QEMU
  2009-04-03  2:12 [Qemu-devel] [RFC] Introduce module API to QEMU Anthony Liguori
                   ` (3 preceding siblings ...)
  2009-04-03 11:35 ` Paul Brook
@ 2009-04-03 14:04 ` Daniel Jacobowitz
  2009-04-03 14:54   ` Avi Kivity
  4 siblings, 1 reply; 29+ messages in thread
From: Daniel Jacobowitz @ 2009-04-03 14:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anthony Liguori, Paul Brook

On Thu, Apr 02, 2009 at 09:12:35PM -0500, Anthony Liguori wrote:
> It uses __attribute__((section)) to make module_init/module_exit work.  I looked
> at making this work by using a parser to find and extract all of these things.
> I'm not sure I know a good way to force the names to be unique via CPP but in
> the very least, I came to the determination that I would need to use something
> like perl or python which would introduce a new dependency to the build.

FYI, I'm pretty sure this will not work in a Windows-hosted QEMU.
There's a mandatory minimum alignment for PE/COFF sections and it will
introduce unexpected padding between your array elements.  I had to
remove a similar trick from prelink when we ported it to Windows.

Yes, this does raise the question of how __attribute__((constructor))
works.  Maybe it's specific to new named sections?  Maybe I'm just
misremembering.  Anyway, whatever you come up with, testing on
Windows would be a good idea if you haven't already.

-- 
Daniel Jacobowitz
CodeSourcery

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [RFC] Introduce module API to QEMU
  2009-04-03 13:09     ` Paul Brook
@ 2009-04-03 14:10       ` Anthony Liguori
  2009-04-03 14:11       ` Anthony Liguori
  1 sibling, 0 replies; 29+ messages in thread
From: Anthony Liguori @ 2009-04-03 14:10 UTC (permalink / raw)
  To: Paul Brook; +Cc: qemu-devel

Paul Brook wrote:
> On Friday 03 April 2009, Anthony Liguori wrote:
>   
>> Paul Brook wrote:
>>     
>>>> This patch introduces a module API similar to what's in the Linux
>>>> kernel. This includes module_init/module_exit functions that register
>>>> functions that are run at init and exit respectively.
>>>>         
>>> Wouldn't it be much simpler to just have a list of device names, and
>>> assume the each device is implemented in $devicename., and provides
>>> $devicename_register ?
>>>
>>> It's then an extremely simple shell script to collate and call these.
>>>       
>> It doesn't generalize very well.  For instance, the VNC server is not a
>> device but it needs to be able to register as a DisplayState backend.
>>     
>
> Hmm, this raises annother issue - we've got to be extremely careful about 
> ordering. It's not inconcievable that the PCI support code would have 
> constructors (e.g. to register a PCI bus type).
>   

That's one reason I started with section instead of constructor.  You 
can control when you initialize with section which means you could have 
multiple types of constructors that allowed some form of ordering.  
Linux allows this via various *_initcall() methods.

Regards,

Anthony Liguori

> Paul
>   

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [RFC] Introduce module API to QEMU
  2009-04-03 13:09     ` Paul Brook
  2009-04-03 14:10       ` Anthony Liguori
@ 2009-04-03 14:11       ` Anthony Liguori
  2009-04-03 14:28         ` Paul Brook
  2009-04-03 14:53         ` Avi Kivity
  1 sibling, 2 replies; 29+ messages in thread
From: Anthony Liguori @ 2009-04-03 14:11 UTC (permalink / raw)
  To: Paul Brook; +Cc: qemu-devel

Paul Brook wrote:
> On Friday 03 April 2009, Anthony Liguori wrote:
>   
>> Paul Brook wrote:
>>     
>>>> This patch introduces a module API similar to what's in the Linux
>>>> kernel. This includes module_init/module_exit functions that register
>>>> functions that are run at init and exit respectively.
>>>>         
>>> Wouldn't it be much simpler to just have a list of device names, and
>>> assume the each device is implemented in $devicename., and provides
>>> $devicename_register ?
>>>
>>> It's then an extremely simple shell script to collate and call these.
>>>       
>> It doesn't generalize very well.  For instance, the VNC server is not a
>> device but it needs to be able to register as a DisplayState backend.
>>     
>
> Hmm, this raises annother issue - we've got to be extremely careful about 
> ordering. It's not inconcievable that the PCI support code would have 
> constructors (e.g. to register a PCI bus type).
>   

Looks like constructor/destructor has explicit support for ordering.  Neat.

Regards,

Anthony Liguori

> Paul
>   

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [RFC] Introduce module API to QEMU
  2009-04-03 14:11       ` Anthony Liguori
@ 2009-04-03 14:28         ` Paul Brook
  2009-04-03 14:53         ` Avi Kivity
  1 sibling, 0 replies; 29+ messages in thread
From: Paul Brook @ 2009-04-03 14:28 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: qemu-devel

> > Hmm, this raises annother issue - we've got to be extremely careful about
> > ordering. It's not inconcievable that the PCI support code would have
> > constructors (e.g. to register a PCI bus type).
>
> Looks like constructor/destructor has explicit support for ordering.  Neat.

IIRC that's not supported on all targets though.

Paul

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [RFC] Introduce module API to QEMU
  2009-04-03 14:11       ` Anthony Liguori
  2009-04-03 14:28         ` Paul Brook
@ 2009-04-03 14:53         ` Avi Kivity
  2009-04-03 16:15           ` Anthony Liguori
  1 sibling, 1 reply; 29+ messages in thread
From: Avi Kivity @ 2009-04-03 14:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paul Brook

Anthony Liguori wrote:
>>
>> Hmm, this raises annother issue - we've got to be extremely careful 
>> about ordering. It's not inconcievable that the PCI support code 
>> would have constructors (e.g. to register a PCI bus type).
>>   
>
> Looks like constructor/destructor has explicit support for ordering.  
> Neat.

I'd avoid it.  Have the constructor do a *pci_module_table++ = blah and 
iterate through that later.

I prefer to have the code explicit somewhere rather than gobs of 
unportable magic.  Use it where you have to, but not elsewhere.


-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [RFC] Introduce module API to QEMU
  2009-04-03 14:04 ` Daniel Jacobowitz
@ 2009-04-03 14:54   ` Avi Kivity
  2009-04-03 15:10     ` Paul Brook
  0 siblings, 1 reply; 29+ messages in thread
From: Avi Kivity @ 2009-04-03 14:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anthony Liguori, Paul Brook

Daniel Jacobowitz wrote:
> Yes, this does raise the question of how __attribute__((constructor))
> works.  Maybe it's specific to new named sections?  Maybe I'm just
> misremembering.  Anyway, whatever you come up with, testing on
> Windows would be a good idea if you haven't already.
>   

I'm guessing it's just a C interface to C++ static constructors.  Since 
gcc supports C++ on Windows, I'd expect this to work.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [RFC] Introduce module API to QEMU
  2009-04-03 14:54   ` Avi Kivity
@ 2009-04-03 15:10     ` Paul Brook
  0 siblings, 0 replies; 29+ messages in thread
From: Paul Brook @ 2009-04-03 15:10 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anthony Liguori, Avi Kivity

> > Yes, this does raise the question of how __attribute__((constructor))
> > works.  Maybe it's specific to new named sections?  Maybe I'm just
> > misremembering.  Anyway, whatever you come up with, testing on
> > Windows would be a good idea if you haven't already.
>
> I'm guessing it's just a C interface to C++ static constructors.  Since
> gcc supports C++ on Windows, I'd expect this to work.

It it has to, gcc will resort to using the equivalent of nm | grep on your 
object files to generate a list of constructors.

Paul

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [RFC] Introduce module API to QEMU
  2009-04-03 14:53         ` Avi Kivity
@ 2009-04-03 16:15           ` Anthony Liguori
  0 siblings, 0 replies; 29+ messages in thread
From: Anthony Liguori @ 2009-04-03 16:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paul Brook

Avi Kivity wrote:
> Anthony Liguori wrote:
>>>
>>> Hmm, this raises annother issue - we've got to be extremely careful 
>>> about ordering. It's not inconcievable that the PCI support code 
>>> would have constructors (e.g. to register a PCI bus type).
>>>   
>>
>> Looks like constructor/destructor has explicit support for ordering.  
>> Neat.
>
> I'd avoid it.  Have the constructor do a *pci_module_table++ = blah 
> and iterate through that later.

Yup, came to that conclusion too.

Regards,

Anthony Liguori

> I prefer to have the code explicit somewhere rather than gobs of 
> unportable magic.  Use it where you have to, but not elsewhere.
>
>

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [RFC] Introduce module API to QEMU
  2009-04-03 12:59       ` Anthony Liguori
@ 2009-04-03 17:00         ` malc
  2009-04-03 17:31           ` Anthony Liguori
  0 siblings, 1 reply; 29+ messages in thread
From: malc @ 2009-04-03 17:00 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paul Brook

On Fri, 3 Apr 2009, Anthony Liguori wrote:

> malc wrote:
> > 4.1.2 comes from C90
> >   
> 
> C99 would be a more reasonable thing to quote from since it updates C89.

Sigh, i provided c&v for both.

> 
> > > So this is exactly the sort of thing that the standard is there to protect
> > > :-)
> > >     
> > 
> > No. Those are reserved for _any_ use. For example, 6.2.5 states:
> > 
> >      31) An implementation may define new keywords that provide
> >          alternative ways to designate  a  basic  (or  any  other)
> >          type;  this does not violate the requirement that all
> >          basic   types   be   different.    Implementation-defined
> >          keywords  shall  have  the form of an identifier reserved
> >          for any use as described in 7.1.3.
> >   
> 
> I still think you're missing the fact that this is a variable that's 
> provided by the compiler.  It's a GCC extension in the same way 
> __attribute__ is.  The reason we have to declare the variables is 
> because it's actually provided by the linker so GCC the front-end has no 
> knowledge of these variables existence.
> 
> This isn't standard C99 but it certainly doesn't violate C99 either.

You are using _identifier_ whose name violates 7.1.3, full stop, you
can not do that in _any_ context.

-- 
mailto:av1474@comtv.ru

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [RFC] Introduce module API to QEMU
  2009-04-03 12:57   ` Anthony Liguori
  2009-04-03 13:09     ` Paul Brook
@ 2009-04-03 17:12     ` Blue Swirl
  1 sibling, 0 replies; 29+ messages in thread
From: Blue Swirl @ 2009-04-03 17:12 UTC (permalink / raw)
  To: qemu-devel

On 4/3/09, Anthony Liguori <anthony@codemonkey.ws> wrote:
> Paul Brook wrote:
>
> >
> > > This patch introduces a module API similar to what's in the Linux
> kernel. This includes module_init/module_exit functions that register
> functions
> > > that are run at init and exit respectively.
> > >
> > >
> >
> > Wouldn't it be much simpler to just have a list of device names, and
> assume the each device is implemented in $devicename., and provides
> $devicename_register ?
> >
> > It's then an extremely simple shell script to collate and call these.
> >
> >
>
>  It doesn't generalize very well.  For instance, the VNC server is not a
> device but it needs to be able to register as a DisplayState backend.
>
>  The other way to do this typically is to have module_init() functions that
> turn a function into a non-static function with some sort of annotation that
> disappears at compile time.  For instance:
>
>  module_init(virtnet_init);
>
>  Would become:
>
>  int qemu__module__init__virtnet_init(void)
>  {
>     return virtnet_init();
>  }
>
>  with #define qemu__init__function
>
>  You then have something that searches for these functions.  The problem
> with this approach is that virtnet_init must be a unique name because it's a
> non-static.  You can add some uniqueness by playing with __LINE__ but that
> doesn't make any guarantees.

The shell script could analyze the module files before compiling and
generate a .h file which contains #defines for the names of the
exported functions. The names can be made unique for example by
prepending the file name with some munging with tr.

This .h file would be included by the module when compiling, it
defines how the exported functions should be named.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [RFC] Introduce module API to QEMU
  2009-04-03 17:00         ` malc
@ 2009-04-03 17:31           ` Anthony Liguori
  2009-04-03 17:46             ` malc
  2009-04-03 17:58             ` M. Warner Losh
  0 siblings, 2 replies; 29+ messages in thread
From: Anthony Liguori @ 2009-04-03 17:31 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paul Brook

malc wrote:
> You are using _identifier_ whose name violates 7.1.3, full stop, you
> can not do that in _any_ context.
>   

So then we cannot use __func__ either or __attribute__ by your logic.

Regards,

Anthony Liguori

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [RFC] Introduce module API to QEMU
  2009-04-03  7:08 ` Mark McLoughlin
  2009-04-03 13:01   ` Anthony Liguori
@ 2009-04-03 17:36   ` Anthony Liguori
  1 sibling, 0 replies; 29+ messages in thread
From: Anthony Liguori @ 2009-04-03 17:36 UTC (permalink / raw)
  To: Mark McLoughlin, qemu-devel; +Cc: Paul Brook

Mark McLoughlin wrote:
> Hey,
>
> Generally looks good to me.
>
> On Thu, 2009-04-02 at 21:12 -0500, Anthony Liguori wrote:
>   
>> diff --git a/hw/mcf_fec.c b/hw/mcf_fec.c
>> index 413c569..49ae69b 100644
>> --- a/hw/mcf_fec.c
>> +++ b/hw/mcf_fec.c
>> @@ -446,8 +446,6 @@ void mcf_fec_init(NICInfo *nd, target_phys_addr_t base, qemu_irq *irq)
>>      mcf_fec_state *s;
>>      int iomemtype;
>>  
>> -    qemu_check_nic_model(nd, "mcf_fec");
>>     
>
> It's obscure, but this line does three things:
>
>   1) Makes 'qemu-system-m68k -net nic,model=?' list the available model
>
>   2) Makes 'qemu-system-m68k -net nic,model=e1000' fail
>
>   3) Makes 'qemu-system-m68k -net nic' have sane 'info network' output 
>      - i.e. the model is listed as mcf_fec
>   

After looking at it, I'm inclined to just ignore these issues.  For the 
targets where this matters, you cannot specify anything other than the 
default nic models.  They all hard code what nics are available and just 
use the number of nics to determine whether to support multiple of those 
nics.

Regards,

Anthony Liguori

> That goes for the other non-PCI NICs too.
>
> Cheers,
> Mark.
>
>
>
>   

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [RFC] Introduce module API to QEMU
  2009-04-03 17:31           ` Anthony Liguori
@ 2009-04-03 17:46             ` malc
  2009-04-03 17:59               ` M. Warner Losh
  2009-04-03 17:58             ` M. Warner Losh
  1 sibling, 1 reply; 29+ messages in thread
From: malc @ 2009-04-03 17:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paul Brook

On Fri, 3 Apr 2009, Anthony Liguori wrote:

> malc wrote:
> > You are using _identifier_ whose name violates 7.1.3, full stop, you
> > can not do that in _any_ context.
> >   
> 
> So then we cannot use __func__ either or __attribute__ by your logic.

a) __func__ is 6.4.2.2#1

b) __attribute__ is exactly the implementation defined keyword that
   6.5.2 31) i referenced earlier describes, so no you can not use
   that in a portable code and yes you can use it in code that is
   GCC specific and properly guarded

-- 
mailto:av1474@comtv.ru

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [RFC] Introduce module API to QEMU
  2009-04-03 17:31           ` Anthony Liguori
  2009-04-03 17:46             ` malc
@ 2009-04-03 17:58             ` M. Warner Losh
  2009-04-03 18:05               ` Anthony Liguori
  2009-04-03 20:12               ` Kevin Wolf
  1 sibling, 2 replies; 29+ messages in thread
From: M. Warner Losh @ 2009-04-03 17:58 UTC (permalink / raw)
  To: qemu-devel, anthony; +Cc: paul

In message: <49D6480F.2000408@codemonkey.ws>
            Anthony Liguori <anthony@codemonkey.ws> writes:
: malc wrote:
: > You are using _identifier_ whose name violates 7.1.3, full stop, you
: > can not do that in _any_ context.
: >   
: 
: So then we cannot use __func__ either or __attribute__ by your logic.

No.  That's not true.  Those are defined by the standard or by the
implementation.  he's objecting to your using an identifier that
starts with __ when really, there's no reason to do that.

Warner

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [RFC] Introduce module API to QEMU
  2009-04-03 17:46             ` malc
@ 2009-04-03 17:59               ` M. Warner Losh
  0 siblings, 0 replies; 29+ messages in thread
From: M. Warner Losh @ 2009-04-03 17:59 UTC (permalink / raw)
  To: qemu-devel, av1474; +Cc: paul

In message: <Pine.LNX.4.64.0904032139110.2406@linmac.oyster.ru>
            malc <av1474@comtv.ru> writes:
: On Fri, 3 Apr 2009, Anthony Liguori wrote:
: 
: > malc wrote:
: > > You are using _identifier_ whose name violates 7.1.3, full stop, you
: > > can not do that in _any_ context.
: > >   
: > 
: > So then we cannot use __func__ either or __attribute__ by your logic.
: 
: a) __func__ is 6.4.2.2#1
: 
: b) __attribute__ is exactly the implementation defined keyword that
:    6.5.2 31) i referenced earlier describes, so no you can not use
:    that in a portable code and yes you can use it in code that is
:    GCC specific and properly guarded

To back this discussion off a notch on the grumpiness scale...

The objection here is that you don't HAVE to use identifiers starting
with __, and using them is dangerous, so why not just remove the __
from the start and get on with your life?

Warner

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [RFC] Introduce module API to QEMU
  2009-04-03 17:58             ` M. Warner Losh
@ 2009-04-03 18:05               ` Anthony Liguori
  2009-04-03 20:12               ` Kevin Wolf
  1 sibling, 0 replies; 29+ messages in thread
From: Anthony Liguori @ 2009-04-03 18:05 UTC (permalink / raw)
  To: M. Warner Losh; +Cc: qemu-devel, paul

M. Warner Losh wrote:
> In message: <49D6480F.2000408@codemonkey.ws>
>             Anthony Liguori <anthony@codemonkey.ws> writes:
> : malc wrote:
> : > You are using _identifier_ whose name violates 7.1.3, full stop, you
> : > can not do that in _any_ context.
> : >   
> : 
> : So then we cannot use __func__ either or __attribute__ by your logic.
>
> No.  That's not true.  Those are defined by the standard or by the
> implementation.  he's objecting to your using an identifier that
> starts with __ when really, there's no reason to do that.
>   

The identifier is defined by the *implementation*.  That's the whole point.

When you use the __attribute__((section,"FOO")), it forces the variable 
to be in section FOO.  For all sections, the linker (IOW the 
*implementation*) creates two variables named __start_SECTION and 
__stop_SECTION.  In this case, it's __start_FOO and __stop_FOO.

You have to *declare* this variable yourself because the linker is not 
the frontend and the frontend doesn't know about these variables.  The 
linker is what *defines* these variables.

It doesn't matter because I've switched to __attribute__((constructor)), 
but this is not a violation of the standard.  You cannot rename these 
variables because you have no control over the definition.  They are 
there whether you like them or not.

Regards,

Anthony Liguori

> Warner
>   

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [RFC] Introduce module API to QEMU
  2009-04-03 20:12               ` Kevin Wolf
@ 2009-04-03 18:30                 ` malc
  0 siblings, 0 replies; 29+ messages in thread
From: malc @ 2009-04-03 18:30 UTC (permalink / raw)
  To: qemu-devel; +Cc: paul

On Fri, 3 Apr 2009, Kevin Wolf wrote:

> Am Freitag, 3. April 2009 19:58 schrieb M. Warner Losh:
> > In message: <49D6480F.2000408@codemonkey.ws>
> >
> >             Anthony Liguori <anthony@codemonkey.ws> writes:
> > : malc wrote:
> > : > You are using _identifier_ whose name violates 7.1.3, full stop, you
> > : > can not do that in _any_ context.
> > :
> > : So then we cannot use __func__ either or __attribute__ by your logic.
> >
> > No.  That's not true.  Those are defined by the standard or by the
> > implementation.  he's objecting to your using an identifier that
> > starts with __ when really, there's no reason to do that.
> 
> Except that it wouldn't work with a different name because these symbols are 
> ld ("the implementation") magic and don't exist with a different name? I 
> actually think this is a quite good reason.

I initially missed that part and should have been less holier than pope
in my postings, so my aplogies to Anthony.

-- 
mailto:av1474@comtv.ru

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [RFC] Introduce module API to QEMU
  2009-04-03 17:58             ` M. Warner Losh
  2009-04-03 18:05               ` Anthony Liguori
@ 2009-04-03 20:12               ` Kevin Wolf
  2009-04-03 18:30                 ` malc
  1 sibling, 1 reply; 29+ messages in thread
From: Kevin Wolf @ 2009-04-03 20:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: paul

Am Freitag, 3. April 2009 19:58 schrieb M. Warner Losh:
> In message: <49D6480F.2000408@codemonkey.ws>
>
>             Anthony Liguori <anthony@codemonkey.ws> writes:
> : malc wrote:
> : > You are using _identifier_ whose name violates 7.1.3, full stop, you
> : > can not do that in _any_ context.
> :
> : So then we cannot use __func__ either or __attribute__ by your logic.
>
> No.  That's not true.  Those are defined by the standard or by the
> implementation.  he's objecting to your using an identifier that
> starts with __ when really, there's no reason to do that.

Except that it wouldn't work with a different name because these symbols are 
ld ("the implementation") magic and don't exist with a different name? I 
actually think this is a quite good reason.

Kevin

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2009-04-03 18:30 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-04-03  2:12 [Qemu-devel] [RFC] Introduce module API to QEMU Anthony Liguori
2009-04-03  2:29 ` malc
2009-04-03  3:36   ` Anthony Liguori
2009-04-03  3:48     ` malc
2009-04-03 12:59       ` Anthony Liguori
2009-04-03 17:00         ` malc
2009-04-03 17:31           ` Anthony Liguori
2009-04-03 17:46             ` malc
2009-04-03 17:59               ` M. Warner Losh
2009-04-03 17:58             ` M. Warner Losh
2009-04-03 18:05               ` Anthony Liguori
2009-04-03 20:12               ` Kevin Wolf
2009-04-03 18:30                 ` malc
2009-04-03  7:08 ` Mark McLoughlin
2009-04-03 13:01   ` Anthony Liguori
2009-04-03 17:36   ` Anthony Liguori
2009-04-03  7:50 ` Avi Kivity
2009-04-03 11:35 ` Paul Brook
2009-04-03 12:57   ` Anthony Liguori
2009-04-03 13:09     ` Paul Brook
2009-04-03 14:10       ` Anthony Liguori
2009-04-03 14:11       ` Anthony Liguori
2009-04-03 14:28         ` Paul Brook
2009-04-03 14:53         ` Avi Kivity
2009-04-03 16:15           ` Anthony Liguori
2009-04-03 17:12     ` Blue Swirl
2009-04-03 14:04 ` Daniel Jacobowitz
2009-04-03 14:54   ` Avi Kivity
2009-04-03 15:10     ` Paul Brook

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).