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

[Pavel Pisa]

Hello Philippe,

On Monday 22 of January 2018 12:35:33 Philippe Mathieu-Daudé wrote:
> Hi Pavel,
>
> On 01/14/2018 05:14 PM, address@hidden 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