search for: vhost_reset_is_le

Currently, under certain circumstances vhost_init_is_le does just a part of the initialization job, and depends on vhost_reset_is_le being called too. For this reason vhost_vq_init_access used to call vhost_reset_is_le when vq->private_data is NULL. This is not only counter intuitive, but also real a problem because it breaks vhost_net. The bug was introduced to vhost_net with commit 2751c9882b94 ("vhost: cross-endian su...

Currently, under certain circumstances vhost_init_is_le does just a part of the initialization job, and depends on vhost_reset_is_le being called too. For this reason vhost_vq_init_access used to call vhost_reset_is_le when vq->private_data is NULL. This is not only counter intuitive, but also real a problem because it breaks vhost_net. The bug was introduced to vhost_net with commit 2751c9882b94 ("vhost: cross-endian su...

On 01/30/2017 08:06 PM, Greg Kurz wrote: >> Currently, under certain circumstances vhost_init_is_le does just a part >> of the initialization job, and depends on vhost_reset_is_le being called >> too. For this reason vhost_vq_init_access used to call vhost_reset_is_le >> when vq->private_data is NULL. This is not only counter intuitive, but >> also real a problem because it breaks vhost_net. The bug was introduced to >> vhost_net with commit 2751c9...

On 01/30/2017 08:06 PM, Greg Kurz wrote: >> Currently, under certain circumstances vhost_init_is_le does just a part >> of the initialization job, and depends on vhost_reset_is_le being called >> too. For this reason vhost_vq_init_access used to call vhost_reset_is_le >> when vq->private_data is NULL. This is not only counter intuitive, but >> also real a problem because it breaks vhost_net. The bug was introduced to >> vhost_net with commit 2751c9...

On Mon, 30 Jan 2017 11:09:36 +0100 Halil Pasic <pasic at linux.vnet.ibm.com> wrote: > Currently, under certain circumstances vhost_init_is_le does just a part > of the initialization job, and depends on vhost_reset_is_le being called > too. For this reason vhost_vq_init_access used to call vhost_reset_is_le > when vq->private_data is NULL. This is not only counter intuitive, but > also real a problem because it breaks vhost_net. The bug was introduced to > vhost_net with commit 2751c9882b94 ("vh...

...t; --- a/drivers/vhost/vhost.c >> +++ b/drivers/vhost/vhost.c >> @@ -1714,10 +1714,8 @@ int vhost_vq_init_access(struct vhost_virtqueue *vq) >> int r; >> bool is_le = vq->is_le; >> >> - if (!vq->private_data) { >> - vhost_reset_is_le(vq); >> + if (!vq->private_data) >> return 0; >> - } >> >> vhost_init_is_le(vq); > > > I think you do need to reset it, just maybe within vhost_init_is_le. > > if (vhost_has_feature(vq, VIRTIO_F_VERSION_1...

...t; --- a/drivers/vhost/vhost.c >> +++ b/drivers/vhost/vhost.c >> @@ -1714,10 +1714,8 @@ int vhost_vq_init_access(struct vhost_virtqueue *vq) >> int r; >> bool is_le = vq->is_le; >> >> - if (!vq->private_data) { >> - vhost_reset_is_le(vq); >> + if (!vq->private_data) >> return 0; >> - } >> >> vhost_init_is_le(vq); > > > I think you do need to reset it, just maybe within vhost_init_is_le. > > if (vhost_has_feature(vq, VIRTIO_F_VERSION_1...

...vhost.c > > >> @@ -1714,10 +1714,8 @@ int vhost_vq_init_access(struct vhost_virtqueue *vq) > > >> int r; > > >> bool is_le = vq->is_le; > > >> > > >> - if (!vq->private_data) { > > >> - vhost_reset_is_le(vq); > > >> + if (!vq->private_data) > > >> return 0; > > >> - } > > >> > > >> vhost_init_is_le(vq); > > > > > > > > > I think you do need to reset it, just maybe within...

...vhost.c > > >> @@ -1714,10 +1714,8 @@ int vhost_vq_init_access(struct vhost_virtqueue *vq) > > >> int r; > > >> bool is_le = vq->is_le; > > >> > > >> - if (!vq->private_data) { > > >> - vhost_reset_is_le(vq); > > >> + if (!vq->private_data) > > >> return 0; > > >> - } > > >> > > >> vhost_init_is_le(vq); > > > > > > > > > I think you do need to reset it, just maybe within...

On Tue, Jan 31, 2017 at 04:56:13PM +0100, Halil Pasic wrote: > > > On 01/30/2017 08:06 PM, Greg Kurz wrote: > >> Currently, under certain circumstances vhost_init_is_le does just a part > >> of the initialization job, and depends on vhost_reset_is_le being called > >> too. For this reason vhost_vq_init_access used to call vhost_reset_is_le > >> when vq->private_data is NULL. This is not only counter intuitive, but > >> also real a problem because it breaks vhost_net. The bug was introduced to > >> vhost_ne...

...tex_unlock(&vq->mutex); } /*[..] rest of the function */ } from drivers/vhost/vhost.c: int vhost_vq_init_access(struct vhost_virtqueue *vq) { __virtio16 last_used_idx; int r; bool is_le = vq->is_le; if (!vq->private_data) { ==> vhost_reset_is_le(vq); /* resets to native endianness and returns */ return 0; } ==> vhost_init_is_le(vq); /* here we init is_le */ r = vhost_update_used_flags(vq); if (r) goto err; vq->signalled_used_valid = false; if (!vq->iotl...

...tex_unlock(&vq->mutex); } /*[..] rest of the function */ } from drivers/vhost/vhost.c: int vhost_vq_init_access(struct vhost_virtqueue *vq) { __virtio16 last_used_idx; int r; bool is_le = vq->is_le; if (!vq->private_data) { ==> vhost_reset_is_le(vq); /* resets to native endianness and returns */ return 0; } ==> vhost_init_is_le(vq); /* here we init is_le */ r = vhost_update_used_flags(vq); if (r) goto err; vq->signalled_used_valid = false; if (!vq->iotl...

...>> +++ b/drivers/vhost/vhost.c > >> @@ -1714,10 +1714,8 @@ int vhost_vq_init_access(struct vhost_virtqueue *vq) > >> int r; > >> bool is_le = vq->is_le; > >> > >> - if (!vq->private_data) { > >> - vhost_reset_is_le(vq); > >> + if (!vq->private_data) > >> return 0; > >> - } > >> > >> vhost_init_is_le(vq); > > > > > > I think you do need to reset it, just maybe within vhost_init_is_le. > > > >...

...function */ > } > > from drivers/vhost/vhost.c: > > int vhost_vq_init_access(struct vhost_virtqueue *vq) > { > __virtio16 last_used_idx; > int r; > bool is_le = vq->is_le; > > if (!vq->private_data) { > ==> vhost_reset_is_le(vq); > /* resets to native endianness and returns */ > return 0; > } > > ==> vhost_init_is_le(vq); > /* here we init is_le */ > > r = vhost_update_used_flags(vq); > if (r) > goto err; > vq-&gt...

This series is a new tentative to have cleaner cross-endian code. Patches 1/3 is new: it fixes a side-effect in case vhost_init_used() fails. Patch 2/3 comes from v1: it renames cross-endian helpers Patch 3/3 is new: it simply renames vhost_init_used() as suggested by Michael. --- Greg Kurz (3): vhost: fix error path in vhost_init_used() vhost: rename cross-endian helpers

This series is a new tentative to have cleaner cross-endian code. Patches 1/3 is new: it fixes a side-effect in case vhost_init_used() fails. Patch 2/3 comes from v1: it renames cross-endian helpers Patch 3/3 is new: it simply renames vhost_init_used() as suggested by Michael. --- Greg Kurz (3): vhost: fix error path in vhost_init_used() vhost: rename cross-endian helpers

...@ -319,13 +326,13 @@ static void vhost_vq_reset(struct vhost_dev *dev, >>> ????? vq->log_base = NULL; >>> ????? vq->error_ctx = NULL; >>> ????? vq->kick = NULL; >>> -??? vq->call_ctx = NULL; >>> ????? vq->log_ctx = NULL; >>> ????? vhost_reset_is_le(vq); >>> ????? vhost_disable_cross_endian(vq); >>> ????? vq->busyloop_timeout = 0; >>> ????? vq->umem = NULL; >>> ????? vq->iotlb = NULL; >>> +??? vhost_vring_call_reset(&vq->call_ctx); >>> ????? __vhost_vq_meta_reset(vq); >&g...

...@ -319,13 +326,13 @@ static void vhost_vq_reset(struct vhost_dev *dev, >>> ????? vq->log_base = NULL; >>> ????? vq->error_ctx = NULL; >>> ????? vq->kick = NULL; >>> -??? vq->call_ctx = NULL; >>> ????? vq->log_ctx = NULL; >>> ????? vhost_reset_is_le(vq); >>> ????? vhost_disable_cross_endian(vq); >>> ????? vq->busyloop_timeout = 0; >>> ????? vq->umem = NULL; >>> ????? vq->iotlb = NULL; >>> +??? vhost_vring_call_reset(&vq->call_ctx); >>> ????? __vhost_vq_meta_reset(vq); >&g...

...dev *dev, > struct vhost_virtqueue *vq) > { > @@ -319,13 +326,13 @@ static void vhost_vq_reset(struct vhost_dev *dev, > vq->log_base = NULL; > vq->error_ctx = NULL; > vq->kick = NULL; > - vq->call_ctx = NULL; > vq->log_ctx = NULL; > vhost_reset_is_le(vq); > vhost_disable_cross_endian(vq); > vq->busyloop_timeout = 0; > vq->umem = NULL; > vq->iotlb = NULL; > + vhost_vring_call_reset(&vq->call_ctx); > __vhost_vq_meta_reset(vq); > } > > @@ -685,8 +692,8 @@ void vhost_dev_cleanup(struct...

...dev *dev, > struct vhost_virtqueue *vq) > { > @@ -319,13 +326,13 @@ static void vhost_vq_reset(struct vhost_dev *dev, > vq->log_base = NULL; > vq->error_ctx = NULL; > vq->kick = NULL; > - vq->call_ctx = NULL; > vq->log_ctx = NULL; > vhost_reset_is_le(vq); > vhost_disable_cross_endian(vq); > vq->busyloop_timeout = 0; > vq->umem = NULL; > vq->iotlb = NULL; > + vhost_call_ctx_reset(&vq->call_ctx); > __vhost_vq_meta_reset(vq); > } > > @@ -685,8 +692,8 @@ void vhost_dev_cleanup(struct vh...