* [PATCH] vhost: move is_le setup to the backend
@ 2016-01-11 14:39 Greg Kurz
2016-01-12 10:01 ` Michael S. Tsirkin
0 siblings, 1 reply; 15+ messages in thread
From: Greg Kurz @ 2016-01-11 14:39 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, linux-kernel, kvm, virtualization
The vq->is_le field is used to fix endianness when accessing the vring via
the cpu_to_vhost16() and vhost16_to_cpu() helpers in the following cases:
1) host is big endian and device is modern virtio
2) host has cross-endian support and device is legacy virtio with a different
endianness than the host
Both cases rely on the VHOST_SET_FEATURES ioctl, but 2) also needs the
VHOST_SET_VRING_ENDIAN ioctl to be called by userspace. Since vq->is_le
is only needed when the backend is active, it was decided to set it at
backend start.
This is currently done in vhost_init_used()->vhost_init_is_le() but it
obfuscates the core vhost code. This patch moves the is_le setup to a
dedicated function that is called from the backend code.
Note vhost_net is the only backend that can pass vq->private_data == NULL to
vhost_init_used(), hence the "if (sock)" branch.
No behaviour change.
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
Hi Michael,
This is a cleanup patch I had posted last year:
http://patchwork.ozlabs.org/patch/538277/
I am just reposting with Cornelia's R-b.
Cheers.
--
Greg
drivers/vhost/net.c | 6 ++++++
drivers/vhost/scsi.c | 3 +++
drivers/vhost/test.c | 2 ++
drivers/vhost/vhost.c | 12 +++++++-----
drivers/vhost/vhost.h | 1 +
5 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 9eda69e40678..d6319cb2664c 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -917,6 +917,12 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
vhost_net_disable_vq(n, vq);
vq->private_data = sock;
+
+ if (sock)
+ vhost_set_is_le(vq);
+ else
+ vq->is_le = virtio_legacy_is_little_endian();
+
r = vhost_init_used(vq);
if (r)
goto err_used;
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 29cfc57d496e..1f4f405ebba8 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -1274,6 +1274,9 @@ vhost_scsi_set_endpoint(struct vhost_scsi *vs,
vq = &vs->vqs[i].vq;
mutex_lock(&vq->mutex);
vq->private_data = vs_tpg;
+
+ vhost_set_is_le(vq);
+
vhost_init_used(vq);
mutex_unlock(&vq->mutex);
}
diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
index f2882ac98726..b1c7df502211 100644
--- a/drivers/vhost/test.c
+++ b/drivers/vhost/test.c
@@ -196,6 +196,8 @@ static long vhost_test_run(struct vhost_test *n, int test)
oldpriv = vq->private_data;
vq->private_data = priv;
+ vhost_set_is_le(vq);
+
r = vhost_init_used(&n->vqs[index]);
mutex_unlock(&vq->mutex);
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index ad2146a9ab2d..e688eb801d43 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -113,6 +113,12 @@ static void vhost_init_is_le(struct vhost_virtqueue *vq)
}
#endif /* CONFIG_VHOST_CROSS_ENDIAN_LEGACY */
+void vhost_set_is_le(struct vhost_virtqueue *vq)
+{
+ vhost_init_is_le(vq);
+}
+EXPORT_SYMBOL_GPL(vhost_set_is_le);
+
static void vhost_poll_func(struct file *file, wait_queue_head_t *wqh,
poll_table *pt)
{
@@ -1156,12 +1162,8 @@ int vhost_init_used(struct vhost_virtqueue *vq)
{
__virtio16 last_used_idx;
int r;
- if (!vq->private_data) {
- vq->is_le = virtio_legacy_is_little_endian();
+ if (!vq->private_data)
return 0;
- }
-
- vhost_init_is_le(vq);
r = vhost_update_used_flags(vq);
if (r)
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index d3f767448a72..af5d33797937 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -162,6 +162,7 @@ bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *);
int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
unsigned int log_num, u64 len);
+void vhost_set_is_le(struct vhost_virtqueue *vq);
#define vq_err(vq, fmt, ...) do { \
pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH] vhost: move is_le setup to the backend
2016-01-11 14:39 [PATCH] vhost: move is_le setup to the backend Greg Kurz
@ 2016-01-12 10:01 ` Michael S. Tsirkin
2016-01-12 10:31 ` Greg Kurz
0 siblings, 1 reply; 15+ messages in thread
From: Michael S. Tsirkin @ 2016-01-12 10:01 UTC (permalink / raw)
To: Greg Kurz; +Cc: netdev, linux-kernel, kvm, virtualization
On Mon, Jan 11, 2016 at 03:39:38PM +0100, Greg Kurz wrote:
> The vq->is_le field is used to fix endianness when accessing the vring via
> the cpu_to_vhost16() and vhost16_to_cpu() helpers in the following cases:
>
> 1) host is big endian and device is modern virtio
>
> 2) host has cross-endian support and device is legacy virtio with a different
> endianness than the host
>
> Both cases rely on the VHOST_SET_FEATURES ioctl, but 2) also needs the
> VHOST_SET_VRING_ENDIAN ioctl to be called by userspace. Since vq->is_le
> is only needed when the backend is active, it was decided to set it at
> backend start.
>
> This is currently done in vhost_init_used()->vhost_init_is_le() but it
> obfuscates the core vhost code. This patch moves the is_le setup to a
> dedicated function that is called from the backend code.
>
> Note vhost_net is the only backend that can pass vq->private_data == NULL to
> vhost_init_used(), hence the "if (sock)" branch.
>
> No behaviour change.
>
> Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> ---
>
> Hi Michael,
>
> This is a cleanup patch I had posted last year:
>
> http://patchwork.ozlabs.org/patch/538277/
>
> I am just reposting with Cornelia's R-b.
>
> Cheers.
>
> --
> Greg
>
> drivers/vhost/net.c | 6 ++++++
> drivers/vhost/scsi.c | 3 +++
> drivers/vhost/test.c | 2 ++
> drivers/vhost/vhost.c | 12 +++++++-----
> drivers/vhost/vhost.h | 1 +
> 5 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 9eda69e40678..d6319cb2664c 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -917,6 +917,12 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
>
> vhost_net_disable_vq(n, vq);
> vq->private_data = sock;
> +
> + if (sock)
> + vhost_set_is_le(vq);
> + else
> + vq->is_le = virtio_legacy_is_little_endian();
> +
> r = vhost_init_used(vq);
> if (r)
> goto err_used;
This part is kind of ugly. I think it's cleaner is the generic code.
How about we teach vhost_set_is_le to test vq->private_data and DTRT?
> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index 29cfc57d496e..1f4f405ebba8 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -1274,6 +1274,9 @@ vhost_scsi_set_endpoint(struct vhost_scsi *vs,
> vq = &vs->vqs[i].vq;
> mutex_lock(&vq->mutex);
> vq->private_data = vs_tpg;
> +
> + vhost_set_is_le(vq);
> +
> vhost_init_used(vq);
> mutex_unlock(&vq->mutex);
> }
> diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
> index f2882ac98726..b1c7df502211 100644
> --- a/drivers/vhost/test.c
> +++ b/drivers/vhost/test.c
> @@ -196,6 +196,8 @@ static long vhost_test_run(struct vhost_test *n, int test)
> oldpriv = vq->private_data;
> vq->private_data = priv;
>
> + vhost_set_is_le(vq);
> +
> r = vhost_init_used(&n->vqs[index]);
>
> mutex_unlock(&vq->mutex);
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index ad2146a9ab2d..e688eb801d43 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -113,6 +113,12 @@ static void vhost_init_is_le(struct vhost_virtqueue *vq)
> }
> #endif /* CONFIG_VHOST_CROSS_ENDIAN_LEGACY */
>
> +void vhost_set_is_le(struct vhost_virtqueue *vq)
> +{
> + vhost_init_is_le(vq);
> +}
> +EXPORT_SYMBOL_GPL(vhost_set_is_le);
> +
> static void vhost_poll_func(struct file *file, wait_queue_head_t *wqh,
> poll_table *pt)
> {
Why do we need this wrapper? Why not just export vhost_init_is_le?
Or rename vhost_init_is_le->vhost_set_is_le if you prefer.
> @@ -1156,12 +1162,8 @@ int vhost_init_used(struct vhost_virtqueue *vq)
> {
> __virtio16 last_used_idx;
> int r;
> - if (!vq->private_data) {
> - vq->is_le = virtio_legacy_is_little_endian();
> + if (!vq->private_data)
> return 0;
> - }
> -
> - vhost_init_is_le(vq);
>
> r = vhost_update_used_flags(vq);
> if (r)
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index d3f767448a72..af5d33797937 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -162,6 +162,7 @@ bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *);
>
> int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
> unsigned int log_num, u64 len);
> +void vhost_set_is_le(struct vhost_virtqueue *vq);
>
> #define vq_err(vq, fmt, ...) do { \
> pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] vhost: move is_le setup to the backend
2016-01-12 10:01 ` Michael S. Tsirkin
@ 2016-01-12 10:31 ` Greg Kurz
2016-01-12 10:48 ` Michael S. Tsirkin
2016-01-12 10:48 ` Michael S. Tsirkin
0 siblings, 2 replies; 15+ messages in thread
From: Greg Kurz @ 2016-01-12 10:31 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, linux-kernel, kvm, virtualization
On Tue, 12 Jan 2016 12:01:32 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Mon, Jan 11, 2016 at 03:39:38PM +0100, Greg Kurz wrote:
> > The vq->is_le field is used to fix endianness when accessing the vring via
> > the cpu_to_vhost16() and vhost16_to_cpu() helpers in the following cases:
> >
> > 1) host is big endian and device is modern virtio
> >
> > 2) host has cross-endian support and device is legacy virtio with a different
> > endianness than the host
> >
> > Both cases rely on the VHOST_SET_FEATURES ioctl, but 2) also needs the
> > VHOST_SET_VRING_ENDIAN ioctl to be called by userspace. Since vq->is_le
> > is only needed when the backend is active, it was decided to set it at
> > backend start.
> >
> > This is currently done in vhost_init_used()->vhost_init_is_le() but it
> > obfuscates the core vhost code. This patch moves the is_le setup to a
> > dedicated function that is called from the backend code.
> >
> > Note vhost_net is the only backend that can pass vq->private_data == NULL to
> > vhost_init_used(), hence the "if (sock)" branch.
> >
> > No behaviour change.
> >
> > Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> > ---
> >
> > Hi Michael,
> >
> > This is a cleanup patch I had posted last year:
> >
> > http://patchwork.ozlabs.org/patch/538277/
> >
> > I am just reposting with Cornelia's R-b.
> >
> > Cheers.
> >
> > --
> > Greg
> >
> > drivers/vhost/net.c | 6 ++++++
> > drivers/vhost/scsi.c | 3 +++
> > drivers/vhost/test.c | 2 ++
> > drivers/vhost/vhost.c | 12 +++++++-----
> > drivers/vhost/vhost.h | 1 +
> > 5 files changed, 19 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> > index 9eda69e40678..d6319cb2664c 100644
> > --- a/drivers/vhost/net.c
> > +++ b/drivers/vhost/net.c
> > @@ -917,6 +917,12 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> >
> > vhost_net_disable_vq(n, vq);
> > vq->private_data = sock;
> > +
> > + if (sock)
> > + vhost_set_is_le(vq);
> > + else
> > + vq->is_le = virtio_legacy_is_little_endian();
> > +
> > r = vhost_init_used(vq);
> > if (r)
> > goto err_used;
>
> This part is kind of ugly. I think it's cleaner is the generic code.
> How about we teach vhost_set_is_le to test vq->private_data and DTRT?
>
You're right. I'll try that.
> > diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> > index 29cfc57d496e..1f4f405ebba8 100644
> > --- a/drivers/vhost/scsi.c
> > +++ b/drivers/vhost/scsi.c
> > @@ -1274,6 +1274,9 @@ vhost_scsi_set_endpoint(struct vhost_scsi *vs,
> > vq = &vs->vqs[i].vq;
> > mutex_lock(&vq->mutex);
> > vq->private_data = vs_tpg;
> > +
> > + vhost_set_is_le(vq);
> > +
> > vhost_init_used(vq);
> > mutex_unlock(&vq->mutex);
> > }
> > diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
> > index f2882ac98726..b1c7df502211 100644
> > --- a/drivers/vhost/test.c
> > +++ b/drivers/vhost/test.c
> > @@ -196,6 +196,8 @@ static long vhost_test_run(struct vhost_test *n, int test)
> > oldpriv = vq->private_data;
> > vq->private_data = priv;
> >
> > + vhost_set_is_le(vq);
> > +
> > r = vhost_init_used(&n->vqs[index]);
> >
> > mutex_unlock(&vq->mutex);
> > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > index ad2146a9ab2d..e688eb801d43 100644
> > --- a/drivers/vhost/vhost.c
> > +++ b/drivers/vhost/vhost.c
> > @@ -113,6 +113,12 @@ static void vhost_init_is_le(struct vhost_virtqueue *vq)
> > }
> > #endif /* CONFIG_VHOST_CROSS_ENDIAN_LEGACY */
> >
> > +void vhost_set_is_le(struct vhost_virtqueue *vq)
> > +{
> > + vhost_init_is_le(vq);
> > +}
> > +EXPORT_SYMBOL_GPL(vhost_set_is_le);
> > +
> > static void vhost_poll_func(struct file *file, wait_queue_head_t *wqh,
> > poll_table *pt)
> > {
>
> Why do we need this wrapper? Why not just export vhost_init_is_le?
This is a tentative workaround to:
WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable
#35: FILE: drivers/vhost/vhost.c:116:
+EXPORT_SYMBOL_GPL(vhost_init_is_le);
Other possibility is to put the EXPORT_SYMBOL_GPL statement in both
conditional compilation blocks. Maybe better ?
> Or rename vhost_init_is_le->vhost_set_is_le if you prefer.
>
> > @@ -1156,12 +1162,8 @@ int vhost_init_used(struct vhost_virtqueue *vq)
> > {
> > __virtio16 last_used_idx;
> > int r;
> > - if (!vq->private_data) {
> > - vq->is_le = virtio_legacy_is_little_endian();
> > + if (!vq->private_data)
> > return 0;
> > - }
> > -
> > - vhost_init_is_le(vq);
> >
> > r = vhost_update_used_flags(vq);
> > if (r)
> > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> > index d3f767448a72..af5d33797937 100644
> > --- a/drivers/vhost/vhost.h
> > +++ b/drivers/vhost/vhost.h
> > @@ -162,6 +162,7 @@ bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *);
> >
> > int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
> > unsigned int log_num, u64 len);
> > +void vhost_set_is_le(struct vhost_virtqueue *vq);
> >
> > #define vq_err(vq, fmt, ...) do { \
> > pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] vhost: move is_le setup to the backend
2016-01-12 10:31 ` Greg Kurz
@ 2016-01-12 10:48 ` Michael S. Tsirkin
2016-01-12 10:48 ` Michael S. Tsirkin
1 sibling, 0 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2016-01-12 10:48 UTC (permalink / raw)
To: Greg Kurz; +Cc: netdev, linux-kernel, kvm, virtualization
On Tue, Jan 12, 2016 at 11:31:00AM +0100, Greg Kurz wrote:
> On Tue, 12 Jan 2016 12:01:32 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > On Mon, Jan 11, 2016 at 03:39:38PM +0100, Greg Kurz wrote:
> > > The vq->is_le field is used to fix endianness when accessing the vring via
> > > the cpu_to_vhost16() and vhost16_to_cpu() helpers in the following cases:
> > >
> > > 1) host is big endian and device is modern virtio
> > >
> > > 2) host has cross-endian support and device is legacy virtio with a different
> > > endianness than the host
> > >
> > > Both cases rely on the VHOST_SET_FEATURES ioctl, but 2) also needs the
> > > VHOST_SET_VRING_ENDIAN ioctl to be called by userspace. Since vq->is_le
> > > is only needed when the backend is active, it was decided to set it at
> > > backend start.
> > >
> > > This is currently done in vhost_init_used()->vhost_init_is_le() but it
> > > obfuscates the core vhost code. This patch moves the is_le setup to a
> > > dedicated function that is called from the backend code.
> > >
> > > Note vhost_net is the only backend that can pass vq->private_data == NULL to
> > > vhost_init_used(), hence the "if (sock)" branch.
> > >
> > > No behaviour change.
> > >
> > > Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> > > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> > > ---
> > >
> > > Hi Michael,
> > >
> > > This is a cleanup patch I had posted last year:
> > >
> > > http://patchwork.ozlabs.org/patch/538277/
> > >
> > > I am just reposting with Cornelia's R-b.
> > >
> > > Cheers.
> > >
> > > --
> > > Greg
> > >
> > > drivers/vhost/net.c | 6 ++++++
> > > drivers/vhost/scsi.c | 3 +++
> > > drivers/vhost/test.c | 2 ++
> > > drivers/vhost/vhost.c | 12 +++++++-----
> > > drivers/vhost/vhost.h | 1 +
> > > 5 files changed, 19 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> > > index 9eda69e40678..d6319cb2664c 100644
> > > --- a/drivers/vhost/net.c
> > > +++ b/drivers/vhost/net.c
> > > @@ -917,6 +917,12 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> > >
> > > vhost_net_disable_vq(n, vq);
> > > vq->private_data = sock;
> > > +
> > > + if (sock)
> > > + vhost_set_is_le(vq);
> > > + else
> > > + vq->is_le = virtio_legacy_is_little_endian();
> > > +
> > > r = vhost_init_used(vq);
> > > if (r)
> > > goto err_used;
> >
> > This part is kind of ugly. I think it's cleaner is the generic code.
> > How about we teach vhost_set_is_le to test vq->private_data and DTRT?
> >
>
> You're right. I'll try that.
>
> > > diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> > > index 29cfc57d496e..1f4f405ebba8 100644
> > > --- a/drivers/vhost/scsi.c
> > > +++ b/drivers/vhost/scsi.c
> > > @@ -1274,6 +1274,9 @@ vhost_scsi_set_endpoint(struct vhost_scsi *vs,
> > > vq = &vs->vqs[i].vq;
> > > mutex_lock(&vq->mutex);
> > > vq->private_data = vs_tpg;
> > > +
> > > + vhost_set_is_le(vq);
> > > +
> > > vhost_init_used(vq);
> > > mutex_unlock(&vq->mutex);
> > > }
> > > diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
> > > index f2882ac98726..b1c7df502211 100644
> > > --- a/drivers/vhost/test.c
> > > +++ b/drivers/vhost/test.c
> > > @@ -196,6 +196,8 @@ static long vhost_test_run(struct vhost_test *n, int test)
> > > oldpriv = vq->private_data;
> > > vq->private_data = priv;
> > >
> > > + vhost_set_is_le(vq);
> > > +
> > > r = vhost_init_used(&n->vqs[index]);
> > >
> > > mutex_unlock(&vq->mutex);
> > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > > index ad2146a9ab2d..e688eb801d43 100644
> > > --- a/drivers/vhost/vhost.c
> > > +++ b/drivers/vhost/vhost.c
> > > @@ -113,6 +113,12 @@ static void vhost_init_is_le(struct vhost_virtqueue *vq)
> > > }
> > > #endif /* CONFIG_VHOST_CROSS_ENDIAN_LEGACY */
> > >
> > > +void vhost_set_is_le(struct vhost_virtqueue *vq)
> > > +{
> > > + vhost_init_is_le(vq);
> > > +}
> > > +EXPORT_SYMBOL_GPL(vhost_set_is_le);
> > > +
> > > static void vhost_poll_func(struct file *file, wait_queue_head_t *wqh,
> > > poll_table *pt)
> > > {
> >
> > Why do we need this wrapper? Why not just export vhost_init_is_le?
>
> This is a tentative workaround to:
>
> WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable
> #35: FILE: drivers/vhost/vhost.c:116:
> +EXPORT_SYMBOL_GPL(vhost_init_is_le);
>
> Other possibility is to put the EXPORT_SYMBOL_GPL statement in both
> conditional compilation blocks. Maybe better ?
That, or ignore the warning.
> > Or rename vhost_init_is_le->vhost_set_is_le if you prefer.
> >
> > > @@ -1156,12 +1162,8 @@ int vhost_init_used(struct vhost_virtqueue *vq)
> > > {
> > > __virtio16 last_used_idx;
> > > int r;
> > > - if (!vq->private_data) {
> > > - vq->is_le = virtio_legacy_is_little_endian();
> > > + if (!vq->private_data)
> > > return 0;
> > > - }
> > > -
> > > - vhost_init_is_le(vq);
> > >
> > > r = vhost_update_used_flags(vq);
> > > if (r)
> > > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> > > index d3f767448a72..af5d33797937 100644
> > > --- a/drivers/vhost/vhost.h
> > > +++ b/drivers/vhost/vhost.h
> > > @@ -162,6 +162,7 @@ bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *);
> > >
> > > int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
> > > unsigned int log_num, u64 len);
> > > +void vhost_set_is_le(struct vhost_virtqueue *vq);
> > >
> > > #define vq_err(vq, fmt, ...) do { \
> > > pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
> >
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] vhost: move is_le setup to the backend
2016-01-12 10:31 ` Greg Kurz
2016-01-12 10:48 ` Michael S. Tsirkin
@ 2016-01-12 10:48 ` Michael S. Tsirkin
1 sibling, 0 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2016-01-12 10:48 UTC (permalink / raw)
To: Greg Kurz; +Cc: netdev, linux-kernel, kvm, virtualization
On Tue, Jan 12, 2016 at 11:31:00AM +0100, Greg Kurz wrote:
> On Tue, 12 Jan 2016 12:01:32 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > On Mon, Jan 11, 2016 at 03:39:38PM +0100, Greg Kurz wrote:
> > > The vq->is_le field is used to fix endianness when accessing the vring via
> > > the cpu_to_vhost16() and vhost16_to_cpu() helpers in the following cases:
> > >
> > > 1) host is big endian and device is modern virtio
> > >
> > > 2) host has cross-endian support and device is legacy virtio with a different
> > > endianness than the host
> > >
> > > Both cases rely on the VHOST_SET_FEATURES ioctl, but 2) also needs the
> > > VHOST_SET_VRING_ENDIAN ioctl to be called by userspace. Since vq->is_le
> > > is only needed when the backend is active, it was decided to set it at
> > > backend start.
> > >
> > > This is currently done in vhost_init_used()->vhost_init_is_le() but it
> > > obfuscates the core vhost code. This patch moves the is_le setup to a
> > > dedicated function that is called from the backend code.
> > >
> > > Note vhost_net is the only backend that can pass vq->private_data == NULL to
> > > vhost_init_used(), hence the "if (sock)" branch.
> > >
> > > No behaviour change.
> > >
> > > Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> > > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> > > ---
> > >
> > > Hi Michael,
> > >
> > > This is a cleanup patch I had posted last year:
> > >
> > > http://patchwork.ozlabs.org/patch/538277/
> > >
> > > I am just reposting with Cornelia's R-b.
> > >
> > > Cheers.
> > >
> > > --
> > > Greg
> > >
> > > drivers/vhost/net.c | 6 ++++++
> > > drivers/vhost/scsi.c | 3 +++
> > > drivers/vhost/test.c | 2 ++
> > > drivers/vhost/vhost.c | 12 +++++++-----
> > > drivers/vhost/vhost.h | 1 +
> > > 5 files changed, 19 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> > > index 9eda69e40678..d6319cb2664c 100644
> > > --- a/drivers/vhost/net.c
> > > +++ b/drivers/vhost/net.c
> > > @@ -917,6 +917,12 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> > >
> > > vhost_net_disable_vq(n, vq);
> > > vq->private_data = sock;
> > > +
> > > + if (sock)
> > > + vhost_set_is_le(vq);
> > > + else
> > > + vq->is_le = virtio_legacy_is_little_endian();
> > > +
> > > r = vhost_init_used(vq);
> > > if (r)
> > > goto err_used;
> >
> > This part is kind of ugly. I think it's cleaner is the generic code.
> > How about we teach vhost_set_is_le to test vq->private_data and DTRT?
> >
>
> You're right. I'll try that.
>
> > > diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> > > index 29cfc57d496e..1f4f405ebba8 100644
> > > --- a/drivers/vhost/scsi.c
> > > +++ b/drivers/vhost/scsi.c
> > > @@ -1274,6 +1274,9 @@ vhost_scsi_set_endpoint(struct vhost_scsi *vs,
> > > vq = &vs->vqs[i].vq;
> > > mutex_lock(&vq->mutex);
> > > vq->private_data = vs_tpg;
> > > +
> > > + vhost_set_is_le(vq);
> > > +
> > > vhost_init_used(vq);
> > > mutex_unlock(&vq->mutex);
> > > }
> > > diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
> > > index f2882ac98726..b1c7df502211 100644
> > > --- a/drivers/vhost/test.c
> > > +++ b/drivers/vhost/test.c
> > > @@ -196,6 +196,8 @@ static long vhost_test_run(struct vhost_test *n, int test)
> > > oldpriv = vq->private_data;
> > > vq->private_data = priv;
> > >
> > > + vhost_set_is_le(vq);
> > > +
> > > r = vhost_init_used(&n->vqs[index]);
> > >
> > > mutex_unlock(&vq->mutex);
> > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > > index ad2146a9ab2d..e688eb801d43 100644
> > > --- a/drivers/vhost/vhost.c
> > > +++ b/drivers/vhost/vhost.c
> > > @@ -113,6 +113,12 @@ static void vhost_init_is_le(struct vhost_virtqueue *vq)
> > > }
> > > #endif /* CONFIG_VHOST_CROSS_ENDIAN_LEGACY */
> > >
> > > +void vhost_set_is_le(struct vhost_virtqueue *vq)
> > > +{
> > > + vhost_init_is_le(vq);
> > > +}
> > > +EXPORT_SYMBOL_GPL(vhost_set_is_le);
> > > +
> > > static void vhost_poll_func(struct file *file, wait_queue_head_t *wqh,
> > > poll_table *pt)
> > > {
> >
> > Why do we need this wrapper? Why not just export vhost_init_is_le?
>
> This is a tentative workaround to:
>
> WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable
> #35: FILE: drivers/vhost/vhost.c:116:
> +EXPORT_SYMBOL_GPL(vhost_init_is_le);
>
> Other possibility is to put the EXPORT_SYMBOL_GPL statement in both
> conditional compilation blocks. Maybe better ?
That, or ignore the warning.
> > Or rename vhost_init_is_le->vhost_set_is_le if you prefer.
> >
> > > @@ -1156,12 +1162,8 @@ int vhost_init_used(struct vhost_virtqueue *vq)
> > > {
> > > __virtio16 last_used_idx;
> > > int r;
> > > - if (!vq->private_data) {
> > > - vq->is_le = virtio_legacy_is_little_endian();
> > > + if (!vq->private_data)
> > > return 0;
> > > - }
> > > -
> > > - vhost_init_is_le(vq);
> > >
> > > r = vhost_update_used_flags(vq);
> > > if (r)
> > > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> > > index d3f767448a72..af5d33797937 100644
> > > --- a/drivers/vhost/vhost.h
> > > +++ b/drivers/vhost/vhost.h
> > > @@ -162,6 +162,7 @@ bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *);
> > >
> > > int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
> > > unsigned int log_num, u64 len);
> > > +void vhost_set_is_le(struct vhost_virtqueue *vq);
> > >
> > > #define vq_err(vq, fmt, ...) do { \
> > > pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
> >
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH] vhost: move is_le setup to the backend
@ 2015-10-30 11:42 Greg Kurz
2015-11-02 20:09 ` David Miller
` (5 more replies)
0 siblings, 6 replies; 15+ messages in thread
From: Greg Kurz @ 2015-10-30 11:42 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, linux-kernel, kvm, virtualization
The vq->is_le field is used to fix endianness when accessing the vring via
the cpu_to_vhost16() and vhost16_to_cpu() helpers in the following cases:
1) host is big endian and device is modern virtio
2) host has cross-endian support and device is legacy virtio with a different
endianness than the host
Both cases rely on the VHOST_SET_FEATURES ioctl, but 2) also needs the
VHOST_SET_VRING_ENDIAN ioctl to be called by userspace. Since vq->is_le
is only needed when the backend is active, it was decided to set it at
backend start.
This is currently done in vhost_init_used()->vhost_init_is_le() but it
obfuscates the core vhost code. This patch moves the is_le setup to a
dedicated function that is called from the backend code.
Note vhost_net is the only backend that can pass vq->private_data == NULL to
vhost_init_used(), hence the "if (sock)" branch.
No behaviour change.
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
drivers/vhost/net.c | 6 ++++++
drivers/vhost/scsi.c | 3 +++
drivers/vhost/test.c | 2 ++
drivers/vhost/vhost.c | 12 +++++++-----
drivers/vhost/vhost.h | 1 +
5 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 9eda69e40678..d6319cb2664c 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -917,6 +917,12 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
vhost_net_disable_vq(n, vq);
vq->private_data = sock;
+
+ if (sock)
+ vhost_set_is_le(vq);
+ else
+ vq->is_le = virtio_legacy_is_little_endian();
+
r = vhost_init_used(vq);
if (r)
goto err_used;
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index e25a23692822..e2644a301fa5 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -1276,6 +1276,9 @@ vhost_scsi_set_endpoint(struct vhost_scsi *vs,
vq = &vs->vqs[i].vq;
mutex_lock(&vq->mutex);
vq->private_data = vs_tpg;
+
+ vhost_set_is_le(vq);
+
vhost_init_used(vq);
mutex_unlock(&vq->mutex);
}
diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
index f2882ac98726..b1c7df502211 100644
--- a/drivers/vhost/test.c
+++ b/drivers/vhost/test.c
@@ -196,6 +196,8 @@ static long vhost_test_run(struct vhost_test *n, int test)
oldpriv = vq->private_data;
vq->private_data = priv;
+ vhost_set_is_le(vq);
+
r = vhost_init_used(&n->vqs[index]);
mutex_unlock(&vq->mutex);
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index eec2f11809ff..6be863dcbd13 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -113,6 +113,12 @@ static void vhost_init_is_le(struct vhost_virtqueue *vq)
}
#endif /* CONFIG_VHOST_CROSS_ENDIAN_LEGACY */
+void vhost_set_is_le(struct vhost_virtqueue *vq)
+{
+ vhost_init_is_le(vq);
+}
+EXPORT_SYMBOL_GPL(vhost_set_is_le);
+
static void vhost_poll_func(struct file *file, wait_queue_head_t *wqh,
poll_table *pt)
{
@@ -1156,12 +1162,8 @@ int vhost_init_used(struct vhost_virtqueue *vq)
{
__virtio16 last_used_idx;
int r;
- if (!vq->private_data) {
- vq->is_le = virtio_legacy_is_little_endian();
+ if (!vq->private_data)
return 0;
- }
-
- vhost_init_is_le(vq);
r = vhost_update_used_flags(vq);
if (r)
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 4772862b71a7..8a62041959fe 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -162,6 +162,7 @@ bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *);
int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
unsigned int log_num, u64 len);
+void vhost_set_is_le(struct vhost_virtqueue *vq);
#define vq_err(vq, fmt, ...) do { \
pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH] vhost: move is_le setup to the backend
2015-10-30 11:42 Greg Kurz
@ 2015-11-02 20:09 ` David Miller
2015-11-02 20:09 ` David Miller
` (4 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: David Miller @ 2015-11-02 20:09 UTC (permalink / raw)
To: gkurz; +Cc: mst, netdev, linux-kernel, kvm, virtualization
From: Greg Kurz <gkurz@linux.vnet.ibm.com>
Date: Fri, 30 Oct 2015 12:42:35 +0100
> The vq->is_le field is used to fix endianness when accessing the vring via
> the cpu_to_vhost16() and vhost16_to_cpu() helpers in the following cases:
>
> 1) host is big endian and device is modern virtio
>
> 2) host has cross-endian support and device is legacy virtio with a different
> endianness than the host
>
> Both cases rely on the VHOST_SET_FEATURES ioctl, but 2) also needs the
> VHOST_SET_VRING_ENDIAN ioctl to be called by userspace. Since vq->is_le
> is only needed when the backend is active, it was decided to set it at
> backend start.
>
> This is currently done in vhost_init_used()->vhost_init_is_le() but it
> obfuscates the core vhost code. This patch moves the is_le setup to a
> dedicated function that is called from the backend code.
>
> Note vhost_net is the only backend that can pass vq->private_data == NULL to
> vhost_init_used(), hence the "if (sock)" branch.
>
> No behaviour change.
>
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
Michael, I'm assuming that you will be the one taking this.
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] vhost: move is_le setup to the backend
2015-10-30 11:42 Greg Kurz
2015-11-02 20:09 ` David Miller
@ 2015-11-02 20:09 ` David Miller
2015-11-12 8:30 ` Greg Kurz
` (3 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: David Miller @ 2015-11-02 20:09 UTC (permalink / raw)
To: gkurz; +Cc: netdev, virtualization, linux-kernel, kvm, mst
From: Greg Kurz <gkurz@linux.vnet.ibm.com>
Date: Fri, 30 Oct 2015 12:42:35 +0100
> The vq->is_le field is used to fix endianness when accessing the vring via
> the cpu_to_vhost16() and vhost16_to_cpu() helpers in the following cases:
>
> 1) host is big endian and device is modern virtio
>
> 2) host has cross-endian support and device is legacy virtio with a different
> endianness than the host
>
> Both cases rely on the VHOST_SET_FEATURES ioctl, but 2) also needs the
> VHOST_SET_VRING_ENDIAN ioctl to be called by userspace. Since vq->is_le
> is only needed when the backend is active, it was decided to set it at
> backend start.
>
> This is currently done in vhost_init_used()->vhost_init_is_le() but it
> obfuscates the core vhost code. This patch moves the is_le setup to a
> dedicated function that is called from the backend code.
>
> Note vhost_net is the only backend that can pass vq->private_data == NULL to
> vhost_init_used(), hence the "if (sock)" branch.
>
> No behaviour change.
>
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
Michael, I'm assuming that you will be the one taking this.
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] vhost: move is_le setup to the backend
2015-10-30 11:42 Greg Kurz
2015-11-02 20:09 ` David Miller
2015-11-02 20:09 ` David Miller
@ 2015-11-12 8:30 ` Greg Kurz
2015-11-12 12:37 ` Cornelia Huck
` (2 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Greg Kurz @ 2015-11-12 8:30 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, linux-kernel, kvm, virtualization
On Fri, 30 Oct 2015 12:42:35 +0100
Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:
> The vq->is_le field is used to fix endianness when accessing the vring via
> the cpu_to_vhost16() and vhost16_to_cpu() helpers in the following cases:
>
> 1) host is big endian and device is modern virtio
>
> 2) host has cross-endian support and device is legacy virtio with a different
> endianness than the host
>
> Both cases rely on the VHOST_SET_FEATURES ioctl, but 2) also needs the
> VHOST_SET_VRING_ENDIAN ioctl to be called by userspace. Since vq->is_le
> is only needed when the backend is active, it was decided to set it at
> backend start.
>
> This is currently done in vhost_init_used()->vhost_init_is_le() but it
> obfuscates the core vhost code. This patch moves the is_le setup to a
> dedicated function that is called from the backend code.
>
> Note vhost_net is the only backend that can pass vq->private_data == NULL to
> vhost_init_used(), hence the "if (sock)" branch.
>
> No behaviour change.
>
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> ---
Ping ?
> drivers/vhost/net.c | 6 ++++++
> drivers/vhost/scsi.c | 3 +++
> drivers/vhost/test.c | 2 ++
> drivers/vhost/vhost.c | 12 +++++++-----
> drivers/vhost/vhost.h | 1 +
> 5 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 9eda69e40678..d6319cb2664c 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -917,6 +917,12 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
>
> vhost_net_disable_vq(n, vq);
> vq->private_data = sock;
> +
> + if (sock)
> + vhost_set_is_le(vq);
> + else
> + vq->is_le = virtio_legacy_is_little_endian();
> +
> r = vhost_init_used(vq);
> if (r)
> goto err_used;
> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index e25a23692822..e2644a301fa5 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -1276,6 +1276,9 @@ vhost_scsi_set_endpoint(struct vhost_scsi *vs,
> vq = &vs->vqs[i].vq;
> mutex_lock(&vq->mutex);
> vq->private_data = vs_tpg;
> +
> + vhost_set_is_le(vq);
> +
> vhost_init_used(vq);
> mutex_unlock(&vq->mutex);
> }
> diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
> index f2882ac98726..b1c7df502211 100644
> --- a/drivers/vhost/test.c
> +++ b/drivers/vhost/test.c
> @@ -196,6 +196,8 @@ static long vhost_test_run(struct vhost_test *n, int test)
> oldpriv = vq->private_data;
> vq->private_data = priv;
>
> + vhost_set_is_le(vq);
> +
> r = vhost_init_used(&n->vqs[index]);
>
> mutex_unlock(&vq->mutex);
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index eec2f11809ff..6be863dcbd13 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -113,6 +113,12 @@ static void vhost_init_is_le(struct vhost_virtqueue *vq)
> }
> #endif /* CONFIG_VHOST_CROSS_ENDIAN_LEGACY */
>
> +void vhost_set_is_le(struct vhost_virtqueue *vq)
> +{
> + vhost_init_is_le(vq);
> +}
> +EXPORT_SYMBOL_GPL(vhost_set_is_le);
> +
> static void vhost_poll_func(struct file *file, wait_queue_head_t *wqh,
> poll_table *pt)
> {
> @@ -1156,12 +1162,8 @@ int vhost_init_used(struct vhost_virtqueue *vq)
> {
> __virtio16 last_used_idx;
> int r;
> - if (!vq->private_data) {
> - vq->is_le = virtio_legacy_is_little_endian();
> + if (!vq->private_data)
> return 0;
> - }
> -
> - vhost_init_is_le(vq);
>
> r = vhost_update_used_flags(vq);
> if (r)
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index 4772862b71a7..8a62041959fe 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -162,6 +162,7 @@ bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *);
>
> int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
> unsigned int log_num, u64 len);
> +void vhost_set_is_le(struct vhost_virtqueue *vq);
>
> #define vq_err(vq, fmt, ...) do { \
> pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] vhost: move is_le setup to the backend
2015-10-30 11:42 Greg Kurz
@ 2015-11-12 12:37 ` Cornelia Huck
2015-11-02 20:09 ` David Miller
` (4 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Cornelia Huck @ 2015-11-12 12:37 UTC (permalink / raw)
To: Greg Kurz; +Cc: Michael S. Tsirkin, netdev, linux-kernel, kvm, virtualization
On Fri, 30 Oct 2015 12:42:35 +0100
Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:
> The vq->is_le field is used to fix endianness when accessing the vring via
> the cpu_to_vhost16() and vhost16_to_cpu() helpers in the following cases:
>
> 1) host is big endian and device is modern virtio
>
> 2) host has cross-endian support and device is legacy virtio with a different
> endianness than the host
>
> Both cases rely on the VHOST_SET_FEATURES ioctl, but 2) also needs the
> VHOST_SET_VRING_ENDIAN ioctl to be called by userspace. Since vq->is_le
> is only needed when the backend is active, it was decided to set it at
> backend start.
>
> This is currently done in vhost_init_used()->vhost_init_is_le() but it
> obfuscates the core vhost code. This patch moves the is_le setup to a
> dedicated function that is called from the backend code.
>
> Note vhost_net is the only backend that can pass vq->private_data == NULL to
> vhost_init_used(), hence the "if (sock)" branch.
>
> No behaviour change.
>
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> ---
> drivers/vhost/net.c | 6 ++++++
> drivers/vhost/scsi.c | 3 +++
> drivers/vhost/test.c | 2 ++
> drivers/vhost/vhost.c | 12 +++++++-----
> drivers/vhost/vhost.h | 1 +
> 5 files changed, 19 insertions(+), 5 deletions(-)
Makes sense.
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] vhost: move is_le setup to the backend
@ 2015-11-12 12:37 ` Cornelia Huck
0 siblings, 0 replies; 15+ messages in thread
From: Cornelia Huck @ 2015-11-12 12:37 UTC (permalink / raw)
To: Greg Kurz; +Cc: netdev, virtualization, linux-kernel, kvm, Michael S. Tsirkin
On Fri, 30 Oct 2015 12:42:35 +0100
Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:
> The vq->is_le field is used to fix endianness when accessing the vring via
> the cpu_to_vhost16() and vhost16_to_cpu() helpers in the following cases:
>
> 1) host is big endian and device is modern virtio
>
> 2) host has cross-endian support and device is legacy virtio with a different
> endianness than the host
>
> Both cases rely on the VHOST_SET_FEATURES ioctl, but 2) also needs the
> VHOST_SET_VRING_ENDIAN ioctl to be called by userspace. Since vq->is_le
> is only needed when the backend is active, it was decided to set it at
> backend start.
>
> This is currently done in vhost_init_used()->vhost_init_is_le() but it
> obfuscates the core vhost code. This patch moves the is_le setup to a
> dedicated function that is called from the backend code.
>
> Note vhost_net is the only backend that can pass vq->private_data == NULL to
> vhost_init_used(), hence the "if (sock)" branch.
>
> No behaviour change.
>
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> ---
> drivers/vhost/net.c | 6 ++++++
> drivers/vhost/scsi.c | 3 +++
> drivers/vhost/test.c | 2 ++
> drivers/vhost/vhost.c | 12 +++++++-----
> drivers/vhost/vhost.h | 1 +
> 5 files changed, 19 insertions(+), 5 deletions(-)
Makes sense.
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] vhost: move is_le setup to the backend
2015-10-30 11:42 Greg Kurz
` (3 preceding siblings ...)
2015-11-12 12:37 ` Cornelia Huck
@ 2015-11-12 13:46 ` Michael S. Tsirkin
2015-11-12 14:28 ` Greg Kurz
2015-11-12 13:46 ` Michael S. Tsirkin
5 siblings, 1 reply; 15+ messages in thread
From: Michael S. Tsirkin @ 2015-11-12 13:46 UTC (permalink / raw)
To: Greg Kurz; +Cc: netdev, linux-kernel, kvm, virtualization
On Fri, Oct 30, 2015 at 12:42:35PM +0100, Greg Kurz wrote:
> The vq->is_le field is used to fix endianness when accessing the vring via
> the cpu_to_vhost16() and vhost16_to_cpu() helpers in the following cases:
>
> 1) host is big endian and device is modern virtio
>
> 2) host has cross-endian support and device is legacy virtio with a different
> endianness than the host
>
> Both cases rely on the VHOST_SET_FEATURES ioctl, but 2) also needs the
> VHOST_SET_VRING_ENDIAN ioctl to be called by userspace. Since vq->is_le
> is only needed when the backend is active, it was decided to set it at
> backend start.
>
> This is currently done in vhost_init_used()->vhost_init_is_le() but it
> obfuscates the core vhost code. This patch moves the is_le setup to a
> dedicated function that is called from the backend code.
>
> Note vhost_net is the only backend that can pass vq->private_data == NULL to
> vhost_init_used(), hence the "if (sock)" branch.
>
> No behaviour change.
>
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
I plan to look at this next week, busy with QEMU 2.5 now.
> ---
> drivers/vhost/net.c | 6 ++++++
> drivers/vhost/scsi.c | 3 +++
> drivers/vhost/test.c | 2 ++
> drivers/vhost/vhost.c | 12 +++++++-----
> drivers/vhost/vhost.h | 1 +
> 5 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 9eda69e40678..d6319cb2664c 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -917,6 +917,12 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
>
> vhost_net_disable_vq(n, vq);
> vq->private_data = sock;
> +
> + if (sock)
> + vhost_set_is_le(vq);
> + else
> + vq->is_le = virtio_legacy_is_little_endian();
> +
> r = vhost_init_used(vq);
> if (r)
> goto err_used;
> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index e25a23692822..e2644a301fa5 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -1276,6 +1276,9 @@ vhost_scsi_set_endpoint(struct vhost_scsi *vs,
> vq = &vs->vqs[i].vq;
> mutex_lock(&vq->mutex);
> vq->private_data = vs_tpg;
> +
> + vhost_set_is_le(vq);
> +
> vhost_init_used(vq);
> mutex_unlock(&vq->mutex);
> }
> diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
> index f2882ac98726..b1c7df502211 100644
> --- a/drivers/vhost/test.c
> +++ b/drivers/vhost/test.c
> @@ -196,6 +196,8 @@ static long vhost_test_run(struct vhost_test *n, int test)
> oldpriv = vq->private_data;
> vq->private_data = priv;
>
> + vhost_set_is_le(vq);
> +
> r = vhost_init_used(&n->vqs[index]);
>
> mutex_unlock(&vq->mutex);
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index eec2f11809ff..6be863dcbd13 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -113,6 +113,12 @@ static void vhost_init_is_le(struct vhost_virtqueue *vq)
> }
> #endif /* CONFIG_VHOST_CROSS_ENDIAN_LEGACY */
>
> +void vhost_set_is_le(struct vhost_virtqueue *vq)
> +{
> + vhost_init_is_le(vq);
> +}
> +EXPORT_SYMBOL_GPL(vhost_set_is_le);
> +
> static void vhost_poll_func(struct file *file, wait_queue_head_t *wqh,
> poll_table *pt)
> {
> @@ -1156,12 +1162,8 @@ int vhost_init_used(struct vhost_virtqueue *vq)
> {
> __virtio16 last_used_idx;
> int r;
> - if (!vq->private_data) {
> - vq->is_le = virtio_legacy_is_little_endian();
> + if (!vq->private_data)
> return 0;
> - }
> -
> - vhost_init_is_le(vq);
>
> r = vhost_update_used_flags(vq);
> if (r)
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index 4772862b71a7..8a62041959fe 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -162,6 +162,7 @@ bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *);
>
> int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
> unsigned int log_num, u64 len);
> +void vhost_set_is_le(struct vhost_virtqueue *vq);
>
> #define vq_err(vq, fmt, ...) do { \
> pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] vhost: move is_le setup to the backend
2015-11-12 13:46 ` Michael S. Tsirkin
@ 2015-11-12 14:28 ` Greg Kurz
2015-12-16 13:52 ` Greg Kurz
0 siblings, 1 reply; 15+ messages in thread
From: Greg Kurz @ 2015-11-12 14:28 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, linux-kernel, kvm, virtualization
On Thu, 12 Nov 2015 15:46:30 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Fri, Oct 30, 2015 at 12:42:35PM +0100, Greg Kurz wrote:
> > The vq->is_le field is used to fix endianness when accessing the vring via
> > the cpu_to_vhost16() and vhost16_to_cpu() helpers in the following cases:
> >
> > 1) host is big endian and device is modern virtio
> >
> > 2) host has cross-endian support and device is legacy virtio with a different
> > endianness than the host
> >
> > Both cases rely on the VHOST_SET_FEATURES ioctl, but 2) also needs the
> > VHOST_SET_VRING_ENDIAN ioctl to be called by userspace. Since vq->is_le
> > is only needed when the backend is active, it was decided to set it at
> > backend start.
> >
> > This is currently done in vhost_init_used()->vhost_init_is_le() but it
> > obfuscates the core vhost code. This patch moves the is_le setup to a
> > dedicated function that is called from the backend code.
> >
> > Note vhost_net is the only backend that can pass vq->private_data == NULL to
> > vhost_init_used(), hence the "if (sock)" branch.
> >
> > No behaviour change.
> >
> > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
>
> I plan to look at this next week, busy with QEMU 2.5 now.
>
I don't have any deadline for this since this is only a cleanup tentative.
Thanks.
> > ---
> > drivers/vhost/net.c | 6 ++++++
> > drivers/vhost/scsi.c | 3 +++
> > drivers/vhost/test.c | 2 ++
> > drivers/vhost/vhost.c | 12 +++++++-----
> > drivers/vhost/vhost.h | 1 +
> > 5 files changed, 19 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> > index 9eda69e40678..d6319cb2664c 100644
> > --- a/drivers/vhost/net.c
> > +++ b/drivers/vhost/net.c
> > @@ -917,6 +917,12 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> >
> > vhost_net_disable_vq(n, vq);
> > vq->private_data = sock;
> > +
> > + if (sock)
> > + vhost_set_is_le(vq);
> > + else
> > + vq->is_le = virtio_legacy_is_little_endian();
> > +
> > r = vhost_init_used(vq);
> > if (r)
> > goto err_used;
> > diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> > index e25a23692822..e2644a301fa5 100644
> > --- a/drivers/vhost/scsi.c
> > +++ b/drivers/vhost/scsi.c
> > @@ -1276,6 +1276,9 @@ vhost_scsi_set_endpoint(struct vhost_scsi *vs,
> > vq = &vs->vqs[i].vq;
> > mutex_lock(&vq->mutex);
> > vq->private_data = vs_tpg;
> > +
> > + vhost_set_is_le(vq);
> > +
> > vhost_init_used(vq);
> > mutex_unlock(&vq->mutex);
> > }
> > diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
> > index f2882ac98726..b1c7df502211 100644
> > --- a/drivers/vhost/test.c
> > +++ b/drivers/vhost/test.c
> > @@ -196,6 +196,8 @@ static long vhost_test_run(struct vhost_test *n, int test)
> > oldpriv = vq->private_data;
> > vq->private_data = priv;
> >
> > + vhost_set_is_le(vq);
> > +
> > r = vhost_init_used(&n->vqs[index]);
> >
> > mutex_unlock(&vq->mutex);
> > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > index eec2f11809ff..6be863dcbd13 100644
> > --- a/drivers/vhost/vhost.c
> > +++ b/drivers/vhost/vhost.c
> > @@ -113,6 +113,12 @@ static void vhost_init_is_le(struct vhost_virtqueue *vq)
> > }
> > #endif /* CONFIG_VHOST_CROSS_ENDIAN_LEGACY */
> >
> > +void vhost_set_is_le(struct vhost_virtqueue *vq)
> > +{
> > + vhost_init_is_le(vq);
> > +}
> > +EXPORT_SYMBOL_GPL(vhost_set_is_le);
> > +
> > static void vhost_poll_func(struct file *file, wait_queue_head_t *wqh,
> > poll_table *pt)
> > {
> > @@ -1156,12 +1162,8 @@ int vhost_init_used(struct vhost_virtqueue *vq)
> > {
> > __virtio16 last_used_idx;
> > int r;
> > - if (!vq->private_data) {
> > - vq->is_le = virtio_legacy_is_little_endian();
> > + if (!vq->private_data)
> > return 0;
> > - }
> > -
> > - vhost_init_is_le(vq);
> >
> > r = vhost_update_used_flags(vq);
> > if (r)
> > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> > index 4772862b71a7..8a62041959fe 100644
> > --- a/drivers/vhost/vhost.h
> > +++ b/drivers/vhost/vhost.h
> > @@ -162,6 +162,7 @@ bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *);
> >
> > int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
> > unsigned int log_num, u64 len);
> > +void vhost_set_is_le(struct vhost_virtqueue *vq);
> >
> > #define vq_err(vq, fmt, ...) do { \
> > pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] vhost: move is_le setup to the backend
2015-11-12 14:28 ` Greg Kurz
@ 2015-12-16 13:52 ` Greg Kurz
0 siblings, 0 replies; 15+ messages in thread
From: Greg Kurz @ 2015-12-16 13:52 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, linux-kernel, kvm, virtualization
On Thu, 12 Nov 2015 15:28:19 +0100
Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:
> On Thu, 12 Nov 2015 15:46:30 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > On Fri, Oct 30, 2015 at 12:42:35PM +0100, Greg Kurz wrote:
> > > The vq->is_le field is used to fix endianness when accessing the vring via
> > > the cpu_to_vhost16() and vhost16_to_cpu() helpers in the following cases:
> > >
> > > 1) host is big endian and device is modern virtio
> > >
> > > 2) host has cross-endian support and device is legacy virtio with a different
> > > endianness than the host
> > >
> > > Both cases rely on the VHOST_SET_FEATURES ioctl, but 2) also needs the
> > > VHOST_SET_VRING_ENDIAN ioctl to be called by userspace. Since vq->is_le
> > > is only needed when the backend is active, it was decided to set it at
> > > backend start.
> > >
> > > This is currently done in vhost_init_used()->vhost_init_is_le() but it
> > > obfuscates the core vhost code. This patch moves the is_le setup to a
> > > dedicated function that is called from the backend code.
> > >
> > > Note vhost_net is the only backend that can pass vq->private_data == NULL to
> > > vhost_init_used(), hence the "if (sock)" branch.
> > >
> > > No behaviour change.
> > >
> > > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> >
> > I plan to look at this next week, busy with QEMU 2.5 now.
> >
>
> I don't have any deadline for this since this is only a cleanup tentative.
>
> Thanks.
>
... but ping anyway since it was a month ago :)
Cheers.
--
Greg
> > > ---
> > > drivers/vhost/net.c | 6 ++++++
> > > drivers/vhost/scsi.c | 3 +++
> > > drivers/vhost/test.c | 2 ++
> > > drivers/vhost/vhost.c | 12 +++++++-----
> > > drivers/vhost/vhost.h | 1 +
> > > 5 files changed, 19 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> > > index 9eda69e40678..d6319cb2664c 100644
> > > --- a/drivers/vhost/net.c
> > > +++ b/drivers/vhost/net.c
> > > @@ -917,6 +917,12 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> > >
> > > vhost_net_disable_vq(n, vq);
> > > vq->private_data = sock;
> > > +
> > > + if (sock)
> > > + vhost_set_is_le(vq);
> > > + else
> > > + vq->is_le = virtio_legacy_is_little_endian();
> > > +
> > > r = vhost_init_used(vq);
> > > if (r)
> > > goto err_used;
> > > diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> > > index e25a23692822..e2644a301fa5 100644
> > > --- a/drivers/vhost/scsi.c
> > > +++ b/drivers/vhost/scsi.c
> > > @@ -1276,6 +1276,9 @@ vhost_scsi_set_endpoint(struct vhost_scsi *vs,
> > > vq = &vs->vqs[i].vq;
> > > mutex_lock(&vq->mutex);
> > > vq->private_data = vs_tpg;
> > > +
> > > + vhost_set_is_le(vq);
> > > +
> > > vhost_init_used(vq);
> > > mutex_unlock(&vq->mutex);
> > > }
> > > diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
> > > index f2882ac98726..b1c7df502211 100644
> > > --- a/drivers/vhost/test.c
> > > +++ b/drivers/vhost/test.c
> > > @@ -196,6 +196,8 @@ static long vhost_test_run(struct vhost_test *n, int test)
> > > oldpriv = vq->private_data;
> > > vq->private_data = priv;
> > >
> > > + vhost_set_is_le(vq);
> > > +
> > > r = vhost_init_used(&n->vqs[index]);
> > >
> > > mutex_unlock(&vq->mutex);
> > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > > index eec2f11809ff..6be863dcbd13 100644
> > > --- a/drivers/vhost/vhost.c
> > > +++ b/drivers/vhost/vhost.c
> > > @@ -113,6 +113,12 @@ static void vhost_init_is_le(struct vhost_virtqueue *vq)
> > > }
> > > #endif /* CONFIG_VHOST_CROSS_ENDIAN_LEGACY */
> > >
> > > +void vhost_set_is_le(struct vhost_virtqueue *vq)
> > > +{
> > > + vhost_init_is_le(vq);
> > > +}
> > > +EXPORT_SYMBOL_GPL(vhost_set_is_le);
> > > +
> > > static void vhost_poll_func(struct file *file, wait_queue_head_t *wqh,
> > > poll_table *pt)
> > > {
> > > @@ -1156,12 +1162,8 @@ int vhost_init_used(struct vhost_virtqueue *vq)
> > > {
> > > __virtio16 last_used_idx;
> > > int r;
> > > - if (!vq->private_data) {
> > > - vq->is_le = virtio_legacy_is_little_endian();
> > > + if (!vq->private_data)
> > > return 0;
> > > - }
> > > -
> > > - vhost_init_is_le(vq);
> > >
> > > r = vhost_update_used_flags(vq);
> > > if (r)
> > > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> > > index 4772862b71a7..8a62041959fe 100644
> > > --- a/drivers/vhost/vhost.h
> > > +++ b/drivers/vhost/vhost.h
> > > @@ -162,6 +162,7 @@ bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *);
> > >
> > > int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
> > > unsigned int log_num, u64 len);
> > > +void vhost_set_is_le(struct vhost_virtqueue *vq);
> > >
> > > #define vq_err(vq, fmt, ...) do { \
> > > pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
> >
>
> _______________________________________________
> Virtualization mailing list
> Virtualization@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/virtualization
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] vhost: move is_le setup to the backend
2015-10-30 11:42 Greg Kurz
` (4 preceding siblings ...)
2015-11-12 13:46 ` Michael S. Tsirkin
@ 2015-11-12 13:46 ` Michael S. Tsirkin
5 siblings, 0 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2015-11-12 13:46 UTC (permalink / raw)
To: Greg Kurz; +Cc: netdev, linux-kernel, kvm, virtualization
On Fri, Oct 30, 2015 at 12:42:35PM +0100, Greg Kurz wrote:
> The vq->is_le field is used to fix endianness when accessing the vring via
> the cpu_to_vhost16() and vhost16_to_cpu() helpers in the following cases:
>
> 1) host is big endian and device is modern virtio
>
> 2) host has cross-endian support and device is legacy virtio with a different
> endianness than the host
>
> Both cases rely on the VHOST_SET_FEATURES ioctl, but 2) also needs the
> VHOST_SET_VRING_ENDIAN ioctl to be called by userspace. Since vq->is_le
> is only needed when the backend is active, it was decided to set it at
> backend start.
>
> This is currently done in vhost_init_used()->vhost_init_is_le() but it
> obfuscates the core vhost code. This patch moves the is_le setup to a
> dedicated function that is called from the backend code.
>
> Note vhost_net is the only backend that can pass vq->private_data == NULL to
> vhost_init_used(), hence the "if (sock)" branch.
>
> No behaviour change.
>
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
I plan to look at this next week, busy with QEMU 2.5 now.
> ---
> drivers/vhost/net.c | 6 ++++++
> drivers/vhost/scsi.c | 3 +++
> drivers/vhost/test.c | 2 ++
> drivers/vhost/vhost.c | 12 +++++++-----
> drivers/vhost/vhost.h | 1 +
> 5 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 9eda69e40678..d6319cb2664c 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -917,6 +917,12 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
>
> vhost_net_disable_vq(n, vq);
> vq->private_data = sock;
> +
> + if (sock)
> + vhost_set_is_le(vq);
> + else
> + vq->is_le = virtio_legacy_is_little_endian();
> +
> r = vhost_init_used(vq);
> if (r)
> goto err_used;
> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index e25a23692822..e2644a301fa5 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -1276,6 +1276,9 @@ vhost_scsi_set_endpoint(struct vhost_scsi *vs,
> vq = &vs->vqs[i].vq;
> mutex_lock(&vq->mutex);
> vq->private_data = vs_tpg;
> +
> + vhost_set_is_le(vq);
> +
> vhost_init_used(vq);
> mutex_unlock(&vq->mutex);
> }
> diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
> index f2882ac98726..b1c7df502211 100644
> --- a/drivers/vhost/test.c
> +++ b/drivers/vhost/test.c
> @@ -196,6 +196,8 @@ static long vhost_test_run(struct vhost_test *n, int test)
> oldpriv = vq->private_data;
> vq->private_data = priv;
>
> + vhost_set_is_le(vq);
> +
> r = vhost_init_used(&n->vqs[index]);
>
> mutex_unlock(&vq->mutex);
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index eec2f11809ff..6be863dcbd13 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -113,6 +113,12 @@ static void vhost_init_is_le(struct vhost_virtqueue *vq)
> }
> #endif /* CONFIG_VHOST_CROSS_ENDIAN_LEGACY */
>
> +void vhost_set_is_le(struct vhost_virtqueue *vq)
> +{
> + vhost_init_is_le(vq);
> +}
> +EXPORT_SYMBOL_GPL(vhost_set_is_le);
> +
> static void vhost_poll_func(struct file *file, wait_queue_head_t *wqh,
> poll_table *pt)
> {
> @@ -1156,12 +1162,8 @@ int vhost_init_used(struct vhost_virtqueue *vq)
> {
> __virtio16 last_used_idx;
> int r;
> - if (!vq->private_data) {
> - vq->is_le = virtio_legacy_is_little_endian();
> + if (!vq->private_data)
> return 0;
> - }
> -
> - vhost_init_is_le(vq);
>
> r = vhost_update_used_flags(vq);
> if (r)
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index 4772862b71a7..8a62041959fe 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -162,6 +162,7 @@ bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *);
>
> int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
> unsigned int log_num, u64 len);
> +void vhost_set_is_le(struct vhost_virtqueue *vq);
>
> #define vq_err(vq, fmt, ...) do { \
> pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2016-01-12 10:48 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-11 14:39 [PATCH] vhost: move is_le setup to the backend Greg Kurz
2016-01-12 10:01 ` Michael S. Tsirkin
2016-01-12 10:31 ` Greg Kurz
2016-01-12 10:48 ` Michael S. Tsirkin
2016-01-12 10:48 ` Michael S. Tsirkin
-- strict thread matches above, loose matches on Subject: below --
2015-10-30 11:42 Greg Kurz
2015-11-02 20:09 ` David Miller
2015-11-02 20:09 ` David Miller
2015-11-12 8:30 ` Greg Kurz
2015-11-12 12:37 ` Cornelia Huck
2015-11-12 12:37 ` Cornelia Huck
2015-11-12 13:46 ` Michael S. Tsirkin
2015-11-12 14:28 ` Greg Kurz
2015-12-16 13:52 ` Greg Kurz
2015-11-12 13:46 ` Michael S. Tsirkin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.