Thread (14 messages) 14 messages, 4 authors, 2013-02-25

Re: [PATCHv2 vringh 1/3] remoteproc: Add support for vringh (Host vrings)

From: Ohad Ben-Cohen <hidden>
Date: 2013-02-21 13:47:35
Also in: lkml, netdev

On Thu, Feb 21, 2013 at 1:01 AM, Sjur Brændeland
[off-list ref] wrote:
[Sjur:]
quoted
quoted
How do you see the in-kernel API for this? I would like to see
something similar to my previous patches, where we extend
the virtqueue API. E.g. something like this:
struct virtqueue *vring_new_virtqueueh(...)...
[Rusty:]
quoted
I was just going to create _kernel variants of all the _user helpers,
and let you drive it directly like that.

If we get a second in-kernel user, we create wrappers (I'd prefer not to
overload struct virtqueue though).
The wrappers suggestion sounds good.

One of the things we truly enjoyed about virtio is how easy it was to
reuse its drivers for communication with a remote processor.

It'd be great if we can stick to this design and keep vringh virtio
drivers decoupled from the low level implementation they are developed
with (specifically caif and remoteproc in this case).
Yes, if you insert a "malicious device" like that you can make it crash,
but wouldn't most drivers do if you try to register a malicious device...?

If we really want to protect from this, we could perhaps validate the vdev
pointer in function rproc_virtio_new_vringh() by looking through the vdevs
of the registered rprocs.
It sounds like we can instead just avoid this altogether if we follow
Rusty's wrappers suggestion.

The result would probably look better, and I suspect it might be very
minimal code.
If some other driver is showing up using the vringh kernel API where
the current API don't fit, I guess it's time to create some abstractions
and wrappers... But I hope the current approach will do for now?
Unless I'm missing something here, adding wrappers should be straight
forward and quick. Coupling a virtio driver with remoteproc doesn't
look great to me, probably because I appreciate so much the elegance
of virtio and how easy we could have reused its vanilla drivers with a
use case it wasn't originally designed to support.
quoted
I have some other questions as well but maybe it's better to discuss
first the bigger picture.
OK, but please don't hesitate to address this. I'm still aiming for this
to go into 3.9.
I was wondering - can you please explain your motivation for using
vringh in caif ?

We have internally discussed supporting multiple remote processors
talking to each other using rpmsg, and in that scenario using vringh
can considerably simplifies the solution (no need to decide in advance
which side is the "guest" and which is the "host"). Was this the
general incentive in using vringh in caif too or something else?

Thanks,
Ohad.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help