Message ID | cover.1515960078.git.pisa@cmp.felk.cvut.cz |
---|---|
Headers | show |
Series | CAN bus support for QEMU (SJA1000 PCI so far) | expand |
Hi Pavel, On 01/14/2018 05:14 PM, pisa@cmp.felk.cvut.cz wrote: > From: Pavel Pisa <pisa@cmp.felk.cvut.cz> > > Basic emulation of CAN bus controller and interconnection for QEMU. > > Patches version 4: > Resolve comments longer than 80 characters to suppress > all warnings reported by scripts/checkpatch.pl. > Follow all suggestions from Frederic Konrad review. > Replace all printf and perror calls by QEMU equivalents. > Include Deniz Eren signed-off confimation. > > Patches version 3: > Support to connect to host SocketCAN interface has been > separated from the core bus implementation. Only simple > statically initialize pointer to the connection function > is used, no QOM concept for now. > SJA1000 message filters redone and code unified where > possible. > Basic documentation added. > QEMU_ALIGNED used in definition of CAN frame structure, > structure and defines are separated from Linux/SocketCAN > API defined ones to allow to keep QEMU message format > independed from host system one. Check for correspondence > to socketcan defines added. > > Patches version 2: > The bus emulation and the SJA1000 chip emulation introduced > by individual patches as suggested by Frederic Konrad. > Simple example board to test SJA1000 as single memory-mapped BAR > has been omitted in a new series because emulation of real > existing boards can provide same functions now. > Conditionalized debug printfs changed to be exposed to compiler > syntax check as suggested in review. > > The work has been started by Jin Yang in the frame of GSoC 2013 slot > contributed by RTEMS project which has been looking for environment > to allow develop and test CAN drivers for multiple CPU architectures. > > I have menthored the project and then done substantial code cleanup > and update to QOM. Deniz Eren then used emulation for SJA1000 base card > driver development for other operating system and contributed > PCM-3680I and MIOe-3680 support. > > Some page about the project > > https://gitlab.fel.cvut.cz/canbus/qemu-canbus/wikis/home > > FEE CTU GitLab repository with can-pci branch for 2.3, 2.4, 2.7, 2.8, 2.10 > and 2.11 QEMU version is available in the repository > > https://gitlab.fel.cvut.cz/canbus/qemu-canbus/tree/can-pci > > mirror at GitHub > > https://github.com/CTU-IIG/qemu > > There are many areas for improvement and extension of the code still > (for example freeze and migration is not implemented. CAN controllers > use proper QOM model but bus/interconnection emulation uses simple broadcast > connection which is required for CAN, but it is not based on QEMU bus model). > I have tried to look into QEMU VLANs implementation but it > does not map straightforward to CAN and I would need some help/opinion > from more advanced developers to decide what is their right > mapping to CAN. I think your series is quite ready to get accepted, although I'm not sure through with tree it will goes. Most of my review comments are not blocking issues and might get addressed later (like having an abstract sja1000). The principal issue I'd like to discuss with Paolo/Marc-André is the chardev backend, they might say it's OK to accept it in this current state and refactor the CANBus backend later. I'd still avoid using directly the socket() syscall and use the QEMU socket API instead (also suggested by Daniel). > > CAN-FD support would be interesting requires other developers/ > companies contributions or setup of some project to allow invite > some students and colleagues from my university into project. > > But I believe that (even in its actual state) provided solution > is great help for embedded systems developers when they can connect > SocketCAN from one or more embedded systems running in virtual > environment together or with Linux host SocketCAN virtual > or real bus interfaces. > > We have even tested our generic CANopen device configured > for CANopen 401 profile for generic I/O running in the virtual > system which can control GPIO inputs/outputs through virtual > industrial I/O card. > > Generally QEMU can be interesting setup which allows > to test complete industrial and automotive applications > in virtual environment even before real hardware is availabe. I have been thinking a bit about how to test some frame operations (rather than the PCI devices) and the Linux vcan driver might be a good option (Virtual Local CAN Interface). This is also useful to test this series without having CAN hardware. How to use vcan might be worth his own paragraph in docs/can.txt. Do you think some of your tests can be added in the QEMU test suite (qtests)? Regards, Phil. > > Deniz Eren (2): > CAN bus PCM-3680I PCI (dual SJA1000 channel) emulation added. > CAN bus MIOe-3680 PCI (dual SJA1000 channel) emulation added. > > Pavel Pisa (5): > CAN bus simple messages transport implementation for QEMU > CAN bus support to connect bust to Linux host SocketCAN interface. > CAN bus SJA1000 chip register level emulation for QEMU > CAN bus Kvaser PCI CAN-S (single SJA1000 channel) emulation added. > QEMU CAN bus emulation documentation > > default-configs/pci.mak | 3 + > docs/can.txt | 78 ++++ > hw/Makefile.objs | 1 + > hw/can/Makefile.objs | 14 + > hw/can/can_core.c | 136 ++++++ > hw/can/can_host_stub.c | 36 ++ > hw/can/can_kvaser_pci.c | 375 +++++++++++++++++ > hw/can/can_mioe3680_pci.c | 336 +++++++++++++++ > hw/can/can_pcm3680_pci.c | 336 +++++++++++++++ > hw/can/can_sja1000.c | 1013 +++++++++++++++++++++++++++++++++++++++++++++ > hw/can/can_sja1000.h | 167 ++++++++ > hw/can/can_socketcan.c | 314 ++++++++++++++ > include/can/can_emu.h | 131 ++++++ > 13 files changed, 2940 insertions(+) > create mode 100644 docs/can.txt > create mode 100644 hw/can/Makefile.objs > create mode 100644 hw/can/can_core.c > create mode 100644 hw/can/can_host_stub.c > create mode 100644 hw/can/can_kvaser_pci.c > create mode 100644 hw/can/can_mioe3680_pci.c > create mode 100644 hw/can/can_pcm3680_pci.c > create mode 100644 hw/can/can_sja1000.c > create mode 100644 hw/can/can_sja1000.h > create mode 100644 hw/can/can_socketcan.c > create mode 100644 include/can/can_emu.h >
Hello Philippe, On Monday 22 of January 2018 12:35:33 Philippe Mathieu-Daudé wrote: > Hi Pavel, > > On 01/14/2018 05:14 PM, pisa@cmp.felk.cvut.cz wrote: > > I think your series is quite ready to get accepted, although I'm not > sure through with tree it will goes. > > Most of my review comments are not blocking issues and might get > addressed later (like having an abstract sja1000). Great, I would be happy to reach acceptable state when development can switch to incremental mode as time allows. I hope than even actual state is usable from reports (at least for some usescases). > The principal issue I'd like to discuss with Paolo/Marc-André is the > chardev backend, they might say it's OK to accept it in this current > state and refactor the CANBus backend later. Do you think QOM based? I would like it to be implemented that way but I need some assistance where to look how this object kind should be implemented and from which base object inherit from. But I prefer to left that for later work. I would definitely like to use some mechanism which allows to get rid of externally visible pointer and need to assign it in the stub. It has been my initial idea and your review sumbled across this hack as well. But I need suggestion what is the preferred way for QEMU. 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. Problem is that there is not module_init type for plain function in include/qemu/module.h MODULE_INIT_BLOCK, MODULE_INIT_OPTS, MODULE_INIT_QOM, MODULE_INIT_TRACE, MODULE_INIT_MAX I expect that QOM object would solve that in future but I would be happy to left it simple for now. What is preferred solution there? > I'd still avoid using directly the socket() syscall and use the QEMU > socket API instead (also suggested by Daniel). I have already switched to qemu_socket(), implementation looks fine and I have tested that it works. I have tested functionality and updated can-pci branch. > I have been thinking a bit about how to test some frame operations > (rather than the PCI devices) and the Linux vcan driver might be a good > option (Virtual Local CAN Interface). This is also useful to test this > series without having CAN hardware. How to use vcan might be worth his > own paragraph in docs/can.txt. > > Do you think some of your tests can be added in the QEMU test suite > (qtests)? I have added some more infomation into docs file + The CAN interface of the host system has to be configured for proper + bitrate and set up. Configuration is not propagated from emulated + devices through bus to the physical host device. Example configuration + for 1 Mbit/s + + ip link set can0 type can bitrate 1000000 + ip link set can0 up + + Virtual (host local only) can interface can be used on the host + side instead of physical interface + + ip link add dev can0 type vcan + + The CAN interface on the host side can be used to analyze CAN + traffic with "candump" command which is included in "can-utils". + + candump can0 As for the automatic testing, iproute2 tools are required on host and guest side (considering use of Linux) and kernel with CAN drivers support. Root access is required on the host side to setup CAN interface. Some simple tool is required. It can be based on can-utils code or our older canping code for example. Best wishes, Pavel
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
Hi Pavel, On 01/24/2018 05:22 PM, Pavel Pisa wrote: > 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. using stubs/monitor.c as a template for stubs/can_host_variant.c doesn't work? > > ---------------------------------------------------------------- > 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 >
Hello Philippe, On Wednesday 24 of January 2018 22:41:16 Philippe Mathieu-Daudé wrote: > Hi Pavel, > > 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. > > using stubs/monitor.c as a template for stubs/can_host_variant.c doesn't > work? If that is preferred then I implement the stub this way. As for the location, can I add stub-obj-y += can_host_stub.o into hw/can/Makefile.objs same as it is in crypto/Makefile.objs to keep CAN stuff together at least for now or it should go to stubs directory? stub-obj-$(CONFIG_CAN_BUS) += can_host_variant.o As for connection to host, again I have weak preference to keep it in hw/can to keep CAN related code together but if you think that it should go t chardev, I would prepare patch that way chardev/can-socketcan.c As for the rest of the remarks You are right that there is some code duplication in the SJA1000 CAN PCI cards support but problem is that KVASER single uses S5920 PCI local bus bridge which requires additional BARs and additional bridge specific interrupt enable support. There are more KVASER variants which combine multiple SJA1000 in a single BAR. pcm3680 and mioe3680 have different BAR structure, each SJA1000 uses one BAR. The first uses I/O mapped SJA1000 and another memory mapped with stride 4. Yes, it all can be combined into one C file. But it would require to to add more more fields into CardX_PCIState structure and some mechanisms and code to select right combination of the BARs, handlers etc for each card. It all can be done, but I am not sure if I find time for such changes now. I expect to have time again in summer when my teaching semester ends. Another disadvantage is that if somebody else wants to implement other card emulation then actual simple can_kvaser_pci.c is easily readable. Code gets much more complex with all variants selection logic and we have already abandoned that simple PCI example (can_pci.c) with dummy PCI ID which as been included in the past. If the code duplication is a problem for now then I vote to include only can_kvaser_pci.c for now. But Deniz Eren would be sad because he uses the cards (which he has contributed) in his test environment. Anyway, I would follow what is proposed. Thanks for your review and time, Pavel
Hi Pavel, Philippe, I’m happy with whatever way is best for the project. However I would personally think merging the different drivers into one C file would not be a very modular way of handling the problem. As you can see from the Advantech drivers for example, the card supplier end can pose difficult Bar allocation logic, which I would think is best isolated within the particular driver model C file. Trying to come up with mechanisms to handle these could prove difficult. Also keep in mind there should be more driver support added in future, which could pose other driver model specific difficulties that are best isolated to their own C files. Having said all that, there might be a nice way of merging them all generically which I fail to see currently. Best regards, Deniz Sent from my iPhone Deniz Eren +61 400 307 762 > On 25 Jan 2018, at 7:24 pm, Pavel Pisa <pisa@cmp.felk.cvut.cz> wrote: > > Hello Philippe, > >> On Wednesday 24 of January 2018 22:41:16 Philippe Mathieu-Daudé wrote: >> Hi Pavel, >>> 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. >> >> using stubs/monitor.c as a template for stubs/can_host_variant.c doesn't >> work? > > If that is preferred then I implement the stub this way. > As for the location, can I add > > stub-obj-y += can_host_stub.o > > into hw/can/Makefile.objs same as it is in crypto/Makefile.objs > to keep CAN stuff together at least for now or it should go > to stubs directory? > > stub-obj-$(CONFIG_CAN_BUS) += can_host_variant.o > > As for connection to host, again I have weak preference > to keep it in hw/can to keep CAN related code together > but if you think that it should go t chardev, I would prepare > patch that way > > chardev/can-socketcan.c > > As for the rest of the remarks > > You are right that there is some code duplication > in the SJA1000 CAN PCI cards support but problem > is that KVASER single uses S5920 PCI local bus bridge > which requires additional BARs and additional bridge > specific interrupt enable support. There are more KVASER > variants which combine multiple SJA1000 in a single BAR. > pcm3680 and mioe3680 have different BAR structure, > each SJA1000 uses one BAR. The first uses I/O mapped > SJA1000 and another memory mapped with stride 4. > Yes, it all can be combined into one C file. > But it would require to to add more more fields > into CardX_PCIState structure and some mechanisms > and code to select right combination of the BARs, > handlers etc for each card. It all can be done, > but I am not sure if I find time for such changes now. > I expect to have time again in summer when my teaching > semester ends. > Another disadvantage is that if somebody else wants > to implement other card emulation then actual simple > can_kvaser_pci.c is easily readable. Code gets much > more complex with all variants selection logic and > we have already abandoned that simple PCI example > (can_pci.c) with dummy PCI ID which as been included > in the past. > > If the code duplication is a problem for now then > I vote to include only can_kvaser_pci.c for now. > But Deniz Eren would be sad because he uses the > cards (which he has contributed) in his test environment. > > Anyway, I would follow what is proposed. > > Thanks for your review and time, > > Pavel
On 23/01/2018 22:42, Pavel Pisa wrote: > Do you think QOM based? I would like it to be implemented > that way but I need some assistance where to look how this > object kind should be implemented and from which base object > inherit from. But I prefer to left that for later work. > > I would definitely like to use some mechanism which allows > to get rid of externally visible pointer and need to assign > it in the stub. It has been my initial idea and your review > sumbled across this hack as well. But I need suggestion what > is the preferred way for QEMU. The best way would be a QOM object. That is, you would do -object can-bus,id=canbus0 -object can-host-socketcan,id=can0-host,canbus=canbus0,if=can0 -device kvaser_pci,canbus=canbus0 In the current version, it's not clear to me: * what it means if multiple controllers have the same canbus * what it means if multiple controllers with the same canbus have different host interfaces Separating the QOM objects is a bit more work, but it makes the semantics clearer. The classes would be: - can-bus and an abstract class can-host, which would inherit directly from TYPE_OBJECT and implement TYPE_USER_CREATABLE - can-host-socketcan, which would inherit from can-host (and take the TYPE_USER_CREATABLE implementation from there) while CanBusClientState and CanBusClientInfo need not be QOMified. can-host's class structure would define a function pointer corresponding to what you have now for the function pointer, more or less---except that allocation is handled by QOM and the method only has to do the connection. You would have something like this: static void can_host_disconnect(CANHost *ch, Error **errp) { CANHostClass *chc = CAN_HOST_GET_CLASS(ch); chc->disconnect(ch); } static void can_host_connect(CANHost *ch, Error **errp) { CANHostClass *chc = CAN_HOST_GET_CLASS(ch); Error *local_err = NULL; chc->connect(ch, &local_err); if (local_err) { error_propagate(errp, local_err); return; } can_bus_insert_client(ch->bus, &ch->bus_client, local_err); if (local_err) { can_host_disconnect(ch); error_propagate(errp, local_err); return; } } and TYPE_USER_CREATABLE's "complete" method would simply invoke can_host_connect. For can-host-socketcan, chc->connect would be assigned something like this: static void can_host_socketcan_connect(CANHost *ch, Error **errp) { CANHostSocketCAN *chs = CAN_HOST_SOCKETCAN(ch); s = socket(PF_CAN, SOCK_RAW, CAN_RAW); if (s < 0) { error_setg_errno(errp, errno "CAN_RAW socket create failed"); return; } addr.can_family = AF_CAN; memset(&ifr.ifr_name, 0, sizeof(ifr.ifr_name)); strcpy(ifr.ifr_name, chs->host_dev_name); if (ioctl(s, SIOCGIFINDEX, &ifr) < 0) { error_setg_errno(errp, "host interface %s not available", chs->host_dev_name); goto fail; } addr.can_ifindex = ifr.ifr_ifindex; .... } In particular, note the difference in error reporting with error_report/exit vs. error_setg/error_propagate. Any call to "exit" is probably grounds for instant rejection of your patch. :) This also means that you have to check for leaks in the failure paths, such as forgetting to close the PF_CAN socket. Thanks, Paolo > 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. > > Problem is that there is not module_init > type for plain function in include/qemu/module.h > > MODULE_INIT_BLOCK, > MODULE_INIT_OPTS, > MODULE_INIT_QOM, > MODULE_INIT_TRACE, > MODULE_INIT_MAX > > I expect that QOM object would solve that in future > but I would be happy to left it simple for now. > > What is preferred solution there? > >> I'd still avoid using directly the socket() syscall and use the QEMU >> socket API instead (also suggested by Daniel). > > I have already switched to qemu_socket(), implementation > looks fine and I have tested that it works. > I have tested functionality and updated can-pci branch. > >> I have been thinking a bit about how to test some frame operations >> (rather than the PCI devices) and the Linux vcan driver might be a good >> option (Virtual Local CAN Interface). This is also useful to test this >> series without having CAN hardware. How to use vcan might be worth his >> own paragraph in docs/can.txt. >> >> Do you think some of your tests can be added in the QEMU test suite >> (qtests)? > > I have added some more infomation into docs file > > + The CAN interface of the host system has to be configured for proper > + bitrate and set up. Configuration is not propagated from emulated > + devices through bus to the physical host device. Example configuration > + for 1 Mbit/s > + > + ip link set can0 type can bitrate 1000000 > + ip link set can0 up > + > + Virtual (host local only) can interface can be used on the host > + side instead of physical interface > + > + ip link add dev can0 type vcan > + > + The CAN interface on the host side can be used to analyze CAN > + traffic with "candump" command which is included in "can-utils". > + > + candump can0 > > As for the automatic testing, iproute2 tools are required > on host and guest side (considering use of Linux) > and kernel with CAN drivers support. > Root access is required on the host side to setup CAN > interface. Some simple tool is required. It can be based > on can-utils code or our older canping code for example. > > Best wishes, > > Pavel >
Hello Paolo, thanks for suggestions. I understand and fully agree with your request to switch to QOM. I have succeed with that for CAN devices some time ago. It worth to be done for the rest of the objects but I fear that I do not find time to complete QOMification in reasonable future. Contributions/suggestions from other are welcomed. I can look for students for GSoC at our university or under other funding. On Thursday 25 of January 2018 14:58:41 Paolo Bonzini wrote: > On 23/01/2018 22:42, Pavel Pisa wrote: > > Do you think QOM based? I would like it to be implemented > > that way but I need some assistance where to look how this > > object kind should be implemented and from which base object > > inherit from. But I prefer to left that for later work. > > > > I would definitely like to use some mechanism which allows > > to get rid of externally visible pointer and need to assign > > it in the stub. It has been my initial idea and your review > > sumbled across this hack as well. But I need suggestion what > > is the preferred way for QEMU. > > The best way would be a QOM object. That is, you would do > > -object can-bus,id=canbus0 > -object can-host-socketcan,id=can0-host,canbus=canbus0,if=can0 > -device kvaser_pci,canbus=canbus0 I would prefer to allow CAN emulation to be used without explicit can-bus object creation specified on the command line. The bus object could be created when requested by can-host-socketcan or device (kvaser_pci) automatically. When multiple QOM object are put on the list then the list content should be preserved over save+restore (mostly hypothetical question for now). There should be probably used some other construct instead of QTAILQ_HEAD(, CanBusClientState) clients; QTAILQ_ENTRY(CanBusState) next; > In the current version, it's not clear to me: > > * what it means if multiple controllers have the same canbus That is fully supported and sane configuration. CAN is publis/subscribe network in principle so sending message from one controller to another one on the host side is intended and can be used to test drivers even if host connection is not available/supported on given OS/setup. Interconnection of multiple controllers with host CAN bus is functional as well. > * what it means if multiple controllers with the same canbus have > different host interfaces It is legitimate but probably not much usesfull/intended setup. It would result is bridging two host CAN busses together. It would work because SocketCAN does not deliver message back to the socket which has been used to send it by default. Connecting twice to the same SocketCAN bus would lead to infinite message loop. Connection of given can bus to the host twice can be prevented if host connection flag is included in CanBusState and if it is already set then next host connection attempt would be reported as error. > Separating the QOM objects is a bit more work, but it makes the > semantics clearer. The classes would be: > > - can-bus and an abstract class can-host, which would inherit directly > from TYPE_OBJECT and implement TYPE_USER_CREATABLE > > - can-host-socketcan, which would inherit from can-host (and take the > TYPE_USER_CREATABLE implementation from there) > > while CanBusClientState and CanBusClientInfo need not be QOMified. May it be, can-host-socketcan can be based directly on TYPE_OBJECT, because only QEMU CAN bus specific part is CanBusClientState which allows it to connect to the CAN bus same way as other CAN controllers connect to the bus. > can-host's class structure would define a function pointer corresponding > to what you have now for the function pointer, more or less---except > that allocation is handled by QOM and the method only has to do the > connection. You would have something like this: > > static void can_host_disconnect(CANHost *ch, Error **errp) > { > CANHostClass *chc = CAN_HOST_GET_CLASS(ch); > > chc->disconnect(ch); > } > > static void can_host_connect(CANHost *ch, Error **errp) > { > CANHostClass *chc = CAN_HOST_GET_CLASS(ch); > Error *local_err = NULL; > > chc->connect(ch, &local_err); > if (local_err) { > error_propagate(errp, local_err); > return; > } > > can_bus_insert_client(ch->bus, &ch->bus_client, local_err); > if (local_err) { > can_host_disconnect(ch); > error_propagate(errp, local_err); > return; > } > } > > and TYPE_USER_CREATABLE's "complete" method would simply invoke > can_host_connect. For can-host-socketcan, chc->connect would be > assigned something like this: > > static void can_host_socketcan_connect(CANHost *ch, Error **errp) > { > CANHostSocketCAN *chs = CAN_HOST_SOCKETCAN(ch); > > s = socket(PF_CAN, SOCK_RAW, CAN_RAW); > if (s < 0) { > error_setg_errno(errp, errno "CAN_RAW socket create failed"); > return; > } > > addr.can_family = AF_CAN; > memset(&ifr.ifr_name, 0, sizeof(ifr.ifr_name)); > strcpy(ifr.ifr_name, chs->host_dev_name); > if (ioctl(s, SIOCGIFINDEX, &ifr) < 0) { > error_setg_errno(errp, "host interface %s not available", > chs->host_dev_name); > goto fail; > } > addr.can_ifindex = ifr.ifr_ifindex; > .... > } Thanks for providing ideas for future development directions. > In particular, note the difference in error reporting with > error_report/exit vs. error_setg/error_propagate. Any call to "exit" is > probably grounds for instant rejection of your patch. :) It seems that it is necessary to switch to use realize() method instead of init() to have initial **errp pointer in the call chain. > This also means that you have to check for leaks in the failure paths, > such as forgetting to close the PF_CAN socket. The socket is closed in can_bus_socketcan_cleanup() in the failure case. g_free(c->rfilter); is there as well. Thanks for ideas and review, Pavel
On 25/01/2018 22:33, Pavel Pisa wrote: > Hello Paolo, > > thanks for suggestions. I understand and fully agree with your > request to switch to QOM. I have succeed with that for CAN devices > some time ago. It worth to be done for the rest of the objects > but I fear that I do not find time to complete QOMification > in reasonable future. Contributions/suggestions from other > are welcomed. I can look for students for GSoC at our university > or under other funding. Coincidentially, I have some time on a flight next Monday. :) I obviously cannot really _test_ anything, but I can at least do the changes below and set up all the QOM boilerplate. > On Thursday 25 of January 2018 14:58:41 Paolo Bonzini wrote: >> On 23/01/2018 22:42, Pavel Pisa wrote: >>> Do you think QOM based? I would like it to be implemented >>> that way but I need some assistance where to look how this >>> object kind should be implemented and from which base object >>> inherit from. But I prefer to left that for later work. >>> >>> I would definitely like to use some mechanism which allows >>> to get rid of externally visible pointer and need to assign >>> it in the stub. It has been my initial idea and your review >>> sumbled across this hack as well. But I need suggestion what >>> is the preferred way for QEMU. >> >> The best way would be a QOM object. That is, you would do >> >> -object can-bus,id=canbus0 >> -object can-host-socketcan,id=can0-host,canbus=canbus0,if=can0 >> -device kvaser_pci,canbus=canbus0 > > I would prefer to allow CAN emulation to be used without > explicit can-bus object creation specified on the command line. > The bus object could be created when requested by > can-host-socketcan or device (kvaser_pci) automatically. It could be fine to allow "-device kvaser_pci" to automatically create a private bus. However, IMO it's better to be explicit in the case where multiple objects (e.g. can-host-socketcan and the kvaser_pci device, or two controllers) want to connect to the same object. >> Separating the QOM objects is a bit more work, but it makes the >> semantics clearer. The classes would be: >> >> - can-bus and an abstract class can-host, which would inherit directly >> from TYPE_OBJECT and implement TYPE_USER_CREATABLE >> >> - can-host-socketcan, which would inherit from can-host (and take the >> TYPE_USER_CREATABLE implementation from there) >> >> while CanBusClientState and CanBusClientInfo need not be QOMified. > > May it be, can-host-socketcan can be based directly on TYPE_OBJECT, > because only QEMU CAN bus specific part is CanBusClientState which > allows it to connect to the CAN bus same way as other CAN controllers > connect to the bus. The abstract class "can-host" is useful to keep can-host-socketcan free of things that are not specific to SocketCAN, but it's just a small detail. Paolo
Hello Paolo, On Friday 26 of January 2018 12:12:32 Paolo Bonzini wrote: > Coincidentially, I have some time on a flight next Monday. :) I > obviously cannot really _test_ anything, but I can at least do the > changes below and set up all the QOM boilerplate. thanks much for offer to help. Deniz Eren or Oleksij Rempel can test your changes as well. I have prepared branch "can-pci-qom" in GitLab repository https://gitlab.fel.cvut.cz/canbus/qemu-canbus https://gitlab.fel.cvut.cz/canbus/qemu-canbus/tree/can-pci-qom and I have updated all PCI devices to use realize() instead of init(), eliminated all uses of exit() and changed error reporting to use error_setg() and error_setg_errno(). So I think that there is no fatal blocker in these files. Problematic is setup of connect_to_host variant can_bus_set_connect_to_host(can_bus_connect_to_host_socketcan); in "hw/can/can_socketcan.c" and location of this file in HW. I keep it there to have all CAN support *.c files in single place for now. I have studied "tests/check-qom-proplist.c" for while but I expect that you will be much more successfull and efficient to define mainline acceptable object model. Thanks again, Pavel > >> The best way would be a QOM object. That is, you would do > >> > >> -object can-bus,id=canbus0 > >> -object can-host-socketcan,id=can0-host,canbus=canbus0,if=can0 > >> -device kvaser_pci,canbus=canbus0 > > > > I would prefer to allow CAN emulation to be used without > > explicit can-bus object creation specified on the command line. > > The bus object could be created when requested by > > can-host-socketcan or device (kvaser_pci) automatically. > > It could be fine to allow "-device kvaser_pci" to automatically create a > private bus. However, IMO it's better to be explicit in the case where > multiple objects (e.g. can-host-socketcan and the kvaser_pci device, or > two controllers) want to connect to the same object. > > >> Separating the QOM objects is a bit more work, but it makes the > >> semantics clearer. The classes would be: > >> > >> - can-bus and an abstract class can-host, which would inherit directly > >> from TYPE_OBJECT and implement TYPE_USER_CREATABLE > >> > >> - can-host-socketcan, which would inherit from can-host (and take the > >> TYPE_USER_CREATABLE implementation from there) > >> > >> while CanBusClientState and CanBusClientInfo need not be QOMified. > > > > May it be, can-host-socketcan can be based directly on TYPE_OBJECT, > > because only QEMU CAN bus specific part is CanBusClientState which > > allows it to connect to the CAN bus same way as other CAN controllers > > connect to the bus. > > The abstract class "can-host" is useful to keep can-host-socketcan free > of things that are not specific to SocketCAN, but it's just a small detail.
Hi, On 28.01.2018 10:02, Pavel Pisa wrote: > Hello Paolo, > > On Friday 26 of January 2018 12:12:32 Paolo Bonzini wrote: >> Coincidentially, I have some time on a flight next Monday. :) I >> obviously cannot really _test_ anything, but I can at least do the >> changes below and set up all the QOM boilerplate. > > thanks much for offer to help. > > Deniz Eren or Oleksij Rempel can test your changes as well. just tell me wen and what should i test :) > I have prepared branch "can-pci-qom" in GitLab repository > > https://gitlab.fel.cvut.cz/canbus/qemu-canbus > > https://gitlab.fel.cvut.cz/canbus/qemu-canbus/tree/can-pci-qom > > and I have updated all PCI devices to use realize() instead of init(), > eliminated all uses of exit() and changed error reporting to use > error_setg() and error_setg_errno(). So I think that there is > no fatal blocker in these files. Problematic is setup > of connect_to_host variant > > can_bus_set_connect_to_host(can_bus_connect_to_host_socketcan); > > in "hw/can/can_socketcan.c" and location of this file > in HW. I keep it there to have all CAN support *.c > files in single place for now. > > I have studied "tests/check-qom-proplist.c" for while > but I expect that you will be much more successfull > and efficient to define mainline acceptable object model. > > Thanks again, > > Pavel > >>>> The best way would be a QOM object. That is, you would do >>>> >>>> -object can-bus,id=canbus0 >>>> -object can-host-socketcan,id=can0-host,canbus=canbus0,if=can0 >>>> -device kvaser_pci,canbus=canbus0 >>> >>> I would prefer to allow CAN emulation to be used without >>> explicit can-bus object creation specified on the command line. >>> The bus object could be created when requested by >>> can-host-socketcan or device (kvaser_pci) automatically. >> >> It could be fine to allow "-device kvaser_pci" to automatically create a >> private bus. However, IMO it's better to be explicit in the case where >> multiple objects (e.g. can-host-socketcan and the kvaser_pci device, or >> two controllers) want to connect to the same object. >> >>>> Separating the QOM objects is a bit more work, but it makes the >>>> semantics clearer. The classes would be: >>>> >>>> - can-bus and an abstract class can-host, which would inherit directly >>>> from TYPE_OBJECT and implement TYPE_USER_CREATABLE >>>> >>>> - can-host-socketcan, which would inherit from can-host (and take the >>>> TYPE_USER_CREATABLE implementation from there) >>>> >>>> while CanBusClientState and CanBusClientInfo need not be QOMified. >>> >>> May it be, can-host-socketcan can be based directly on TYPE_OBJECT, >>> because only QEMU CAN bus specific part is CanBusClientState which >>> allows it to connect to the CAN bus same way as other CAN controllers >>> connect to the bus. >> >> The abstract class "can-host" is useful to keep can-host-socketcan free >> of things that are not specific to SocketCAN, but it's just a small detail. > >
On 25/01/2018 22:33, Pavel Pisa wrote: > Hello Paolo, > > thanks for suggestions. I understand and fully agree with your > request to switch to QOM. I have succeed with that for CAN devices > some time ago. It worth to be done for the rest of the objects > but I fear that I do not find time to complete QOMification > in reasonable future. Contributions/suggestions from other > are welcomed. I can look for students for GSoC at our university > or under other funding. Please take a look at branch can-pci-qom of github.com/bonzini/qemu.git. Apart from QOMification of the backend include, I simplified the IRQ handling in can_kvaser_pci (fixing bugs too I think), and removed an unnecessary mutex. I also moved the files to net/can and hw/net/can so that in the future Jason (networking maintainer) can take care of pull requests. I might have broken something, and the top commit in particular is completely untested. Paolo
Hello Paolo, thanks much for conversion to acceptable QOM model. On Tuesday 30 of January 2018 15:15:22 Paolo Bonzini wrote: > On 25/01/2018 22:33, Pavel Pisa wrote: > > Hello Paolo, > > > > thanks for suggestions. I understand and fully agree with your > > request to switch to QOM. I have succeed with that for CAN devices > > some time ago. It worth to be done for the rest of the objects > > but I fear that I do not find time to complete QOMification > > in reasonable future. Contributions/suggestions from other > > are welcomed. I can look for students for GSoC at our university > > or under other funding. > > Please take a look at branch can-pci-qom of github.com/bonzini/qemu.git. > Apart from QOMification of the backend include, I simplified the IRQ > handling in can_kvaser_pci (fixing bugs too I think), and removed an > unnecessary mutex. I also moved the files to net/can and hw/net/can so > that in the future Jason (networking maintainer) can take care of pull > requests. > > I might have broken something, and the top commit in particular is > completely untested. I have run basic test with Linux kernel on both sides for one kavser_pci card on guest side and vcan (no real interface) on host side. Mesages exchange tests passed and looks OK. I have used next parameters -object can-bus,id=canbus0 \ -device kvaser_pci,canbus=canbus0 \ -object can-host-socketcan,if=can0,canbus=canbus0,id=canbus0-socketcan The id parameter is required for "can-host-socketcan" object. Else next error is printed qemu-system-x86_64: -object can-host-socketcan,if=can0,canbus=canbus0: Parameter 'id' is missing If "-object can-bus,id=canbus0" is missing then next error is reported qemu-system-x86_64: -object can-host-socketcan,if=can0,canbus=canbus0,id=canbus0-socketcan: Device 'canbus0' not found I have inspected through monitor the state of objects (qemu) qom-list /objects canbus0-socketcan (child<can-host-socketcan>) type (string) canbus0 (child<can-bus>) (qemu) info qom-tree /machine (pc-i440fx-2.12-machine) ... /peripheral-anon (container) /device[1] (kvaser_pci) /bus master[0] (qemu:memory-region) /kvaser_pci-xilinx[0] (qemu:memory-region) /kvaser_pci-s5920[0] (qemu:memory-region) /kvaser_pci-sja[0] (qemu:memory-region) /bus master container[0] (qemu:memory-region) ... (qemu) qom-list /objects canbus0-socketcan (child<can-host-socketcan>) type (string) canbus0 (child<can-bus>) (qemu) qom-list /machine/peripheral-anon/device[1] bus master container[0] (child<qemu:memory-region>) canbus (link<can-bus>) rombar (uint32) hotpluggable (bool) x-pcie-lnksta-dllla (bool) kvaser_pci-sja[0] (child<qemu:memory-region>) multifunction (bool) hotplugged (bool) parent_bus (link<bus>) romfile (str) kvaser_pci-s5920[0] (child<qemu:memory-region>) x-pcie-extcap-init (bool) command_serr_enable (bool) addr (int32) type (string) legacy-addr (str) kvaser_pci-xilinx[0] (child<qemu:memory-region>) realized (bool) bus master[0] (child<qemu:memory-region>) From the user point of view, it would be nice if "can-bus" can be populated when required automatically. I am not sure, but may it be that it would worth to push can-bus objects under some category/specific container. The path /objects is quite wide. Into something like /object/can-bus or /net/can. But generally thanks much, the progress you have made in one day is really great. I hope that others check your branch. I have pushed your unmodified version into "can-pci-qom" branch of my repo https://gitlab.fel.cvut.cz/canbus/qemu-canbus/tree/can-pci-qom It would be great if others can check that everything works in their setup. I think that then it can be pushed into mainline and some usability improvements can be done/experiment with later. Thanks much, Pavel Pisa
Hi Pavel, Paolo, I tried to rerun my environment to test however it seems the interface has changed a little and my standard program options cause complaints. Unfortunately I don’t have too much time to dig through at the moment. My standard startup command is: $ ./qemu-local/bin/qemu-system-i386 -hda sdd2gb-uno1483-16.04-2.0-dev.img -boot d -k en-us -device mioe3680_pci,canbus1=canbus0,host1=vcan0,canbus2=canbus1,host2=vcan1 -m size=2048 -netdev user,id=user.0 -device e1000,netdev=user.0 -redir tcp:5022::22 -enable-kvm & Best regards, Deniz Sent from my iPhone Deniz Eren +61 400 307 762 > On 31 Jan 2018, at 9:12 am, Pavel Pisa <pisa@cmp.felk.cvut.cz> wrote: > > Hello Paolo, > > thanks much for conversion to acceptable QOM model. > >> On Tuesday 30 of January 2018 15:15:22 Paolo Bonzini wrote: >>> On 25/01/2018 22:33, Pavel Pisa wrote: >>> Hello Paolo, >>> >>> thanks for suggestions. I understand and fully agree with your >>> request to switch to QOM. I have succeed with that for CAN devices >>> some time ago. It worth to be done for the rest of the objects >>> but I fear that I do not find time to complete QOMification >>> in reasonable future. Contributions/suggestions from other >>> are welcomed. I can look for students for GSoC at our university >>> or under other funding. >> >> Please take a look at branch can-pci-qom of github.com/bonzini/qemu.git. >> Apart from QOMification of the backend include, I simplified the IRQ >> handling in can_kvaser_pci (fixing bugs too I think), and removed an >> unnecessary mutex. I also moved the files to net/can and hw/net/can so >> that in the future Jason (networking maintainer) can take care of pull >> requests. >> >> I might have broken something, and the top commit in particular is >> completely untested. > > I have run basic test with Linux kernel on both sides > for one kavser_pci card on guest side and vcan (no real interface) > on host side. > > Mesages exchange tests passed and looks OK. > > I have used next parameters > > -object can-bus,id=canbus0 \ > -device kvaser_pci,canbus=canbus0 \ > -object can-host-socketcan,if=can0,canbus=canbus0,id=canbus0-socketcan > > The id parameter is required for "can-host-socketcan" object. > Else next error is printed > > qemu-system-x86_64: -object can-host-socketcan,if=can0,canbus=canbus0: Parameter 'id' is missing > > If "-object can-bus,id=canbus0" is missing then next error is reported > > qemu-system-x86_64: -object can-host-socketcan,if=can0,canbus=canbus0,id=canbus0-socketcan: Device 'canbus0' not found > > I have inspected through monitor the state of objects > > (qemu) qom-list /objects > canbus0-socketcan (child<can-host-socketcan>) > type (string) > canbus0 (child<can-bus>) > > (qemu) info qom-tree > /machine (pc-i440fx-2.12-machine) > ... > /peripheral-anon (container) > /device[1] (kvaser_pci) > /bus master[0] (qemu:memory-region) > /kvaser_pci-xilinx[0] (qemu:memory-region) > /kvaser_pci-s5920[0] (qemu:memory-region) > /kvaser_pci-sja[0] (qemu:memory-region) > /bus master container[0] (qemu:memory-region) > ... > > > (qemu) qom-list /objects > canbus0-socketcan (child<can-host-socketcan>) > type (string) > canbus0 (child<can-bus>) > > (qemu) qom-list /machine/peripheral-anon/device[1] > bus master container[0] (child<qemu:memory-region>) > canbus (link<can-bus>) > rombar (uint32) > hotpluggable (bool) > x-pcie-lnksta-dllla (bool) > kvaser_pci-sja[0] (child<qemu:memory-region>) > multifunction (bool) > hotplugged (bool) > parent_bus (link<bus>) > romfile (str) > kvaser_pci-s5920[0] (child<qemu:memory-region>) > x-pcie-extcap-init (bool) > command_serr_enable (bool) > addr (int32) > type (string) > legacy-addr (str) > kvaser_pci-xilinx[0] (child<qemu:memory-region>) > realized (bool) > bus master[0] (child<qemu:memory-region>) > > From the user point of view, it would be nice if "can-bus" > can be populated when required automatically. > > I am not sure, but may it be that it would worth to > push can-bus objects under some category/specific > container. The path /objects is quite wide. > Into something like /object/can-bus or /net/can. > > But generally thanks much, the progress you have made > in one day is really great. I hope that others check > your branch. I have pushed your unmodified version into > "can-pci-qom" branch of my repo > > https://gitlab.fel.cvut.cz/canbus/qemu-canbus/tree/can-pci-qom > > It would be great if others can check that everything > works in their setup. I think that then it can be pushed > into mainline and some usability improvements can be > done/experiment with later. > > Thanks much, > > > Pavel Pisa
On 30/01/2018 19:13, Deniz Eren wrote: > Hi Pavel, Paolo, > > I tried to rerun my environment to test however it seems the interface has changed a little and my standard program options cause complaints. Unfortunately I don’t have too much time to dig through at the moment. > > My standard startup command is: > > $ ./qemu-local/bin/qemu-system-i386 -hda sdd2gb-uno1483-16.04-2.0-dev.img -boot d -k en-us -device mioe3680_pci,canbus1=canbus0,host1=vcan0,canbus2=canbus1,host2=vcan1 -m size=2048 -netdev user,id=user.0 -device e1000,netdev=user.0 -redir tcp:5022::22 -enable-kvm & Yep, it's now like this: ./qemu-local/bin/qemu-system-i386 \ -hda sdd2gb-uno1483-16.04-2.0-dev.img -boot d -k en-us \ -object can-bus,id=canbus0 \ -object can-bus,id=canbus1 \ -object can-host-socketcan,id=canhost0,canbus=canbus0,ifname=vcan0 \ -object can-host-socketcan,id=canhost1,canbus=canbus1,ifname=vcan1 \ -device mioe3680_pci,canbus0=canbus0,canbus1=canbus1 \ -m size=2048 -netdev user,id=user.0 -device e1000,netdev=user.0 \ -redir tcp:5022::22 -enable-kvm Thanks, Paolo > > > > Best regards, > Deniz > > Sent from my iPhone > > Deniz Eren > +61 400 307 762 > >> On 31 Jan 2018, at 9:12 am, Pavel Pisa <pisa@cmp.felk.cvut.cz> wrote: >> >> Hello Paolo, >> >> thanks much for conversion to acceptable QOM model. >> >>> On Tuesday 30 of January 2018 15:15:22 Paolo Bonzini wrote: >>>> On 25/01/2018 22:33, Pavel Pisa wrote: >>>> Hello Paolo, >>>> >>>> thanks for suggestions. I understand and fully agree with your >>>> request to switch to QOM. I have succeed with that for CAN devices >>>> some time ago. It worth to be done for the rest of the objects >>>> but I fear that I do not find time to complete QOMification >>>> in reasonable future. Contributions/suggestions from other >>>> are welcomed. I can look for students for GSoC at our university >>>> or under other funding. >>> >>> Please take a look at branch can-pci-qom of github.com/bonzini/qemu.git. >>> Apart from QOMification of the backend include, I simplified the IRQ >>> handling in can_kvaser_pci (fixing bugs too I think), and removed an >>> unnecessary mutex. I also moved the files to net/can and hw/net/can so >>> that in the future Jason (networking maintainer) can take care of pull >>> requests. >>> >>> I might have broken something, and the top commit in particular is >>> completely untested. >> >> I have run basic test with Linux kernel on both sides >> for one kavser_pci card on guest side and vcan (no real interface) >> on host side. >> >> Mesages exchange tests passed and looks OK. >> >> I have used next parameters >> >> -object can-bus,id=canbus0 \ >> -device kvaser_pci,canbus=canbus0 \ >> -object can-host-socketcan,if=can0,canbus=canbus0,id=canbus0-socketcan >> >> The id parameter is required for "can-host-socketcan" object. >> Else next error is printed >> >> qemu-system-x86_64: -object can-host-socketcan,if=can0,canbus=canbus0: Parameter 'id' is missing >> >> If "-object can-bus,id=canbus0" is missing then next error is reported >> >> qemu-system-x86_64: -object can-host-socketcan,if=can0,canbus=canbus0,id=canbus0-socketcan: Device 'canbus0' not found >> >> I have inspected through monitor the state of objects >> >> (qemu) qom-list /objects >> canbus0-socketcan (child<can-host-socketcan>) >> type (string) >> canbus0 (child<can-bus>) >> >> (qemu) info qom-tree >> /machine (pc-i440fx-2.12-machine) >> ... >> /peripheral-anon (container) >> /device[1] (kvaser_pci) >> /bus master[0] (qemu:memory-region) >> /kvaser_pci-xilinx[0] (qemu:memory-region) >> /kvaser_pci-s5920[0] (qemu:memory-region) >> /kvaser_pci-sja[0] (qemu:memory-region) >> /bus master container[0] (qemu:memory-region) >> ... >> >> >> (qemu) qom-list /objects >> canbus0-socketcan (child<can-host-socketcan>) >> type (string) >> canbus0 (child<can-bus>) >> >> (qemu) qom-list /machine/peripheral-anon/device[1] >> bus master container[0] (child<qemu:memory-region>) >> canbus (link<can-bus>) >> rombar (uint32) >> hotpluggable (bool) >> x-pcie-lnksta-dllla (bool) >> kvaser_pci-sja[0] (child<qemu:memory-region>) >> multifunction (bool) >> hotplugged (bool) >> parent_bus (link<bus>) >> romfile (str) >> kvaser_pci-s5920[0] (child<qemu:memory-region>) >> x-pcie-extcap-init (bool) >> command_serr_enable (bool) >> addr (int32) >> type (string) >> legacy-addr (str) >> kvaser_pci-xilinx[0] (child<qemu:memory-region>) >> realized (bool) >> bus master[0] (child<qemu:memory-region>) >> >> From the user point of view, it would be nice if "can-bus" >> can be populated when required automatically. >> >> I am not sure, but may it be that it would worth to >> push can-bus objects under some category/specific >> container. The path /objects is quite wide. >> Into something like /object/can-bus or /net/can. >> >> But generally thanks much, the progress you have made >> in one day is really great. I hope that others check >> your branch. I have pushed your unmodified version into >> "can-pci-qom" branch of my repo >> >> https://gitlab.fel.cvut.cz/canbus/qemu-canbus/tree/can-pci-qom >> >> It would be great if others can check that everything >> works in their setup. I think that then it can be pushed >> into mainline and some usability improvements can be >> done/experiment with later. >> >> Thanks much, >> >> >> Pavel Pisa
On 30/01/2018 20:08, Paolo Bonzini wrote: > On 30/01/2018 19:13, Deniz Eren wrote: >> Hi Pavel, Paolo, >> >> I tried to rerun my environment to test however it seems the interface has changed a little and my standard program options cause complaints. Unfortunately I don’t have too much time to dig through at the moment. >> >> My standard startup command is: >> >> $ ./qemu-local/bin/qemu-system-i386 -hda sdd2gb-uno1483-16.04-2.0-dev.img -boot d -k en-us -device mioe3680_pci,canbus1=canbus0,host1=vcan0,canbus2=canbus1,host2=vcan1 -m size=2048 -netdev user,id=user.0 -device e1000,netdev=user.0 -redir tcp:5022::22 -enable-kvm & > > Yep, it's now like this: > > ./qemu-local/bin/qemu-system-i386 \ > -hda sdd2gb-uno1483-16.04-2.0-dev.img -boot d -k en-us \ > -object can-bus,id=canbus0 \ > -object can-bus,id=canbus1 \ > -object can-host-socketcan,id=canhost0,canbus=canbus0,ifname=vcan0 \ > -object can-host-socketcan,id=canhost1,canbus=canbus1,ifname=vcan1 \ > -device mioe3680_pci,canbus0=canbus0,canbus1=canbus1 \ > -m size=2048 -netdev user,id=user.0 -device e1000,netdev=user.0 \ > -redir tcp:5022::22 -enable-kvm Sorry, all "ifname" are "if". Paolo > Thanks, > > Paolo > >> >> >> >> Best regards, >> Deniz >> >> Sent from my iPhone >> >> Deniz Eren >> +61 400 307 762 >> >>> On 31 Jan 2018, at 9:12 am, Pavel Pisa <pisa@cmp.felk.cvut.cz> wrote: >>> >>> Hello Paolo, >>> >>> thanks much for conversion to acceptable QOM model. >>> >>>> On Tuesday 30 of January 2018 15:15:22 Paolo Bonzini wrote: >>>>> On 25/01/2018 22:33, Pavel Pisa wrote: >>>>> Hello Paolo, >>>>> >>>>> thanks for suggestions. I understand and fully agree with your >>>>> request to switch to QOM. I have succeed with that for CAN devices >>>>> some time ago. It worth to be done for the rest of the objects >>>>> but I fear that I do not find time to complete QOMification >>>>> in reasonable future. Contributions/suggestions from other >>>>> are welcomed. I can look for students for GSoC at our university >>>>> or under other funding. >>>> >>>> Please take a look at branch can-pci-qom of github.com/bonzini/qemu.git. >>>> Apart from QOMification of the backend include, I simplified the IRQ >>>> handling in can_kvaser_pci (fixing bugs too I think), and removed an >>>> unnecessary mutex. I also moved the files to net/can and hw/net/can so >>>> that in the future Jason (networking maintainer) can take care of pull >>>> requests. >>>> >>>> I might have broken something, and the top commit in particular is >>>> completely untested. >>> >>> I have run basic test with Linux kernel on both sides >>> for one kavser_pci card on guest side and vcan (no real interface) >>> on host side. >>> >>> Mesages exchange tests passed and looks OK. >>> >>> I have used next parameters >>> >>> -object can-bus,id=canbus0 \ >>> -device kvaser_pci,canbus=canbus0 \ >>> -object can-host-socketcan,if=can0,canbus=canbus0,id=canbus0-socketcan >>> >>> The id parameter is required for "can-host-socketcan" object. >>> Else next error is printed >>> >>> qemu-system-x86_64: -object can-host-socketcan,if=can0,canbus=canbus0: Parameter 'id' is missing >>> >>> If "-object can-bus,id=canbus0" is missing then next error is reported >>> >>> qemu-system-x86_64: -object can-host-socketcan,if=can0,canbus=canbus0,id=canbus0-socketcan: Device 'canbus0' not found >>> >>> I have inspected through monitor the state of objects >>> >>> (qemu) qom-list /objects >>> canbus0-socketcan (child<can-host-socketcan>) >>> type (string) >>> canbus0 (child<can-bus>) >>> >>> (qemu) info qom-tree >>> /machine (pc-i440fx-2.12-machine) >>> ... >>> /peripheral-anon (container) >>> /device[1] (kvaser_pci) >>> /bus master[0] (qemu:memory-region) >>> /kvaser_pci-xilinx[0] (qemu:memory-region) >>> /kvaser_pci-s5920[0] (qemu:memory-region) >>> /kvaser_pci-sja[0] (qemu:memory-region) >>> /bus master container[0] (qemu:memory-region) >>> ... >>> >>> >>> (qemu) qom-list /objects >>> canbus0-socketcan (child<can-host-socketcan>) >>> type (string) >>> canbus0 (child<can-bus>) >>> >>> (qemu) qom-list /machine/peripheral-anon/device[1] >>> bus master container[0] (child<qemu:memory-region>) >>> canbus (link<can-bus>) >>> rombar (uint32) >>> hotpluggable (bool) >>> x-pcie-lnksta-dllla (bool) >>> kvaser_pci-sja[0] (child<qemu:memory-region>) >>> multifunction (bool) >>> hotplugged (bool) >>> parent_bus (link<bus>) >>> romfile (str) >>> kvaser_pci-s5920[0] (child<qemu:memory-region>) >>> x-pcie-extcap-init (bool) >>> command_serr_enable (bool) >>> addr (int32) >>> type (string) >>> legacy-addr (str) >>> kvaser_pci-xilinx[0] (child<qemu:memory-region>) >>> realized (bool) >>> bus master[0] (child<qemu:memory-region>) >>> >>> From the user point of view, it would be nice if "can-bus" >>> can be populated when required automatically. >>> >>> I am not sure, but may it be that it would worth to >>> push can-bus objects under some category/specific >>> container. The path /objects is quite wide. >>> Into something like /object/can-bus or /net/can. >>> >>> But generally thanks much, the progress you have made >>> in one day is really great. I hope that others check >>> your branch. I have pushed your unmodified version into >>> "can-pci-qom" branch of my repo >>> >>> https://gitlab.fel.cvut.cz/canbus/qemu-canbus/tree/can-pci-qom >>> >>> It would be great if others can check that everything >>> works in their setup. I think that then it can be pushed >>> into mainline and some usability improvements can be >>> done/experiment with later. >>> >>> Thanks much, >>> >>> >>> Pavel Pisa >
From: Pavel Pisa <pisa@cmp.felk.cvut.cz> Basic emulation of CAN bus controller and interconnection for QEMU. Patches version 4: Resolve comments longer than 80 characters to suppress all warnings reported by scripts/checkpatch.pl. Follow all suggestions from Frederic Konrad review. Replace all printf and perror calls by QEMU equivalents. Include Deniz Eren signed-off confimation. Patches version 3: Support to connect to host SocketCAN interface has been separated from the core bus implementation. Only simple statically initialize pointer to the connection function is used, no QOM concept for now. SJA1000 message filters redone and code unified where possible. Basic documentation added. QEMU_ALIGNED used in definition of CAN frame structure, structure and defines are separated from Linux/SocketCAN API defined ones to allow to keep QEMU message format independed from host system one. Check for correspondence to socketcan defines added. Patches version 2: The bus emulation and the SJA1000 chip emulation introduced by individual patches as suggested by Frederic Konrad. Simple example board to test SJA1000 as single memory-mapped BAR has been omitted in a new series because emulation of real existing boards can provide same functions now. Conditionalized debug printfs changed to be exposed to compiler syntax check as suggested in review. The work has been started by Jin Yang in the frame of GSoC 2013 slot contributed by RTEMS project which has been looking for environment to allow develop and test CAN drivers for multiple CPU architectures. I have menthored the project and then done substantial code cleanup and update to QOM. Deniz Eren then used emulation for SJA1000 base card driver development for other operating system and contributed PCM-3680I and MIOe-3680 support. Some page about the project https://gitlab.fel.cvut.cz/canbus/qemu-canbus/wikis/home FEE CTU GitLab repository with can-pci branch for 2.3, 2.4, 2.7, 2.8, 2.10 and 2.11 QEMU version is available in the repository https://gitlab.fel.cvut.cz/canbus/qemu-canbus/tree/can-pci mirror at GitHub https://github.com/CTU-IIG/qemu There are many areas for improvement and extension of the code still (for example freeze and migration is not implemented. CAN controllers use proper QOM model but bus/interconnection emulation uses simple broadcast connection which is required for CAN, but it is not based on QEMU bus model). I have tried to look into QEMU VLANs implementation but it does not map straightforward to CAN and I would need some help/opinion from more advanced developers to decide what is their right mapping to CAN. CAN-FD support would be interesting requires other developers/ companies contributions or setup of some project to allow invite some students and colleagues from my university into project. But I believe that (even in its actual state) provided solution is great help for embedded systems developers when they can connect SocketCAN from one or more embedded systems running in virtual environment together or with Linux host SocketCAN virtual or real bus interfaces. We have even tested our generic CANopen device configured for CANopen 401 profile for generic I/O running in the virtual system which can control GPIO inputs/outputs through virtual industrial I/O card. Generally QEMU can be interesting setup which allows to test complete industrial and automotive applications in virtual environment even before real hardware is availabe. Deniz Eren (2): CAN bus PCM-3680I PCI (dual SJA1000 channel) emulation added. CAN bus MIOe-3680 PCI (dual SJA1000 channel) emulation added. Pavel Pisa (5): CAN bus simple messages transport implementation for QEMU CAN bus support to connect bust to Linux host SocketCAN interface. CAN bus SJA1000 chip register level emulation for QEMU CAN bus Kvaser PCI CAN-S (single SJA1000 channel) emulation added. QEMU CAN bus emulation documentation default-configs/pci.mak | 3 + docs/can.txt | 78 ++++ hw/Makefile.objs | 1 + hw/can/Makefile.objs | 14 + hw/can/can_core.c | 136 ++++++ hw/can/can_host_stub.c | 36 ++ hw/can/can_kvaser_pci.c | 375 +++++++++++++++++ hw/can/can_mioe3680_pci.c | 336 +++++++++++++++ hw/can/can_pcm3680_pci.c | 336 +++++++++++++++ hw/can/can_sja1000.c | 1013 +++++++++++++++++++++++++++++++++++++++++++++ hw/can/can_sja1000.h | 167 ++++++++ hw/can/can_socketcan.c | 314 ++++++++++++++ include/can/can_emu.h | 131 ++++++ 13 files changed, 2940 insertions(+) create mode 100644 docs/can.txt create mode 100644 hw/can/Makefile.objs create mode 100644 hw/can/can_core.c create mode 100644 hw/can/can_host_stub.c create mode 100644 hw/can/can_kvaser_pci.c create mode 100644 hw/can/can_mioe3680_pci.c create mode 100644 hw/can/can_pcm3680_pci.c create mode 100644 hw/can/can_sja1000.c create mode 100644 hw/can/can_sja1000.h create mode 100644 hw/can/can_socketcan.c create mode 100644 include/can/can_emu.h