On Sun, 30 Nov 2014 17:11:49 +0200
"Michael S. Tsirkin" [off-list ref] wrote:
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/vhost/vhost.c | 93 +++++++++++++++++++++++++++++++--------------------
1 file changed, 56 insertions(+), 37 deletions(-)
quoted hunk
@@ -1113,18 +1120,19 @@ static int get_indirect(struct vhost_virtqueue *vq,
{
struct vring_desc desc;
unsigned int i = 0, count, found = 0;
+ u32 len = vhost32_to_cpu(vq, indirect->len);
int ret;
/* Sanity check */
- if (unlikely(indirect->len % sizeof desc)) {
+ if (unlikely(len % sizeof desc)) {
vq_err(vq, "Invalid length in indirect descriptor: "
"len 0x%llx not multiple of 0x%zx\n",
- (unsigned long long)indirect->len,
+ (unsigned long long)vhost32_to_cpu(vq, indirect->len),
Can't you use len here?
sizeof desc);
return -EINVAL;
}
- ret = translate_desc(vq, indirect->addr, indirect->len, vq->indirect,
+ ret = translate_desc(vq, vhost64_to_cpu(vq, indirect->addr), len, vq->indirect,
UIO_MAXIOV);
if (unlikely(ret < 0)) {
vq_err(vq, "Translation failure %d in indirect.\n", ret);
quoted hunk
@@ -1404,7 +1422,7 @@ int vhost_add_used_n(struct vhost_virtqueue *vq, struct vring_used_elem *heads,
/* Make sure buffer is written before we update index. */
smp_wmb();
- if (put_user(vq->last_used_idx, &vq->used->idx)) {
+ if (__put_user(cpu_to_vhost16(vq, vq->last_used_idx), &vq->used->idx)) {
Why s/put_user/__put_user/ - I don't see how endianness conversions
should have an influence there?
vq_err(vq, "Failed to increment used idx");
return -EFAULT;
}
quoted hunk
@@ -1449,11 +1468,11 @@ static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
if (unlikely(!v))
return true;
- if (get_user(event, vhost_used_event(vq))) {
+ if (__get_user(event, vhost_used_event(vq))) {
Dito: why the change?
vq_err(vq, "Failed to get used event idx");
return true;
}
- return vring_need_event(event, new, old);
+ return vring_need_event(vhost16_to_cpu(vq, event), new, old);
}
/* This actually signals the guest, using eventfd. */
On Mon, Dec 01, 2014 at 01:33:53PM +0100, Cornelia Huck wrote:
On Sun, 30 Nov 2014 17:11:49 +0200
"Michael S. Tsirkin" [off-list ref] wrote:
quoted
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/vhost/vhost.c | 93 +++++++++++++++++++++++++++++++--------------------
1 file changed, 56 insertions(+), 37 deletions(-)
quoted
@@ -1113,18 +1120,19 @@ static int get_indirect(struct vhost_virtqueue *vq,
{
struct vring_desc desc;
unsigned int i = 0, count, found = 0;
+ u32 len = vhost32_to_cpu(vq, indirect->len);
int ret;
/* Sanity check */
- if (unlikely(indirect->len % sizeof desc)) {
+ if (unlikely(len % sizeof desc)) {
vq_err(vq, "Invalid length in indirect descriptor: "
"len 0x%llx not multiple of 0x%zx\n",
- (unsigned long long)indirect->len,
+ (unsigned long long)vhost32_to_cpu(vq, indirect->len),
Can't you use len here?
Not if I want error message to be readable.
quoted
sizeof desc);
return -EINVAL;
}
- ret = translate_desc(vq, indirect->addr, indirect->len, vq->indirect,
+ ret = translate_desc(vq, vhost64_to_cpu(vq, indirect->addr), len, vq->indirect,
UIO_MAXIOV);
if (unlikely(ret < 0)) {
vq_err(vq, "Translation failure %d in indirect.\n", ret);
quoted
@@ -1404,7 +1422,7 @@ int vhost_add_used_n(struct vhost_virtqueue *vq, struct vring_used_elem *heads,
/* Make sure buffer is written before we update index. */
smp_wmb();
- if (put_user(vq->last_used_idx, &vq->used->idx)) {
+ if (__put_user(cpu_to_vhost16(vq, vq->last_used_idx), &vq->used->idx)) {
Why s/put_user/__put_user/ - I don't see how endianness conversions
should have an influence there?
We should generally to __ variants since addresses are pre-validated.
But I agree - should be a separate patch.
quoted
vq_err(vq, "Failed to increment used idx");
return -EFAULT;
}
quoted
@@ -1449,11 +1468,11 @@ static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
if (unlikely(!v))
return true;
- if (get_user(event, vhost_used_event(vq))) {
+ if (__get_user(event, vhost_used_event(vq))) {
Dito: why the change?
Same. Will split this out, it's unrelated to virtio 1.0.
quoted
vq_err(vq, "Failed to get used event idx");
return true;
}
- return vring_need_event(event, new, old);
+ return vring_need_event(vhost16_to_cpu(vq, event), new, old);
}
/* This actually signals the guest, using eventfd. */
On Mon, 1 Dec 2014 14:37:01 +0200
"Michael S. Tsirkin" [off-list ref] wrote:
On Mon, Dec 01, 2014 at 01:33:53PM +0100, Cornelia Huck wrote:
quoted
On Sun, 30 Nov 2014 17:11:49 +0200
"Michael S. Tsirkin" [off-list ref] wrote:
quoted
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/vhost/vhost.c | 93 +++++++++++++++++++++++++++++++--------------------
1 file changed, 56 insertions(+), 37 deletions(-)
quoted
@@ -1113,18 +1120,19 @@ static int get_indirect(struct vhost_virtqueue *vq,
{
struct vring_desc desc;
unsigned int i = 0, count, found = 0;
+ u32 len = vhost32_to_cpu(vq, indirect->len);
int ret;
/* Sanity check */
- if (unlikely(indirect->len % sizeof desc)) {
+ if (unlikely(len % sizeof desc)) {
vq_err(vq, "Invalid length in indirect descriptor: "
"len 0x%llx not multiple of 0x%zx\n",
- (unsigned long long)indirect->len,
+ (unsigned long long)vhost32_to_cpu(vq, indirect->len),
Can't you use len here?
Not if I want error message to be readable.
Huh? Both have the same value.
On Mon, Dec 01, 2014 at 01:42:47PM +0100, Cornelia Huck wrote:
On Mon, 1 Dec 2014 14:37:01 +0200
"Michael S. Tsirkin" [off-list ref] wrote:
quoted
On Mon, Dec 01, 2014 at 01:33:53PM +0100, Cornelia Huck wrote:
quoted
On Sun, 30 Nov 2014 17:11:49 +0200
"Michael S. Tsirkin" [off-list ref] wrote:
quoted
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/vhost/vhost.c | 93 +++++++++++++++++++++++++++++++--------------------
1 file changed, 56 insertions(+), 37 deletions(-)
quoted
@@ -1113,18 +1120,19 @@ static int get_indirect(struct vhost_virtqueue *vq,
{
struct vring_desc desc;
unsigned int i = 0, count, found = 0;
+ u32 len = vhost32_to_cpu(vq, indirect->len);
int ret;
/* Sanity check */
- if (unlikely(indirect->len % sizeof desc)) {
+ if (unlikely(len % sizeof desc)) {
vq_err(vq, "Invalid length in indirect descriptor: "
"len 0x%llx not multiple of 0x%zx\n",
- (unsigned long long)indirect->len,
+ (unsigned long long)vhost32_to_cpu(vq, indirect->len),
Can't you use len here?
Not if I want error message to be readable.
Huh? Both have the same value.
Ah, good point.
On Mon, Dec 01, 2014 at 01:33:53PM +0100, Cornelia Huck wrote:
On Sun, 30 Nov 2014 17:11:49 +0200
"Michael S. Tsirkin" [off-list ref] wrote:
quoted
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/vhost/vhost.c | 93 +++++++++++++++++++++++++++++++--------------------
1 file changed, 56 insertions(+), 37 deletions(-)
quoted
@@ -1113,18 +1120,19 @@ static int get_indirect(struct vhost_virtqueue *vq,
{
struct vring_desc desc;
unsigned int i = 0, count, found = 0;
+ u32 len = vhost32_to_cpu(vq, indirect->len);
int ret;
/* Sanity check */
- if (unlikely(indirect->len % sizeof desc)) {
+ if (unlikely(len % sizeof desc)) {
vq_err(vq, "Invalid length in indirect descriptor: "
"len 0x%llx not multiple of 0x%zx\n",
- (unsigned long long)indirect->len,
+ (unsigned long long)vhost32_to_cpu(vq, indirect->len),
Can't you use len here?
quoted
sizeof desc);
return -EINVAL;
}
- ret = translate_desc(vq, indirect->addr, indirect->len, vq->indirect,
+ ret = translate_desc(vq, vhost64_to_cpu(vq, indirect->addr), len, vq->indirect,
UIO_MAXIOV);
if (unlikely(ret < 0)) {
vq_err(vq, "Translation failure %d in indirect.\n", ret);
quoted
@@ -1404,7 +1422,7 @@ int vhost_add_used_n(struct vhost_virtqueue *vq, struct vring_used_elem *heads,
/* Make sure buffer is written before we update index. */
smp_wmb();
- if (put_user(vq->last_used_idx, &vq->used->idx)) {
+ if (__put_user(cpu_to_vhost16(vq, vq->last_used_idx), &vq->used->idx)) {
Why s/put_user/__put_user/ - I don't see how endianness conversions
should have an influence there?
quoted
vq_err(vq, "Failed to increment used idx");
return -EFAULT;
}
quoted
@@ -1449,11 +1468,11 @@ static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
if (unlikely(!v))
return true;
- if (get_user(event, vhost_used_event(vq))) {
+ if (__get_user(event, vhost_used_event(vq))) {
Dito: why the change?
I remember now.
put_user/get_user macros don't work well
with __virtio tags.
As __ are a good idea anyway, I just switched to that
everywhere.
quoted
vq_err(vq, "Failed to get used event idx");
return true;
}
- return vring_need_event(event, new, old);
+ return vring_need_event(vhost16_to_cpu(vq, event), new, old);
}
/* This actually signals the guest, using eventfd. */