Re: [Qemu-devel] [PATCH V4 0/7] CAN bus support for QEMU (SJA1000 PCI so far)

[Pavel Pisa]

Hello everybody,

On Tuesday 23 of January 2018 22:42:31 Pavel Pisa wrote:
> When Linux specific object file is linked in then some local
> function needs to be called before QOM instances population.
> I know how to do that GCC specific/non-portable way
>
> static void __attribute__((constructor)) can_socketcan_setup_variant(void)
> {
>
> }
>
> but I expect that something like
>
> module_init()
>
> in can_socketcan.c should be used.


I have experimented with code changes to get rid of stub for
non Linux systems. type_init() is used because it is more
portable than constructor attribute.

I have seen that a few other type_init-s do more
than simple sequence of type_register_static().
Is it acceptable to use type_init for registration
to CAN core by function call for now? Conversion simplifies
makefiles and unnecessary stub file is removed.

But I would use attribute if that solution is preferred because
it is allways present on Linux where SocketCAN is used anyway
and it is used in other Qemu subsystems as well.

----------------------------------------------------------------
Solution with attribute

#ifdef TOOLCHAIN_SUPPORTS_ATTRIBUTE_CONSTRUCTOR
static void __attribute__((constructor))
can_bus_socketcan_setup(void)
{
    can_bus_set_connect_to_host(can_bus_connect_to_host_socketcan);
}
#endif

----------------------------------------------------------------
Solution with type_init
branch https://gitlab.fel.cvut.cz/canbus/qemu-canbus/tree/can-pci-socketcan-init

diff --git a/hw/can/can_socketcan.c b/hw/can/can_socketcan.c
index 42099fb696..fb41853c2b 100644
--- a/hw/can/can_socketcan.c
+++ b/hw/can/can_socketcan.c
@@ -309,5 +309,14 @@ static int can_bus_connect_to_host_socketcan(CanBusState 
*bus,
     return 0;
 }
 
-int (*can_bus_connect_to_host_variant)(CanBusState *bus, const char *name) =
-        can_bus_connect_to_host_socketcan;
+static void can_bus_socketcan_type_init(void)
+{
+    /*
+     * There should be object registration when CanBusSocketcanConnectState
+     * is converted into QOM object. Use for registration of host
+     * can bus access for now.
+     */
+    can_bus_set_connect_to_host(can_bus_connect_to_host_socketcan);
+}
+
+type_init(can_bus_socketcan_type_init);


diff --git a/hw/can/Makefile.objs b/hw/can/Makefile.objs
index 1ce9950de0..14c2369718 100644
--- a/hw/can/Makefile.objs
+++ b/hw/can/Makefile.objs
@@ -2,11 +2,7 @@
 
 ifeq ($(CONFIG_CAN_BUS),y)
 common-obj-y += can_core.o
-ifeq ($(CONFIG_LINUX),y)
-common-obj-y += can_socketcan.o
-else
-common-obj-y += can_host_stub.o
-endif
+common-obj-$(CONFIG_LINUX) += can_socketcan.o
 common-obj-$(CONFIG_CAN_SJA1000) += can_sja1000.o
 common-obj-$(CONFIG_CAN_PCI) += can_kvaser_pci.o
 common-obj-$(CONFIG_CAN_PCI) += can_pcm3680_pci.o
diff --git a/hw/can/can_core.c b/hw/can/can_core.c
index 41c458c792..c14807b188 100644
--- a/hw/can/can_core.c
+++ b/hw/can/can_core.c
@@ -34,6 +34,8 @@
 static QTAILQ_HEAD(, CanBusState) can_buses =
     QTAILQ_HEAD_INITIALIZER(can_buses);
 
+static can_bus_connect_to_host_t can_bus_connect_to_host_fnc;
+
 CanBusState *can_bus_find_by_name(const char *name, bool create_missing)
 {
     CanBusState *bus;
@@ -127,10 +129,15 @@ int can_bus_client_set_filters(CanBusClientState *client,
 
 int can_bus_connect_to_host_device(CanBusState *bus, const char *name)
 {
-    if (can_bus_connect_to_host_variant == NULL) {
+    if (can_bus_connect_to_host_fnc == NULL) {
         error_report("CAN bus connect to host device is not "
                      "supported on this system");
         exit(1);
     }
-    return can_bus_connect_to_host_variant(bus, name);
+    return can_bus_connect_to_host_fnc(bus, name);
+}
+
+void can_bus_set_connect_to_host(can_bus_connect_to_host_t connect_fnc)
+{
+    can_bus_connect_to_host_fnc = connect_fnc;
 }
diff --git a/hw/can/can_host_stub.c b/hw/can/can_host_stub.c
deleted file mode 100644
index 748d25f995..0000000000
--- a/hw/can/can_host_stub.c
+++ /dev/null
@@ -1,36 +0,0 @@
....
....
....
-
-
-int (*can_bus_connect_to_host_variant)(CanBusState *bus, const char *name) =
-        NULL;

diff --git a/include/can/can_emu.h b/include/can/can_emu.h
index 85237ee3c9..7f0705e49f 100644
--- a/include/can/can_emu.h
+++ b/include/can/can_emu.h
@@ -107,8 +107,9 @@ struct CanBusState {
     QTAILQ_ENTRY(CanBusState) next;
 };
 
-extern int (*can_bus_connect_to_host_variant)(CanBusState *bus,
-                                              const char *name);
+typedef int (*can_bus_connect_to_host_t)(CanBusState *bus, const char *name);
+
+void can_bus_set_connect_to_host(can_bus_connect_to_host_t connect_fnc);
 
 int can_bus_filter_match(struct qemu_can_filter *filter, qemu_canid_t can_id);
----------------------------------------------------------------

Best wishes,

Pavel