Re: [PATCH] virtio_console: Use virtio device index to generate port name
Amit Shah writes: > On (Thu) 17 Jan 2013 [13:23:17], sjur.brandel...@stericsson.com wrote: >> From: Sjur Brændeland >> >> Use virtio device index for creating unique device port names. >> Current index allocation in virtio is based on a monotonically >> increasing variable "index". A better handling of this is to >> use device index which is allocated by ida. >> >> Signed-off-by: Sjur Brændeland > > Acked-by: Amit Shah > > Rusty, please pick this one up. In my virtio-next queue now... Thanks, Rusty. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [Xen-devel] [PATCH linux-next] xen/multicall: xen_mc_callback(): avoid buffer overflow
On 08/02/2013 20:34, Tim Gardner wrote:
> This buffer overflow was introduced with
> 91e0c5f3dad47838cb2ecc1865ce789a0b7182b1
> (2.6.24).
There's no buffer overflow here. xen_mc_flush() resets b->cbidx.
David
> arch/x86/xen/multicalls.c |3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/xen/multicalls.c b/arch/x86/xen/multicalls.c
> index 0d82003..5270407 100644
> --- a/arch/x86/xen/multicalls.c
> +++ b/arch/x86/xen/multicalls.c
> @@ -195,9 +195,10 @@ void xen_mc_callback(void (*fn)(void *), void *data)
> struct mc_buffer *b = &__get_cpu_var(mc_buffer);
> struct callback *cb;
>
> - if (b->cbidx == MC_BATCH) {
> + if (b->cbidx >= MC_BATCH) {
> trace_xen_mc_flush_reason(XEN_MC_FL_CALLBACK);
> xen_mc_flush();
> + return;
> }
>
> trace_xen_mc_callback(fn, data);
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH linux-next] xen/multicall: xen_mc_callback(): avoid buffer overflow
This buffer overflow was introduced with
91e0c5f3dad47838cb2ecc1865ce789a0b7182b1
(2.6.24).
Cc: Konrad Rzeszutek Wilk
Cc: Jeremy Fitzhardinge
Cc: Thomas Gleixner
Cc: Ingo Molnar
Cc: "H. Peter Anvin"
Cc: x...@kernel.org
Cc: xen-de...@lists.xensource.com
Cc: virtualization@lists.linux-foundation.org
Cc: sta...@vger.kernel.org
Signed-off-by: Tim Gardner
---
This patch applies cleanly to 3.1.10 and newer. Requires a backport
for 2.6.24-3.2.y
arch/x86/xen/multicalls.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/xen/multicalls.c b/arch/x86/xen/multicalls.c
index 0d82003..5270407 100644
--- a/arch/x86/xen/multicalls.c
+++ b/arch/x86/xen/multicalls.c
@@ -195,9 +195,10 @@ void xen_mc_callback(void (*fn)(void *), void *data)
struct mc_buffer *b = &__get_cpu_var(mc_buffer);
struct callback *cb;
- if (b->cbidx == MC_BATCH) {
+ if (b->cbidx >= MC_BATCH) {
trace_xen_mc_flush_reason(XEN_MC_FL_CALLBACK);
xen_mc_flush();
+ return;
}
trace_xen_mc_callback(fn, data);
--
1.7.9.5
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
CFP: Workshops in CISTI'2013 - 8th Iberian Conference on IST, Lisbon, June 19-22
*** WORKSHOPS in CISTI'2013 8th Iberian Conference on Information Systems and Technologies Lisbon, Portugal, June 19 - 22, 2013 http://www.aisti.eu/cisti2013/index.php?option=com_content&view=article&id=82&Itemid=67&lang=en *** Workshops in CISTI'2013 (8th Iberian Conference on Information Systems and Technologies) with CFP open: > IAwDQ 2013 - Fourth Ibero-American Workshop on Data Quality > SGaMePlay 2013 - Third Iberian Workshop on Serious Games and Meaningful Play > TICAMES 2013 - First Workshop on Information and Communication Technology in > Higher Education: Learning Mathematics > WIA 2013 - Primero Workshop en Innovación Abierta > WISA 2013 - Fifth Workshop on Intelligent Systems and Applications > WISIS 2013 - Third Workshop on Information Systems for Interactive Spaces > WSEQP 2013 - First Workshop in Software Engineering and Quality Process URL: http://www.aisti.eu/cisti2013/index.php?option=com_content&view=article&id=82&Itemid=67&lang=en Best regards, Maria Lemos AISTI / CISTI'2013 http://www.aisti.eu/cisti2013 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] virtio_console: Use virtio device index to generate port name
On (Thu) 17 Jan 2013 [13:23:17], sjur.brandel...@stericsson.com wrote: > From: Sjur Brændeland > > Use virtio device index for creating unique device port names. > Current index allocation in virtio is based on a monotonically > increasing variable "index". A better handling of this is to > use device index which is allocated by ida. > > Signed-off-by: Sjur Brændeland Acked-by: Amit Shah Rusty, please pick this one up. Thanks, Amit ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] virtio_console: Use virtio device index to generate port name
On (Mon) 11 Feb 2013 [14:12:52], Sjur Brændeland wrote: > On Fri, Jan 25, 2013 at 10:20 AM, Amit Shah wrote: > > > On (Thu) 17 Jan 2013 [13:23:17], sjur.brandel...@stericsson.com wrote: > > > From: Sjur Brændeland > > > > > > Use virtio device index for creating unique device port names. > > > Current index allocation in virtio is based on a monotonically > > > increasing variable "index". A better handling of this is to > > > use device index which is allocated by ida. > > > > This should be fine. I'm wondering, though, if there's something else > > you need from the naming scheme (esp. considering the next patch)? > > This current patch solves the most critical issue - that the device name > changes for every modem reboot. This is a show-stopper for me. So it would > be > great if this patch could go upstream. The other problem that device name > is non-deterministic is currently only a theoretical problem as we have > only one > remoteproc device. OK - I've no problem with the patch. Amit ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 1/1] VSOCK: Introduce VM Sockets
On 02/07/13 01:23, Andy King wrote:
> +struct vsock_transport_recv_notify_data {
> + u64 data1; /* Transport-defined. */
> + u64 data2; /* Transport-defined. */
> + bool notify_on_block;
> +};
> +
> +struct vsock_transport_send_notify_data {
> + u64 data1; /* Transport-defined. */
> + u64 data2; /* Transport-defined. */
> +};
> +
> +struct vsock_transport {
> + /* Initialize/tear-down socket. */
> + int (*init)(struct vsock_sock *, struct vsock_sock *);
> + void (*destruct)(struct vsock_sock *);
> + void (*release)(struct vsock_sock *);
> +
> + /* Connections. */
> + int (*connect)(struct vsock_sock *);
> +
> + /* DGRAM. */
> + int (*dgram_bind)(struct vsock_sock *, struct sockaddr_vm *);
> + int (*dgram_dequeue)(struct kiocb *kiocb, struct vsock_sock *vsk,
> + struct msghdr *msg, size_t len, int flags);
> + int (*dgram_enqueue)(struct vsock_sock *, struct sockaddr_vm *,
> + struct iovec *, size_t len);
> + bool (*dgram_allow)(u32 cid, u32 port);
> +
> + /* STREAM. */
> + /* TODO: stream_bind() */
> + ssize_t (*stream_dequeue)(struct vsock_sock *, struct iovec *,
> + size_t len, int flags);
> + ssize_t (*stream_enqueue)(struct vsock_sock *, struct iovec *,
> + size_t len);
> + s64 (*stream_has_data)(struct vsock_sock *);
> + s64 (*stream_has_space)(struct vsock_sock *);
> + u64 (*stream_rcvhiwat)(struct vsock_sock *);
> + bool (*stream_is_active)(struct vsock_sock *);
> + bool (*stream_allow)(u32 cid, u32 port);
> +
> + /* Notification. */
> + int (*notify_poll_in)(struct vsock_sock *, size_t, bool *);
> + int (*notify_poll_out)(struct vsock_sock *, size_t, bool *);
> + int (*notify_recv_init)(struct vsock_sock *, size_t,
> + struct vsock_transport_recv_notify_data *);
> + int (*notify_recv_pre_block)(struct vsock_sock *, size_t,
> + struct vsock_transport_recv_notify_data *);
> + int (*notify_recv_pre_dequeue)(struct vsock_sock *, size_t,
> + struct vsock_transport_recv_notify_data *);
> + int (*notify_recv_post_dequeue)(struct vsock_sock *, size_t,
> + ssize_t, bool, struct vsock_transport_recv_notify_data *);
> + int (*notify_send_init)(struct vsock_sock *,
> + struct vsock_transport_send_notify_data *);
> + int (*notify_send_pre_block)(struct vsock_sock *,
> + struct vsock_transport_send_notify_data *);
> + int (*notify_send_pre_enqueue)(struct vsock_sock *,
> + struct vsock_transport_send_notify_data *);
> + int (*notify_send_post_enqueue)(struct vsock_sock *, ssize_t,
> + struct vsock_transport_send_notify_data *);
> +
> + /* Shutdown. */
> + int (*shutdown)(struct vsock_sock *, int);
> +
> + /* Buffer sizes. */
> + void (*set_buffer_size)(struct vsock_sock *, u64);
> + void (*set_min_buffer_size)(struct vsock_sock *, u64);
> + void (*set_max_buffer_size)(struct vsock_sock *, u64);
> + u64 (*get_buffer_size)(struct vsock_sock *);
> + u64 (*get_min_buffer_size)(struct vsock_sock *);
> + u64 (*get_max_buffer_size)(struct vsock_sock *);
> +
> + /* Addressing. */
> + u32 (*get_local_cid)(void);
> +};
> +
Whoa. This has grown *alot*. Care to explain this please? Patch
creating a Documentation/virtual/vsock.txt would be cool.
*_enqueue + *_dequeue is sending/receiving data, ok.
stream_has_data + stream_has_space + stream_rcvhiwat look like they are
needed for buffer management. Details please (especially for
stream_rcvhiwat).
What is stream_is_active?
What is *_allow?
What are all those notify_* calls?
Why do you need vsock_transport_{send,recv}_notify_data structs?
Can't this live in vsock_sock->trans?
thanks,
Gerd
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] virtio: Add module driver macro for virtio drivers.
From: Sjur Brændeland Add helper macro for drivers that don't do anything special in module init/exit. Signed-off-by: Sjur Brændeland --- Hi Rusty, Here is the patch for module driver macro. I will update caif_virtio accordingly in my next patch-set. Thanks, Sjur include/linux/virtio.h |9 + 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/include/linux/virtio.h b/include/linux/virtio.h index cf8adb1..00ccc40 100644 --- a/include/linux/virtio.h +++ b/include/linux/virtio.h @@ -126,4 +126,13 @@ static inline struct virtio_driver *drv_to_virtio(struct device_driver *drv) int register_virtio_driver(struct virtio_driver *drv); void unregister_virtio_driver(struct virtio_driver *drv); + +/* module_virtio_driver() - Helper macro for drivers that don't do + * anything special in module init/exit. This eliminates a lot of + * boilerplate. Each module may only use this macro once, and + * calling it replaces module_init() and module_exit() + */ +#define module_virtio_driver(__virtio_driver) \ + module_driver(__virtio_driver, register_virtio_driver, \ + unregister_virtio_driver) #endif /* _LINUX_VIRTIO_H */ -- 1.7.5.4 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] virtio_console: Use virtio device index to generate port name
On Fri, Jan 25, 2013 at 10:20 AM, Amit Shah wrote: > On (Thu) 17 Jan 2013 [13:23:17], sjur.brandel...@stericsson.com wrote: > > From: Sjur Brændeland > > > > Use virtio device index for creating unique device port names. > > Current index allocation in virtio is based on a monotonically > > increasing variable "index". A better handling of this is to > > use device index which is allocated by ida. > > This should be fine. I'm wondering, though, if there's something else > you need from the naming scheme (esp. considering the next patch)? > This current patch solves the most critical issue - that the device name changes for every modem reboot. This is a show-stopper for me. So it would be great if this patch could go upstream. The other problem that device name is non-deterministic is currently only a theoretical problem as we have only one remoteproc device. Regards, Sjur ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
RE: [PATCH vringh 2/2] caif_virtio: Introduce caif over virtio
Hi Rusty,
> From: Rusty Russell [mailto:ru...@rustcorp.com.au]
> sjur.brandel...@stericsson.com writes:
> > From: Vikram ARV
> >
> > Add the the Virtio shared memory driver for STE Modems.
> > caif_virtio is implemented utilizing the virtio framework
> > for data transport and is managed with the remoteproc frameworks.
> >
> > The Virtio queue is used for transmitting data to the modem, and
> > the new vringh implementation is receiving data over the vring.
>
> OK, let's refine vringh now we can see another user:
>
> > + * @head: Last descriptor ID we received from vringh_getdesc_kern.
> > + * We use this to put descriptor back on the used ring. USHRT_MAX is
> > + * used to indicate invalid head-id.
>
> > + */
> > +struct cfv_napi_context {
> > + struct vringh_kiov riov;
> > + unsigned short head;
> > +};
>
> Usually we use an int, and -1. I imagine it'll take no more space,
> due to padding.
I'm passing a pointer to "head" to vringh_getdesc_kern() directly, are you
suggesting to change the head argument in vringh_getdesc_kern()
to a int pointer as well then?
> > +static inline void ctx_prep_iov(struct cfv_napi_context *ctx)
> > +{
> > + if (ctx->riov.allocated) {
> > + kfree(ctx->riov.iov);
> > + ctx->riov.iov = NULL;
> > + ctx->riov.allocated = false;
> > + }
> > + ctx->riov.iov = NULL;
> > + ctx->riov.i = 0;
> > + ctx->riov.max = 0;
> > +}
>
> Hmm, we should probably make sure you don't have to do this: that if
> allocated once, you can simply reuse it by setting i = 0.
Yes, I had problems getting the alloc/free of iov right. This is
perhaps the least intuitively part of the API. I maybe it's just me, but
I think some more helper functions and support from vringh in this
area would be great.
>
> (This requires some care in the error handling paths, so we don't
> free it from under you)...
>
> And you probably want to free this up in cfv_remove() instead?
OK, I'll look into this when you have a some more code ready...
>
> I'll create and test a patch now.
>
> > + if (riov->i == riov->max) {
> > + if (cfv->ctx.head != USHRT_MAX) {
> > + vringh_complete_kern(cfv->vr_rx,
> > +cfv->ctx.head,
> > +0);
> > + cfv->ctx.head = USHRT_MAX;
> > + }
> > +
> > + ctx_prep_iov(&cfv->ctx);
> > + err = vringh_getdesc_kern(
> > + cfv->vr_rx,
> > + riov,
> > + &wiov,
> > + &cfv->ctx.head,
> > + GFP_ATOMIC);
> > +
> > + if (err <= 0)
> > + goto out;
> > +
> > + if (wiov.max != 0) {
> > + /* CAIF does not use write descriptors */
> > + err = -EPROTO;
> > + goto out;
> > + }
>
> This is probably a common case, so we should generate this error inside
> vringh_getdesc (if wiov is NULL).
>
> I'll do that too...
>
> > +module_driver(caif_virtio_driver, register_virtio_driver,
> > + unregister_virtio_driver);
> > +MODULE_DEVICE_TABLE(virtio, id_table);
>
> The comment on module_driver says:
>
> * Use this macro to construct bus specific macros for registering
> * drivers, and do not use it on its own.
>
> Want to send me a patch to define module_virtio_driver?
Sure, I can do that.
Thanks a lot Rusty.
Regards,
Sjur
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
